Re: [PATCH] common/rc: add the function _require_noatime

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

 




----- Original Message -----
> From: "Steve French" <smfrench@xxxxxxxxx>
> To: "Xiaoli Feng" <xifeng@xxxxxxxxxx>
> Cc: "Eryu Guan" <guaneryu@xxxxxxxxx>, "CIFS" <linux-cifs@xxxxxxxxxxxxxxx>, fstests@xxxxxxxxxxxxxxx
> Sent: Tuesday, May 22, 2018 7:54:45 AM
> Subject: Re: [PATCH] common/rc: add the function _require_noatime
> 
> Should this be fixed by changing cifs.ko to accept the mount parm but ignore
> it?

Yes, it should. For cifs, atime/diratime/relatime/norelatime/nostrictatime can be mounted without effect.
But mount failed with noatime/nodiatime/strictatime. Seems they don't maintain consistency. I will file a bug
about it. 

Thanks Steve & Eryu.

> 
> If this test works on NFS (the noatime mount option has no meaning for
> NFS apparently) we should do the same
> 
> Quoting the NFS man page:
> 
>        In particular, the atime/noatime, diratime/nodiratime,
>        relatime/norela‐
>        time, and strictatime/nostrictatime mount options have no effect on
>        NFS
>        mounts.
> 
> On Mon, May 21, 2018 at 3:50 AM, Xiaoli Feng <xifeng@xxxxxxxxxx> wrote:
> >
> > [add linux-cifs@xxxxxxxxxxxxxxx to cc list]
> >
> > ----- Forwarded Message -----
> > From: "Eryu Guan" <guaneryu@xxxxxxxxx>
> > To: "XiaoLi Feng" <xifeng@xxxxxxxxxx>
> > Cc: fstests@xxxxxxxxxxxxxxx
> > Sent: Monday, May 21, 2018 3:50:27 PM
> > Subject: Re: [PATCH] common/rc: add the function _require_noatime
> >
> > [add linux-cifs@xxxxxxxxxxxxxxx to cc list]
> >
> > On Fri, May 18, 2018 at 07:10:29PM +0800, Xiaoli Feng wrote:
> >> From: xiaoli feng <xifeng@xxxxxxxxxx>
> >>
> >> In the generic/120, it will make the test not-pass if the filesystem
> >> mounts failed with noatime. Now change this result to norun. The
> >> filesystem cifs doesn't support noatime. Just make the test norun
> >> until it supports noatime.
> >>
> >> Signed-off-by: xiaoli feng <xifeng@xxxxxxxxxx>
> >> ---
> >>  common/rc         | 8 +++++++-
> >>  tests/generic/120 | 7 +------
> >>  2 files changed, 8 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/common/rc b/common/rc
> >> index ffe5323..9c45f1b 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -3244,7 +3244,13 @@ _require_atime()
> >>       _exclude_scratch_mount_option "noatime"
> >>       if [ "$FSTYP" == "nfs" ]; then
> >>               _notrun "atime related mount options have no effect on NFS"
> >> -     fi
> >
> > I'm not sure what's the expected behavior from CIFS on atime/noatime, so
> > I added linux-cifs list for input.
> >
> > If CIFS behaves similarly to NFS, looks like that you could simply add
> > another check for cifs in _require_atime(), as what we already do for
> > nfs, so all tests that _require_atime() will _notrun on cifs.
> >
> >> +}
> >> +
> >> +_require_noatime()
> >> +{
> >> +     _exclude_scratch_mount_option "atime"
> >> +      _try_scratch_mount -o noatime || \
> >> +                _notrun "noatime not supported by the current tested
> >> filesystem"
> >>  }
> >>
> >>  _require_relatime()
> >> diff --git a/tests/generic/120 b/tests/generic/120
> >> index 1180c10..ddd61b3 100755
> >> --- a/tests/generic/120
> >> +++ b/tests/generic/120
> >> @@ -60,12 +60,7 @@ _compare_access_times()
> >>
> >>  }
> >>
> >> -if ! _try_scratch_mount "-o noatime" >$tmp.out 2>&1
> >> -then
> >> -    cat $tmp.out
> >> -    echo "!!! mount failed"
> >> -    exit
> >> -fi
> >> +_require_noatime
> >
> > Anyway, failing the test when "noatime" mount fails is one of the
> > purposes of the test, and it shouldn't be removed, as we've already made
> > sure current FSTYP supports atime/noatime (by _require rules), so a
> > noatime mount failure indicates a bug in the filesystem.
> >
> > Thanks,
> > Eryu
> >
> >>
> >>  #executable file
> >>  echo "*** copying file ***"
> >> --
> >> 1.8.3.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe fstests" in
> >> the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> --
> Thanks,
> 
> Steve
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux