Re: [PATCH 1/2] NFS: Clean up nfs_update_request()

[Chuck Lever <chuck.lever@oracle.com>]

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

Trond Myklebust wrote:
> On Wed, 2008-06-18 at 16:40 -0400, Chuck Lever wrote:
>>> No. If the page is being written out, then we _must_ release all locks,
>>> wait on the request, and try again. This is not a change compared to the
>>> previous incarnation.
>> Right, understood... but I think the logic added here is harder to 
>> understand and thus more prone to break when something changes than the 
>> previous incarnation was.
>
> I don't understand: The behaviour and the results are exactly the same
> as before.  All that has happened is that the existing code to update a
> request got shuffled into a separate function. If the request is locked,
> so that we can't update it, we still wait for the request to become
> unlocked, then release it and try the lookup again. There is no new
> logic...

Sorry, I'm not being clear.  I do realize that this patch is intended as 
a clean up and not a behavioral change.

What I'm trying to say is the new function with the EAGAIN return code 
is spaghetti.  Don't get me wrong, I like Italian food.

I do agree that nfs_update_request() in its current state deserves some 
clarification and clean up.  I'm not sure I like the new code any more 
than the old, however.

> One alternative might be to move the code that waits on the lock into
> nfs_try_to_update_request(), but I don't like that idea for two reasons:
> 
>      1. waiting on the lock isn't related to updating the request
>      2. Once the request has been unlocked, you _still_ have to return
>         to nfs_setup_write_request() because the old request might have
>         completed, and been removed from the inode altogether. IOW: you
>         still don't get rid of the need to signal the EAGAIN.

[-- Attachment #2: chuck_lever.vcf --]
[-- Type: text/x-vcard, Size: 259 bytes --]

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
version:2.1
end:vcard