Re: [RFC][PATCH 1/2] vfs: update overlay inode times on write

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 2, 2018 at 6:51 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> On Tue, Jan 02, 2018 at 02:26:48PM +0200, Amir Goldstein wrote:
>> Currently with overlayfs, the real upper inode's i_mtime is updated on
>> write, but not overlay vfs inode. The vfs inode's i_mtime is used by nfsd
>> to check if nfs client cache is stale, so updating the overlay vfs inode
>> i_mtime on write is required for overlayfs NFS export support.
>>
>> The non uptodate mtime issue was found and verified with the
>> nfstest_posix test when run over NFS exported overlayfs:
>>
>>  $ nfstest_posix --runtest=write
>>  ...
>>  FAIL: write - file st_mtime should be updated
>>
>> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>> ---
>>  fs/inode.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/inode.c b/fs/inode.c
>> index 03102d6ef044..a252256f4e51 100644
>> --- a/fs/inode.c
>> +++ b/fs/inode.c
>> @@ -1567,8 +1567,7 @@ EXPORT_SYMBOL(bmap);
>>  /*
>>   * Update times in overlayed inode from underlying real inode
>>   */
>> -static void update_ovl_inode_times(struct dentry *dentry, struct inode *inode,
>> -                            bool rcu)
>> +static void update_ovl_d_inode_times(struct dentry *dentry, bool rcu)
>>  {
>>       struct dentry *upperdentry;
>>
>> @@ -1585,6 +1584,7 @@ static void update_ovl_inode_times(struct dentry *dentry, struct inode *inode,
>>        * stale mtime/ctime.
>>        */
>>       if (upperdentry) {
>> +             struct inode *inode = d_inode(dentry);
>>               struct inode *realinode = d_inode(upperdentry);
>>
>>               if ((!timespec_equal(&inode->i_mtime, &realinode->i_mtime) ||
>> @@ -1607,7 +1607,8 @@ static int relatime_need_update(const struct path *path, struct inode *inode,
>>       if (!(path->mnt->mnt_flags & MNT_RELATIME))
>>               return 1;
>>
>> -     update_ovl_inode_times(path->dentry, inode, rcu);
>> +     update_ovl_d_inode_times(path->dentry, rcu);
>> +
>
> Hi Amir,
>
> If we update overlay inode mtime, ctime in write path, then do we still
> need this call in relatime_need_update() path? I mean what other path
> is there where real inode mtime/ctime will be out of sync with overlay
> inode mtime/ctime.
>

Specifically, in the mentioned nfstest_posix test, i_mtime was not
updated on write of regular file and i_mtime of parent was not updated on link.
I guess there are more cases where i_mtime is updated not in the
write() path. I can think of punch hole for regular files and even file system
specific ioctls, which overlayfs is not aware of.
To handle those cases, I added the 2nd patch, which is "mtime reader
oriented", just like the relatime_need_update() case.

The problem is that AFAIK there is no simple place where all
i_mtime updates can be intercepted. Perhaps the recent work by Jeff
to sort out i_version readers/writers will provide a better place
to intercept all mtime updates?? At least for underlying filesystems that
support i_version, we could update overlay inode times only if i_version
was updated, but I don't even know what would be the best implementation
choice for relatime_need_update() in that case.

Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux