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... The only 'change' is a cosmetic one: the new routine has somehow to signal to the caller that the lock attempt failed, so it returns an error code EAGAIN to do so. 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. -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@xxxxxxxxxx www.netapp.com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html