On Thu, Aug 21, 2014 at 2:15 PM, David Jeffery <djeffery@xxxxxxxxxx> wrote: > On 08/21/2014 11:50 AM, Trond Myklebust wrote: >> On Thu, Aug 21, 2014 at 11:40 AM, David Jeffery <djeffery@xxxxxxxxxx> wrote: >>> On 08/20/2014 08:28 PM, Trond Myklebust wrote: >>>> >>>> What guarantees that this does not lead to silent corruption of the file >>>> if there are outstanding write requests? >>>> >>> >>> Do you have a particular scenario in mind you are concerned about? >>> >>> Right before the code the patch modifies, nfs_sync_mapping() is called. >>> Any writes started before the unlock operation began have already been >>> flushed, so we shouldn't have a corruption case of writes from before >>> the unlock began being sent after the unlock is complete. >>> >>> Are you concerned about some other nfs4 writes being started while we >>> initially waited on the counter? Such a write racing with the unlock >> >> No. I'm worried about the writes that have been started, but which are >> now completing in the background while the lock is being freed. >> >>> going ahead instead of erroring out could initially fail from a wrong >>> state ID, but should retry with the new state. Is there something I've >>> overlooked? >> >> Loss of lock atomicity due to the fact that the writes are completing >> after the lock was released. >> > > I don't think my patches break lock atomicity, unless I've completely > misunderstood nfs_sync_mapping()'s behavior. > > The background writes should have already been waited for by > nfs_sync_mapping() at the beginning of do_unlk(). nfs_sync_mapping() > would call nfs_wb_all() which calls sync_inode() with WB_SYNC_ALL, so > it's going to push out any dirty data and wait for any writes to > complete. Only after the writes are complete do we go on to call > nfs_iocounter_wait(). What if nfs_wb_all() is interrupted? Currently, that doesn't matter precisely because of the call to nfs_iocounter_wait. > If there is a write causing us to want to wait in nfs_iocounter_wait() > and which my patches no longer wait for, the write was done after > nfs_sync_mapping() started, which means the write occurred after the > unlock-initiating call began. Such a write should have no atomicity > guarantee with holding the lock, and may complete before or after the > lock is released. The writes which require atomicity are already > completed before getting to the call of nfs_iocounter_wait() and the > code my patch changes. See above. -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@xxxxxxxxxxxxxxx -- 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
