diff mbox series

[printk,v4,38/39] printk: relieve console_lock of list synchronization duties

Message ID 20221114162932.141883-39-john.ogness@linutronix.de
State New
Headers
Series reduce console_lock scope |

Commit Message

John Ogness Nov. 14, 2022, 4:29 p.m. UTC 
  The console_list_lock provides synchronization for console list and
console->flags updates. All call sites that were using the console_lock
for this synchronization have either switched to use the
console_list_lock or the SRCU list iterator.

Remove console_lock usage for console list updates and console->flags
updates.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 kernel/printk/printk.c | 42 ++++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

Comments

Petr Mladek Nov. 15, 2022, 3:34 p.m. UTC | #1 
On Mon 2022-11-14 17:35:31, John Ogness wrote:
> The console_list_lock provides synchronization for console list and
> console->flags updates. All call sites that were using the console_lock
> for this synchronization have either switched to use the
> console_list_lock or the SRCU list iterator.
> 
> Remove console_lock usage for console list updates and console->flags
> updates.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -3334,6 +3330,11 @@ void register_console(struct console *newcon)
>  		 * boot console that is the furthest behind.
>  		 */
>  		if (bootcon_registered && !keep_bootcon) {
> +			/*
> +			 * Hold the console_lock to guarantee safe access to
> +			 * console->seq.
> +			 */
> +			console_lock();
>  			for_each_console(con) {
>  				if ((con->flags & CON_BOOT) &&
>  				    (con->flags & CON_ENABLED) &&
> @@ -3341,6 +3342,7 @@ void register_console(struct console *newcon)
>  					newcon->seq = con->seq;
>  				}
>  			}
> +			console_unlock();

Thinking more about it. This console_unlock() will actually cause
flushing the boot consoles. A solution would be to call
console_flush_all() here.

And we could/should solve this in a separate patch. This code was not locked
before. It is a corner case. It could be solved later.

>  		}
>  	}
>  

Best Regards,
Petr
John Ogness Nov. 15, 2022, 4:41 p.m. UTC | #2 
On 2022-11-15, Petr Mladek <pmladek@suse.com> wrote:
> On Mon 2022-11-14 17:35:31, John Ogness wrote:
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -3334,6 +3330,11 @@ void register_console(struct console *newcon)
>>  		 * boot console that is the furthest behind.
>>  		 */
>>  		if (bootcon_registered && !keep_bootcon) {
>> +			/*
>> +			 * Hold the console_lock to guarantee safe access to
>> +			 * console->seq.
>> +			 */
>> +			console_lock();
>>  			for_each_console(con) {
>>  				if ((con->flags & CON_BOOT) &&
>>  				    (con->flags & CON_ENABLED) &&
>> @@ -3341,6 +3342,7 @@ void register_console(struct console *newcon)
>>  					newcon->seq = con->seq;
>>  				}
>>  			}
>> +			console_unlock();
>
> Thinking more about it. This console_unlock() will actually cause
> flushing the boot consoles. A solution would be to call
> console_flush_all() here.

console_flush_all() requires the console_lock, so I don't think it would
be different.

The correct solution would be to recognize if nextcon is taking over a
bootcon. If yes, that bootcon could be unregistered right here with
unregister_console_locked() and then seq for nextcon set appropriately
to perfectly take over.

But we will need to think about how we could recognize the same
device. I was thinking about if consoles hat some attribute showing
their io-membase or something so that it could be clear that the two are
the same hardware.

> And we could/should solve this in a separate patch. This code was not
> locked before. It is a corner case. It could be solved later.

Agreed.

John
Petr Mladek Nov. 15, 2022, 4:41 p.m. UTC | #3 
On Tue 2022-11-15 16:34:10, Petr Mladek wrote:
> On Mon 2022-11-14 17:35:31, John Ogness wrote:
> > The console_list_lock provides synchronization for console list and
> > console->flags updates. All call sites that were using the console_lock
> > for this synchronization have either switched to use the
> > console_list_lock or the SRCU list iterator.
> > 
> > Remove console_lock usage for console list updates and console->flags
> > updates.
> > 
> > Signed-off-by: John Ogness <john.ogness@linutronix.de>
> 
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -3334,6 +3330,11 @@ void register_console(struct console *newcon)
> >  		 * boot console that is the furthest behind.
> >  		 */
> >  		if (bootcon_registered && !keep_bootcon) {
> > +			/*
> > +			 * Hold the console_lock to guarantee safe access to
> > +			 * console->seq.
> > +			 */
> > +			console_lock();
> >  			for_each_console(con) {
> >  				if ((con->flags & CON_BOOT) &&
> >  				    (con->flags & CON_ENABLED) &&
> > @@ -3341,6 +3342,7 @@ void register_console(struct console *newcon)
> >  					newcon->seq = con->seq;
> >  				}
> >  			}
> > +			console_unlock();
> 
> Thinking more about it. This console_unlock() will actually cause
> flushing the boot consoles. A solution would be to call
> console_flush_all() here.
> 
> And we could/should solve this in a separate patch. This code was not locked
> before. It is a corner case. It could be solved later.
> 
> >  		}
> >  	}
> >  

The rest of the patch looks fine. I checked hopefully console_list walks
and console flags manipulations and everything looks good.

So, without the above two hunks:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr
John Ogness Nov. 15, 2022, 5:15 p.m. UTC | #4 
On 2022-11-15, Petr Mladek <pmladek@suse.com> wrote:
>>> --- a/kernel/printk/printk.c
>>> +++ b/kernel/printk/printk.c
>>> @@ -3334,6 +3330,11 @@ void register_console(struct console *newcon)
>>>  		 * boot console that is the furthest behind.
>>>  		 */
>>>  		if (bootcon_registered && !keep_bootcon) {
>>> +			/*
>>> +			 * Hold the console_lock to guarantee safe access to
>>> +			 * console->seq.
>>> +			 */
>>> +			console_lock();
>>>  			for_each_console(con) {
>>>  				if ((con->flags & CON_BOOT) &&
>>>  				    (con->flags & CON_ENABLED) &&
>>> @@ -3341,6 +3342,7 @@ void register_console(struct console *newcon)
>>>  					newcon->seq = con->seq;
>>>  				}
>>>  			}
>>> +			console_unlock();
>
> So, without the above two hunks:
>
> Reviewed-by: Petr Mladek <pmladek@suse.com>

Note that we actually need those hunks to guarantee a consistent @seq
value. The console_lock is the only synchronization mechanism available
to read console->seq.

John
Petr Mladek Nov. 15, 2022, 6:23 p.m. UTC | #5 
On Tue 2022-11-15 17:47:12, John Ogness wrote:
> On 2022-11-15, Petr Mladek <pmladek@suse.com> wrote:
> > On Mon 2022-11-14 17:35:31, John Ogness wrote:
> >> --- a/kernel/printk/printk.c
> >> +++ b/kernel/printk/printk.c
> >> @@ -3334,6 +3330,11 @@ void register_console(struct console *newcon)
> >>  		 * boot console that is the furthest behind.
> >>  		 */
> >>  		if (bootcon_registered && !keep_bootcon) {
> >> +			/*
> >> +			 * Hold the console_lock to guarantee safe access to
> >> +			 * console->seq.
> >> +			 */
> >> +			console_lock();
> >>  			for_each_console(con) {
> >>  				if ((con->flags & CON_BOOT) &&
> >>  				    (con->flags & CON_ENABLED) &&
> >> @@ -3341,6 +3342,7 @@ void register_console(struct console *newcon)
> >>  					newcon->seq = con->seq;
> >>  				}
> >>  			}
> >> +			console_unlock();
> >
> > Thinking more about it. This console_unlock() will actually cause
> > flushing the boot consoles. A solution would be to call
> > console_flush_all() here.
> 
> console_flush_all() requires the console_lock, so I don't think it would
> be different.

I meant to keep the console_lock(), something like:

		if (bootcon_registered && !keep_bootcon) {
			/*
			 * Hold the console_lock to guarantee safe access to
			 * console->seq.
			 */
			console_lock();
			/* Try to sync all consoles. */
			console_flush_all();
			/* Check if some boot console is still behind. */
			for_each_console(con) {
				if ((con->flags & CON_BOOT) &&
				    (con->flags & CON_ENABLED) &&
				    con->seq < newcon->seq) {
					newcon->seq = con->seq;
				}
			}
			console_unlock();
		}

It is not that easy because console_flush_all() might handover the
console_lock(). Also some new messages might appear so that we
should re-read prb_next_seq().

Maybe, the best solution would be to call pr_flush():

		if (bootcon_registered && !keep_bootcon) {
			/*
			 * Try to sync all consoles because we do not
			 * know which one is going to be replaced
			 */
			pr_flush();
			/*
			 * Hold the console_lock to guarantee safe access to
			 * console->seq.
			 */
			console_lock();
			/* Check if some boot console is still behind. */
			for_each_console(con) {
				if ((con->flags & CON_BOOT) &&
				    (con->flags & CON_ENABLED) &&
				    con->seq < newcon->seq) {
					newcon->seq = con->seq;
				}
			}
			console_unlock();
		}

It was always just the best effort. It does not need to be perfect.
On the other hand, we should not make it much worse because people
report duplicated messages from time to time.

> The correct solution would be to recognize if nextcon is taking over a
> bootcon. If yes, that bootcon could be unregistered right here with
> unregister_console_locked() and then seq for nextcon set appropriately
> to perfectly take over.
> 
> But we will need to think about how we could recognize the same
> device. I was thinking about if consoles hat some attribute showing
> their io-membase or something so that it could be clear that the two are
> the same hardware.

Interesting idea. Well, it is out of scope of this patchset.

Best Regards,
Petr
Petr Mladek Nov. 15, 2022, 6:36 p.m. UTC | #6 
On Tue 2022-11-15 18:21:34, John Ogness wrote:
> On 2022-11-15, Petr Mladek <pmladek@suse.com> wrote:
> >>> --- a/kernel/printk/printk.c
> >>> +++ b/kernel/printk/printk.c
> >>> @@ -3334,6 +3330,11 @@ void register_console(struct console *newcon)
> >>>  		 * boot console that is the furthest behind.
> >>>  		 */
> >>>  		if (bootcon_registered && !keep_bootcon) {
> >>> +			/*
> >>> +			 * Hold the console_lock to guarantee safe access to
> >>> +			 * console->seq.
> >>> +			 */
> >>> +			console_lock();
> >>>  			for_each_console(con) {
> >>>  				if ((con->flags & CON_BOOT) &&
> >>>  				    (con->flags & CON_ENABLED) &&
> >>> @@ -3341,6 +3342,7 @@ void register_console(struct console *newcon)
> >>>  					newcon->seq = con->seq;
> >>>  				}
> >>>  			}
> >>> +			console_unlock();
> >
> > So, without the above two hunks:
> >
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
> 
> Note that we actually need those hunks to guarantee a consistent @seq
> value. The console_lock is the only synchronization mechanism available
> to read console->seq.

Yes, we need a solution. But it does not need to be in this patch.

This patch removes console_lock() on some locations. But this
particular code was called without console_lock() even before
this patch.

Note that the regression was added in the 3rd patch that moved
this code outside console_lock().

Maybe, the easiest solution would be to do in the 3rd patch [*]:

	} else {
		/* Begin with next message. */
		newcon->seq = prb_next_seq(prb);
		/*
		 * Try hard to show the pending messages on boot consoles.
		 * so that the new console does not start too late.
		 */
		pr_flush();
	}

It should behave as good and as bad as the original code.

[*] Or move the code and add this change before the 3rd patch
    to keep this questionable solution separated and avoid
    the regression.

Best Regards,
Petr
John Ogness Nov. 16, 2022, 8:52 a.m. UTC | #7 
On 2022-11-15, Petr Mladek <pmladek@suse.com> wrote:
> It is not that easy because console_flush_all() might handover the
> console_lock(). Also some new messages might appear so that we
> should re-read prb_next_seq().

OK. Taking all this into account, how about:

	if (newcon->flags & (CON_PRINTBUFFER | CON_BOOT)) {
		/* Get a consistent copy of @syslog_seq. */
		mutex_lock(&syslog_lock);
		newcon->seq = syslog_seq;
		mutex_unlock(&syslog_lock);
	} else {
		/* Begin with next message added to ringbuffer. */
		newcon->seq = prb_next_seq(prb);

		/*
		 * If any enabled boot consoles are due to be unregistered
		 * shortly, some may not be caught up and may be the same
		 * device as @newcon. Since it is not known which boot console
		 * is the same device, flush all consoles and, if necessary,
		 * start with the message of the enabled boot console that is
		 * the furthest behind.
		 */
		if (bootcon_registered && !keep_bootcon) {
			bool handover;

			/*
			 * Hold the console_lock to guarantee safe access to
			 * console->seq.
			 */
			console_lock();

			/*
			 * Flush all consoles and set the console to start at
			 * the next unprinted sequence number.
			 */
			if (!console_flush_all(true, &newcon->seq, &handover)) {
				/*
				 * Flushing failed. Just choose the lowest
				 * sequence of the enabled boot consoles.
				 */
				newcon->seq = prb_next_seq(prb);
				for_each_console(con) {
					if ((con->flags & CON_BOOT) &&
					    (con->flags & CON_ENABLED) &&
					    con->seq < newcon->seq) {
						newcon->seq = con->seq;
					}
				}
			}

			console_unlock();
		}
	}

John Ogness
John Ogness Nov. 16, 2022, 8:56 a.m. UTC | #8 
Hi,

I forgot to re-lock the console... See below.

On 2022-11-16, John Ogness <john.ogness@linutronix.de> wrote:
> 	if (newcon->flags & (CON_PRINTBUFFER | CON_BOOT)) {
> 		/* Get a consistent copy of @syslog_seq. */
> 		mutex_lock(&syslog_lock);
> 		newcon->seq = syslog_seq;
> 		mutex_unlock(&syslog_lock);
> 	} else {
> 		/* Begin with next message added to ringbuffer. */
> 		newcon->seq = prb_next_seq(prb);
>
> 		/*
> 		 * If any enabled boot consoles are due to be unregistered
> 		 * shortly, some may not be caught up and may be the same
> 		 * device as @newcon. Since it is not known which boot console
> 		 * is the same device, flush all consoles and, if necessary,
> 		 * start with the message of the enabled boot console that is
> 		 * the furthest behind.
> 		 */
> 		if (bootcon_registered && !keep_bootcon) {
> 			bool handover;
>
> 			/*
> 			 * Hold the console_lock to guarantee safe access to
> 			 * console->seq.
> 			 */
> 			console_lock();
>
> 			/*
> 			 * Flush all consoles and set the console to start at
> 			 * the next unprinted sequence number.
> 			 */
> 			if (!console_flush_all(true, &newcon->seq, &handover)) {
> 				/*
> 				 * Flushing failed. Just choose the lowest
> 				 * sequence of the enabled boot consoles.
> 				 */

Sorry. Here we lost the console_lock. Another acquire is needed here.

				console_lock();

> 				newcon->seq = prb_next_seq(prb);
> 				for_each_console(con) {
> 					if ((con->flags & CON_BOOT) &&
> 					    (con->flags & CON_ENABLED) &&
> 					    con->seq < newcon->seq) {
> 						newcon->seq = con->seq;
> 					}
> 				}
> 			}
>
> 			console_unlock();
> 		}
> 	}

John Ogness
John Ogness Nov. 16, 2022, 9:08 a.m. UTC | #9 
Hi Petr,

Sorry, console_flush_all() only loses the console_lock if there was a
handover. Here is a new complete suggestion from me.

	if (newcon->flags & (CON_PRINTBUFFER | CON_BOOT)) {
		/* Get a consistent copy of @syslog_seq. */
		mutex_lock(&syslog_lock);
		newcon->seq = syslog_seq;
		mutex_unlock(&syslog_lock);
	} else {
		/* Begin with next message added to ringbuffer. */
		newcon->seq = prb_next_seq(prb);

		/*
		 * If any enabled boot consoles are due to be unregistered
		 * shortly, some may not be caught up and may be the same
		 * device as @newcon. Since it is not known which boot console
		 * is the same device, flush all consoles and, if necessary,
		 * start with the message of the enabled boot console that is
		 * the furthest behind.
		 */
		if (bootcon_registered && !keep_bootcon) {
			bool handover;

			/*
			 * Hold the console_lock to guarantee safe access to
			 * console->seq.
			 */
			console_lock();

			/*
			 * Flush all consoles and set the console to start at
			 * the next unprinted sequence number.
			 */
			if (!console_flush_all(true, &newcon->seq, &handover)) {
				/*
				 * Flushing failed. Just choose the lowest
				 * sequence of the enabled boot consoles.
				 */

				/*
				 * If there was a handover, this context no
				 * longer holds the console_lock.
				 */
				if (handover)
					console_lock();

				newcon->seq = prb_next_seq(prb);
				for_each_console(con) {
					if ((con->flags & CON_BOOT) &&
					    (con->flags & CON_ENABLED) &&
					    con->seq < newcon->seq) {
						newcon->seq = con->seq;
					}
				}
			}

			console_unlock();
		}

John Ogness
Petr Mladek Nov. 16, 2022, 9:48 a.m. UTC | #10 
On Wed 2022-11-16 10:14:35, John Ogness wrote:
> Hi Petr,
> 
> Sorry, console_flush_all() only loses the console_lock if there was a
> handover. Here is a new complete suggestion from me.
> 
> 	if (newcon->flags & (CON_PRINTBUFFER | CON_BOOT)) {
> 		/* Get a consistent copy of @syslog_seq. */
> 		mutex_lock(&syslog_lock);
> 		newcon->seq = syslog_seq;
> 		mutex_unlock(&syslog_lock);
> 	} else {
> 		/* Begin with next message added to ringbuffer. */
> 		newcon->seq = prb_next_seq(prb);
> 
> 		/*
> 		 * If any enabled boot consoles are due to be unregistered
> 		 * shortly, some may not be caught up and may be the same
> 		 * device as @newcon. Since it is not known which boot console
> 		 * is the same device, flush all consoles and, if necessary,
> 		 * start with the message of the enabled boot console that is
> 		 * the furthest behind.
> 		 */
> 		if (bootcon_registered && !keep_bootcon) {
> 			bool handover;
> 
> 			/*
> 			 * Hold the console_lock to guarantee safe access to
> 			 * console->seq.
> 			 */
> 			console_lock();
> 
> 			/*
> 			 * Flush all consoles and set the console to start at
> 			 * the next unprinted sequence number.
> 			 */
> 			if (!console_flush_all(true, &newcon->seq, &handover)) {
> 				/*
> 				 * Flushing failed. Just choose the lowest
> 				 * sequence of the enabled boot consoles.
> 				 */
> 
> 				/*
> 				 * If there was a handover, this context no
> 				 * longer holds the console_lock.
> 				 */
> 				if (handover)
> 					console_lock();
> 
> 				newcon->seq = prb_next_seq(prb);
> 				for_each_console(con) {
> 					if ((con->flags & CON_BOOT) &&
> 					    (con->flags & CON_ENABLED) &&
> 					    con->seq < newcon->seq) {
> 						newcon->seq = con->seq;
> 					}
> 				}
> 			}
> 
> 			console_unlock();
> 		}

It looks good to me.

Now, we just need to agree how to add this into the patchset.
My proposal is to:

1. patch: hide the original code into a function, .e.g. console_init_seq()
2. patch: move console_init_seq() and add the above trickery

Do both before the 3rd patch in this patchset. It moved the code
outside console_lock() guarded section.

Best Regards,
Petr
diff mbox series

Patch

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index dff76c1cef80..8d635467882f 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -86,8 +86,8 @@  EXPORT_SYMBOL(oops_in_progress);
 static DEFINE_MUTEX(console_mutex);
 
 /*
- * console_sem protects console_list and console->flags updates, and also
- * provides serialization for access to the entire console driver system.
+ * console_sem protects updates to console->seq and console_suspended,
+ * and also provides serialization for console printing.
  */
 static DEFINE_SEMAPHORE(console_sem);
 HLIST_HEAD(console_list);
@@ -2639,10 +2639,10 @@  static int console_cpu_notify(unsigned int cpu)
 }
 
 /**
- * console_lock - lock the console system for exclusive use.
+ * console_lock - block the console subsystem from printing
  *
- * Acquires a lock which guarantees that the caller has
- * exclusive access to the console system and console_list.
+ * Acquires a lock which guarantees that no consoles will
+ * be in or enter their write() callback.
  *
  * Can sleep, returns nothing.
  */
@@ -2659,10 +2659,10 @@  void console_lock(void)
 EXPORT_SYMBOL(console_lock);
 
 /**
- * console_trylock - try to lock the console system for exclusive use.
+ * console_trylock - try to block the console subsystem from printing
  *
- * Try to acquire a lock which guarantees that the caller has exclusive
- * access to the console system and console_list.
+ * Try to acquire a lock which guarantees that no consoles will
+ * be in or enter their write() callback.
  *
  * returns 1 on success, and 0 on failure to acquire the lock.
  */
@@ -2919,10 +2919,10 @@  static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handove
 }
 
 /**
- * console_unlock - unlock the console system
+ * console_unlock - unblock the console subsystem from printing
  *
- * Releases the console_lock which the caller holds on the console system
- * and the console driver list.
+ * Releases the console_lock which the caller holds to block printing of
+ * the console subsystem.
  *
  * While the console_lock was held, console output may have been buffered
  * by printk().  If this is the case, console_unlock(); emits
@@ -3109,9 +3109,7 @@  void console_stop(struct console *console)
 {
 	__pr_flush(console, 1000, true);
 	console_list_lock();
-	console_lock();
 	console_srcu_write_flags(console, console->flags & ~CON_ENABLED);
-	console_unlock();
 	console_list_unlock();
 
 	/*
@@ -3127,9 +3125,7 @@  EXPORT_SYMBOL(console_stop);
 void console_start(struct console *console)
 {
 	console_list_lock();
-	console_lock();
 	console_srcu_write_flags(console, console->flags | CON_ENABLED);
-	console_unlock();
 	console_list_unlock();
 	__pr_flush(console, 1000, true);
 }
@@ -3334,6 +3330,11 @@  void register_console(struct console *newcon)
 		 * boot console that is the furthest behind.
 		 */
 		if (bootcon_registered && !keep_bootcon) {
+			/*
+			 * Hold the console_lock to guarantee safe access to
+			 * console->seq.
+			 */
+			console_lock();
 			for_each_console(con) {
 				if ((con->flags & CON_BOOT) &&
 				    (con->flags & CON_ENABLED) &&
@@ -3341,6 +3342,7 @@  void register_console(struct console *newcon)
 					newcon->seq = con->seq;
 				}
 			}
+			console_unlock();
 		}
 	}
 
@@ -3348,7 +3350,6 @@  void register_console(struct console *newcon)
 	 * Put this console in the list - keep the
 	 * preferred driver at the head of the list.
 	 */
-	console_lock();
 	if (hlist_empty(&console_list)) {
 		/* Ensure CON_CONSDEV is always set for the head. */
 		newcon->flags |= CON_CONSDEV;
@@ -3362,7 +3363,6 @@  void register_console(struct console *newcon)
 	} else {
 		hlist_add_behind_rcu(&newcon->node, console_list.first);
 	}
-	console_unlock();
 
 	/*
 	 * No need to synchronize SRCU here! The caller does not rely
@@ -3410,15 +3410,11 @@  static int unregister_console_locked(struct console *console)
 	if (res > 0)
 		return 0;
 
-	console_lock();
-
 	/* Disable it unconditionally */
 	console_srcu_write_flags(console, console->flags & ~CON_ENABLED);
 
-	if (!console_is_registered_locked(console)) {
-		console_unlock();
+	if (!console_is_registered_locked(console))
 		return -ENODEV;
-	}
 
 	hlist_del_init_rcu(&console->node);
 
@@ -3434,8 +3430,6 @@  static int unregister_console_locked(struct console *console)
 	if (!hlist_empty(&console_list) && console->flags & CON_CONSDEV)
 		console_srcu_write_flags(console_first(), console_first()->flags | CON_CONSDEV);
 
-	console_unlock();
-
 	/*
 	 * Ensure that all SRCU list walks have completed. All contexts
 	 * must not be able to see this console in the list so that any