Re: 2.6.24-rc2-mm1: kcryptd vs lockdep

[Johannes Berg]

Hi,

> > Ok, then I have question: Is the following pseudocode correct
> > (and problem is in lock validation which checks something
> > already initialized for another queue) or reusing work_struct
> > is not permitted from inside called work function ?
> >
> > (Note comment in code "It is permissible to free the struct
> > work_struct from inside the function that is called from it".)
> >
> > struct work_struct work;
> > struct workqueue_struct *a, *b;
> >
> > do_b(*work)
> > {
> >         /* do something else */
> > }
> >
> > do_a(*work)
> > {
> >         /* do something */
> >         INIT_WORK(&work, do_b);
> >         queue_work(b, &work);
> > }
> >
> >
> > INIT_WORK(&work, do_a);
> > queue_work(a, &work);
> 
> (just in case, in that particular case PREPARE_WORK() should be used)
> 
> INIT_WORK(w) can be used if we know that "w" is not pending, and nobody
> else can write to this work (say, queue_work(w) or cancel_work_sync(w)).
> So currently the code above should work correctly.
> 
> However, I'd say it is not correct, INIT_WORK() can throw out some debug
> info for example, or the implementation could be changed.
> 
> I'm not sure about CONFIG_LOCKDEP (Johannes cc'ed). INIT_WORK() does
> lockdep_init_map(->lockdep_map) but run_workqueue() has a local copy,
> looks ok.

We explicitly need to use a copy of the lockdep_map for "locking" the
work struct as per the quoted comment. So as far as I can tell, what
INIT_WORK() is doing here is changing an at that point unused copy of
the lockdep map so I think it should be fine. Not sure about the other
fine points nor why you'd want this though :)

johannes

Attachment: signature.asc
Description: This is a digitally signed message part