Messages in this thread | | | Date | Tue, 7 Mar 2017 22:27:40 +0800 | From | Boqun Feng <> | Subject | Re: rcu: WARNING in rcu_seq_end |
| |
On Tue, Mar 07, 2017 at 08:05:19AM +0100, Dmitry Vyukov wrote: [...] > >> > >> What is that mutex? And what locks/unlocks provide synchronization? I > >> see that one uses exp_mutex and another -- exp_wake_mutex. > > > > Both of them. > > > > ->exp_mutex is acquired by the task requesting the grace period, and > > the counter's first increment is done by that task under that mutex. > > This task then schedules a workqueue, which drives forward the grace > > period. Upon grace-period completion, the workqueue handler does the > > second increment (the one that your patch addressed). The workqueue > > handler then acquires ->exp_wake_mutex and wakes the task that holds > > ->exp_mutex (along with all other tasks waiting for this grace period), > > and that task releases ->exp_mutex, which allows the next grace period to > > start (and the first increment for that next grace period to be carried > > out under that lock). The workqueue handler releases ->exp_wake_mutex > > after finishing its wakeups. > > > Then we need the following for the case when task requesting the grace > period does not block, right? >
Won't be necessary I think, as the smp_mb() in rcu_seq_end() and the smp_mb__before_atomic() in sync_exp_work_done() already provide the required ordering, no?
Regards, Boqun
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index d80c2587bed8..aa7ba83f6a56 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -3534,7 +3534,7 @@ static void rcu_seq_start(unsigned long *sp) > static void rcu_seq_end(unsigned long *sp) > { > smp_mb(); /* Ensure update-side operation before counter increment. */ > - WRITE_ONCE(*sp, *sp + 1); > + smp_store_release(sp, *sp + 1); > WARN_ON_ONCE(*sp & 0x1); > } > > @@ -3554,7 +3554,7 @@ static unsigned long rcu_seq_snap(unsigned long *sp) > */ > static bool rcu_seq_done(unsigned long *sp, unsigned long s) > { > - return ULONG_CMP_GE(READ_ONCE(*sp), s); > + return ULONG_CMP_GE(smp_load_acquire(sp), s); > > } [unhandled content-type:application/pgp-signature] | |