Messages in this thread | | | Date | Tue, 07 Jul 2009 09:29:52 -0400 | From | Gregory Haskins <> | Subject | Re: [PATCH v4] slow-work: add (module*)work->ops->owner to fix races with module clients |
| |
David Howells wrote: > Gregory Haskins <ghaskins@novell.com> wrote: > > >> + struct module *owner = work->ops->owner; >> + >> + work->ops->put_ref(work); >> + module_put(owner); >> > > Hmmm... There needs to be an smp_mb() between the read of the module owner > and the call to put_ref(), lest the CPU reorder things... However, if > put_ref(), say, calls atomic_dec_and_test(), then inserting one here would be > superfluous. > > I think documenting this will be enough - perhaps something like: > > (*) Release a reference on an item: > > void (*put_ref)(struct slow_work *work); > > This allows the thread pool to unpin an item by releasing the reference on > it. The thread pool will not touch the item again once this has been > called. > > This function must interpolate a general SMP memory barrier before freeing > or re-using the work struct as the caller may have read the module > pointer. Implying a barrier with something like atomic_dec_and_test() is > sufficient. > > Do you agree? > > David > Hi David, I agree, and think that looks good. If you want to just fold that into the patch or something, feel free. Conversely, if you would like me to submit a new patch, let me know.
Regards, -Greg
[unhandled content-type:application/pgp-signature] | |