Messages in this thread | | | Date | Fri, 8 Aug 2008 12:37:47 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] printk: robustify printk |
| |
On Fri, 08 Aug 2008 21:21:08 +0200 Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Fri, 2008-08-08 at 12:14 -0700, Andrew Morton wrote: > > On Fri, 08 Aug 2008 20:14:28 +0200 > > Peter Zijlstra <a.p.zijlstra@chello.nl> wrote: > > > > > void wake_up_klogd(void) > > > { > > > - if (!oops_in_progress && waitqueue_active(&log_wait)) > > > - wake_up_interruptible(&log_wait); > > > + unsigned long flags; > > > + struct klogd_wakeup_state *kws; > > > + > > > + if (!waitqueue_active(&log_wait)) > > > + return; > > > + > > > + local_irq_save(flags); > > > + kws = &__get_cpu_var(kws); > > > + if (!kws->pending) { > > > + kws->pending = 1; > > > + call_rcu(&kws->head, __wake_up_klogd); > > > + } > > > + local_irq_restore(flags); > > > } > > > > Note that kernel/rcupreempt.c's flavour of call_rcu() takes > > RCU_DATA_ME().lock, so there are still code sites from which a printk > > can deadlock. Only now, it is config-dependent. > > > > From a quick look it appears that large amounts of kernel/rcupreempt.c > > are now a printk-free zone. > > Drad, missed that bit, I did look at the calling end, but forgot the > call_rcu() end :-/ > > The initial printk_tick() based implementation didn't suffer this > problem, should we revert to that scheme?
Dunno. Perhaps we could convert RCU_DATA_ME's spinlock_t into an rwlock and do read_lock() in call_rcu()? Then we can should be able to call printk from inside that read_lock(), but not inside write_lock(), which, with suitable warning comments might be acceptable.
afacit everything in call_rcu()'s *rdp is cpu-local and is protected by local_irq_save(). rcu_ctrlblk.completed and rcu_flipped need some protection, but a) rdp->lock isn't sufficient anyway and b) read_lock protection would suffice. Maybe other CPUs can alter *rdp while __rcu_advance_callbacks() is running.
Anyway, that's all handwaving. My point is that making rcupreempt.c more robust and more concurrent might be an alternative fix, and might be beneficial in its own right. Working out the details is what we have Pauls for ;)
| |