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

Chuck Lever
2006-05-19T18:56:23Z

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