* Fwd: [PATCH] common/rc: add the function _require_noatime
2018-05-21 23:54 ` Steve French
@ 2018-05-21 23:55 ` Steve French
2018-05-22 2:21 ` Eryu Guan
2018-05-22 2:54 ` Xiaoli Feng
2 siblings, 0 replies; 7+ messages in thread
From: Steve French @ 2018-05-21 23:55 UTC (permalink / raw)
To: linux-fsdevel
Adding fsdevel for this question
---------- Forwarded message ----------
From: Steve French <smfrench@gmail.com>
Date: Mon, May 21, 2018 at 6:54 PM
Subject: Re: [PATCH] common/rc: add the function _require_noatime
To: Xiaoli Feng <xifeng@redhat.com>
Cc: Eryu Guan <guaneryu@gmail.com>, CIFS <linux-cifs@vger.kernel.org>,
fstests@vger.kernel.org
Should this be fixed by changing cifs.ko to accept the mount parm but ignore it?
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@redhat.com> wrote:
>
> [add linux-cifs@vger.kernel.org to cc list]
>
> ----- Forwarded Message -----
> From: "Eryu Guan" <guaneryu@gmail.com>
> To: "XiaoLi Feng" <xifeng@redhat.com>
> Cc: fstests@vger.kernel.org
> Sent: Monday, May 21, 2018 3:50:27 PM
> Subject: Re: [PATCH] common/rc: add the function _require_noatime
>
> [add linux-cifs@vger.kernel.org to cc list]
>
> On Fri, May 18, 2018 at 07:10:29PM +0800, Xiaoli Feng wrote:
>> From: xiaoli feng <xifeng@redhat.com>
>>
>> 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@redhat.com>
>> ---
>> 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@vger.kernel.org
>> 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@vger.kernel.org
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
Steve
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] common/rc: add the function _require_noatime
2018-05-21 23:54 ` Steve French
2018-05-21 23:55 ` Fwd: " Steve French
@ 2018-05-22 2:21 ` Eryu Guan
2018-05-22 2:54 ` Xiaoli Feng
2 siblings, 0 replies; 7+ messages in thread
From: Eryu Guan @ 2018-05-22 2:21 UTC (permalink / raw)
To: Steve French; +Cc: Xiaoli Feng, CIFS, fstests
On Mon, May 21, 2018 at 06:54:45PM -0500, Steve French wrote:
> Should this be fixed by changing cifs.ko to accept the mount parm but ignore it?
>
> If this test works on NFS (the noatime mount option has no meaning for
> NFS apparently) we should do the same
NFS works because tests that call _require_atime will _notrun on NFS.
>
> 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.
Yeah, the NFS check in _require_atime was based on this description. If
atime related mount options have no effect on CIFS too, we could simply,
from fstests' prospect of view, _notrun for CIFS in _require_atime too.
Thanks,
Eryu
>
> On Mon, May 21, 2018 at 3:50 AM, Xiaoli Feng <xifeng@redhat.com> wrote:
> >
> > [add linux-cifs@vger.kernel.org to cc list]
> >
> > ----- Forwarded Message -----
> > From: "Eryu Guan" <guaneryu@gmail.com>
> > To: "XiaoLi Feng" <xifeng@redhat.com>
> > Cc: fstests@vger.kernel.org
> > Sent: Monday, May 21, 2018 3:50:27 PM
> > Subject: Re: [PATCH] common/rc: add the function _require_noatime
> >
> > [add linux-cifs@vger.kernel.org to cc list]
> >
> > On Fri, May 18, 2018 at 07:10:29PM +0800, Xiaoli Feng wrote:
> >> From: xiaoli feng <xifeng@redhat.com>
> >>
> >> 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@redhat.com>
> >> ---
> >> 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@vger.kernel.org
> >> 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@vger.kernel.org
> > 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@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Thanks,
>
> Steve
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] common/rc: add the function _require_noatime
2018-05-21 23:54 ` Steve French
2018-05-21 23:55 ` Fwd: " Steve French
2018-05-22 2:21 ` Eryu Guan
@ 2018-05-22 2:54 ` Xiaoli Feng
2 siblings, 0 replies; 7+ messages in thread
From: Xiaoli Feng @ 2018-05-22 2:54 UTC (permalink / raw)
To: Steve French; +Cc: Eryu Guan, CIFS, fstests
----- Original Message -----
> From: "Steve French" <smfrench@gmail.com>
> To: "Xiaoli Feng" <xifeng@redhat.com>
> Cc: "Eryu Guan" <guaneryu@gmail.com>, "CIFS" <linux-cifs@vger.kernel.org>, fstests@vger.kernel.org
> 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@redhat.com> wrote:
> >
> > [add linux-cifs@vger.kernel.org to cc list]
> >
> > ----- Forwarded Message -----
> > From: "Eryu Guan" <guaneryu@gmail.com>
> > To: "XiaoLi Feng" <xifeng@redhat.com>
> > Cc: fstests@vger.kernel.org
> > Sent: Monday, May 21, 2018 3:50:27 PM
> > Subject: Re: [PATCH] common/rc: add the function _require_noatime
> >
> > [add linux-cifs@vger.kernel.org to cc list]
> >
> > On Fri, May 18, 2018 at 07:10:29PM +0800, Xiaoli Feng wrote:
> >> From: xiaoli feng <xifeng@redhat.com>
> >>
> >> 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@redhat.com>
> >> ---
> >> 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@vger.kernel.org
> >> 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@vger.kernel.org
> > 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@vger.kernel.org
> > 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 7+ messages in thread