On Thu, Feb 08, 2024 at 01:56:10AM -0800, Paul E. McKenney wrote:
> On Thu, Feb 08, 2024 at 03:10:32AM +0100, Frederic Weisbecker wrote:
> This ordering is not needed. The lock orders addition to this
> list against removal from tasklist. If we hold this lock, either
> the task is already on this list or our holding this lock prevents
> it from removing itself from the tasklist.
>
> We have already scanned the task list, and we have already done
> whatever update we are worried about.
>
> So, if the task was on the tasklist when we scanned, well and
> good. If the task was created after we scanned the tasklist,
> then it cannot possibly access whatever we removed.
>
> But please double-check!!!
Heh, right, another new pattern for me to discover :-/
C r-LOCK
{
}
P0(spinlock_t *LOCK, int *X, int *Y)
{
int r1;
int r2;
r1 = READ_ONCE(*X);
spin_lock(LOCK);
r2 = READ_ONCE(*Y);
spin_unlock(LOCK);
}
P1(spinlock_t *LOCK, int *X, int *Y)
{
spin_lock(LOCK);
WRITE_ONCE(*Y, 1);
spin_unlock(LOCK);
WRITE_ONCE(*X, 1);
}
exists (0:r1=1 /\ 0:r2=0) (* never *)
>
> > > synchronize_rcu_tasks() do_exit()
> > > ---------------------- ---------
> > > //for_each_process_thread()
> > > READ tasklist WRITE rtpcp->rtp_exit_list
> > > LOCK rtpcp->lock UNLOCK rtpcp->lock
> > > smp_mb__after_unlock_lock() WRITE tasklist //unhash_process()
> > > READ rtpcp->rtp_exit_list
> > >
> > > Does this work? Hmm, I'll play with litmus once I have a fresh brain...
>
> First, thank you very much for the review!!!
>
> > ie: does smp_mb__after_unlock_lock() order only what precedes the UNLOCK with
> > the UNLOCK itself? (but then the UNLOCK itself can be reordered with anything
> > that follows)? Or does it also order what follows the UNLOCK with the UNLOCK
> > itself? If both, then it looks ok, otherwise...
>
> If you have this:
>
> earlier_accesses();
> spin_lock(...);
> ill_considered_memory_accesses();
> smp_mb__after_unlock_lock();
> later_accesses();
>
> Then earlier_accesses() will be ordered against later_accesses(), but
> ill_considered_memory_accesses() won't necessarily be ordered. Also,
> any accesses before any prior release of that same lock will be ordered
> against later_accesses().
>
> (In real life, ill_considered_memory_accesses() will be fully ordered
> against either spin_lock() on the one hand or smp_mb__after_unlock_lock()
> on the other, with x86 doing the first and PowerPC doing the second.
> So please try to avoid any ill_considered_memory_accesses().)
Thanks a lot for that explanation!
>
> > Also on the other end, does LOCK/smp_mb__after_unlock_lock() order against what
> > precedes the LOCK? That also is necessary for the above to work.
>
> It looks like an smp_mb__after_spinlock() would also be needed, for
> example, on ARMv8.
>
> > Of course by the time I'm writing this email, litmus would have told me
> > already...
>
> ;-) ;-) ;-)
>
> But I believe that simple locking covers this case. Famous last words...
Indeed, looks right!
Thanks!
> Thanx, Paul
