Re: [RFC][PATCH 2/2] vfs: update overlay inode times on lease_get_mtime()

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

 



On Wed, Jan 3, 2018 at 8:48 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> On Wed, Jan 3, 2018 at 12:54 AM, J . Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
>> On Tue, Jan 02, 2018 at 02:26:49PM +0200, Amir Goldstein wrote:
>>> Instead of looking for all the places that update inode's m_time,
>>> update overlay inode i_mtime just before nfsd needs to read it.
>>>
>>> The non uptodate mtime issue was found and verified with the
>>> nfstest_posix test when run over NFS exported overlayfs:
>>>
>>>  $ nfstest_posix --runtest=link,write
>>>  ...
>>>  FAIL: link - parent directory st_mtime should be updated
>>>  FAIL: write - file st_mtime should be updated
>>
>> Grepping for users of i_mtime in fs/nfsd/....
>>
>> Looks like we might need something similar for fill_pre_wcc() ?  (And
>> maybe nfsd4_block_commit_blocks() ?)
>>
>> I don't think it's necessary for do_nfsd_create(), but it should be
>> harmless if we want to just make it a rule that nfsd shouldn't be
>> accessing i_mtime directly.
>>
>
> All right. I'll do that.
> While at it, do you think it will be fine to call vfs_getattr() from a helper
> nfsd_get_inode_mtime() instead of having to special case overlayfs?
> Or is there some reason why accessing i_mtime is a requirement?
>

Looking closer... in the callers of lease_get_mtime() vfs_getattr() is
already called before accessing i_mtime, but stat->mtime is ignored,
so I will change lease_get_mtime() to only *update* mtime for the
has_lease case and leave it at the stat->mtime value otherwise.

Regarding fill_pre_wcc() it is quite unbalanced that fill_pre_wcc()
reads i_mtime, while fill_post_wcc() reads stat->mtime.
I will change fill_pre_wcc() to use stat->mtime as well.

Regarding nfsd4_block_commit_blocks(), there is no concern with
respect to overlayfs and I have no means to test it, so I rather leave it be.

Patches to follow...

Thanks for your inputs,
Amir.



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux