Messages in this thread | | | Date | Wed, 22 Apr 2009 17:07:18 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH] Document that wake_up(), complete() and co. imply a full memory barrier |
| |
On 04/22, Ingo Molnar wrote: > > * Oleg Nesterov <oleg@redhat.com> wrote: > > > So, I think that try_to_wake_up() implies that the LOADS after it > > can't be reordered with STOREs before it (and wmb() of course). > > Note that the patch David sent says "full memory barrier", not "full > memory barrier wrt. task->state": > > + (*) wake_up(), try_to_wake_up() and co. imply a full memory barrier. > + > + (*) complete() and co. imply a full memory barrier. > > These statements are not true in that form, as this code does not > imply a full memory barrier. It does imply one on task->state > _alone_ (and a couple of other wq-internal variables it happens to > read for sure). > > But even that one isnt entirely true in the two sub-cases i noted: > the !wq case (which can happen in object state teardown) and the > special ->func handler (which can happen in custom wakeup code a'la > eventpoll). > > So adding a comment that says "this is a full memory barrier" is > simply not true to that extent, and is easily misunderstood. Adding > "this is a fully memory barrier for task->state dependent data flow" > would be more correct. (with a 'as long as wq is not NULL, and as > long as the code using this isnt overriding ->func) > > Agreed?
Yes sure.
Except... not that it really matters, but the reading of ->state is not "special". I mean,
STORE; try_to_wake_up(); LOAD;
in this case try_to_wake_up() acts as a barrier for STORE/LOAD. But probably we should not rely on this. So personally I agree with "for task->state dependent data flow" above.
Oleg.
| |