On Thu, 07 Feb 2008 10:16:56 -0500
Trond Myklebust <[email protected]> wrote:
>
> On Thu, 2008-02-07 at 06:37 -0500, Jeff Layton wrote:
> > On Wed, 06 Feb 2008 18:01:16 -0500
> > Trond Myklebust <[email protected]> wrote:
> >
> > >
> > > On Wed, 2008-02-06 at 14:09 -0500, Jeff Layton wrote:
> > > > On Wed, 06 Feb 2008 13:52:34 -0500
> > > > Trond Myklebust <[email protected]> wrote:
> > > >
> > > > >
> > > > > On Wed, 2008-02-06 at 13:47 -0500, Jeff Layton wrote:
> > > > > > There's no guarantee that kthread_stop() won't wake up lockd before
> > > > > > schedule_timeout() gets called, but after the last check for
> > > > > > kthread_should_stop().
> > > > >
> > > > > Doesn't the BKL pretty much eliminate this race? (assuming you
> > > > > transform that call to 'if (!kthread_should_stop())
> > > > > schedule_timeout();')
> > > > >
> > > > > Trond
> > > > >
> > > >
> > > > I don't think so. That would require that lockd_down is always called
> > > > with the BKL held, and I don't think it is, is it?
> > >
> > > Nothing stops you from grabbing the BKL inside lockd_down, though :-)
> > >
> >
> > True, but what's worse -- keeping signaling in the codepath or
> > increasing reliance on the BKL? :-)
> >
> > I think in the near term, you're probably right that taking the BKL in
> > lockd_down is the best scheme to prevent these races. I think the best
> > long term solution though is to add a way for kthread_stop to signal
> > the task before calling wait_for_completion.
>
> Doh! Where are my brains?
>
> You don't need the BKL either here. Just make sure that the test for
> 'kthread_should_stop()' occurs after the call to set_current_state().
> wake_up_process() will reset the state to TASK_RUNNING, and so the
> process is kept on the run queue when schedule_timeout() is called.
>
> > I'll keep plugging away on this for now. If I can get a patch
> > upstream to add a kthread_stop_sig() call then we might be able to
> > avoid relying on the BKL here...
>
> No need for signals. We can get rid of them altogether (and good
> riddance!).
>
Good catch! That makes total sense and does seem to be the best
solution for this. I'm working on a respin now and should hopefully
have something posted in a day or so.
Thanks for the input!
--
Jeff Layton <[email protected]>