lkml.org 
[lkml]   [2020]   [Oct]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v7 6/6] rcu/segcblist: Add additional comments to explain smp_mb()
On Wed, Oct 21, 2020 at 11:57:04AM -0700, Joel Fernandes wrote:
> On Mon, Oct 19, 2020 at 5:37 AM Frederic Weisbecker <frederic@kernel.org> wrote:
> > Now, reading the documentation of rcu_barrier() (thanks to you!):
> >
> > Pseudo-code using rcu_barrier() is as follows:
> >
> > 1. Prevent any new RCU callbacks from being posted.
> > 2. Execute rcu_barrier().
> > 3. Allow the module to be unloaded.
> >
>
> Basically, you are saying that if all CPUs agree that len == 0
> henceforth (through other memory barriers), then callback enqueuing
> does not need a memory barrier before setting length to 0.

I think setting length to 0 isn't much an issue. At worst we send a useless
IPI and queue a needless callback. But incrementing from 0 to 1 is precisely
what we don't want to miss.

> I think that makes sense but is it worth removing the memory barrier
> before WRITE(len, 1) and hoping after #1, the caller would have
> ensured things are fine? Also I am not sure if the above is the only
> usecase for rcu_barrier().

I'm not sure either. Also I need to check your scenario again.

> > cancel_work_sync() also seem to really sync as well. I'm less sure about del_timer_sync().
> >
> > Say we have:
> >
> > expire_timers (CPU 0) CPU 1
> > ------------- -----------
> > detach_timer(timer)
> > raw_spin_unlock(&base->lock);
> > call_timer_fn(timer, fn, baseclk);
> > -> enqueue callback
> > //would need at least smp_wmb() here

Aah, my bad, the smp_mb() after inc_len does that.

> > base->running_timer = NULL;
> >
> > del_timer_sync() {
> > raw_spin_lock(&base->lock);
> > if (base->running_timer != timer)
> > ret = detach_if_pending(timer, base, true);
> > if (!timer_pending())
> > return 0;
> > raw_spin_unlock(&base->lock);
> > }
> > //would need at least smp_rmb() here

And rcu_seq_start() implies that, although I'm not sure that's what was intended.
So we are good.

>
> Regarding "would need at least smp_rmb.." :
> But the rcu_barrier() has the control dependency we discussed in last
> emails, between READ(len) and whatever follows the rcu_barrier().
> That itself will provide the ordering right?

I'm not sure that was enough. The len itself has to be synchronized against
whatever callback enqueuer that got stopped.

> I could be missing something too :-/. But I'll include this patch in
> my next posting anyway and let us also maybe see if Paul disagrees.

Ok.

Thanks!

\
 
 \ /
  Last update: 2020-10-21 23:17    [W:0.065 / U:2.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site