Hey Neil,
Attached is a patch that fixes some potential SMP races
in the lockd code that were identified by the SLEEP_ON_BKLCHECK
that was (at one time) in the -mm tree...
Signed-Off-By: Steve Dickson <[email protected]>
P? m? , 04/10/2004 klokka 18:24, skreiv Steve Dickson:
> Hey Neil,
Hey! This is the client side NLM code... 8-)
> clear_thread_flag(TIF_SIGPENDING);
> - interruptible_sleep_on_timeout(&lockd_exit, HZ);
> - if (nlmsvc_pid) {
> + set_current_state(TASK_UNINTERRUPTIBLE);
Nope. Those clearly are not the same.
Note that you probably also want to move the call to
set_current_state(TASK_INTERRUPTIBLE) inside the loop. In that case you
can also remove the call to set_current_state(TASK_RUNNING) ('cos
schedule_timeout() will do that for you).
Also, why aren't you using the more standard DECLARE_WAITQUEUE(__wait)?
Cheers,
Trond
P? m? , 04/10/2004 klokka 18:24, skreiv Steve Dickson:
> Hey Neil,
>
> Attached is a patch that fixes some potential SMP races
> in the lockd code that were identified by the SLEEP_ON_BKLCHECK
> that was (at one time) in the -mm tree...
Just for the record: the "SMP race condition" argument given here is
completely bogus. sleep_on_* is quite safe to use when the SMP races are
being handled using the BKL, as is the case here.
That said, I agree that the patch is of interest given the long term
goal of removing the BKL completely. Perhaps you could therefore also
amend your changelog entry text to reflect this motive?
Cheers,
Trond
On Mon, 2004-10-04 at 20:01, Trond Myklebust wrote:
> På må , 04/10/2004 klokka 18:24, skreiv Steve Dickson:
> > Hey Neil,
> >
> > Attached is a patch that fixes some potential SMP races
> > in the lockd code that were identified by the SLEEP_ON_BKLCHECK
> > that was (at one time) in the -mm tree...
>
> Just for the record: the "SMP race condition" argument given here is
> completely bogus. sleep_on_* is quite safe to use when the SMP races are
> being handled using the BKL, as is the case here.
actually this triggered because there was NO bkl... if you hold the bkl
the warning doesn't trigger.....
P? m? , 04/10/2004 klokka 20:13, skreiv Arjan van de Ven:
> actually this triggered because there was NO bkl... if you hold the bkl
> the warning doesn't trigger.....
Then the fix is downright wrong.
We *must* be holding the BKL upon entry to nlmclnt_lock(). All sorts of
other things depend on it.
Cheers,
Trond
Trond Myklebust wrote:
>P? m? , 04/10/2004 klokka 18:24, skreiv Steve Dickson:
>
>
>
>>Hey Neil,
>>
>>
>
>Hey! This is the client side NLM code... 8-)
>
>
Sorry buddy.... I'm having one of those days!!!! :-\
>Note that you probably also want to move the call to
>set_current_state(TASK_INTERRUPTIBLE) inside the loop. In that case you
>can also remove the call to set_current_state(TASK_RUNNING) ('cos
>schedule_timeout() will do that for you).
>
>
>
Ok...
>Also, why aren't you using the more standard DECLARE_WAITQUEUE(__wait)?
>
>
I guess I didn't realize that would be a better way to do it... I'll
look into to...
thanks,
SteveD.