Re: [PATCH 1/6] nfs: "open code" the NFS direct write rescheduler

[Chuck Lever <cel@citi.umich.edu>]

Andrew Morton wrote:
> Chuck Lever <cel@citi.umich.edu> wrote:
>> Andrew Morton wrote:
>>> Chuck Lever <cel@netapp.com> wrote:
>>>> +	 * Prevent I/O completion while we're still rescheduling
>>>> +	 */
>>>> +	dreq->outstanding++;
>>>> +
>>> No locking.
>>>
>>>>  	dreq->count = 0;
>>>> +	list_for_each(pos, &dreq->rewrite_list) {
>>>> +		struct nfs_write_data *data =
>>>> +			list_entry(dreq->rewrite_list.next, struct nfs_write_data, pages);
>>>> +
>>>> +		spin_lock(&dreq->lock);
>>>> +		dreq->outstanding++;
>>>> +		spin_unlock(&dreq->lock);
>>> Locking.
>>>
>>> Deliberate?
>> Yes.  At the top of the loop, there is no outstanding I/O, so no locking 
>> is needed while updating "outstanding."  Inside the loop, we've 
>> dispatched some I/O against "dreq" so locking is needed to ensure 
>> outstanding is updated properly.
>>
> 
> OK.  Well if I asked, then others will wonder about it.  A comment would
> cure that problem ;)

Or, I could code defensively and just add locking there too, even though 
it is not needed.  This path is not a performance path, and things could 
get changed at some point so my assumption that is no longer valid.

-- 
corporate:	cel at netapp dot com
personal:	chucklever at bigfoot dot com