All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Abort pending request for RAID10
@ 2016-01-19  8:55 Hannes Reinecke
  2016-01-19  9:05 ` kbuild test robot
  2016-01-19 18:53 ` Shaohua Li
  0 siblings, 2 replies; 3+ messages in thread
From: Hannes Reinecke @ 2016-01-19  8:55 UTC (permalink / raw)
  To: Neil Brown; +Cc: Shaohua Li, linux-raid, Hannes Reinecke

RAID10 delays the write until the bitmap has been updated.
So it really should check if the device is still working
before sending requests, otherwise it'll happily sending
I/O to a known faulty device.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/md/raid10.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index d4efed7..74369c2 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -902,12 +902,29 @@ static void flush_pending_writes(struct r10conf *conf)
 
 		while (bio) { /* submit pending writes */
 			struct bio *next = bio->bi_next;
+			struct r10bio *r10_bio = bio->bi_private;
+			struct md_rdev *rdev = NULL;
+			int dev, slot, repl;
 			bio->bi_next = NULL;
+			dev = find_bio_disk(conf, r10_bio, bio, &slot, &repl);
+			if (repl)
+				rdev = conf->mirrors[dev].replacement;
+			if (!rdev) {
+				smp_rmb();
+				repl = 0;
+				rdev = conf->mirrors[dev].rdev;
+			}
 			if (unlikely((bio->bi_rw & REQ_DISCARD) &&
 			    !blk_queue_discard(bdev_get_queue(bio->bi_bdev))))
 				/* Just ignore it */
 				bio_endio(bio);
-			else
+			else if (test_bit(Faulty, &rdev->flags)) {
+				if (test_bit(Timeout, &rdev->flags))
+					bio->bi_error = -ETIMEDOUT;
+				else
+					bio->bi_error = -EIO;
+				bio_endio(bio);
+			} else
 				generic_make_request(bio);
 			bio = next;
 		}
@@ -1079,12 +1096,29 @@ static void raid10_unplug(struct blk_plug_cb *cb, bool from_schedule)
 
 	while (bio) { /* submit pending writes */
 		struct bio *next = bio->bi_next;
+		struct r10bio *r10_bio = bio->bi_private;
+		struct md_rdev *rdev = NULL;
+		int dev, slot, repl;
 		bio->bi_next = NULL;
+		dev = find_bio_disk(conf, r10_bio, bio, &slot, &repl);
+		if (repl)
+			rdev = conf->mirrors[dev].replacement;
+		if (!rdev) {
+			smp_rmb();
+			repl = 0;
+			rdev = conf->mirrors[dev].rdev;
+		}
 		if (unlikely((bio->bi_rw & REQ_DISCARD) &&
 		    !blk_queue_discard(bdev_get_queue(bio->bi_bdev))))
 			/* Just ignore it */
 			bio_endio(bio);
-		else
+		else if (test_bit(Faulty, &rdev->flags)) {
+			if (test_bit(Timeout, &rdev->flags))
+				bio->bi_error = -ETIMEDOUT;
+			else
+				bio->bi_error = -EIO;
+			bio_endio(bio);
+		} else
 			generic_make_request(bio);
 		bio = next;
 	}
-- 
1.8.5.6


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Abort pending request for RAID10
  2016-01-19  8:55 [PATCH] Abort pending request for RAID10 Hannes Reinecke
@ 2016-01-19  9:05 ` kbuild test robot
  2016-01-19 18:53 ` Shaohua Li
  1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2016-01-19  9:05 UTC (permalink / raw)
  Cc: kbuild-all, Neil Brown, Shaohua Li, linux-raid, Hannes Reinecke

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

Hi Hannes,

[auto build test ERROR on v4.4-rc8]
[also build test ERROR on next-20160119]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Hannes-Reinecke/Abort-pending-request-for-RAID10/20160119-165822
config: x86_64-randconfig-x012-01180513 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from include/asm-generic/bug.h:4:0,
                    from arch/x86/include/asm/bug.h:35,
                    from include/linux/bug.h:4,
                    from include/linux/mmdebug.h:4,
                    from include/linux/gfp.h:4,
                    from include/linux/slab.h:14,
                    from drivers/md/raid10.c:21:
   drivers/md/raid10.c: In function 'flush_pending_writes':
>> drivers/md/raid10.c:884:18: error: 'Timeout' undeclared (first use in this function)
        if (test_bit(Timeout, &rdev->flags))
                     ^
   include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                               ^
>> drivers/md/raid10.c:884:5: note: in expansion of macro 'if'
        if (test_bit(Timeout, &rdev->flags))
        ^
>> drivers/md/raid10.c:884:9: note: in expansion of macro 'test_bit'
        if (test_bit(Timeout, &rdev->flags))
            ^
   drivers/md/raid10.c:884:18: note: each undeclared identifier is reported only once for each function it appears in
        if (test_bit(Timeout, &rdev->flags))
                     ^
   include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                               ^
>> drivers/md/raid10.c:884:5: note: in expansion of macro 'if'
        if (test_bit(Timeout, &rdev->flags))
        ^
>> drivers/md/raid10.c:884:9: note: in expansion of macro 'test_bit'
        if (test_bit(Timeout, &rdev->flags))
            ^
   drivers/md/raid10.c: In function 'raid10_unplug':
   drivers/md/raid10.c:1077:17: error: 'Timeout' undeclared (first use in this function)
       if (test_bit(Timeout, &rdev->flags))
                    ^
   include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                               ^
   drivers/md/raid10.c:1077:4: note: in expansion of macro 'if'
       if (test_bit(Timeout, &rdev->flags))
       ^
   drivers/md/raid10.c:1077:8: note: in expansion of macro 'test_bit'
       if (test_bit(Timeout, &rdev->flags))
           ^

vim +/Timeout +884 drivers/md/raid10.c

   878				}
   879				if (unlikely((bio->bi_rw & REQ_DISCARD) &&
   880				    !blk_queue_discard(bdev_get_queue(bio->bi_bdev))))
   881					/* Just ignore it */
   882					bio_endio(bio);
   883				else if (test_bit(Faulty, &rdev->flags)) {
 > 884					if (test_bit(Timeout, &rdev->flags))
   885						bio->bi_error = -ETIMEDOUT;
   886					else
   887						bio->bi_error = -EIO;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 19773 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Abort pending request for RAID10
  2016-01-19  8:55 [PATCH] Abort pending request for RAID10 Hannes Reinecke
  2016-01-19  9:05 ` kbuild test robot
@ 2016-01-19 18:53 ` Shaohua Li
  1 sibling, 0 replies; 3+ messages in thread
From: Shaohua Li @ 2016-01-19 18:53 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Neil Brown, linux-raid

On Tue, Jan 19, 2016 at 09:55:26AM +0100, Hannes Reinecke wrote:
> RAID10 delays the write until the bitmap has been updated.
> So it really should check if the device is still working
> before sending requests, otherwise it'll happily sending
> I/O to a known faulty device.

Hi,

__make_request already checks the faulty bit. That's possible the disk becomes
faulty after __make_request check though. But faulty bit can be set any time,
for example, after the check with your patch. There is no guarantee pending
writes get canceled immediately after the disk is marked faulty. I'm wondering
what's the real problem.

Thanks,
Shaohua

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-01-19 18:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-19  8:55 [PATCH] Abort pending request for RAID10 Hannes Reinecke
2016-01-19  9:05 ` kbuild test robot
2016-01-19 18:53 ` Shaohua Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.