Messages in this thread | | | Date | Fri, 18 Feb 2022 18:49:23 +0100 | From | Sebastian Andrzej Siewior <> | Subject | Re: [PATCH] pstore: Don't use semaphores in always-atomic-context code |
| |
On 2022-02-18 18:29:01 [+0100], Jann Horn wrote: … > Lockdep probably never caught the problem because it's very rare that you > actually hit the contended case, and lockdep can't check a
I appears that after ´a' a few words would follow.
> Fixes: ea84b580b955 ("pstore: Convert buf_lock to semaphore") > Cc: stable@vger.kernel.org > Signed-off-by: Jann Horn <jannh@google.com> > --- > Testing on 5.15.24 (latest stable), with CONFIG_PREEMPT=y, when I trigger > a BUG() via LKDTM ("echo BUG > /sys/kernel/debug/provoke-crash/DIRECT"), > I first get the expected BUG splat, followed by this RCU warning: > > Voluntary context switch within RCU read-side critical section!
Right, scheduling not allowed within a RCU read-side section. I'm not sure if lockdep cares but if you enable CONFIG_DEBUG_ATOMIC_SLEEP then the might_sleep() will complain if you are in a RCU section.
> This patch makes the RCU context warning go away.
Makes sense. Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> --- a/fs/pstore/platform.c > +++ b/fs/pstore/platform.c > @@ -143,27 +143,26 @@ static void pstore_timer_kick(void) > mod_timer(&pstore_timer, jiffies + msecs_to_jiffies(pstore_update_ms)); > } > > -/* > - * Should pstore_dump() wait for a concurrent pstore_dump()? If > - * not, the current pstore_dump() will report a failure to dump > - * and return. > - */ > -static bool pstore_cannot_wait(enum kmsg_dump_reason reason) > +bool pstore_cannot_block_path(enum kmsg_dump_reason reason) > { > - /* In NMI path, pstore shouldn't block regardless of reason. */ > + /* > + * In case of NMI path, pstore shouldn't be blocked > + * regardless of reason. > + */ > if (in_nmi()) > return true; > > switch (reason) { > /* In panic case, other cpus are stopped by smp_send_stop(). */ > case KMSG_DUMP_PANIC: > - /* Emergency restart shouldn't be blocked. */ > + /* Emergency restart shouldn't be blocked by spin lock. */
by spinning on pstore_info::buf_lock
> case KMSG_DUMP_EMERG: > return true; > default: > return false; > } > } > +EXPORT_SYMBOL_GPL(pstore_cannot_block_path); > > #if IS_ENABLED(CONFIG_PSTORE_DEFLATE_COMPRESS) > static int zbufsize_deflate(size_t size)
Sebastian
| |