Re: [PATCH V2] libcfs: Fix a sleep-in-atomic bug in cfs_wi_exit
On Wed, May 31, 2017 at 04:00:41PM +0800, Jia-Ju Bai wrote: > The driver may sleep under a spin lock, and the function call path is: > cfs_wi_exit (acquire the lock by spin_lock) > LASSERT > lbug_with_loc > libcfs_debug_dumplog > schedule and kthread_run --> may sleep > > To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock. > > Signed-off-by: Jia-Ju Bai > --- > drivers/staging/lustre/lnet/libcfs/workitem.c | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) What changed from v1? You have to always tell us that. Please redo all of these, if they are still valid patches, and send them as a patch series, so I know what order they should be applied in. thanks, greg k-h
Re: [PATCH V2] libcfs: Fix a sleep-in-atomic bug in cfs_wi_exit
Hello! On May 31, 2017, at 4:00 AM, Jia-Ju Bai wrote: > The driver may sleep under a spin lock, and the function call path is: > cfs_wi_exit (acquire the lock by spin_lock) > LASSERT >lbug_with_loc > libcfs_debug_dumplog >schedule and kthread_run --> may sleep > > To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock. > > Signed-off-by: Jia-Ju Bai > --- > drivers/staging/lustre/lnet/libcfs/workitem.c | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/lustre/lnet/libcfs/workitem.c > b/drivers/staging/lustre/lnet/libcfs/workitem.c > index dbc2a9b..928d06d 100644 > --- a/drivers/staging/lustre/lnet/libcfs/workitem.c > +++ b/drivers/staging/lustre/lnet/libcfs/workitem.c > @@ -111,22 +111,23 @@ struct cfs_wi_sched { > { > LASSERT(!in_interrupt()); /* because we use plain spinlock */ > LASSERT(!sched->ws_stopping); > + LASSERT(wi->wi_running); > + if (wi->wi_scheduled) { > + LASSERT(!list_empty(&wi->wi_list)); > + LASSERT(sched->ws_nscheduled > 0); > + } Similarly here and in all other patches about LASSERT calls under spinlocks() from you, just think of them as a panic() call, no operations are expected to continue after it triggers. Thanks. > > spin_lock(&sched->ws_lock); > > - LASSERT(wi->wi_running); > if (wi->wi_scheduled) { /* cancel pending schedules */ > - LASSERT(!list_empty(&wi->wi_list)); > list_del_init(&wi->wi_list); > - > - LASSERT(sched->ws_nscheduled > 0); > sched->ws_nscheduled--; > } > > - LASSERT(list_empty(&wi->wi_list)); > - > wi->wi_scheduled = 1; /* LBUG future schedule attempts */ > spin_unlock(&sched->ws_lock); > + > + LASSERT(list_empty(&wi->wi_list)); > } > EXPORT_SYMBOL(cfs_wi_exit); > > -- > 1.7.9.5 >
[PATCH V2] libcfs: Fix a sleep-in-atomic bug in cfs_wi_exit
The driver may sleep under a spin lock, and the function call path is: cfs_wi_exit (acquire the lock by spin_lock) LASSERT lbug_with_loc libcfs_debug_dumplog schedule and kthread_run --> may sleep To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock. Signed-off-by: Jia-Ju Bai --- drivers/staging/lustre/lnet/libcfs/workitem.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/workitem.c b/drivers/staging/lustre/lnet/libcfs/workitem.c index dbc2a9b..928d06d 100644 --- a/drivers/staging/lustre/lnet/libcfs/workitem.c +++ b/drivers/staging/lustre/lnet/libcfs/workitem.c @@ -111,22 +111,23 @@ struct cfs_wi_sched { { LASSERT(!in_interrupt()); /* because we use plain spinlock */ LASSERT(!sched->ws_stopping); + LASSERT(wi->wi_running); + if (wi->wi_scheduled) { + LASSERT(!list_empty(&wi->wi_list)); + LASSERT(sched->ws_nscheduled > 0); + } spin_lock(&sched->ws_lock); - LASSERT(wi->wi_running); if (wi->wi_scheduled) { /* cancel pending schedules */ - LASSERT(!list_empty(&wi->wi_list)); list_del_init(&wi->wi_list); - - LASSERT(sched->ws_nscheduled > 0); sched->ws_nscheduled--; } - LASSERT(list_empty(&wi->wi_list)); - wi->wi_scheduled = 1; /* LBUG future schedule attempts */ spin_unlock(&sched->ws_lock); + + LASSERT(list_empty(&wi->wi_list)); } EXPORT_SYMBOL(cfs_wi_exit); -- 1.7.9.5