Linux RAID subsystem development
 help / color / mirror / Atom feed
* [PATCH 3/4] mdadm/test: Get rid of tests/testdev
From: Zhilong Liu @ 2017-07-19 11:11 UTC (permalink / raw)
  To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
In-Reply-To: <1500462675-6463-1-git-send-email-zlliu@suse.com>

It's time to get rid of the tests/testdev due
to it has covered by testdev() in 'test'.

Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
 tests/testdev | 13 -------------
 1 file changed, 13 deletions(-)
 delete mode 100644 tests/testdev

diff --git a/tests/testdev b/tests/testdev
deleted file mode 100644
index 8b6e6f0..0000000
--- a/tests/testdev
+++ /dev/null
@@ -1,13 +0,0 @@
-dev=$1
-cnt=$2
-size=$3
-chunk=$4
-mkfs -j $dev > /dev/null 2>&1 && fsck -fn $dev >&2
-dsize=$[size/chunk]
-dsize=$[dsize*chunk]
-rasize=$[dsize*2*cnt]
-if [ $rasize -ne `/sbin/blockdev --getsize $dev` ]
-then
-  echo "ERROR: size is wrong for $dev: $cnt * $size (chunk=$chunk) = $rasize, not `/sbin/blockdev --getsize $dev`"
-  exit 1;
-fi
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 2/4] mdadm/test: Fixed the condition parsing error for array[]
From: Zhilong Liu @ 2017-07-19 11:11 UTC (permalink / raw)
  To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
In-Reply-To: <1500462675-6463-1-git-send-email-zlliu@suse.com>

it complains too many arguments when array[] has few
members, it's proper to check whether or not array[0]
is zero.

Reported-by: Xiao Ni <xni@redhat.com>
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
 test | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test b/test
