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);
>
> }
signature.asc
Description: PGP signature
