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

[Gregory Haskins]

David Howells wrote:
> Gregory Haskins <ghaskins@xxxxxxxxxx> 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: signature.asc
Description: OpenPGP digital signature