index 87e2df2..25268a0 100755
--- a/test
+++ b/test
@@ -75,7 +75,7 @@ save_log() {
 		echo
 	elif [ "$DEVTYPE" == 'loop' -o "$DEVTYPE" == 'disk' ]
 	then
-		if [ ! -z ${array[@]} -a ${#array[@]} -ge 1 ]
+		if [ ! -z "$array" -a ${#array[@]} -ge 1 ]
 		then
 			md_disks=($($mdadm -D -Y ${array[@]} | grep "/dev/" | cut -d'=' -f2))
 			cat /proc/mdstat | grep -q "bitmap"
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 1/4] test/00readonly: Add unit case for testing readonly/readwrite
From: Zhilong Liu @ 2017-07-19 11:11 UTC (permalink / raw)
  To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
In-Reply-To: <1500462675-6463-1-git-send-email-zlliu@suse.com>

This is a test case for testing --readonly and
--readwrite feature, it covers common metadata
versions and raid levels.

Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
 tests/00readonly | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 tests/00readonly

diff --git a/tests/00readonly b/tests/00readonly
new file mode 100644
index 0000000..8467da7
--- /dev/null
+++ b/tests/00readonly
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+for metadata in 0.9 1.0 1.1 1.2
+do
+	for level in linear raid0 raid1 raid4 raid5 raid6 raid10
+	do
+		mdadm -CR $md0 -l $level -n 4 --metadata=$metadata \
+			$dev1 $dev2 $dev3 $dev4 --assume-clean
+		check nosync
+		check $level
+		mdadm -ro $md0
+		check readonly &&
+			[ "$(cat /sys/block/md0/md/array_state)" == "readonly" ]
+		mdadm -w $md0
+		check $level
+		mdadm -S $md0
+	done
+done
+
+exit 0
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 0/4] mdadm/test: Update for mdadm test part
From: Zhilong Liu @ 2017-07-19 11:11 UTC (permalink / raw)
  To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu

1. add a new unit case for testing readonly/readwrite feature, it
   covers common metadata versions and raid levels.
2. fixed a issue reported by Xiao Ni, shell got parsing error when
   array[] has few members, so just check the array[0] is enough.
3. get rid of the useless test files.

Thanks,
Zhilong

Zhilong Liu (4):
  test/00readonly: Add unit case for testing readonly/readwrite
  mdadm/test: Fixed the condition parsing error for array[]
  mdadm/test: Get rid of tests/testdev
  mdadm/test: Get rid of tests/check

 test             |  2 +-
 tests/00readonly | 20 ++++++++++++++++++++
 tests/check      | 35 -----------------------------------
 tests/testdev    | 13 -------------
 4 files changed, 21 insertions(+), 49 deletions(-)
 create mode 100644 tests/00readonly
 delete mode 100644 tests/check
 delete mode 100644 tests/testdev

-- 
1.8.3.1


^ permalink raw reply

* Re: [PATCH -v3 1/1] mdadm/test: Add one test case for raid5 reshape
From: Zhilong Liu @ 2017-07-19  9:14 UTC (permalink / raw)
  To: Xiao Ni; +Cc: linux-raid, jes sorensen
In-Reply-To: <571877982.32043460.1500443653100.JavaMail.zimbra@redhat.com>



On 07/19/2017 01:54 PM, Xiao Ni wrote:
>
> ----- Original Message -----
>> From: "Zhilong Liu" <zlliu@suse.com>
>> To: "Xiao Ni" <xni@redhat.com>, linux-raid@vger.kernel.org
>> Cc: "jes sorensen" <jes.sorensen@gmail.com>
>> Sent: Tuesday, July 18, 2017 6:34:33 PM
>> Subject: Re: [PATCH -v3 1/1] mdadm/test: Add one test case for raid5 reshape
>>
>>
>>
>> On 07/18/2017 04:55 PM, Xiao Ni wrote:
>>> This case trys to allow raid5 reshape to use backwards direction.
>>> It changes chunksize after reshape and stop the raid. Then start
>>> the raid again.
>>>
>>> Signed-off-by: Xiao Ni <xni@redhat.com>
>>> Suggested-by:  Jes Sorensen <jes.sorensen@gmail.com>
>>> Suggested-by:  Zhilong Liu <zlliu@suse.com>
>>> ---
>>>    test           |  8 ++++++++
>>>    tests/02r5grow | 16 ++++++++++++++++
>>>    2 files changed, 24 insertions(+)
>>>
>>> diff --git a/test b/test
>>> index 87e2df2..51da2dc 100755
>>> --- a/test
>>> +++ b/test
>>> @@ -320,6 +320,14 @@ check() {
>>>    		grep -sq "inactive" /proc/mdstat ||
>>>    			die "array is not inactive!"
>>>    		;;
>>> +	# It only can be used when there is only one raid
>>> +	chunk )
>>> +		chunk_size=`cat /proc/mdstat  | awk -F',' '/chunk/{print $2}' | awk
>>> '{print $1}' | sed s/k//g`
>> Hi,
>>     only from my opinion, how about shrink the command:
>> # awk -F',' '/chunk/{print $2}' /proc/mdstat | awk -F'[a-z]' '{print $1}'
>>> +		if [ $chunk_size -ne $2 ]; then
>>> +			echo >&2 "chunksize is not right"
>>> +			exit 1
>>> +		fi
> Yes, you are a shell expert :)
>
>> how about using the func die()
>> [  "$chunk_size" -ne "$2" ] &&
>>     die "chunk should be $2, but is $chunk_size"
> I tried this and did a simple test. It gave a error:
> [root@dell-per210-01 mdadm]# ./test --tests=02r5grow
> Testing on linux-3.10.0-685.el7.x86_64 kernel
> tests/02r5grow... FAILED - see /var/tmp/02r5grow.log and /var/tmp/fail02r5grow.log for details
> ./test: line 80: [: too many arguments
>
> It was introduced by this 20d10b4b. It looks like the check have errors:
> if [ ! -z ${array[@]} -a ${#array[@]} -ge 1 ]

Yes, thanks for catching this, this condition would get parsing
error when array[] has few members. it should correct as:

[ ! -z "$array" -a ${#array[@]}] -ge 1 ]

check the array[index](first member) is enough, I should fix it.

Thanks,
-Zhilong

>
> Regards
> Xiao
>> Thanks,
>> -Zhilong
>>
>>> +		;;
>>>    	* )
>>>    		die "unknown check $1"
>>>    		;;
>>> diff --git a/tests/02r5grow b/tests/02r5grow
>>> index 386e82e..20db4cd 100644
>>> --- a/tests/02r5grow
>>> +++ b/tests/02r5grow
>>> @@ -34,3 +34,19 @@ check nosync
>>>    sh tests/testdev $md0 3 $[size/2] 128
>>>    
>>>    mdadm -S $md0
>>> +
>>> +# create a raid5 array and change the chunk
>>> +mdadm -CR $md0 --level raid5 --metadata=1.1 --chunk=32 --raid-disks 3
>>> --size $[size/2] $dev1 $dev2 $dev3
>>> +check wait
>>> +check state UUU
>>> +check chunk 32
>>> +
>>> +mdadm $md0 --grow --chunk=64
>>> +check reshape
>>> +check wait
>>> +check chunk 64
>>> +
>>> +mdadm -S $md0
>>> +mdadm -As
>>> +check state UUU
>>> +check chunk 64
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" 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-raid" 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

* Re: [PATCH -v3 1/1] mdadm/test: Add one test case for raid5 reshape
From: Xiao Ni @ 2017-07-19  5:54 UTC (permalink / raw)
  To: Zhilong Liu; +Cc: linux-raid, jes sorensen
In-Reply-To: <c966357c-2a59-0cd1-af4c-d2861ff782be@suse.com>



----- Original Message -----
> From: "Zhilong Liu" <zlliu@suse.com>
> To: "Xiao Ni" <xni@redhat.com>, linux-raid@vger.kernel.org
> Cc: "jes sorensen" <jes.sorensen@gmail.com>
> Sent: Tuesday, July 18, 2017 6:34:33 PM
> Subject: Re: [PATCH -v3 1/1] mdadm/test: Add one test case for raid5 reshape
> 
> 
> 
> On 07/18/2017 04:55 PM, Xiao Ni wrote:
> > This case trys to allow raid5 reshape to use backwards direction.
> > It changes chunksize after reshape and stop the raid. Then start
> > the raid again.
> >
> > Signed-off-by: Xiao Ni <xni@redhat.com>
> > Suggested-by:  Jes Sorensen <jes.sorensen@gmail.com>
> > Suggested-by:  Zhilong Liu <zlliu@suse.com>
> > ---
> >   test           |  8 ++++++++
> >   tests/02r5grow | 16 ++++++++++++++++
> >   2 files changed, 24 insertions(+)
> >
> > diff --git a/test b/test
> > index 87e2df2..51da2dc 100755
> > --- a/test
> > +++ b/test
> > @@ -320,6 +320,14 @@ check() {
> >   		grep -sq "inactive" /proc/mdstat ||
> >   			die "array is not inactive!"
> >   		;;
> > +	# It only can be used when there is only one raid
> > +	chunk )
> > +		chunk_size=`cat /proc/mdstat  | awk -F',' '/chunk/{print $2}' | awk
> > '{print $1}' | sed s/k//g`
> 
> Hi,
>    only from my opinion, how about shrink the command:
> # awk -F',' '/chunk/{print $2}' /proc/mdstat | awk -F'[a-z]' '{print $1}'
> > +		if [ $chunk_size -ne $2 ]; then
> > +			echo >&2 "chunksize is not right"
> > +			exit 1
> > +		fi

Yes, you are a shell expert :)

> 
> how about using the func die()
> [  "$chunk_size" -ne "$2" ] &&
>    die "chunk should be $2, but is $chunk_size"

I tried this and did a simple test. It gave a error:
[root@dell-per210-01 mdadm]# ./test --tests=02r5grow
Testing on linux-3.10.0-685.el7.x86_64 kernel
tests/02r5grow... FAILED - see /var/tmp/02r5grow.log and /var/tmp/fail02r5grow.log for details
./test: line 80: [: too many arguments

It was introduced by this 20d10b4b. It looks like the check have errors:
if [ ! -z ${array[@]} -a ${#array[@]} -ge 1 ]


Regards
Xiao
> 
> Thanks,
> -Zhilong
> 
> > +		;;
> >   	* )
> >   		die "unknown check $1"
> >   		;;
> > diff --git a/tests/02r5grow b/tests/02r5grow
> > index 386e82e..20db4cd 100644
> > --- a/tests/02r5grow
> > +++ b/tests/02r5grow
> > @@ -34,3 +34,19 @@ check nosync
> >   sh tests/testdev $md0 3 $[size/2] 128
> >   
> >   mdadm -S $md0
> > +
> > +# create a raid5 array and change the chunk
> > +mdadm -CR $md0 --level raid5 --metadata=1.1 --chunk=32 --raid-disks 3
> > --size $[size/2] $dev1 $dev2 $dev3
> > +check wait
> > +check state UUU
> > +check chunk 32
> > +
> > +mdadm $md0 --grow --chunk=64
> > +check reshape
> > +check wait
> > +check chunk 64
> > +
> > +mdadm -S $md0
> > +mdadm -As
> > +check state UUU
> > +check chunk 64
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" 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

* Re: [PATCH] md/raid1: fix writebehind bio clone
From: Ming Lei @ 2017-07-19  2:13 UTC (permalink / raw)
  To: Shaohua Li
  Cc: open list:SOFTWARE RAID (Multiple Disks) SUPPORT, Shaohua Li,
	4.12+
In-Reply-To: <20170718230437.xdlmxjzedhm4jilf@kernel.org>

On Wed, Jul 19, 2017 at 7:04 AM, Shaohua Li <shli@kernel.org> wrote:
> On Wed, Jul 19, 2017 at 06:51:06AM +0800, Ming Lei wrote:
>> On Wed, Jul 19, 2017 at 1:21 AM, Shaohua Li <shli@kernel.org> wrote:
>> > From: Shaohua Li <shli@fb.com>
>> >
>> > After bio is submitted, we should not clone it as its bi_iter might be
>> > invalid by driver. This is the case of behind_master_bio. In certain
>> > situration, we could dispatch behind_master_bio immediately for the
>> > first disk and then clone it for other disks.
>>
>> If I understand it correctly, it should be caused by dispatching
>> the master bio during the loop either via flushing plug or md_wakeup_thread().
>>
>> If so, could we just add the bios into one temp pending list inside
>> the loop? Then
>> merge the temp list into conf->pending_bio_list after loop and
>> schedule to dispatch
>> them all? This way looks a bit more efficient.
>
> that's possible, but narrow_write_error will clone it after we already dispatch
> the bio, so I thought it's more clean to just clone one bio.

OK, looks fine:

Reviewed-by: Ming Lei <ming.lei@redhat.com>


Thanks,
Ming Lei

^ permalink raw reply

* Re: Have I any chance of restoring my Raid6 data?
From: Bogo Mipps @ 2017-07-19  2:01 UTC (permalink / raw)
  To: Joe Landman, Linux Raid
In-Reply-To: <56444baf-d0a9-13ab-6397-b5da7b9ed28a@gmail.com>

On 07/17/2017 10:28 AM, Joe Landman wrote:

> Why did you recreate the RAID (though you used --assume-clean) ? Did you 
> try mounting it before that?

Yes, and data appeared to be missing

>> Is there a chance of restoring my data?
> 
> Depends ... if you 'fsck -n /dev/md0' does it say there are errors?

Yes.  Unsuccessful.  Could find no superblocks, hence no ability to check.

>>
>> P.S. This line looks ominous? <md0: detected capacity change from 
>> 4000528203776 to 0> !!!
> 
> That is because md0 was stopped.

Ah - thanks. Your input appreciated. Not looking good, having now 
followed Peter's post.



^ permalink raw reply

* Re: Advice please re failed Raid6
From: Bogo Mipps @ 2017-07-19  1:52 UTC (permalink / raw)
  To: Peter Grandi, Linux Raid
In-Reply-To: <22892.649.826246.644975@tree.ty.sabi.co.uk>

On 07/17/2017 12:19 PM, Peter Grandi wrote:
> The line that worries a bit is this:
> 
>    Jul 5 21:06:18 keruru mdadm[2497]: RebuildFinished event
>    detected on md  device /dev/md0, component device mismatches
>    found: 1847058224 (on raid  level 6)
> 
> That seems to indicate that pretty much every block was a
> mismatch. Which would have happened if you put in a blank drive
> and then used '--create --assume-clean' instead of '--assemble
> --force'. But '--assume-clean" explicitly skips a rebuild, and I
> wonder whether you omitted that yoiu have triggered a "repair"
> in 'sync_action'. Also the message is reported by 'mdadm', and
> it may be that 'mdadm' was running in daemon mode and triggering
> a periodic "repair". I can't remember the defaults.
> 
> HOWEVER there is a very subtle detail: the order of the devices
> from '--examine' is: 0: 'sdb', 1: 'sdc', 2: 'sdd', 3: 'sde' but
> you recreated the set in a different order. The order of the
> devices does not matter if they have MD superblocks,.but here
> you are using '--create' to make new superblocks, and they order
> must match exactly the original order.
> 
>> root@keruru:/var/log# mdadm --create --assume-clean --level=6 --raid-devices=4 --size=1953382912 /dev/md0 /dev/sdb /dev/sde /dev/sdd /dev/sdc
> 
> Probably the best thing you can do is to rerun this with members
> "missing /dev/sdc /dev/sdd /dev/sde". and then use 'blkid /dev/md0'
> to check whether the data in it is recognized again. If so add
> '/dev/sdb'. I did a quick test here of something close to that
> and it worked...

Thanks for responding, Peter.  Much appreciated.

Did I do it right? (See below)

root@keruru:~# mdadm --create --assume-clean --level=6 --raid-devices=4 
--size=1953382912 /dev/md0 missing /dev/sdc /dev/sdd /dev/sde
mdadm: /dev/sdc appears to be part of a raid array:
     level=raid6 devices=4 ctime=Tue Jul 11 17:33:12 2017
mdadm: /dev/sdd appears to be part of a raid array:
     level=raid6 devices=4 ctime=Tue Jul 11 17:33:12 2017
mdadm: /dev/sde appears to be part of a raid array:
     level=raid6 devices=4 ctime=Tue Jul 11 17:33:12 2017
Continue creating array? y
mdadm: Defaulting to version 1.2 metadata
mdadm: array /dev/md0 started.

root@keruru:~# blkid /dev/md0
root@keruru:~#

root@keruru:~# cat /proc/mdstat
Personalities : [raid6] [raid5] [raid4]
md0 : active (auto-read-only) raid6 sde[3] sdd[2] sdc[1]
       3906765824 blocks super 1.2 level 6, 512k chunk, algorithm 2 
[4/3] [_UUU]

unused devices: <none>

No reading from blkid, so I am assuming 'not good'.  But I've done 
nothing further in case I misunderstood your suggestion.




^ permalink raw reply

* Re: [PATCH] md/raid1: fix writebehind bio clone
From: Shaohua Li @ 2017-07-18 23:04 UTC (permalink / raw)
  To: Ming Lei
  Cc: open list:SOFTWARE RAID (Multiple Disks) SUPPORT, Shaohua Li,
	4.12+
In-Reply-To: <CACVXFVNkTKhGqooGNww1iqSpw1dBCsDTJ6E1QX8a_P=CACG_1A@mail.gmail.com>

On Wed, Jul 19, 2017 at 06:51:06AM +0800, Ming Lei wrote:
> On Wed, Jul 19, 2017 at 1:21 AM, Shaohua Li <shli@kernel.org> wrote:
> > From: Shaohua Li <shli@fb.com>
> >
> > After bio is submitted, we should not clone it as its bi_iter might be
> > invalid by driver. This is the case of behind_master_bio. In certain
> > situration, we could dispatch behind_master_bio immediately for the
> > first disk and then clone it for other disks.
> 
> If I understand it correctly, it should be caused by dispatching
> the master bio during the loop either via flushing plug or md_wakeup_thread().
> 
> If so, could we just add the bios into one temp pending list inside
> the loop? Then
> merge the temp list into conf->pending_bio_list after loop and
> schedule to dispatch
> them all? This way looks a bit more efficient.

that's possible, but narrow_write_error will clone it after we already dispatch
the bio, so I thought it's more clean to just clone one bio.

Thanks,
Shaohua
 
> Thanks,
> Ming
> 
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=196383
> >
> > Reported-by: Markus <m4rkusxxl@web.de>
> > Cc: Ming Lei <tom.leiming@gmail.com>
> > Fix: 841c1316c7da(md: raid1: improve write behind)
> > Cc: stable@vger.kernel.org (4.12+)
> > Signed-off-by: Shaohua Li <shli@fb.com>
> > ---
> >  drivers/md/raid1.c | 30 ++++++++++--------------------
> >  1 file changed, 10 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> > index 8387eb1540cd..5622f521830b 100644
> > --- a/drivers/md/raid1.c
> > +++ b/drivers/md/raid1.c
> > @@ -484,10 +484,6 @@ static void raid1_end_write_request(struct bio *bio)
> >         }
> >
> >         if (behind) {
> > -               /* we release behind master bio when all write are done */
> > -               if (r1_bio->behind_master_bio == bio)
> > -                       to_put = NULL;
> > -
> >                 if (test_bit(WriteMostly, &rdev->flags))
> >                         atomic_dec(&r1_bio->behind_remaining);
> >
> > @@ -1080,7 +1076,7 @@ static void unfreeze_array(struct r1conf *conf)
> >         wake_up(&conf->wait_barrier);
> >  }
> >
> > -static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
> > +static void alloc_behind_master_bio(struct r1bio *r1_bio,
> >                                            struct bio *bio)
> >  {
> >         int size = bio->bi_iter.bi_size;
> > @@ -1090,7 +1086,7 @@ static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
> >
> >         behind_bio = bio_alloc_mddev(GFP_NOIO, vcnt, r1_bio->mddev);
> >         if (!behind_bio)
> > -               goto fail;
> > +               return;
> >
> >         /* discard op, we don't support writezero/writesame yet */
> >         if (!bio_has_data(bio))
> > @@ -1115,14 +1111,13 @@ static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
> >         r1_bio->behind_master_bio = behind_bio;;
> >         set_bit(R1BIO_BehindIO, &r1_bio->state);
> >
> > -       return behind_bio;
> > +       return;
> >
> >  free_pages:
> >         pr_debug("%dB behind alloc failed, doing sync I/O\n",
> >                  bio->bi_iter.bi_size);
> >         bio_free_pages(behind_bio);
> > -fail:
> > -       return behind_bio;
> > +       bio_put(behind_bio);
> >  }
> >
> >  struct raid1_plug_cb {
> > @@ -1475,7 +1470,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> >                             (atomic_read(&bitmap->behind_writes)
> >                              < mddev->bitmap_info.max_write_behind) &&
> >                             !waitqueue_active(&bitmap->behind_wait)) {
> > -                               mbio = alloc_behind_master_bio(r1_bio, bio);
> > +                               alloc_behind_master_bio(r1_bio, bio);
> >                         }
> >
> >                         bitmap_startwrite(bitmap, r1_bio->sector,
> > @@ -1485,14 +1480,11 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> >                         first_clone = 0;
> >                 }
> >
> > -               if (!mbio) {
> > -                       if (r1_bio->behind_master_bio)
> > -                               mbio = bio_clone_fast(r1_bio->behind_master_bio,
> > -                                                     GFP_NOIO,
> > -                                                     mddev->bio_set);
> > -                       else
> > -                               mbio = bio_clone_fast(bio, GFP_NOIO, mddev->bio_set);
> > -               }
> > +               if (r1_bio->behind_master_bio)
> > +                       mbio = bio_clone_fast(r1_bio->behind_master_bio,
> > +                                             GFP_NOIO, mddev->bio_set);
> > +               else
> > +                       mbio = bio_clone_fast(bio, GFP_NOIO, mddev->bio_set);
> >
> >                 if (r1_bio->behind_master_bio) {
> >                         if (test_bit(WriteMostly, &conf->mirrors[i].rdev->flags))
> > @@ -2346,8 +2338,6 @@ static int narrow_write_error(struct r1bio *r1_bio, int i)
> >                         wbio = bio_clone_fast(r1_bio->behind_master_bio,
> >                                               GFP_NOIO,
> >                                               mddev->bio_set);
> > -                       /* We really need a _all clone */
> > -                       wbio->bi_iter = (struct bvec_iter){ 0 };
> >                 } else {
> >                         wbio = bio_clone_fast(r1_bio->master_bio, GFP_NOIO,
> >                                               mddev->bio_set);
> > --
> > 2.11.0
> >

^ permalink raw reply

* Re: [PATCH] md/raid1: fix writebehind bio clone
From: Ming Lei @ 2017-07-18 22:51 UTC (permalink / raw)
  To: Shaohua Li
  Cc: open list:SOFTWARE RAID (Multiple Disks) SUPPORT, Shaohua Li,
	4.12+
In-Reply-To: <90e5fed8cd2b5dff7b393de121460671ebdd1260.1500398441.git.shli@fb.com>

On Wed, Jul 19, 2017 at 1:21 AM, Shaohua Li <shli@kernel.org> wrote:
> From: Shaohua Li <shli@fb.com>
>
> After bio is submitted, we should not clone it as its bi_iter might be
> invalid by driver. This is the case of behind_master_bio. In certain
> situration, we could dispatch behind_master_bio immediately for the
> first disk and then clone it for other disks.

If I understand it correctly, it should be caused by dispatching
the master bio during the loop either via flushing plug or md_wakeup_thread().

If so, could we just add the bios into one temp pending list inside
the loop? Then
merge the temp list into conf->pending_bio_list after loop and
schedule to dispatch
them all? This way looks a bit more efficient.

Thanks,
Ming

>
> https://bugzilla.kernel.org/show_bug.cgi?id=196383
>
> Reported-by: Markus <m4rkusxxl@web.de>
> Cc: Ming Lei <tom.leiming@gmail.com>
> Fix: 841c1316c7da(md: raid1: improve write behind)
> Cc: stable@vger.kernel.org (4.12+)
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
>  drivers/md/raid1.c | 30 ++++++++++--------------------
>  1 file changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 8387eb1540cd..5622f521830b 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -484,10 +484,6 @@ static void raid1_end_write_request(struct bio *bio)
>         }
>
>         if (behind) {
> -               /* we release behind master bio when all write are done */
> -               if (r1_bio->behind_master_bio == bio)
> -                       to_put = NULL;
> -
>                 if (test_bit(WriteMostly, &rdev->flags))
>                         atomic_dec(&r1_bio->behind_remaining);
>
> @@ -1080,7 +1076,7 @@ static void unfreeze_array(struct r1conf *conf)
>         wake_up(&conf->wait_barrier);
>  }
>
> -static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
> +static void alloc_behind_master_bio(struct r1bio *r1_bio,
>                                            struct bio *bio)
>  {
>         int size = bio->bi_iter.bi_size;
> @@ -1090,7 +1086,7 @@ static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
>
>         behind_bio = bio_alloc_mddev(GFP_NOIO, vcnt, r1_bio->mddev);
>         if (!behind_bio)
> -               goto fail;
> +               return;
>
>         /* discard op, we don't support writezero/writesame yet */
>         if (!bio_has_data(bio))
> @@ -1115,14 +1111,13 @@ static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
>         r1_bio->behind_master_bio = behind_bio;;
>         set_bit(R1BIO_BehindIO, &r1_bio->state);
>
> -       return behind_bio;
> +       return;
>
>  free_pages:
>         pr_debug("%dB behind alloc failed, doing sync I/O\n",
>                  bio->bi_iter.bi_size);
>         bio_free_pages(behind_bio);
> -fail:
> -       return behind_bio;
> +       bio_put(behind_bio);
>  }
>
>  struct raid1_plug_cb {
> @@ -1475,7 +1470,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>                             (atomic_read(&bitmap->behind_writes)
>                              < mddev->bitmap_info.max_write_behind) &&
>                             !waitqueue_active(&bitmap->behind_wait)) {
> -                               mbio = alloc_behind_master_bio(r1_bio, bio);
> +                               alloc_behind_master_bio(r1_bio, bio);
>                         }
>
>                         bitmap_startwrite(bitmap, r1_bio->sector,
> @@ -1485,14 +1480,11 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>                         first_clone = 0;
>                 }
>
> -               if (!mbio) {
> -                       if (r1_bio->behind_master_bio)
> -                               mbio = bio_clone_fast(r1_bio->behind_master_bio,
> -                                                     GFP_NOIO,
> -                                                     mddev->bio_set);
> -                       else
> -                               mbio = bio_clone_fast(bio, GFP_NOIO, mddev->bio_set);
> -               }
> +               if (r1_bio->behind_master_bio)
> +                       mbio = bio_clone_fast(r1_bio->behind_master_bio,
> +                                             GFP_NOIO, mddev->bio_set);
> +               else
> +                       mbio = bio_clone_fast(bio, GFP_NOIO, mddev->bio_set);
>
>                 if (r1_bio->behind_master_bio) {
>                         if (test_bit(WriteMostly, &conf->mirrors[i].rdev->flags))
> @@ -2346,8 +2338,6 @@ static int narrow_write_error(struct r1bio *r1_bio, int i)
>                         wbio = bio_clone_fast(r1_bio->behind_master_bio,
>                                               GFP_NOIO,
>                                               mddev->bio_set);
> -                       /* We really need a _all clone */
> -                       wbio->bi_iter = (struct bvec_iter){ 0 };
>                 } else {
>                         wbio = bio_clone_fast(r1_bio->master_bio, GFP_NOIO,
>                                               mddev->bio_set);
> --
> 2.11.0
>

^ permalink raw reply

* Re: 3-disk fail on raid-6, examining my options...
From: Maarten @ 2017-07-18 21:29 UTC (permalink / raw)
  To: linux-raid
In-Reply-To: <20170718202550.GA2533@animx.eu.org>

On 07/18/2017 10:25 PM, Wakko Warner wrote:
> Wols Lists wrote:
>> On 18/07/17 18:20, Maarten wrote:
>>> Now from what I've gathered over the years and from earlier incidents, I
>>> have now 1 (one) chance left to rescue data off this array; by hopefully
>>> cloning the bad 3rd-failed drive with the aid of dd_rescue and
>>> re-assembling --force the fully-degraded array. (Only IF that drive is
>>> still responsive and can be cloned)
>>
>> If it clones successfully, great. If it clones, but with badblocks, I
>> keep on asking - is there any way we can work together to turn
>> dd-rescue's log into a utility that will flag failed blocks as "unreadable"?
> 
> I wrote a shell script that will output a device mapper table to do this. 
> It will do either zero or error targets for failed blocks.  It's not
> automatic and does require a block device (loop for files).  I've used this
> several times at work and works for me.
> 
> I'm not sure if this is what you're talking about or not, but if you want
> the script, I'll post it.

For me, I don't think it will make much difference. On top of the array
there are a number of LVM volumes. Of most of them I have full and
current backups. Some of it is [now] free space. There are two volumes
that hold data that is both important to me and not backed up recently
enough.

Those two volumes together take up about 33%-40% of the total size. So
the chances of bad sectors affecting these are also (somewhat) smaller.
And the data will still be valuable to me, even when it has some silent
corruption.

No, my main question that I seek a definitive answer to is whether the
two drives that failed earlier hold anything of worth, or that any
salvaging data using them is out of the question.

In the mean time, I occupy my time with copying the data I wanted to
copy onto the array to a remote system, and to make sure all my backups
and copies that were not redundant get proper redundancy. I will not
'touch' the machine with the broken array until all that is sorted (it
has an other raid-6 array, which is healthy... for now at least).

I hope the 3rd failed drive won't deteriorate during that time, but
under the circumstances, I'm going to take that risk nonetheless.

regards,
Maarten

^ permalink raw reply

* Re: 3-disk fail on raid-6, examining my options...
From: Wakko Warner @ 2017-07-18 20:25 UTC (permalink / raw)
  To: Wols Lists; +Cc: Maarten, linux-raid
In-Reply-To: <596E6D72.8050108@youngman.org.uk>

Wols Lists wrote:
> On 18/07/17 18:20, Maarten wrote:
> > Now from what I've gathered over the years and from earlier incidents, I
> > have now 1 (one) chance left to rescue data off this array; by hopefully
> > cloning the bad 3rd-failed drive with the aid of dd_rescue and
> > re-assembling --force the fully-degraded array. (Only IF that drive is
> > still responsive and can be cloned)
> 
> If it clones successfully, great. If it clones, but with badblocks, I
> keep on asking - is there any way we can work together to turn
> dd-rescue's log into a utility that will flag failed blocks as "unreadable"?

I wrote a shell script that will output a device mapper table to do this. 
It will do either zero or error targets for failed blocks.  It's not
automatic and does require a block device (loop for files).  I've used this
several times at work and works for me.

I'm not sure if this is what you're talking about or not, but if you want
the script, I'll post it.

-- 
 Microsoft has beaten Volkswagen's world record.  Volkswagen only created 22
 million bugs.

^ permalink raw reply

* Re: 3-disk fail on raid-6, examining my options...
From: Wols Lists @ 2017-07-18 20:20 UTC (permalink / raw)
  To: Maarten, linux-raid
In-Reply-To: <07b77b80-4bee-3820-6a0d-3323ef06a3f3@ultratux.net>

On 18/07/17 18:20, Maarten wrote:
> Now from what I've gathered over the years and from earlier incidents, I
> have now 1 (one) chance left to rescue data off this array; by hopefully
> cloning the bad 3rd-failed drive with the aid of dd_rescue and
> re-assembling --force the fully-degraded array. (Only IF that drive is
> still responsive and can be cloned)

If it clones successfully, great. If it clones, but with badblocks, I
keep on asking - is there any way we can work together to turn
dd-rescue's log into a utility that will flag failed blocks as "unreadable"?

This project is mentioned on the wiki, the idea being that you can tell
the hard drive to return a read error if the computer tries to access
certain blocks. This "fake" read error means that we can then use a
partially copied disk to rebuild an array knowing that we will get an
error rather than silent corruption if we try and access the faulty block.

At least then, we know what is damaged rather than trying to
integrity-check an entire disk in the hope that we'll detect the corruption.

Cheers,
Wol

^ permalink raw reply

* [GIT PULL] MD update for 4.13-rc2
From: Shaohua Li @ 2017-07-18 18:49 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, linux-raid

Hi,
Please pull 3 MD fixes:
- raid5-ppl fix by Artur. This one is introduced in this release cycle.
- raid5 reshape fix by Xiao. This is an old bug and will be added to stable.
- Bitmap fix by Guoqing.

Thanks,
Shaohua

The following changes since commit af3c8d98508d37541d4bf57f13a984a7f73a328c:

  Merge tag 'drm-for-v4.13' of git://people.freedesktop.org/~airlied/linux (2017-07-09 18:48:37 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/shli/md.git tags/md/4.13-rc2

for you to fetch changes up to 6409e84ec58fc4c0085d8921f8e01815dc871971:

  raid5-ppl: use BIOSET_NEED_BVECS when creating bioset (2017-07-12 09:26:37 -0700)

----------------------------------------------------------------
Artur Paszkiewicz (1):
      raid5-ppl: use BIOSET_NEED_BVECS when creating bioset

Guoqing Jiang (1):
      md/bitmap: don't read page from device with Bitmap_sync

Xiao Ni (1):
      Raid5 should update rdev->sectors after reshape

 drivers/md/bitmap.c    | 3 ++-
 drivers/md/md.h        | 4 +++-
 drivers/md/raid5-ppl.c | 2 +-
 drivers/md/raid5.c     | 4 +---
 4 files changed, 7 insertions(+), 6 deletions(-)

^ permalink raw reply

* [PATCH] md/raid1: fix writebehind bio clone
From: Shaohua Li @ 2017-07-18 17:21 UTC (permalink / raw)
  To: linux-raid; +Cc: Shaohua Li, Ming Lei, 4.12+

From: Shaohua Li <shli@fb.com>

After bio is submitted, we should not clone it as its bi_iter might be
invalid by driver. This is the case of behind_master_bio. In certain
situration, we could dispatch behind_master_bio immediately for the
first disk and then clone it for other disks.

https://bugzilla.kernel.org/show_bug.cgi?id=196383

Reported-by: Markus <m4rkusxxl@web.de>
Cc: Ming Lei <tom.leiming@gmail.com>
Fix: 841c1316c7da(md: raid1: improve write behind)
Cc: stable@vger.kernel.org (4.12+)
Signed-off-by: Shaohua Li <shli@fb.com>
---
 drivers/md/raid1.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 8387eb1540cd..5622f521830b 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -484,10 +484,6 @@ static void raid1_end_write_request(struct bio *bio)
 	}
 
 	if (behind) {
-		/* we release behind master bio when all write are done */
-		if (r1_bio->behind_master_bio == bio)
-			to_put = NULL;
-
 		if (test_bit(WriteMostly, &rdev->flags))
 			atomic_dec(&r1_bio->behind_remaining);
 
@@ -1080,7 +1076,7 @@ static void unfreeze_array(struct r1conf *conf)
 	wake_up(&conf->wait_barrier);
 }
 
-static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
+static void alloc_behind_master_bio(struct r1bio *r1_bio,
 					   struct bio *bio)
 {
 	int size = bio->bi_iter.bi_size;
@@ -1090,7 +1086,7 @@ static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
 
 	behind_bio = bio_alloc_mddev(GFP_NOIO, vcnt, r1_bio->mddev);
 	if (!behind_bio)
-		goto fail;
+		return;
 
 	/* discard op, we don't support writezero/writesame yet */
 	if (!bio_has_data(bio))
@@ -1115,14 +1111,13 @@ static struct bio *alloc_behind_master_bio(struct r1bio *r1_bio,
 	r1_bio->behind_master_bio = behind_bio;;
 	set_bit(R1BIO_BehindIO, &r1_bio->state);
 
-	return behind_bio;
+	return;
 
 free_pages:
 	pr_debug("%dB behind alloc failed, doing sync I/O\n",
 		 bio->bi_iter.bi_size);
 	bio_free_pages(behind_bio);
-fail:
-	return behind_bio;
+	bio_put(behind_bio);
 }
 
 struct raid1_plug_cb {
@@ -1475,7 +1470,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
 			    (atomic_read(&bitmap->behind_writes)
 			     < mddev->bitmap_info.max_write_behind) &&
 			    !waitqueue_active(&bitmap->behind_wait)) {
-				mbio = alloc_behind_master_bio(r1_bio, bio);
+				alloc_behind_master_bio(r1_bio, bio);
 			}
 
 			bitmap_startwrite(bitmap, r1_bio->sector,
@@ -1485,14 +1480,11 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
 			first_clone = 0;
 		}
 
-		if (!mbio) {
-			if (r1_bio->behind_master_bio)
-				mbio = bio_clone_fast(r1_bio->behind_master_bio,
-						      GFP_NOIO,
-						      mddev->bio_set);
-			else
-				mbio = bio_clone_fast(bio, GFP_NOIO, mddev->bio_set);
-		}
+		if (r1_bio->behind_master_bio)
+			mbio = bio_clone_fast(r1_bio->behind_master_bio,
+					      GFP_NOIO, mddev->bio_set);
+		else
+			mbio = bio_clone_fast(bio, GFP_NOIO, mddev->bio_set);
 
 		if (r1_bio->behind_master_bio) {
 			if (test_bit(WriteMostly, &conf->mirrors[i].rdev->flags))
@@ -2346,8 +2338,6 @@ static int narrow_write_error(struct r1bio *r1_bio, int i)
 			wbio = bio_clone_fast(r1_bio->behind_master_bio,
 					      GFP_NOIO,
 					      mddev->bio_set);
-			/* We really need a _all clone */
-			wbio->bi_iter = (struct bvec_iter){ 0 };
 		} else {
 			wbio = bio_clone_fast(r1_bio->master_bio, GFP_NOIO,
 					      mddev->bio_set);
-- 
2.11.0

^ permalink raw reply related

* 3-disk fail on raid-6, examining my options...
From: Maarten @ 2017-07-18 17:20 UTC (permalink / raw)
  To: linux-raid

Argh.. Murphy can be such a troll... :(

Hi all,

While I was in the process of migrating all my raid-6 arrays to raid-1
arrays (with either two or three member disks), I got stung severely.
(Obviously I shouldn't have been so stupid as to write to an array not
yet fully copied, but that is now too late to undo)


  What probably happened:
A six-disk raid-6 array suffered a simultaneous two-disk failure which
went unnoticed for a number of hours, and then inevitably got hit by a
-catastrophic- 3rd disk failure during the following night.


The first two disks that failed have exactly identical event counters
according to mdadm -E <disk device> which leads me to believe that it is
probably the SATA card/controller that failed/oops'ed, not the disks
themselves. But at this point that has not yet been verified.

The third disk, and the array, have a substantially higher event
counter. This makes complete sense, since the array was being actively
_written_ to at the time. (Yes, alas...) *Bangs head against desk*


Now from what I've gathered over the years and from earlier incidents, I
have now 1 (one) chance left to rescue data off this array; by hopefully
cloning the bad 3rd-failed drive with the aid of dd_rescue and
re-assembling --force the fully-degraded array. (Only IF that drive is
still responsive and can be cloned)

My feeling is, the two ``good'' drives with the lower event counter are
now more useful as paperweights than to help restore any data... But I
like to have certainty before I try other ways to restore (or recreate)
data...

Is there any hope?


Here are some snippets from mdadm:

md0 : active raid6 sdh1[10] sdj1[8] sdm1[9] sdb1[3](F) sde1[7](F) sdd1[6](F)
7799470080 blocks super 1.2 level 6, 512k chunk, algorithm 2 [6/3] [U___UU]


mdadm -E /dev/sde1:
    Update Time : Mon Jul 17 15:49:44 2017
       Checksum : fdc7fdd7 - correct
         Events : 58235
    Array State : AAAAAA ('A' == active, '.' == missing)

mdadm -E /dev/sdb1:
    Update Time : Mon Jul 17 15:49:44 2017
       Checksum : cd97800c - correct
         Events : 58235
    Array State : AAAAAA ('A' == active, '.' == missing)

mdadm -E /dev/sdd1:
    Update Time : Tue Jul 18 01:47:33 2017
       Checksum : d00eff1d - correct
         Events : 69129
    Array State : AA..AA ('A' == active, '.' == missing)

mdadm --detail /dev/md0
 Failed Devices : 3
         Events : 69132


Thanks for any insights...

regards,
Maarten

^ permalink raw reply

* Re: [PATCH -v3 1/1] mdadm/test: Add one test case for raid5 reshape
From: Zhilong Liu @ 2017-07-18 10:34 UTC (permalink / raw)
  To: Xiao Ni, linux-raid; +Cc: jes.sorensen
In-Reply-To: <1500368140-23563-1-git-send-email-xni@redhat.com>



On 07/18/2017 04:55 PM, Xiao Ni wrote:
> This case trys to allow raid5 reshape to use backwards direction.
> It changes chunksize after reshape and stop the raid. Then start
> the raid again.
>
> Signed-off-by: Xiao Ni <xni@redhat.com>
> Suggested-by:  Jes Sorensen <jes.sorensen@gmail.com>
> Suggested-by:  Zhilong Liu <zlliu@suse.com>
> ---
>   test           |  8 ++++++++
>   tests/02r5grow | 16 ++++++++++++++++
>   2 files changed, 24 insertions(+)
>
> diff --git a/test b/test
> index 87e2df2..51da2dc 100755
> --- a/test
> +++ b/test
> @@ -320,6 +320,14 @@ check() {
>   		grep -sq "inactive" /proc/mdstat ||
>   			die "array is not inactive!"
>   		;;
> +	# It only can be used when there is only one raid
> +	chunk )
> +		chunk_size=`cat /proc/mdstat  | awk -F',' '/chunk/{print $2}' | awk '{print $1}' | sed s/k//g`

Hi,
   only from my opinion, how about shrink the command:
# awk -F',' '/chunk/{print $2}' /proc/mdstat | awk -F'[a-z]' '{print $1}'
> +		if [ $chunk_size -ne $2 ]; then
> +			echo >&2 "chunksize is not right"
> +			exit 1
> +		fi

how about using the func die()
[  "$chunk_size" -ne "$2" ] &&
   die "chunk should be $2, but is $chunk_size"

Thanks,
-Zhilong

> +		;;
>   	* )
>   		die "unknown check $1"
>   		;;
> diff --git a/tests/02r5grow b/tests/02r5grow
> index 386e82e..20db4cd 100644
> --- a/tests/02r5grow
> +++ b/tests/02r5grow
> @@ -34,3 +34,19 @@ check nosync
>   sh tests/testdev $md0 3 $[size/2] 128
>   
>   mdadm -S $md0
> +
> +# create a raid5 array and change the chunk
> +mdadm -CR $md0 --level raid5 --metadata=1.1 --chunk=32 --raid-disks 3 --size $[size/2] $dev1 $dev2 $dev3
> +check wait
> +check state UUU
> +check chunk 32
> +
> +mdadm $md0 --grow --chunk=64
> +check reshape
> +check wait
> +check chunk 64
> +
> +mdadm -S $md0
> +mdadm -As
> +check state UUU
> +check chunk 64


^ permalink raw reply

* Re: [PATCH -v3 1/1] mdadm/test: Add one test case for raid5 reshape
From: Xiao Ni @ 2017-07-18  9:48 UTC (permalink / raw)
  To: Paul Menzel; +Cc: linux-raid, Jes Sorensen, Zhilong Liu
In-Reply-To: <e76ccf61-238e-00da-a00f-48354264c162@molgen.mpg.de>



----- Original Message -----
> From: "Paul Menzel" <pmenzel@molgen.mpg.de>
> To: "Xiao Ni" <xni@redhat.com>
> Cc: linux-raid@vger.kernel.org, "Jes Sorensen" <jes.sorensen@gmail.com>, "Zhilong Liu" <zlliu@suse.com>
> Sent: Tuesday, July 18, 2017 5:03:11 PM
> Subject: Re: [PATCH -v3 1/1] mdadm/test: Add one test case for raid5 reshape
> 
> Dear Xiao,
> 
> 
> On 07/18/17 10:55, Xiao Ni wrote:
> > This case trys to allow raid5 reshape to use backwards direction.
> 
> s/trys/tries/
> 
> > It changes chunksize after reshape and stop the raid. Then start
> 
> stop*s* and start*s*

Thanks for pointing these errors. I should be more careful. 

> 
> > the raid again.
> > 
> > Signed-off-by: Xiao Ni <xni@redhat.com>
> > Suggested-by:  Jes Sorensen <jes.sorensen@gmail.com>
> > Suggested-by:  Zhilong Liu <zlliu@suse.com>
> > ---
> >   test           |  8 ++++++++
> >   tests/02r5grow | 16 ++++++++++++++++
> >   2 files changed, 24 insertions(+)
> > 
> > diff --git a/test b/test
> > index 87e2df2..51da2dc 100755
> > --- a/test
> > +++ b/test
> > @@ -320,6 +320,14 @@ check() {
> >   		grep -sq "inactive" /proc/mdstat ||
> >   			die "array is not inactive!"
> >   		;;
> > +	# It only can be used when there is only one raid
> > +	chunk )
> > +		chunk_size=`cat /proc/mdstat  | awk -F',' '/chunk/{print $2}' | awk
> > '{print $1}' | sed s/k//g`
> > +		if [ $chunk_size -ne $2 ]; then
> > +			echo >&2 "chunksize is not right"
> 
> Maybe output the two different values?
> 
>  > chunksize is not right. It should be … but is ….

Sure, it's better to give the values. 
> 
> > +			exit 1
> > +		fi
> > +		;;
> >   	* )
> >   		die "unknown check $1"
> >   		;;
> > diff --git a/tests/02r5grow b/tests/02r5grow
> > index 386e82e..20db4cd 100644
> > --- a/tests/02r5grow
> > +++ b/tests/02r5grow
> > @@ -34,3 +34,19 @@ check nosync
> >   sh tests/testdev $md0 3 $[size/2] 128
> >   
> >   mdadm -S $md0
> > +
> > +# create a raid5 array and change the chunk
> > +mdadm -CR $md0 --level raid5 --metadata=1.1 --chunk=32 --raid-disks 3
> > --size $[size/2] $dev1 $dev2 $dev3
> > +check wait
> > +check state UUU
> > +check chunk 32
> > +
> > +mdadm $md0 --grow --chunk=64
> > +check reshape
> > +check wait
> > +check chunk 64
> > +
> > +mdadm -S $md0
> > +mdadm -As
> > +check state UUU
> > +check chunk 64
> 
> Do you need to run `mdadm -S $md0` after the assembly again?

Yes, it should be stopped again.

Regards
Xiao
> 
> 
> Kind regards,
> 
> Paul
> 

^ permalink raw reply

* Re: [PATCH -v3 1/1] mdadm/test: Add one test case for raid5 reshape
From: Paul Menzel @ 2017-07-18  9:03 UTC (permalink / raw)
  To: Xiao Ni; +Cc: linux-raid, Jes Sorensen, Zhilong Liu
In-Reply-To: <1500368140-23563-1-git-send-email-xni@redhat.com>

Dear Xiao,


On 07/18/17 10:55, Xiao Ni wrote:
> This case trys to allow raid5 reshape to use backwards direction.

s/trys/tries/

> It changes chunksize after reshape and stop the raid. Then start

stop*s* and start*s*

> the raid again.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> Suggested-by:  Jes Sorensen <jes.sorensen@gmail.com>
> Suggested-by:  Zhilong Liu <zlliu@suse.com>
> ---
>   test           |  8 ++++++++
>   tests/02r5grow | 16 ++++++++++++++++
>   2 files changed, 24 insertions(+)
> 
> diff --git a/test b/test
> index 87e2df2..51da2dc 100755
> --- a/test
> +++ b/test
> @@ -320,6 +320,14 @@ check() {
>   		grep -sq "inactive" /proc/mdstat ||
>   			die "array is not inactive!"
>   		;;
> +	# It only can be used when there is only one raid
> +	chunk )
> +		chunk_size=`cat /proc/mdstat  | awk -F',' '/chunk/{print $2}' | awk '{print $1}' | sed s/k//g`
> +		if [ $chunk_size -ne $2 ]; then
> +			echo >&2 "chunksize is not right"

Maybe output the two different values?

 > chunksize is not right. It should be … but is ….

> +			exit 1
> +		fi
> +		;;
>   	* )
>   		die "unknown check $1"
>   		;;
> diff --git a/tests/02r5grow b/tests/02r5grow
> index 386e82e..20db4cd 100644
> --- a/tests/02r5grow
> +++ b/tests/02r5grow
> @@ -34,3 +34,19 @@ check nosync
>   sh tests/testdev $md0 3 $[size/2] 128
>   
>   mdadm -S $md0
> +
> +# create a raid5 array and change the chunk
> +mdadm -CR $md0 --level raid5 --metadata=1.1 --chunk=32 --raid-disks 3 --size $[size/2] $dev1 $dev2 $dev3
> +check wait
> +check state UUU
> +check chunk 32
> +
> +mdadm $md0 --grow --chunk=64
> +check reshape
> +check wait
> +check chunk 64
> +
> +mdadm -S $md0
> +mdadm -As
> +check state UUU
> +check chunk 64

Do you need to run `mdadm -S $md0` after the assembly again?


Kind regards,

Paul

^ permalink raw reply

* Re: Linear device of two arrays
From: Veljko @ 2017-07-18  8:58 UTC (permalink / raw)
  To: linux-raid
In-Reply-To: <38e015fa-a07c-8f1a-3a04-384d973966be@gmail.com>

On 07/17/2017 12:16 PM, Veljko wrote:
> On 07/15/2017 02:12 AM, NeilBrown wrote:
>
>> So command should be
>>
>>>>   dd if=/dev/md2 of=SOMEWHERE/SAFE bs=1M skip=5626590
>>
>> and expect it to create a 3M file.
>>
>> Use this 'skip' number of the 'seek' number later.
>>
>> NeilBrown
>
> Thanks, Neil, now it makes more sense.
>
> I tried to create new linear device, but mdadm is complaining about
> data-offset:
>
> # mdadm -C /dev/md4 -l linear -n 2 --rounding=1M -e 1.0 --data-offset=0
> /dev/md2 /dev/md3
>  mdadm: invalid data-offset: 0
>
> I'm using Debian 8.8 if it makes any difference.
>
> # mdadm -V
>  mdadm - v3.3.2 - 21st August 2014
>
> What could be the problem?

I noticed that md2 and md3 use 1.2 metadata. Can that be the issue? 
Trying to create 1.0 metadata for md4?


^ permalink raw reply

* [PATCH -v3 1/1] mdadm/test: Add one test case for raid5 reshape
From: Xiao Ni @ 2017-07-18  8:55 UTC (permalink / raw)
  To: linux-raid; +Cc: jes.sorensen, zlliu

This case trys to allow raid5 reshape to use backwards direction.
It changes chunksize after reshape and stop the raid. Then start
the raid again.

Signed-off-by: Xiao Ni <xni@redhat.com>
Suggested-by:  Jes Sorensen <jes.sorensen@gmail.com>
Suggested-by:  Zhilong Liu <zlliu@suse.com> 
---
 test           |  8 ++++++++
 tests/02r5grow | 16 ++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/test b/test
index 87e2df2..51da2dc 100755
--- a/test
+++ b/test
@@ -320,6 +320,14 @@ check() {
 		grep -sq "inactive" /proc/mdstat ||
 			die "array is not inactive!"
 		;;
+	# It only can be used when there is only one raid
+	chunk )
+		chunk_size=`cat /proc/mdstat  | awk -F',' '/chunk/{print $2}' | awk '{print $1}' | sed s/k//g`
+		if [ $chunk_size -ne $2 ]; then
+			echo >&2 "chunksize is not right"
+			exit 1
+		fi
+		;;
 	* )
 		die "unknown check $1"
 		;;
diff --git a/tests/02r5grow b/tests/02r5grow
index 386e82e..20db4cd 100644
--- a/tests/02r5grow
+++ b/tests/02r5grow
@@ -34,3 +34,19 @@ check nosync
 sh tests/testdev $md0 3 $[size/2] 128
 
 mdadm -S $md0
+
+# create a raid5 array and change the chunk
+mdadm -CR $md0 --level raid5 --metadata=1.1 --chunk=32 --raid-disks 3 --size $[size/2] $dev1 $dev2 $dev3
+check wait
+check state UUU
+check chunk 32
+
+mdadm $md0 --grow --chunk=64
+check reshape
+check wait
+check chunk 64
+
+mdadm -S $md0
+mdadm -As
+check state UUU
+check chunk 64
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH -v2 1/1] mdadm/test: Add one test case for raid5 reshape
From: Xiao Ni @ 2017-07-18  8:43 UTC (permalink / raw)
  To: Zhilong Liu; +Cc: linux-raid, jes sorensen
In-Reply-To: <3d22eddd-79cc-4877-8c0d-497ea80e829b@suse.com>



----- Original Message -----
> From: "Zhilong Liu" <zlliu@suse.com>
> To: "Xiao Ni" <xni@redhat.com>, linux-raid@vger.kernel.org
> Cc: "jes sorensen" <jes.sorensen@gmail.com>
> Sent: Tuesday, July 18, 2017 12:37:32 PM
> Subject: Re: [PATCH -v2 1/1] mdadm/test: Add one test case for raid5 reshape
> 
> 
> 
> On 07/17/2017 05:27 PM, Xiao Ni wrote:
> > This case trys to allow raid5 reshape to use backwards direction.
> > It changes chunksize after reshape and stop the raid. Then start
> > the raid again.
> >
> > Signed-off-by: Xiao Ni <xni@redhat.com>
> > ---
> >   tests/02r5grow | 13 +++++++++++++
> >   1 file changed, 13 insertions(+)
> >
> > diff --git a/tests/02r5grow b/tests/02r5grow
> > index 386e82e..64c5599 100644
> > --- a/tests/02r5grow
> > +++ b/tests/02r5grow
> > @@ -34,3 +34,16 @@ check nosync
> >   sh tests/testdev $md0 3 $[size/2] 128
> >   
> >   mdadm -S $md0
> > +
> > +# create a raid5 array and change the chunk
> > +mdadm -CR $md0 --level raid5 --metadata=1.1 --chunk=32 --raid-disks 3
> > --size $[size/2] $dev1 $dev2 $dev3
> > +check wait
> > +check state UUU
> > +
> 
> Hi,
>     Just my opinion, it should check the chunk-size properly if you
> wanna test 'chunk'.
>     Maybe can do following three things in a function.
> 
>  From sysfs:
> new_size=$(cat /sys/block/md0/md/chunk_size)
> if [ "$(expr 32 \* 1024)" -eq "$new_size" ]; then
> ...
> fi
> 
>  From procfs:
> # cat /proc/mdstat  | awk -F',' '/chunk/{print $2}'
> 32k chunk
> 
>  From superblock:
> mdadm -D /dev/md0 | grep "Chunk Size"
> 
>    It's important for testing based on expected environment.
> 
> > +mdadm $md0 --grow --chunk=64
> > +check reshape
> > +check wait
> 
> Here too.
> 
> > +
> > +mdadm -S $md0
> > +mdadm -As
> > +check state UUU
> 
> Also here.

Thanks for these suggestions. I'll add this check too.

Regards
Xiao
> 
> Thanks,
> -Zhilong
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" 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

* Re: [PATCH -v2 1/1] mdadm/test: Add one test case for raid5 reshape
From: Zhilong Liu @ 2017-07-18  4:37 UTC (permalink / raw)
  To: Xiao Ni, linux-raid; +Cc: jes.sorensen
In-Reply-To: <1500283627-15195-1-git-send-email-xni@redhat.com>



On 07/17/2017 05:27 PM, Xiao Ni wrote:
> This case trys to allow raid5 reshape to use backwards direction.
> It changes chunksize after reshape and stop the raid. Then start
> the raid again.
>
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
>   tests/02r5grow | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/tests/02r5grow b/tests/02r5grow
> index 386e82e..64c5599 100644
> --- a/tests/02r5grow
> +++ b/tests/02r5grow
> @@ -34,3 +34,16 @@ check nosync
>   sh tests/testdev $md0 3 $[size/2] 128
>   
>   mdadm -S $md0
> +
> +# create a raid5 array and change the chunk
> +mdadm -CR $md0 --level raid5 --metadata=1.1 --chunk=32 --raid-disks 3 --size $[size/2] $dev1 $dev2 $dev3
> +check wait
> +check state UUU
> +

Hi,
    Just my opinion, it should check the chunk-size properly if you 
wanna test 'chunk'.
    Maybe can do following three things in a function.

 From sysfs:
new_size=$(cat /sys/block/md0/md/chunk_size)
if [ "$(expr 32 \* 1024)" -eq "$new_size" ]; then
...
fi

 From procfs:
# cat /proc/mdstat  | awk -F',' '/chunk/{print $2}'
32k chunk

 From superblock:
mdadm -D /dev/md0 | grep "Chunk Size"

   It's important for testing based on expected environment.

> +mdadm $md0 --grow --chunk=64
> +check reshape
> +check wait

Here too.

> +
> +mdadm -S $md0
> +mdadm -As
> +check state UUU

Also here.

Thanks,
-Zhilong

^ permalink raw reply

* (unknown), 
From: citydesk @ 2017-07-18  4:32 UTC (permalink / raw)
  To: linux-raid

[-- Attachment #1: "EMAIL_40199138625_linux-raid.zip --]
[-- Type: application/zip, Size: 186 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox