autofs: don't stuck in a loop if vfs_write returns an error

PATCH In review Andrey Vagin 3 participants 10 messages Apr 1, 2016 – Jun 16, 2016 Kernel automounter

lore

Share & tools

lore

Andrey Vagin

author

2016-04-01 05:12 UTC #0 lore raw

PATCH 1/1 autofs: don't stuck in a loop if vfs_write returns an error

From: Andrey Vagin <[email protected]>

__vfs_write() returns a negative value in a error case.

Cc Ian Kent <[email protected]>

Signed-off-by Andrey Vagin <[email protected]>

---
 fs/autofs4/waitq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

fs/autofs4/waitq.c +4 -3

index 0146d91..631f155 100644
@@ -66,11 +66,12 @@ static int autofs4_write(struct autofs_sb_info *sbi,
 	set_fs(KERNEL_DS);
 
 	mutex_lock(&sbi->pipe_mutex);
-	wr = __vfs_write(file, data, bytes, &file->f_pos);
-	while (bytes && wr) {
+	while (bytes) {
+		wr = __vfs_write(file, data, bytes, &file->f_pos);
+		if (wr <= 0)
+			break;
 		data += wr;
 		bytes -= wr;
-		wr = __vfs_write(file, data, bytes, &file->f_pos);
 	}
 	mutex_unlock(&sbi->pipe_mutex);

Signature · 4 lines

2.5.0

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Ian Kent

maintainer

2016-04-01 07:37 UTC #1 lore raw

Quoted 3 lines · Andrey Vagin

From: Andrey Vagin <[email protected]>
__vfs_write() returns a negative value in a error case.

Ha, right, I'll send this along to Andrew with my next series which should be soon.

Quoted 18 lines

Cc Ian Kent <[email protected]>

Signed-off-by Andrey Vagin <[email protected]>

---
 fs/autofs4/waitq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

fs/autofs4/waitq.c +0 -1

index 0146d91..631f155 100644
@@ -66,11 +66,12 @@ static int autofs4_write(struct autofs_sb_info
 *sbi,
 	set_fs(KERNEL_DS);
 
 	mutex_lock(&sbi->pipe_mutex);
-	wr = __vfs_write(file, data, bytes, &file->f_pos);

fs/autofs4/waitq.c static int autofs4_write(struct autofs_sb_info · line 77

Quoted 10 lines

fs/autofs4/waitq.c +4 -2

-	while (bytes && wr) {
+	while (bytes) {
+		wr = __vfs_write(file, data, bytes, &file->f_pos);
+		if (wr <= 0)
+			break;
 		data += wr;
 		bytes -= wr;
-		wr = __vfs_write(file, data, bytes, &file->f_pos);
 	}
 	mutex_unlock(&sbi->pipe_mutex);

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Andrei Vagin

author

2016-05-23 21:50 UTC #2 lore raw

Hi Ian,

When are you going to apply this patch? We can't test linux-next without it.

Thanks,
Andrew

Quoted 23 lines · Ian Kent

Quoted 3 lines · Andrey Vagin
From: Andrey Vagin <[email protected]>
__vfs_write() returns a negative value in a error case.
Ha, right, I'll send this along to Andrew with my next series which should be soon.
Quoted 15 lines
Cc Ian Kent <[email protected]>
Signed-off-by Andrey Vagin <[email protected]>
---
 fs/autofs4/waitq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/autofs4/waitq.c +0 -0
index 0146d91..631f155 100644
@@ -66,11 +66,12 @@ static int autofs4_write(struct autofs_sb_info
 *sbi,
      set_fs(KERNEL_DS);
 

Quoted 13 lines

fs/autofs4/waitq.c static int autofs4_write(struct autofs_sb_info *sbi, · line 76

Quoted 13 lines

fs/autofs4/waitq.c +4 -3

      mutex_lock(&sbi->pipe_mutex);
-     wr = __vfs_write(file, data, bytes, &file->f_pos);
-     while (bytes && wr) {
+     while (bytes) {
+             wr = __vfs_write(file, data, bytes, &file->f_pos);
+             if (wr <= 0)
+                     break;
              data += wr;
              bytes -= wr;
-             wr = __vfs_write(file, data, bytes, &file->f_pos);
      }
      mutex_unlock(&sbi->pipe_mutex);

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Ian Kent

maintainer

2016-05-24 01:34 UTC #3 lore raw

Quoted 3 lines · Andrei Vagin

Hi Ian,
When are you going to apply this patch? We can't test linux-next without it.

I though I sent this with the last series but I can't see that I have.

I have the rest of that series to send over to Andrew which I was planning to do after the current merge window closes (which is about now I guess).

I'll include it in that series.
Sorry for the hold up, ;)

Ian

Quoted 29 lines

Thanks,
Andrew
Quoted 23 lines · Ian Kent
Quoted 3 lines · Andrey Vagin
From: Andrey Vagin <[email protected]>
__vfs_write() returns a negative value in a error case.
Ha, right, I'll send this along to Andrew with my next series which should be soon.
Quoted 15 lines
Cc Ian Kent <[email protected]>
Signed-off-by Andrey Vagin <[email protected]>
---
 fs/autofs4/waitq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/autofs4/waitq.c +0 -0
index 0146d91..631f155 100644
@@ -66,11 +66,12 @@ static int autofs4_write(struct autofs_sb_info
 *sbi,
      set_fs(KERNEL_DS);
 

Quoted 13 lines

Quoted 5 lines

fs/autofs4/waitq.c +2 -2

      mutex_lock(&sbi->pipe_mutex);
-     wr = __vfs_write(file, data, bytes, &file->f_pos);
-     while (bytes && wr) {
+     while (bytes) {
+             wr = __vfs_write(file, data, bytes, &file->f_pos);

Quoted 8 lines

fs/autofs4/waitq.c static int autofs4_write(struct autofs_sb_info *sbi, · line 76

Quoted 8 lines

fs/autofs4/waitq.c +2 -1

+             if (wr <= 0)
+                     break;
              data += wr;
              bytes -= wr;
-             wr = __vfs_write(file, data, bytes, &file->f_pos);
      }
      mutex_unlock(&sbi->pipe_mutex);

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Ian Kent

maintainer

2016-05-30 05:52 UTC #4 lore raw

Quoted 5 lines · Ian Kent

Quoted 3 lines · Andrei Vagin
Hi Ian,
When are you going to apply this patch? We can't test linux-next without it.

Quoted 8 lines

I though I sent this with the last series but I can't see that I have.
I have the rest of that series to send over to Andrew which I was planning to do
after the current merge window closes (which is about now I guess).
I'll include it in that series.
Sorry for the hold up, ;)

Some bad news I'm afraid.

I was getting ready to send these over to Andrew and found that opendir(3) is failing on a number of tests (51 of 230, 9 fails are expected) with 4.6.0.

It's not the patches, yours or mine and it doesn't happen with 4.4.x kernels.

Looks like I'm going to have to bisect to work out what's going on and that will take a while.

Quoted 32 lines

Ian
Quoted 29 lines
Thanks,
Andrew
Quoted 23 lines · Ian Kent
Quoted 3 lines · Andrey Vagin
From: Andrey Vagin <[email protected]>
__vfs_write() returns a negative value in a error case.
Ha, right, I'll send this along to Andrew with my next series which should be soon.
Quoted 15 lines
Cc Ian Kent <[email protected]>
Signed-off-by Andrey Vagin <[email protected]>
---
 fs/autofs4/waitq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/autofs4/waitq.c +0 -0
index 0146d91..631f155 100644
@@ -66,11 +66,12 @@ static int autofs4_write(struct autofs_sb_info
 *sbi,
      set_fs(KERNEL_DS);
 

Quoted 12 lines

Quoted 4 lines

fs/autofs4/waitq.c +1 -2

      mutex_lock(&sbi->pipe_mutex);
-     wr = __vfs_write(file, data, bytes, &file->f_pos);
-     while (bytes && wr) {
+     while (bytes) {

Quoted 8 lines

fs/autofs4/waitq.c static int autofs4_write(struct autofs_sb_info *sbi, · line 76

Quoted 8 lines

fs/autofs4/waitq.c +3 -1

+             wr = __vfs_write(file, data, bytes, &file->f_pos);
+             if (wr <= 0)
+                     break;
              data += wr;
              bytes -= wr;
-             wr = __vfs_write(file, data, bytes, &file->f_pos);
      }
      mutex_unlock(&sbi->pipe_mutex);

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Ian Kent

maintainer

2016-06-09 01:23 UTC #5 lore raw

Quoted 19 lines · Ian Kent

Quoted 8 lines · Ian Kent
Quoted 4 lines · Andrei Vagin
Hi Ian,
When are you going to apply this patch? We can't test linux-next without it.
I though I sent this with the last series but I can't see that I have.
Quoted 7 lines
I have the rest of that series to send over to Andrew which I was planning to
do
after the current merge window closes (which is about now I guess).
I'll include it in that series.
Sorry for the hold up, ;)
Some bad news I'm afraid.

Quoted 8 lines

I was getting ready to send these over to Andrew and found that opendir(3) is failing on a number of tests (51 of 230, 9 fails are expected) with 4.6.0.
It's not the patches, yours or mine and it doesn't happen with 4.4.x kernels.
Looks like I'm going to have to bisect to work out what's going on and that will
take a while.

The regression has been fixed now.

Al Viro sent a patch for it to Linus yesterday, it's commit e6ec03a25f1 in the Linux tree.

I can send my patches to Andrew (after re-testing) but any autofs related testing of linux.next will need the above commit.

Andrew, surely this isn't the first time this type of problem has happened, how is it usually handled, what do I need to do to make this go smoothly?

Quoted 33 lines

Quoted 33 lines
Ian
Quoted 29 lines
Thanks,
Andrew
Quoted 23 lines · Ian Kent
Quoted 3 lines · Andrey Vagin
From: Andrey Vagin <[email protected]>
__vfs_write() returns a negative value in a error case.
Ha, right, I'll send this along to Andrew with my next series which should be soon.
Quoted 15 lines
Cc Ian Kent <[email protected]>
Signed-off-by Andrey Vagin <[email protected]>
---
 fs/autofs4/waitq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/autofs4/waitq.c +0 -0
index 0146d91..631f155 100644
@@ -66,11 +66,12 @@ static int autofs4_write(struct autofs_sb_info
 *sbi,
      set_fs(KERNEL_DS);
 

Quoted 14 lines

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
fs/autofs4/waitq.c +1 -2
      mutex_lock(&sbi->pipe_mutex);
-     wr = __vfs_write(file, data, bytes, &file->f_pos);
-     while (bytes && wr) {
+     while (bytes) {
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
fs/autofs4/waitq.c static int autofs4_write(struct autofs_sb_info *sbi, · line 76
Quoted 8 lines
fs/autofs4/waitq.c +3 -1
+             wr = __vfs_write(file, data, bytes, &file->f_pos);
+             if (wr <= 0)
+                     break;
              data += wr;
              bytes -= wr;
-             wr = __vfs_write(file, data, bytes, &file->f_pos);
      }
      mutex_unlock(&sbi->pipe_mutex);
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Andrei Vagin

author

2016-06-09 17:27 UTC #6 lore raw

Quoted 25 lines · Ian Kent

Quoted 24 lines · Ian Kent
Quoted 8 lines · Ian Kent
Quoted 4 lines · Andrei Vagin
Hi Ian,
When are you going to apply this patch? We can't test linux-next without it.
I though I sent this with the last series but I can't see that I have.
Quoted 7 lines
I have the rest of that series to send over to Andrew which I was planning to
do
after the current merge window closes (which is about now I guess).
I'll include it in that series.
Sorry for the hold up, ;)
Some bad news I'm afraid.
I was getting ready to send these over to Andrew and found that opendir(3) is failing on a number of tests (51 of 230, 9 fails are expected) with 4.6.0.
It's not the patches, yours or mine and it doesn't happen with 4.4.x kernels.

Quoted 8 lines

Quoted 3 lines
Looks like I'm going to have to bisect to work out what's going on and that will
take a while.
The regression has been fixed now.
Al Viro sent a patch for it to Linus yesterday, it's commit e6ec03a25f1 in the Linux tree.

It's good!

Quoted 6 lines

I can send my patches to Andrew (after re-testing) but any autofs related testing of linux.next will need the above commit.
Andrew, surely this isn't the first time this type of problem has happened, how is it usually handled, what do I need to do to make this go smoothly?

In linux-next we catch two sorts of bugs.

1. If bugs is triggered very often, we report it, when it isn't in Linus' tree. And such bugs are fixed very fast.
2. If bugs is triggered rarely. In this case we may detect this bug too late, when it's in Linus' tree. In this case we may need to workaraound it.

Here the problem belongs to the second type. It is triggered only when one or more tests failed and we try to kill all test processes. Actually it doesn't affect regular runs of tests, but it's annoying when we are investigating something.

Ian, I don't think that you need to do anything special. Thank you for handling this patch!

Thanks,
Andrew

Quoted 48 lines

Quoted 48 lines
Quoted 34 lines
Ian
Quoted 29 lines
Thanks,
Andrew
Quoted 23 lines · Ian Kent
Quoted 3 lines · Andrey Vagin
From: Andrey Vagin <[email protected]>
__vfs_write() returns a negative value in a error case.
Ha, right, I'll send this along to Andrew with my next series which should be soon.
Quoted 15 lines
Cc Ian Kent <[email protected]>
Signed-off-by Andrey Vagin <[email protected]>
---
 fs/autofs4/waitq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/autofs4/waitq.c +0 -0
index 0146d91..631f155 100644
@@ -66,11 +66,12 @@ static int autofs4_write(struct autofs_sb_info
 *sbi,
      set_fs(KERNEL_DS);
 
Quoted 12 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
fs/autofs4/waitq.c +1 -2
      mutex_lock(&sbi->pipe_mutex);
-     wr = __vfs_write(file, data, bytes, &file->f_pos);
-     while (bytes && wr) {
+     while (bytes) {
Quoted 8 lines
Quoted 8 lines
fs/autofs4/waitq.c static int autofs4_write(struct autofs_sb_info *sbi, · line 76
Quoted 8 lines
fs/autofs4/waitq.c +3 -1
+             wr = __vfs_write(file, data, bytes, &file->f_pos);
+             if (wr <= 0)
+                     break;
              data += wr;
              bytes -= wr;
-             wr = __vfs_write(file, data, bytes, &file->f_pos);
      }
      mutex_unlock(&sbi->pipe_mutex);
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Ian Kent

maintainer

2016-06-10 08:45 UTC #7 lore raw

Quoted 59 lines · Andrei Vagin

Quoted 34 lines · Ian Kent
Quoted 25 lines · Ian Kent
Quoted 9 lines · Ian Kent
Quoted 5 lines · Andrei Vagin
Hi Ian,
When are you going to apply this patch? We can't test linux-next without
it.
I though I sent this with the last series but I can't see that I have.
Quoted 8 lines
I have the rest of that series to send over to Andrew which I was planning
to
do
after the current merge window closes (which is about now I guess).
I'll include it in that series.
Sorry for the hold up, ;)
Some bad news I'm afraid.
I was getting ready to send these over to Andrew and found that opendir(3) is
failing on a number of tests (51 of 230, 9 fails are expected) with 4.6.0.
Quoted 7 lines
It's not the patches, yours or mine and it doesn't happen with 4.4.x kernels.
Looks like I'm going to have to bisect to work out what's going on and that
will
take a while.
Quoted 5 lines
The regression has been fixed now.
Al Viro sent a patch for it to Linus yesterday, it's commit e6ec03a25f1 in the
Linux tree.
It's good!
Quoted 7 lines
I can send my patches to Andrew (after re-testing) but any autofs related testing of linux.next will need the above commit.
Andrew, surely this isn't the first time this type of problem has happened, how
is it usually handled, what do I need to do to make this go smoothly?
In linux-next we catch two sorts of bugs.
1. If bugs is triggered very often, we report it, when it isn't in Linus' tree. And such bugs are fixed very fast.
2. If bugs is triggered rarely. In this case we may detect this bug too late, when it's in Linus' tree. In this case we may need to workaraound it.

Quoted 7 lines

Here the problem belongs to the second type. It is triggered only when one or more tests failed and we try to kill all test processes. Actually it doesn't affect regular runs of tests, but it's annoying when we are investigating something.
Ian, I don't think that you need to do anything special. Thank you for handling this patch!

LOL, it's good that it was my standard (relatively simple) testing that exposed it.

I'll wait a little longer in case Andrew has anything to add then send the patch series to him.

Quoted 52 lines

Thanks,
Andrew
Quoted 48 lines
Quoted 48 lines
Quoted 34 lines
Ian
Quoted 29 lines
Thanks,
Andrew
Quoted 23 lines · Ian Kent
Quoted 3 lines · Andrey Vagin
From: Andrey Vagin <[email protected]>
__vfs_write() returns a negative value in a error case.
Ha, right, I'll send this along to Andrew with my next series which should be soon.
Quoted 15 lines
Cc Ian Kent <[email protected]>
Signed-off-by Andrey Vagin <[email protected]>
---
 fs/autofs4/waitq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/autofs4/waitq.c +0 -0
index 0146d91..631f155 100644
@@ -66,11 +66,12 @@ static int autofs4_write(struct autofs_sb_info
 *sbi,
      set_fs(KERNEL_DS);
 
Quoted 12 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
fs/autofs4/waitq.c +1 -2
      mutex_lock(&sbi->pipe_mutex);
-     wr = __vfs_write(file, data, bytes, &file->f_pos);
-     while (bytes && wr) {
+     while (bytes) {
Quoted 8 lines
Quoted 8 lines
fs/autofs4/waitq.c static int autofs4_write(struct autofs_sb_info · line 77
Quoted 8 lines
fs/autofs4/waitq.c +3 -1
+             wr = __vfs_write(file, data, bytes, &file->f_pos);
+             if (wr <= 0)
+                     break;
              data += wr;
              bytes -= wr;
-             wr = __vfs_write(file, data, bytes, &file->f_pos);
      }
      mutex_unlock(&sbi->pipe_mutex);
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Andrew Morton

2016-06-10 19:07 UTC #8 lore raw

In reply to Ian Kent

Quoted 11 lines · On Thu, 09 Jun 2016 09:23:26 +0800 Ian Kent <[email protected]> wrote:

Quoted 8 lines
I was getting ready to send these over to Andrew and found that opendir(3) is failing on a number of tests (51 of 230, 9 fails are expected) with 4.6.0.
It's not the patches, yours or mine and it doesn't happen with 4.4.x kernels.
Looks like I'm going to have to bisect to work out what's going on and that will
take a while.
The regression has been fixed now.

Quoted 8 lines

Al Viro sent a patch for it to Linus yesterday, it's commit e6ec03a25f1 in the Linux tree.
I can send my patches to Andrew (after re-testing) but any autofs related testing of linux.next will need the above commit.
Andrew, surely this isn't the first time this type of problem has happened, how is it usually handled, what do I need to do to make this go smoothly?

e6ec03a25f1 is in Linus's tree now so everything should be good. --
To unsubscribe from this list: send the line "unsubscribe autofs" in

Ian Kent

maintainer

2016-06-16 07:58 UTC #9 lore raw

Quoted 20 lines · Andrew Morton

Quoted 18 lines · On Thu, 09 Jun 2016 09:23:26 +0800 Ian Kent <[email protected]> wrote:
Quoted 11 lines
I was getting ready to send these over to Andrew and found that opendir(3) is
failing on a number of tests (51 of 230, 9 fails are expected) with 4.6.0.
It's not the patches, yours or mine and it doesn't happen with 4.4.x kernels.
Looks like I'm going to have to bisect to work out what's going on and that
will
take a while.
The regression has been fixed now.
Al Viro sent a patch for it to Linus yesterday, it's commit e6ec03a25f1 in the
Linux tree.

Quoted 8 lines

Quoted 6 lines
I can send my patches to Andrew (after re-testing) but any autofs related testing of linux.next will need the above commit.
Andrew, surely this isn't the first time this type of problem has happened, how
is it usually handled, what do I need to do to make this go smoothly?
e6ec03a25f1 is in Linus's tree now so everything should be good.

Everything was good.

There's another autofs patch in Al's for-linus tree now and I don't know when it will go to Linus.

That one will definitely cause merge conflicts.

After thinking about it, it's probably best to defer the module rename series until after the next merge window, after all it's not urgent (apart from being needed for so long).

I'll send over Andrei's patch alone soonish.

Thanks all for your patience,
Ian

--
To unsubscribe from this list: send the line "unsubscribe autofs" in