Messages in this thread | | | Subject | Re: 2.6.24-rc2-mm1: kcryptd vs lockdep | From | Johannes Berg <> | Date | Wed, 21 Nov 2007 17:06:13 +0100 |
| |
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 [unhandled content-type:application/pgp-signature] | |