| Message ID | 1414502171-10319-4-git-send-email-eguan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 28, 2014 at 09:16:10PM +0800, Eryu Guan wrote: > >From nfs(5) we can know that atime related mount options have no effect > on NFS mounts, so add _require_atime() helper to skip atime tests on NFS I' like to use this opportunity to start a discussion on NFS atime handlign again, which I think is broken. I think relatime is perfectly fine default semantics for NFS, and not supporting it can cause all kidns of application breakage. Supporting normal atime semantics when explicity requested is also something NFS shouldn't sneak out of. > Also change the way how _require_relatime() mount $SCRATCH_DEV, use > _scratch_mount helper so it's mounted with selinux context, to avoid > "same superblock, different selinux context" failure. Can you please split this part into a separate patch? -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 30, 2014 at 02:03:06AM -0700, Christoph Hellwig wrote: > On Tue, Oct 28, 2014 at 09:16:10PM +0800, Eryu Guan wrote: > > >From nfs(5) we can know that atime related mount options have no effect > > on NFS mounts, so add _require_atime() helper to skip atime tests on NFS > > I' like to use this opportunity to start a discussion on NFS atime > handlign again, which I think is broken. I think relatime is perfectly > fine default semantics for NFS, and not supporting it can cause all > kidns of application breakage. Supporting normal atime semantics when > explicity requested is also something NFS shouldn't sneak out of. > > > Also change the way how _require_relatime() mount $SCRATCH_DEV, use > > _scratch_mount helper so it's mounted with selinux context, to avoid > > "same superblock, different selinux context" failure. > > Can you please split this part into a separate patch? > Sure, will do in v2. Thanks, Eryu -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 30, 2014 at 11:03 AM, Christoph Hellwig <hch@infradead.org> wrote: > On Tue, Oct 28, 2014 at 09:16:10PM +0800, Eryu Guan wrote: >> >From nfs(5) we can know that atime related mount options have no effect >> on NFS mounts, so add _require_atime() helper to skip atime tests on NFS > > I' like to use this opportunity to start a discussion on NFS atime > handlign again, which I think is broken. I think relatime is perfectly > fine default semantics for NFS, and not supporting it can cause all > kidns of application breakage. Supporting normal atime semantics when > explicity requested is also something NFS shouldn't sneak out of. If there is no read on the wire, then there is no way to update the atime without doing an explicit SETATTR. Courtesy of POSIX filesystem semantics on the server, that means we get a bonus change attribute and ctime update (no extra charge). Unless there are new suggestions for how to solve the atime issue that do not involve introducing this or similar regressions, then the standing NACK applies. Cheers Trond
On 10/31/2014 01:31 PM, Trond Myklebust wrote: <> > If there is no read on the wire, then there is no way to update the > atime without doing an explicit SETATTR. Courtesy of POSIX filesystem > semantics on the server, that means we get a bonus change attribute > and ctime update (no extra charge). > > Unless there are new suggestions for how to solve the atime issue that > do not involve introducing this or similar regressions, then the > standing NACK applies. > Say the user asks, realy (realy^3) nicely, can we instead of sending SETATTR (BAD) send in its place an async READ of say one byte (or one word) It will do what we want. Just need to collect the updates and atime vs rel-atime correctly, and to not hurt performance as well. (Like only send when no real READS went through) > Cheers > Trond > Cheers Boaz -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 31, 2014 at 01:31:36PM +0200, Trond Myklebust wrote: > If there is no read on the wire, then there is no way to update the > atime without doing an explicit SETATTR. Courtesy of POSIX filesystem > semantics on the server, that means we get a bonus change attribute > and ctime update (no extra charge). > > Unless there are new suggestions for how to solve the atime issue that > do not involve introducing this or similar regressions, then the > standing NACK applies. I think various network filesystems are fairly lax on updating the atime on the server. What xfstests generic/192 tests is that we don't lose an atime update after an unmount/remount. I think we should be able to expect local atime updates, and updates to the server on unmount. Or at least claim that the filesystem is mounted by noatime if it is so that users (including xfstests) notice, and can enable strict atime mode if really needed, including the above mentioned drawbacks. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/common/rc b/common/rc index 42f00cb..b50577f 100644 --- a/common/rc +++ b/common/rc @@ -2379,10 +2379,17 @@ _verify_reflink() || echo "$1 and $2 are not reflinks: different extents" } +_require_atime() +{ + if [ "$FSTYP" == "nfs" ]; then + _notrun "atime related mount options have no effect on NFS" + fi +} + _require_relatime() { _scratch_mkfs > /dev/null 2>&1 - _mount -t $FSTYP -o relatime $SCRATCH_DEV $SCRATCH_MNT || \ + _scratch_mount -o relatime || \ _notrun "relatime not supported by the current kernel" _scratch_unmount } diff --git a/tests/generic/003 b/tests/generic/003 index 83d6f90..7ffd09a 100755 --- a/tests/generic/003 +++ b/tests/generic/003 @@ -47,6 +47,7 @@ _cleanup() _supported_fs generic _supported_os Linux _require_scratch +_require_atime _require_relatime rm -f $seqres.full diff --git a/tests/generic/192 b/tests/generic/192 index b2da358..5b6cfbc 100755 --- a/tests/generic/192 +++ b/tests/generic/192 @@ -54,6 +54,7 @@ is_noatime_set() { _supported_fs generic _supported_os Linux _require_test +_require_atime #delay=150 #delay=75 #delay=60
From nfs(5) we can know that atime related mount options have no effect on NFS mounts, so add _require_atime() helper to skip atime tests on NFS Also change the way how _require_relatime() mount $SCRATCH_DEV, use _scratch_mount helper so it's mounted with selinux context, to avoid "same superblock, different selinux context" failure. Signed-off-by: Eryu Guan <eguan@redhat.com> --- common/rc | 9 ++++++++- tests/generic/003 | 1 + tests/generic/192 | 1 + 3 files changed, 10 insertions(+), 1 deletion(-)