Re: [PATCH xfstests] dirstress: Allow ESTALE as well as ENOENT

Re: [PATCH xfstests] dirstress: Allow ESTALE as well as ENOENT

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1934 bytes --]

On Nov 23, 2016, at 3:20 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> 
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> In the NFS case an attempt to chdir out of a removed directory could result
> in ESTALE, and that should not cause the test to fail.
> 
> This was causing intermittent failures on generic/011 on NFS.
> 
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
> src/dirstress.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
> 
> Sorry, who do xfstests patches go to?--b.

They are sent to fstests@vger.kernel.org

Cheers, Andreas

> 
> diff --git a/src/dirstress.c b/src/dirstress.c
> index f8f7355..33a166c 100644
> --- a/src/dirstress.c
> +++ b/src/dirstress.c
> @@ -203,8 +203,10 @@ dirstress(
>         if (verbose) fprintf(stderr,"** [%d] chdir ..\n", pid);
> 	error = chdir("..");
> 	if (error) {
> -		/* If this is multithreaded, then expecting a ENOENT here is fine */
> -		if (nprocs_per_dir > 1 && errno == ENOENT) {
> +		/* If this is multithreaded, then expecting a ENOENT here is fine,
> +		 * and ESTALE is normal in the NFS case. */
> +		if (nprocs_per_dir > 1 &&
> +				(errno == ENOENT || errno = ESTALE)) {
> 			return 0;
> 		}
> 
> @@ -224,8 +226,10 @@ dirstress(
>         if (verbose) fprintf(stderr,"** [%d] chdir ..\n", pid);
> 	error = chdir("..");
> 	if (error) {
> -		/* If this is multithreaded, then expecting a ENOENT here is fine */
> -		if (nprocs_per_dir > 1 && errno == ENOENT) {
> +		/* If this is multithreaded, then expecting a ENOENT here is fine,
> +		 * and ESTALE is normal in the NFS case. */
> +		if (nprocs_per_dir > 1 &&
> +				(errno == ENOENT || errno = ESTALE)) {
> 			return 0;
> 		}
> 
> --
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH xfstests] dirstress: Allow ESTALE as well as ENOENT

[J. Bruce Fields]

On Wed, Nov 23, 2016 at 04:19:11PM -0700, Andreas Dilger wrote:
> On Nov 23, 2016, at 3:20 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > 
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > In the NFS case an attempt to chdir out of a removed directory could result
> > in ESTALE, and that should not cause the test to fail.
> > 
> > This was causing intermittent failures on generic/011 on NFS.
> > 
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> > src/dirstress.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > Sorry, who do xfstests patches go to?--b.
> 
> They are sent to fstests@vger.kernel.org

Thanks!

Might be worth something like this too?

--b.

commit 0d61e7b9779d
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Thu Nov 24 22:41:26 2016 -0500

    Document fstests mailing list
    
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/README b/README
index d1f6788a17a3..0a6ac6ece5aa 100644
--- a/README
+++ b/README
@@ -248,3 +248,9 @@ Pass/failure:
     The recent pass/fail history is maintained in the file "check.log".
     The elapsed time for the most recent pass for each test is kept
     in "check.time".
+
+__________________
+SUBMITTING PATCHES
+__________________
+
+Send patches to the fstests mailing list at fstests@vger.kernel.org.

Re: [PATCH xfstests] dirstress: Allow ESTALE as well as ENOENT

[Eryu Guan]

Hi Bruce,

On Thu, Nov 24, 2016 at 10:47:06PM -0500, J. Bruce Fields wrote:
> On Wed, Nov 23, 2016 at 04:19:11PM -0700, Andreas Dilger wrote:
> > On Nov 23, 2016, at 3:20 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > > 
> > > From: "J. Bruce Fields" <bfields@redhat.com>
> > > 
> > > In the NFS case an attempt to chdir out of a removed directory could result
> > > in ESTALE, and that should not cause the test to fail.
> > > 
> > > This was causing intermittent failures on generic/011 on NFS.
> > > 
> > > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > > ---
> > > src/dirstress.c | 12 ++++++++----
> > > 1 file changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > Sorry, who do xfstests patches go to?--b.
> > 
> > They are sent to fstests@vger.kernel.org
> 
> Thanks!
> 
> Might be worth something like this too?

This looks fine to me. Can you please resend as a formal patch? Also the
patch to fix dirstress.c.

Thanks,
Eryu

[PATCH 1/2] Document fstests mailing list

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 README | 6 ++++++
 1 file changed, 6 insertions(+)

> This looks fine to me. Can you please resend as a formal patch? Also the
> patch to fix dirstress.c.

Sure, here you go.--b.

diff --git a/README b/README
index 8a362bd51f49..9f20884ecd37 100644
--- a/README
+++ b/README
@@ -243,3 +243,9 @@ Pass/failure:
     The recent pass/fail history is maintained in the file "check.log".
     The elapsed time for the most recent pass for each test is kept
     in "check.time".
+
+__________________
+SUBMITTING PATCHES
+__________________
+
+Send patches to the fstests mailing list at fstests@vger.kernel.org.
-- 
2.9.3

[PATCH 2/2] dirstress: Allow ESTALE as well as ENOENT

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

In the NFS case an attempt to chdir out of a removed directory could result
in ESTALE, and that should not cause the test to fail.

This was causing intermittent failures on generic/011 on NFS.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 src/dirstress.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/dirstress.c b/src/dirstress.c
index f8f73553a1bc..33a166c51819 100644
--- a/src/dirstress.c
+++ b/src/dirstress.c
@@ -203,8 +203,10 @@ dirstress(
         if (verbose) fprintf(stderr,"** [%d] chdir ..\n", pid);
 	error = chdir("..");
 	if (error) {
-		/* If this is multithreaded, then expecting a ENOENT here is fine */
-		if (nprocs_per_dir > 1 && errno == ENOENT) {
+		/* If this is multithreaded, then expecting a ENOENT here is fine,
+		 * and ESTALE is normal in the NFS case. */
+		if (nprocs_per_dir > 1 &&
+				(errno == ENOENT || errno = ESTALE)) {
 			return 0;
 		}
 
@@ -224,8 +226,10 @@ dirstress(
         if (verbose) fprintf(stderr,"** [%d] chdir ..\n", pid);
 	error = chdir("..");
 	if (error) {
-		/* If this is multithreaded, then expecting a ENOENT here is fine */
-		if (nprocs_per_dir > 1 && errno == ENOENT) {
+		/* If this is multithreaded, then expecting a ENOENT here is fine,
+		 * and ESTALE is normal in the NFS case. */
+		if (nprocs_per_dir > 1 &&
+				(errno == ENOENT || errno = ESTALE)) {
 			return 0;
 		}
 
-- 
2.9.3

Re: [PATCH 2/2] dirstress: Allow ESTALE as well as ENOENT

[Eryu Guan]

On Fri, Nov 25, 2016 at 08:39:39PM -0500, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> In the NFS case an attempt to chdir out of a removed directory could result
> in ESTALE, and that should not cause the test to fail.
> 
> This was causing intermittent failures on generic/011 on NFS.
> 
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  src/dirstress.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/src/dirstress.c b/src/dirstress.c
> index f8f73553a1bc..33a166c51819 100644
> --- a/src/dirstress.c
> +++ b/src/dirstress.c
> @@ -203,8 +203,10 @@ dirstress(
>          if (verbose) fprintf(stderr,"** [%d] chdir ..\n", pid);
>  	error = chdir("..");
>  	if (error) {
> -		/* If this is multithreaded, then expecting a ENOENT here is fine */
> -		if (nprocs_per_dir > 1 && errno == ENOENT) {
> +		/* If this is multithreaded, then expecting a ENOENT here is fine,
> +		 * and ESTALE is normal in the NFS case. */
> +		if (nprocs_per_dir > 1 &&
> +				(errno == ENOENT || errno = ESTALE)) {
                                                         ^^^ this should be ==
>  			return 0;
>  		}
>  
> @@ -224,8 +226,10 @@ dirstress(
>          if (verbose) fprintf(stderr,"** [%d] chdir ..\n", pid);
>  	error = chdir("..");
>  	if (error) {
> -		/* If this is multithreaded, then expecting a ENOENT here is fine */
> -		if (nprocs_per_dir > 1 && errno == ENOENT) {
> +		/* If this is multithreaded, then expecting a ENOENT here is fine,
> +		 * and ESTALE is normal in the NFS case. */
> +		if (nprocs_per_dir > 1 &&
> +				(errno == ENOENT || errno = ESTALE)) {
                                                         ^^^ same here

I can fix them at commit time, no resend is needed.

Thanks,
Eryu

>  			return 0;
>  		}
>  
> -- 
> 2.9.3
> 
> --
> 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

Re: [PATCH 2/2] dirstress: Allow ESTALE as well as ENOENT

[J. Bruce Fields]

On Tue, Nov 29, 2016 at 02:43:06PM +0800, Eryu Guan wrote:
> On Fri, Nov 25, 2016 at 08:39:39PM -0500, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > In the NFS case an attempt to chdir out of a removed directory could result
> > in ESTALE, and that should not cause the test to fail.
> > 
> > This was causing intermittent failures on generic/011 on NFS.
> > 
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> >  src/dirstress.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/dirstress.c b/src/dirstress.c
> > index f8f73553a1bc..33a166c51819 100644
> > --- a/src/dirstress.c
> > +++ b/src/dirstress.c
> > @@ -203,8 +203,10 @@ dirstress(
> >          if (verbose) fprintf(stderr,"** [%d] chdir ..\n", pid);
> >  	error = chdir("..");
> >  	if (error) {
> > -		/* If this is multithreaded, then expecting a ENOENT here is fine */
> > -		if (nprocs_per_dir > 1 && errno == ENOENT) {
> > +		/* If this is multithreaded, then expecting a ENOENT here is fine,
> > +		 * and ESTALE is normal in the NFS case. */
> > +		if (nprocs_per_dir > 1 &&
> > +				(errno == ENOENT || errno = ESTALE)) {
>                                                          ^^^ this should be ==
> >  			return 0;
> >  		}
> >  
> > @@ -224,8 +226,10 @@ dirstress(
> >          if (verbose) fprintf(stderr,"** [%d] chdir ..\n", pid);
> >  	error = chdir("..");
> >  	if (error) {
> > -		/* If this is multithreaded, then expecting a ENOENT here is fine */
> > -		if (nprocs_per_dir > 1 && errno == ENOENT) {
> > +		/* If this is multithreaded, then expecting a ENOENT here is fine,
> > +		 * and ESTALE is normal in the NFS case. */
> > +		if (nprocs_per_dir > 1 &&
> > +				(errno == ENOENT || errno = ESTALE)) {
>                                                          ^^^ same here
> 
> I can fix them at commit time, no resend is needed.

Oh, good grief.  Thanks for catching that!

I can confirm that my problem's still fixed with the corrected patch.

--b.

> 
> Thanks,
> Eryu
> 
> >  			return 0;
> >  		}
> >  
> > -- 
> > 2.9.3
> > 
> > --
> > 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