Re: [PATCH v4] slow-work: add (module*)work->ops->owner to fix races with module clients

Gregory Haskins
2009-07-07T13:30:26Z

[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 266 bytes --]