| Message ID | 20170114062825.GB699@tigerII.localdomain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat 2017-01-14 15:28:25, Sergey Senozhatsky wrote: > On (01/13/17 14:15), Petr Mladek wrote: > > Some console drivers code calls console_conditional_schedule() > > that looks at @console_may_schedule. The value must be cleared > > when the drivers are called from console_unlock() with > > interrupts disabled. But rescheduling is fine when the same > > code is called, for example, from tty operations where the > > console semaphore is taken via console_lock(). > > > > This is why @console_may_schedule is cleared before calling console > > drivers. The original value is stored to decide if we could sleep > > between lines. > > > > Now, @console_may_schedule is not cleared when we call > > console_trylock() and jump back to the "again" goto label. > > This has become a problem, since the commit 6b97a20d3a7909daa066 > > ("printk: set may_schedule for some of console_trylock() callers"). > > so I think I'd prefer to revert that commit. > > the reason I added the commit in question was to reduce the number of > printk() soft lockups that I observed back then. however, it obviously > didn't solve all of the printk() problems. Interesting idea! > now printk() is moving in a > completely different direction in term of lockups and deadlocks. there > will be no console_trylock() call in vprintk_emit() at all. we will > either do console_lock() from scheduleable printk_kthread or > console_trylock() from IRQ work. so 6b97a20d3a7909daa066 didn't buy us > a lot, and it still doesn't (+ it introduced a bug). Well, console_trylock() still will be there for the sync mode. Or do I miss anything? > apart from that, Tetsuo wasn't really happy with the patch > http://www.spinics.net/lists/linux-mm/msg103099.html The complain is questionable. If a code is sensitive for preemption, it should disable preemption. Another question is if people expect that printk() would call cond_resched() or preempt. > so let's just return the old behavior back. > > --- > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 7180088cbb23..ddfbd47398f8 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -2078,20 +2078,7 @@ int console_trylock(void) > return 0; > } > console_locked = 1; > - /* > - * When PREEMPT_COUNT disabled we can't reliably detect if it's > - * safe to schedule (e.g. calling printk while holding a spin_lock), > - * because preempt_disable()/preempt_enable() are just barriers there > - * and preempt_count() is always 0. > - * > - * RCU read sections have a separate preemption counter when > - * PREEMPT_RCU enabled thus we must take extra care and check > - * rcu_preempt_depth(), otherwise RCU read sections modify > - * preempt_count(). > - */ > - console_may_schedule = !oops_in_progress && > - preemptible() && > - !rcu_preempt_depth(); > + console_may_schedule = 0; > return 1; > } > EXPORT_SYMBOL(console_trylock); This would revert the change only for non-preemptive kernel. The commit 6b97a20d3a7909daa06625 ("printk: set may_schedule for some of console_trylock() callers" also enabled preemption which still affects preemtible kernel. Do we want to behave differently in preemptive and non-preemtive kernel? Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On (01/16/17 12:38), Petr Mladek wrote: [..] > > > Now, @console_may_schedule is not cleared when we call > > > console_trylock() and jump back to the "again" goto label. > > > This has become a problem, since the commit 6b97a20d3a7909daa066 > > > ("printk: set may_schedule for some of console_trylock() callers"). > > > > so I think I'd prefer to revert that commit. > > > > the reason I added the commit in question was to reduce the number of > > printk() soft lockups that I observed back then. however, it obviously > > didn't solve all of the printk() problems. > > Interesting idea! > > > now printk() is moving in a > > completely different direction in term of lockups and deadlocks. there > > will be no console_trylock() call in vprintk_emit() at all. we will > > either do console_lock() from scheduleable printk_kthread or > > console_trylock() from IRQ work. so 6b97a20d3a7909daa066 didn't buy us > > a lot, and it still doesn't (+ it introduced a bug). > > Well, console_trylock() still will be there for the sync mode. > Or do I miss anything? you mean in console_unlock()? there we inherit may_schedule from the original console_sem lock path, which sould be console_lock() in async printk case (IOW, preemptible). other then that - from printk POV, I don't think we will care that much. anything that directly calls console_lock()/console_trylock will be doing console_unlock(). those paths are not addressed by async printk anyway. I have some plans on addressing it, as you know, but that's a later work. so let's return good ol' bhaviour: -- console_trylock is always "no resched" -- console_lock is always "enable resched" (regardless of console_trylock calls from console_unlock()). > > apart from that, Tetsuo wasn't really happy with the patch > > http://www.spinics.net/lists/linux-mm/msg103099.html > > The complain is questionable. If a code is sensitive for preemption, > it should disable preemption. > > Another question is if people expect that printk() would call > cond_resched() or preempt. my assumption would be that probably people expect printk to work asap. [..] > This would revert the change only for non-preemptive kernel. > > The commit 6b97a20d3a7909daa06625 ("printk: set may_schedule for some > of console_trylock() callers" also enabled preemption which still > affects preemtible kernel. > > Do we want to behave differently in preemptive and non-preemtive > kernel? not sure I'm following here. in non-preemptible kernels console_trylock() always sets console_may_schedule to 0, just like it did before. in preemptible kernels we now will also set console_may_schedule to 0. just like before. in any case, we return back the old behavior. there should be no issues (tm) -ss -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 2017-01-16 20:58:44, Sergey Senozhatsky wrote: > On (01/16/17 12:38), Petr Mladek wrote: > [..] > > > > Now, @console_may_schedule is not cleared when we call > > > > console_trylock() and jump back to the "again" goto label. > > > > This has become a problem, since the commit 6b97a20d3a7909daa066 > > > > ("printk: set may_schedule for some of console_trylock() callers"). > > > > > > so I think I'd prefer to revert that commit. > > > > > > the reason I added the commit in question was to reduce the number of > > > printk() soft lockups that I observed back then. however, it obviously > > > didn't solve all of the printk() problems. > > > > Interesting idea! > > > > > now printk() is moving in a > > > completely different direction in term of lockups and deadlocks. there > > > will be no console_trylock() call in vprintk_emit() at all. we will > > > either do console_lock() from scheduleable printk_kthread or > > > console_trylock() from IRQ work. so 6b97a20d3a7909daa066 didn't buy us > > > a lot, and it still doesn't (+ it introduced a bug). > > > > Well, console_trylock() still will be there for the sync mode. > > Or do I miss anything? > > you mean in console_unlock()? there we inherit may_schedule from the > original console_sem lock path, which sould be console_lock() in async > printk case (IOW, preemptible). The async printk code looks like this: vprintk_emit(...) { if (can_printk_async()) { /* Offload printing to a schedulable context. */ printk_kthread_need_flush_console = true; wake_up_process(printk_kthread); } else { /* * Try to acquire and then immediately release the * console semaphore. The release will print out * buffers and wake up /dev/kmsg and syslog() users. */ if (console_trylock()) console_unlock(); } So, there is still the console_trylock() for the sync mode. Or do I see an outdated variant? > other then that - from printk POV, I don't think we will care that much. > anything that directly calls console_lock()/console_trylock will be doing > console_unlock(). those paths are not addressed by async printk anyway. > I have some plans on addressing it, as you know, but that's a later work. > > so let's return good ol' bhaviour: > -- console_trylock is always "no resched" Then you would need to revert the entire commit 6b97a20d3a7909daa06625 ("printk: set may_schedule for some of console_trylock() callers") to disable preemption also in preemptive kernel. > -- console_lock is always "enable resched" (regardless of > console_trylock calls from console_unlock()). This was always broken. If we want to fix this, we need some variant of my patch. > > > apart from that, Tetsuo wasn't really happy with the patch > > > http://www.spinics.net/lists/linux-mm/msg103099.html > > > > The complain is questionable. If a code is sensitive for preemption, > > it should disable preemption. > > > > Another question is if people expect that printk() would call > > cond_resched() or preempt. > > my assumption would be that probably people expect printk to work > asap. Sure. But this will be solved by the async mode. If people force sync mode there always will be a risk that printk() might take long. IMHO, if a code takes a long time and it is called in preemtible context it should get preempted. => We should keep that cond_resched() and allow to call it for the synchronous mode. > [..] > > This would revert the change only for non-preemptive kernel. > > > > The commit 6b97a20d3a7909daa06625 ("printk: set may_schedule for some > > of console_trylock() callers" also enabled preemption which still > > affects preemtible kernel. > > > > Do we want to behave differently in preemptive and non-preemtive > > kernel? > > not sure I'm following here. in non-preemptible kernels console_trylock() > always sets console_may_schedule to 0, just like it did before. No, if CONFIG_PREEMPT_COUNT is enabled then we are able to detect preemtible context even on non-preemtible kernel. Then console_may_schedule = !oops_in_progress && preemptible() && !rcu_preempt_depth(); might eventually allow scheduling. > preemptible kernels we now will also set console_may_schedule to 0. > just like before. Only, the following part of the commit 6b97a20d3a7909d was important for preemtible kernel: @@ -1758,20 +1758,12 @@ asmlinkage int vprintk_emit(int facility, int level, if (!in_sched) { lockdep_off(); /* - * Disable preemption to avoid being preempted while holding - * console_sem which would prevent anyone from printing to - * console - */ - preempt_disable(); - - /* * Try to acquire and then immediately release the console * semaphore. The release will print out buffers and wake up * /dev/kmsg and syslog() users. */ if (console_trylock()) console_unlock(); - preempt_enable(); lockdep_on(); } Note that cond_resched() is a non-op in preemtible kernel. See the following code is in current Linus' tree in include/linux/sched.h: #ifndef CONFIG_PREEMPT extern int _cond_resched(void); #else static inline int _cond_resched(void) { return 0; } #endif It makes perfect sense. The following code is needed for non-preemtible kernel: local_irq_restore(flags); cond_resched() but the following code does the same job in preemtible kernel: local_irq_restore(flags); If there is a pending interrupt/timer that would cause preemption in preemtible kernel, it will happen immediately when interrupts are enabled. We do not need to call cond_resched() for this. Also if the interrupt/timers is not pending, it does not make sense to call cond_resched() because the time for the task has not elapsed yet. My proposal: 1. Keep the commit 6b97a20d3a7909d as is. As I wrote above. If a function takes a long and it is called in preemtible context, it should preempt. The fact that printk() might take long is bad. But this will get solved by async mode. The cond_resched still makes sense in sync mode. 2. Fix clearing/storing console_might_schedule in console_unlock(). It makes sense for keeping the setting from console_lock() even if console_trylock() always set 0. Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On (01/16/17 13:48), Petr Mladek wrote: [..] > The async printk code looks like this: > > vprintk_emit(...) > { > > > if (can_printk_async()) { > /* Offload printing to a schedulable context. */ > printk_kthread_need_flush_console = true; > wake_up_process(printk_kthread); > } else { > /* > * Try to acquire and then immediately release the > * console semaphore. The release will print out > * buffers and wake up /dev/kmsg and syslog() users. > */ > if (console_trylock()) > console_unlock(); > } > > So, there is still the console_trylock() for the sync mode. Or do I > see an outdated variant? no, it doesn't. ASYNC printk looks like a wake_up of printk_kthread from deferred printk handler. and printk_kthread does a while-loop, calling console_lock() and doing console_unlock(). SYNC printk mode is also handled in deferred printk callback and does console_trylock()/console_unlock(). > > other then that - from printk POV, I don't think we will care that much. > > anything that directly calls console_lock()/console_trylock will be doing > > console_unlock(). those paths are not addressed by async printk anyway. > > I have some plans on addressing it, as you know, but that's a later work. > > > > so let's return good ol' bhaviour: > > -- console_trylock is always "no resched" > > Then you would need to revert the entire commit 6b97a20d3a7909daa06625 > ("printk: set may_schedule for some of console_trylock() callers") > to disable preemption also in preemptive kernel. we check can_use_console() in console_unlock(), with console_sem acquired. it's not like the CPU will suddenly go offline from under us. > > -- console_lock is always "enable resched" (regardless of > > console_trylock calls from console_unlock()). > > This was always broken. If we want to fix this, we need > some variant of my patch. you mean the "console_trylock calls from console_unlock()" part. well, may be. it sort of works for me. we safe may_schedule from the original context and then don't care about console_trylock(). it's a bit fragile, though. took me 1 year to find out that I accidentally broke it. [..] > > not sure I'm following here. in non-preemptible kernels console_trylock() > > always sets console_may_schedule to 0, just like it did before. > > No, if CONFIG_PREEMPT_COUNT is enabled then we are able to detect > preemtible context even on non-preemtible kernel. Then > > console_may_schedule = !oops_in_progress && > preemptible() && > !rcu_preempt_depth(); > > might eventually allow scheduling. yes. well, that was the idea behind those lines. the question is - do we really need it after all? given that there won't be console_trylock() in printk path any more. my call -- we probably don't need it. thus I'm proposing to return back console_trylock() we used to have. > My proposal: > > 1. Keep the commit 6b97a20d3a7909d as is. As I wrote above. If > a function takes a long and it is called in preemtible context, > it should preempt. > > The fact that printk() might take long is bad. But this will > get solved by async mode. The cond_resched still makes sense in > sync mode. > > > 2. Fix clearing/storing console_might_schedule in console_unlock(). > It makes sense for keeping the setting from console_lock() even > if console_trylock() always set 0. well, we can add that *_orig/etc, but is it really worth it? console_trylock() won't be used that often. not in printk at least. -ss -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sergey Senozhatsky wrote: > On (01/16/17 12:38), Petr Mladek wrote: > > > apart from that, Tetsuo wasn't really happy with the patch > > > http://www.spinics.net/lists/linux-mm/msg103099.html > > > > The complain is questionable. If a code is sensitive for preemption, > > it should disable preemption. > > > > Another question is if people expect that printk() would call > > cond_resched() or preempt. > > my assumption would be that probably people expect printk to work > asap. The code executed with oom_lock held is sensitive for preemption. I tried to disable preemption, but it was not accepted. What is so sorry is that this is not really a problem of printk() because sleeping for minutes (presumably forever) with oom_lock held is triggerable without printk(). It is a problem of memory page allocator which consumes a lot of CPU time for pointless direct reclaim rather than giving CPU time to a thread which held the oom_lock. I tried to wait for oom_lock in order to give CPU time to the thread holding the oom_lock, but it was not accepted neither. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On (01/16/17 22:26), Sergey Senozhatsky wrote: > On (01/16/17 13:48), Petr Mladek wrote: > [..] > > The async printk code looks like this: > > > > vprintk_emit(...) > > { > > > > > > if (can_printk_async()) { > > /* Offload printing to a schedulable context. */ > > printk_kthread_need_flush_console = true; > > wake_up_process(printk_kthread); > > } else { > > /* > > * Try to acquire and then immediately release the > > * console semaphore. The release will print out > > * buffers and wake up /dev/kmsg and syslog() users. > > */ > > if (console_trylock()) > > console_unlock(); > > } > > > > So, there is still the console_trylock() for the sync mode. Or do I > > see an outdated variant? > > no, it doesn't. I meant that none of this is happening in vprintk_emit(). there is no console_sem in vprintk_emit() at all. eveything is done in deferred printk. -ss -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 2017-01-16 22:26:33, Sergey Senozhatsky wrote: > On (01/16/17 13:48), Petr Mladek wrote: > [..] > > The async printk code looks like this: > > > > vprintk_emit(...) > > { > > > > > > if (can_printk_async()) { > > /* Offload printing to a schedulable context. */ > > printk_kthread_need_flush_console = true; > > wake_up_process(printk_kthread); > > } else { > > /* > > * Try to acquire and then immediately release the > > * console semaphore. The release will print out > > * buffers and wake up /dev/kmsg and syslog() users. > > */ > > if (console_trylock()) > > console_unlock(); > > } > > > > So, there is still the console_trylock() for the sync mode. Or do I > > see an outdated variant? > > no, it doesn't. > > ASYNC printk looks like a wake_up of printk_kthread from deferred > printk handler. and printk_kthread does a while-loop, calling > console_lock() and doing console_unlock(). Yup. > SYNC printk mode is also handled in deferred printk callback and > does console_trylock()/console_unlock(). I am confused by the sentence. If it is a synchronous mode then console_trylock()/console_unlock() must be called directly from printk()/vprintk_emit(). If you move this to a deferred callback, it is not longer synchronous. > > > other then that - from printk POV, I don't think we will care that much. > > > anything that directly calls console_lock()/console_trylock will be doing > > > console_unlock(). those paths are not addressed by async printk anyway. > > > I have some plans on addressing it, as you know, but that's a later work. > > > > > > so let's return good ol' bhaviour: > > > -- console_trylock is always "no resched" > > > > Then you would need to revert the entire commit 6b97a20d3a7909daa06625 > > ("printk: set may_schedule for some of console_trylock() callers") > > to disable preemption also in preemptive kernel. > > we check can_use_console() in console_unlock(), with console_sem acquired. > it's not like the CPU will suddenly go offline from under us. I am not sure how this is related to the above. It talked about the "no resched" behavior. If you want to avoid preemtion in preemtible kernel, you need to disable preemtion. If you want to avoid preemtion in non-preemtible kernel, it is enough to do _not_ call cond_resched()/schedule(). Your mail https://lkml.kernel.org/r/20170114062825.GB699@tigerII.localdomain suggested to avoid calling cond_resched(). This reverted the original behavior only for non-preemtible kernel. If we wanted to restore the original behavior also for preemtible kernel, we would need to disable preemtion around console_trylock/ console_unlock() calls again. > > > -- console_lock is always "enable resched" (regardless of > > > console_trylock calls from console_unlock()). > > > > This was always broken. If we want to fix this, we need > > some variant of my patch. > > you mean the "console_trylock calls from console_unlock()" part. > well, may be. it sort of works for me. we safe may_schedule from > the original context and then don't care about console_trylock(). > > it's a bit fragile, though. took me 1 year to find out that I > accidentally broke it. That only means that the problem is a corner case. Note that Tetsuo found it by some stress test that puts the machine beyond usability. I personally think that the enabled preemtion and automatic detection of the context makes perfect sense. [...] > > My proposal: > > > > 1. Keep the commit 6b97a20d3a7909d as is. As I wrote above. If > > a function takes a long and it is called in preemtible context, > > it should preempt. > > > > The fact that printk() might take long is bad. But this will > > get solved by async mode. The cond_resched still makes sense in > > sync mode. > > > > > > 2. Fix clearing/storing console_might_schedule in console_unlock(). > > It makes sense for keeping the setting from console_lock() even > > if console_trylock() always set 0. > > well, we can add that *_orig/etc, but is it really worth it? > console_trylock() won't be used that often. not in printk at > least. IMHO, it is worth it because both patches go into the right direction. Your commit allows to preemt a potentially long call in preemtible context. It makes perfect sense. And it will be usable in the sync mode. My patch is needed if we keep your commit and I vote for it. Also it might safe someone a time in the future if he wanted to solve the prepemtion again. Note how much effort it took us to understand the side effects and the console_trylock()/goto-again influence. Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On (01/16/17 15:14), Petr Mladek wrote: [..] > > SYNC printk mode is also handled in deferred printk callback and > > does console_trylock()/console_unlock(). > > I am confused by the sentence. > > If it is a synchronous mode then console_trylock()/console_unlock() must > be called directly from printk()/vprintk_emit(). > > If you move this to a deferred callback, it is not longer synchronous. yes, everything is about to move to the deferred printk() handler. it has been discussed during the LPC/KS session. Linus proposed it, to be exact. and I was quite sure that everyone in the room, including you, agreed. we either do everything asking scheduled for help (wake_up()), which is async printk. or do the print out from deferred printk() handler /** 1) in case of panic() there is a console_flush_on_panic() call. 2) once the system stores at least one EMERG loglevel message, we don't wake_up() printk_kthread from deferred printk handler. **/ but, well, probably this is not related to this thread. > it is not longer synchronous. well, a silly and boring note is that there is nothing that guarantees or provides 'synchronous' printk(). console_sem simply can be locked and printk() will just log_store(), while the actual printing will happen sometime later (if ever) from whatever context that held the console_sem. can be IRQ or anything else. *may be* we need a new 'terminology' here. 'sync printk' does not reflect the actual behavior and can give false expectations. just a note. > > > Then you would need to revert the entire commit 6b97a20d3a7909daa06625 > > > ("printk: set may_schedule for some of console_trylock() callers") > > > to disable preemption also in preemptive kernel. > > > > we check can_use_console() in console_unlock(), with console_sem acquired. > > it's not like the CPU will suddenly go offline from under us. > > I am not sure how this is related to the above. It talked about > the "no resched" behavior. > > If you want to avoid preemtion in preemtible kernel, you need to > disable preemtion. > > If you want to avoid preemtion in non-preemtible kernel, it is > enough to do _not_ call cond_resched()/schedule(). [..] > Your mail > https://lkml.kernel.org/r/20170114062825.GB699@tigerII.localdomain > suggested to avoid calling cond_resched(). This reverted the > original behavior only for non-preemtible kernel. > If we wanted to restore the original behavior also for preemtible > kernel, we would need to disable preemtion around console_trylock/ > console_unlock() calls again. ah, ok. the code there is not even a patch. so yes, could be incomplete/wrong "revert". [..] > > well, we can add that *_orig/etc, but is it really worth it? > > console_trylock() won't be used that often. not in printk at > > least. > > IMHO, it is worth it because both patches go into the right direction. well, ok. if you insist :) -ss -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On (01/17/17 00:19), Sergey Senozhatsky wrote: [..] > > I am confused by the sentence. > > > > If it is a synchronous mode then console_trylock()/console_unlock() must > > be called directly from printk()/vprintk_emit(). > > > > If you move this to a deferred callback, it is not longer synchronous. > > yes, everything is about to move to the deferred printk() handler. > it has been discussed during the LPC/KS session. Linus proposed it, > to be exact. and I was quite sure that everyone in the room, > including you, agreed. we either do everything asking scheduled for > help (wake_up()), which is async printk. or do the print out from > deferred printk() handler /** 1) in case of panic() there is a > console_flush_on_panic() call. 2) once the system stores at least > one EMERG loglevel message, we don't wake_up() printk_kthread from > deferred printk handler. **/ gosh.. sorry. I need some rest. badly. the above is not completely accurate. "make printk always behave like printk deferred" was in my presentation and proposal. the difference was in EMERG loglevel handling. Linus suggested to handle it in deferred printk handler as well, while I initially wanted to handle EMERG messages in vprintk_emit() as a special case/exception. but I like the idea of doing everything in printk deferred handler better. printk is moving there anyway. otherwise there is no way of resolving printk deadlocks on "external" locks. -ss -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue 2017-01-17 00:43:43, Sergey Senozhatsky wrote: > On (01/17/17 00:19), Sergey Senozhatsky wrote: > [..] > > > I am confused by the sentence. > > > > > > If it is a synchronous mode then console_trylock()/console_unlock() must > > > be called directly from printk()/vprintk_emit(). > > > > > > If you move this to a deferred callback, it is not longer synchronous. > > > > yes, everything is about to move to the deferred printk() handler. > > it has been discussed during the LPC/KS session. Linus proposed it, > > to be exact. and I was quite sure that everyone in the room, > > including you, agreed. we either do everything asking scheduled for > > help (wake_up()), which is async printk. or do the print out from > > deferred printk() handler /** 1) in case of panic() there is a > > console_flush_on_panic() call. 2) once the system stores at least > > one EMERG loglevel message, we don't wake_up() printk_kthread from > > deferred printk handler. **/ > > gosh.. sorry. I need some rest. badly. the above is not completely > accurate. "make printk always behave like printk deferred" was in > my presentation and proposal. the difference was in EMERG loglevel > handling. Linus suggested to handle it in deferred printk handler > as well, while I initially wanted to handle EMERG messages in > vprintk_emit() as a special case/exception. but I like the idea of > doing everything in printk deferred handler better. printk is moving > there anyway. otherwise there is no way of resolving printk deadlocks > on "external" locks. Yes, this is the direction. But I can not imagine to do all this in one big step. It is even possible that it will simply not work. Just remember the problems with suspend, sysrq, kexec. There are more paths where we need to make sure that the messages are flushed. And we must not rely only on deferred solutions (IRQ, kthread) because they simply need not happen. My expectation is that we will keep the sync mode as it is now for some (long time). And we will do more and more deferring in the async mode step by step. If the async mode proves to be perfectly usable and people do not need the sync mode any longer, we could remove the sync mode but only then. By other words, I expect that we would first push a solution similar to v12. It was tested for years in SUSE. Any bigger changes would just cause another huge delay. I am sorry but I am not a crazy jumper. The async printk idea was blocked for years. We should not go into the other extreme now. Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 7180088cbb23..ddfbd47398f8 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2078,20 +2078,7 @@ int console_trylock(void) return 0; } console_locked = 1; - /* - * When PREEMPT_COUNT disabled we can't reliably detect if it's - * safe to schedule (e.g. calling printk while holding a spin_lock), - * because preempt_disable()/preempt_enable() are just barriers there - * and preempt_count() is always 0. - * - * RCU read sections have a separate preemption counter when - * PREEMPT_RCU enabled thus we must take extra care and check - * rcu_preempt_depth(), otherwise RCU read sections modify - * preempt_count(). - */ - console_may_schedule = !oops_in_progress && - preemptible() && - !rcu_preempt_depth(); + console_may_schedule = 0; return 1; } EXPORT_SYMBOL(console_trylock);