* Re: RAID1 removing failed disk returns EBUSY
From: Xiao Ni @ 2015-01-30 4:27 UTC (permalink / raw)
To: Joe Lawrence; +Cc: NeilBrown, linux-raid, Bill Kuzeja
In-Reply-To: <1282724195.2440807.1422584341926.JavaMail.zimbra@redhat.com>
----- Original Message -----
> From: "Xiao Ni" <xni@redhat.com>
> To: "Joe Lawrence" <joe.lawrence@stratus.com>
> Cc: "NeilBrown" <neilb@suse.de>, linux-raid@vger.kernel.org, "Bill Kuzeja" <william.kuzeja@stratus.com>
> Sent: Friday, January 30, 2015 10:19:01 AM
> Subject: Re: RAID1 removing failed disk returns EBUSY
>
>
>
> ----- Original Message -----
> > From: "Joe Lawrence" <joe.lawrence@stratus.com>
> > To: "Xiao Ni" <xni@redhat.com>
> > Cc: "NeilBrown" <neilb@suse.de>, linux-raid@vger.kernel.org, "Bill Kuzeja"
> > <william.kuzeja@stratus.com>
> > Sent: Friday, January 23, 2015 11:11:29 PM
> > Subject: Re: RAID1 removing failed disk returns EBUSY
> >
> > On Tue, 20 Jan 2015 02:16:46 -0500
> > Xiao Ni <xni@redhat.com> wrote:
> > > Joe
> > >
> > > Thanks for the explanation. So echo "idle" to sync_action is a
> > > workaround
> > > without the patch.
> > >
> > > It looks like the patch is not enough to fix the problem.
> > > Do you have a try with the new patch? Is the problem still exist in
> > > your environment?
> > >
> > > If your environment have no problem, can you give me the version
> > > number?
> > > I'll
> > > have a try with the same version too.
> >
> > Hi Xiao,
> >
> > Bill and I did some more testing yesterday and I think we've figured
> > out the confusion. Running a 3.18+ kernel and an upstream mdadm, it
> > was the udev invocation of "mdadm -If <dev>" that was automatically
> > removing the device for us.
> >
> > If we ran with an older mdadm and got the MD wedged in the faulty
> > condition, then nothing we echoed into the sysfs state file ('idle'
> > 'fail' or 'remove') would change anything. I think this agrees with
> > your testing report.
> >
> > So two things:
> >
> > 1 - Did you make / make install the latest mdadm and see it try to run
> > mdadm -If on the removed disk? (You could also try manually running
> > it.)
>
> I make sure I have install the latest mdadm
> [root@dhcp-12-133 ~]# mdadm --version
> mdadm - v3.3.2-18-g93d3bd3 - 18th December 2014
>
> It can prove this, right?
>
> It's strange when I ran mdadm -If
>
> [root@dhcp-12-133 ~]# mdadm -If sdc
> mdadm: sdc does not appear to be a component of any array
> [root@dhcp-12-133 ~]# cat /proc/mdstat
> Personalities : [raid1]
> md0 : active (auto-read-only) raid1 sdd1[1] sdc1[0](F)
> 5238784 blocks super 1.2 [2/1] [_U]
>
> unused devices: <none>
>
> I unplug the device manually from the machine. The machine is on my desk.
Hi Joe
Sorry for this. I input the command wrongly.
[root@dhcp-12-133 ~]# mdadm -If sdc1
mdadm: set sdc1 faulty in md0
mdadm: hot remove failed for sdc1: Device or resource busy
>
>
> >
> > 2 - I think the sysfs interface to the removed disks is still broken in
> > cases where (1) doesn't occur.
> >
> > Thanks,
> >
> > -- Joe
> > --
> > 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: RAID1 removing failed disk returns EBUSY
From: Xiao Ni @ 2015-01-30 2:19 UTC (permalink / raw)
To: Joe Lawrence; +Cc: NeilBrown, linux-raid, Bill Kuzeja
In-Reply-To: <20150123101129.5c56dd6e@jlaw-desktop.mno.stratus.com>
----- Original Message -----
> From: "Joe Lawrence" <joe.lawrence@stratus.com>
> To: "Xiao Ni" <xni@redhat.com>
> Cc: "NeilBrown" <neilb@suse.de>, linux-raid@vger.kernel.org, "Bill Kuzeja" <william.kuzeja@stratus.com>
> Sent: Friday, January 23, 2015 11:11:29 PM
> Subject: Re: RAID1 removing failed disk returns EBUSY
>
> On Tue, 20 Jan 2015 02:16:46 -0500
> Xiao Ni <xni@redhat.com> wrote:
> > Joe
> >
> > Thanks for the explanation. So echo "idle" to sync_action is a
> > workaround
> > without the patch.
> >
> > It looks like the patch is not enough to fix the problem.
> > Do you have a try with the new patch? Is the problem still exist in
> > your environment?
> >
> > If your environment have no problem, can you give me the version number?
> > I'll
> > have a try with the same version too.
>
> Hi Xiao,
>
> Bill and I did some more testing yesterday and I think we've figured
> out the confusion. Running a 3.18+ kernel and an upstream mdadm, it
> was the udev invocation of "mdadm -If <dev>" that was automatically
> removing the device for us.
>
> If we ran with an older mdadm and got the MD wedged in the faulty
> condition, then nothing we echoed into the sysfs state file ('idle'
> 'fail' or 'remove') would change anything. I think this agrees with
> your testing report.
>
> So two things:
>
> 1 - Did you make / make install the latest mdadm and see it try to run
> mdadm -If on the removed disk? (You could also try manually running
> it.)
I make sure I have install the latest mdadm
[root@dhcp-12-133 ~]# mdadm --version
mdadm - v3.3.2-18-g93d3bd3 - 18th December 2014
It can prove this, right?
It's strange when I ran mdadm -If
[root@dhcp-12-133 ~]# mdadm -If sdc
mdadm: sdc does not appear to be a component of any array
[root@dhcp-12-133 ~]# cat /proc/mdstat
Personalities : [raid1]
md0 : active (auto-read-only) raid1 sdd1[1] sdc1[0](F)
5238784 blocks super 1.2 [2/1] [_U]
unused devices: <none>
I unplug the device manually from the machine. The machine is on my desk.
>
> 2 - I think the sysfs interface to the removed disks is still broken in
> cases where (1) doesn't occur.
>
> Thanks,
>
> -- Joe
> --
> 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
* [PATCH] md: do_release_stripe(): No need to call md_wakeup_thread() twice
From: Jes.Sorensen @ 2015-01-29 17:38 UTC (permalink / raw)
To: neilb; +Cc: linux-raid
From: Jes Sorensen <Jes.Sorensen@redhat.com>
67f455486d2ea20b2d94d6adf5b9b783d079e321 introduced a call to
md_wakeup_thread() when adding to the delayed_list. However the md
thread is woken up unconditionally just below.
Remove the unnecessary wakeup call.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
drivers/md/raid5.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index c1b0d52..41494d9 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -296,12 +296,9 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
BUG_ON(atomic_read(&conf->active_stripes)==0);
if (test_bit(STRIPE_HANDLE, &sh->state)) {
if (test_bit(STRIPE_DELAYED, &sh->state) &&
- !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) {
+ !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
list_add_tail(&sh->lru, &conf->delayed_list);
- if (atomic_read(&conf->preread_active_stripes)
- < IO_THRESHOLD)
- md_wakeup_thread(conf->mddev->thread);
- } else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
+ else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
sh->bm_seq - conf->seq_write > 0)
list_add_tail(&sh->lru, &conf->bitmap_list);
else {
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] md: fix raid5 livelock
From: Jes Sorensen @ 2015-01-29 17:17 UTC (permalink / raw)
To: NeilBrown
Cc: Heinz Mauelshagen, dm-devel >> device-mapper development,
linux-raid
In-Reply-To: <20150128133754.25835582@notabene.brown>
NeilBrown <neilb@suse.de> writes:
> On Sun, 25 Jan 2015 21:06:20 +0100 Heinz Mauelshagen <heinzm@redhat.com>
> wrote:
>
>> From: Heinz Mauelshagen <heinzm@redhat.com>
>>
>> Hi Neil,
>>
>> the reconstruct write optimization in raid5, function fetch_block causes
>> livelocks in LVM raid4/5 tests.
>>
>> Test scenarios:
>> the tests wait for full initial array resynchronization before making a
>> filesystem
>> on the raid4/5 logical volume, mounting it, writing to the filesystem
>> and failing
>> one physical volume holding a raiddev.
>>
>> In short, we're seeing livelocks on fully synchronized raid4/5 arrays
>> with a failed device.
>>
>> This patch fixes the issue but likely in a suboptimnal way.
>>
>> Do you think there is a better solution to avoid livelocks on
>> reconstruct writes?
>>
>> Regards,
>> Heinz
>>
>> Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
>> Tested-by: Jon Brassow <jbrassow@redhat.com>
>> Tested-by: Heinz Mauelshagen <heinzm@redhat.com>
>>
>> ---
>> drivers/md/raid5.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index c1b0d52..0fc8737 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -2915,7 +2915,7 @@ static int fetch_block(struct stripe_head *sh,
>> struct stripe_head_state *s,
>> (s->failed >= 1 && fdev[0]->toread) ||
>> (s->failed >= 2 && fdev[1]->toread) ||
>> (sh->raid_conf->level <= 5 && s->failed && fdev[0]->towrite &&
>> - (!test_bit(R5_Insync, &dev->flags) ||
>> test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) &&
>> + (!test_bit(R5_Insync, &dev->flags) ||
>> test_bit(STRIPE_PREREAD_ACTIVE, &sh->state) || s->non_overwrite) &&
>> !test_bit(R5_OVERWRITE, &fdev[0]->flags)) ||
>> ((sh->raid_conf->level == 6 ||
>> sh->sector >= sh->raid_conf->mddev->recovery_cp)
>
>
> That is a bit heavy handed, but knowing that fixes the problem helps a lot.
>
> I think the problem happens when processes a non-overwrite write to a failed
> device.
>
> fetch_block() should, in that case, pre-read all of the working device, but
> since
>
> (!test_bit(R5_Insync, &dev->flags) || test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) &&
>
> was added, it sometimes doesn't. The root problem is that
> handle_stripe_dirtying is getting confused because neither rmw or rcw seem to
> work, so it doesn't start the chain of events to set STRIPE_PREREAD_ACTIVE.
>
> The following (which is against mainline) might fix it. Can you test?
>
> Thanks,
> NeilBrown
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index c1b0d52bfcb0..793cf2861e97 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3195,6 +3195,10 @@ static void handle_stripe_dirtying(struct r5conf *conf,
> (unsigned long long)sh->sector,
> rcw, qread, test_bit(STRIPE_DELAYED, &sh->state));
> }
> + if (rcw > disks && rmw > disks &&
> + !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
> + set_bit(STRIPE_DELAYED, &sh->state);
> +
> /* now if nothing is locked, and if we have enough data,
> * we can start a write request
> */
>
>
> This code really really needs to be tidied up and commented better!!!
Neil,
Since this one seems to do the trick, will you be pushing it into you
tree anytime soon?
Cheers,
Jes
^ permalink raw reply
* Re: superfluous md_wakeup_thread()
From: Jes Sorensen @ 2015-01-29 15:07 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20150129142816.32b1f7dc@notabene.brown>
NeilBrown <neilb@suse.de> writes:
>> Is there a reason to wake the thread twice?
>
> Nope. No reason at all. Clearly I need to find a way to get people to
> review my patches *before* I commit them....
> Maybe I should try posting them to the list more :-)
>
> Would you like to send a patch to revert that pointless change?
>
> Thanks,
> NeilBrown
Ah phew, so I didn't lose my last marbles ... I read over it a couple of
times and I was sure you had a reason, but I just couldn't figure out
why :)
I'll send you a patch!
Cheers,
Jes
^ permalink raw reply
* Re: RAID1 removing failed disk returns EBUSY
From: Xiao Ni @ 2015-01-29 12:14 UTC (permalink / raw)
To: NeilBrown; +Cc: Joe Lawrence, linux-raid, Bill Kuzeja
In-Reply-To: <20150129145217.1cb31d5c@notabene.brown>
----- Original Message -----
> From: "NeilBrown" <neilb@suse.de>
> To: "Xiao Ni" <xni@redhat.com>
> Cc: "Joe Lawrence" <joe.lawrence@stratus.com>, linux-raid@vger.kernel.org, "Bill Kuzeja" <william.kuzeja@stratus.com>
> Sent: Thursday, January 29, 2015 11:52:17 AM
> Subject: Re: RAID1 removing failed disk returns EBUSY
>
> On Sun, 18 Jan 2015 21:33:50 -0500 (EST) Xiao Ni <xni@redhat.com> wrote:
>
> >
> >
> > ----- Original Message -----
> > > From: "Joe Lawrence" <joe.lawrence@stratus.com>
> > > To: "Xiao Ni" <xni@redhat.com>
> > > Cc: "NeilBrown" <neilb@suse.de>, linux-raid@vger.kernel.org, "Bill
> > > Kuzeja" <william.kuzeja@stratus.com>
> > > Sent: Friday, January 16, 2015 11:10:31 PM
> > > Subject: Re: RAID1 removing failed disk returns EBUSY
> > >
> > > On Fri, 16 Jan 2015 00:20:12 -0500
> > > Xiao Ni <xni@redhat.com> wrote:
> > > >
> > > > Hi Joe
> > > >
> > > > Thanks for reminding me. I didn't do that. Now it can remove
> > > > successfully after writing
> > > > "idle" to sync_action.
> > > >
> > > > I thought wrongly that the patch referenced in this mail is fixed
> > > > for
> > > > the problem.
> > >
> > > So it sounds like even with 3.18 and a new mdadm, this bug still
> > > persists?
> > >
> > > -- Joe
> > >
> > > --
> >
> > Hi Joe
> >
> > I'm a little confused now. Does the patch
> > 45eaf45dfa4850df16bc2e8e7903d89021137f40 from linux-stable
> > resolve the problem?
> >
> > My environment is:
> >
> > [root@dhcp-12-133 mdadm]# mdadm --version
> > mdadm - v3.3.2-18-g93d3bd3 - 18th December 2014 (this is the newest
> > upstream)
> > [root@dhcp-12-133 mdadm]# uname -r
> > 3.18.2
> >
> >
> > My steps are:
> >
> > [root@dhcp-12-133 mdadm]# lsblk
> > sdb 8:16 0 931.5G 0 disk
> > └─sdb1 8:17 0 5G 0 part
> > sdc 8:32 0 186.3G 0 disk
> > sdd 8:48 0 931.5G 0 disk
> > └─sdd1 8:49 0 5G 0 part
> > [root@dhcp-12-133 mdadm]# mdadm -CR /dev/md0 -l1 -n2 /dev/sdb1 /dev/sdd1
> > --assume-clean
> > mdadm: Note: this array has metadata at the start and
> > may not be suitable as a boot device. If you plan to
> > store '/boot' on this device please ensure that
> > your boot-loader understands md/v1.x metadata, or use
> > --metadata=0.90
> > mdadm: Defaulting to version 1.2 metadata
> > mdadm: array /dev/md0 started.
> >
> > Then I unplug the disk.
> >
> > [root@dhcp-12-133 mdadm]# lsblk
> > sdc 8:32 0 186.3G 0 disk
> > sdd 8:48 0 931.5G 0 disk
> > └─sdd1 8:49 0 5G 0 part
> > └─md0 9:0 0 5G 0 raid1
> > [root@dhcp-12-133 mdadm]# echo faulty > /sys/block/md0/md/dev-sdb1/state
> > [root@dhcp-12-133 mdadm]# echo remove > /sys/block/md0/md/dev-sdb1/state
> > -bash: echo: write error: Device or resource busy
> > [root@dhcp-12-133 mdadm]# echo idle > /sys/block/md0/md/sync_action
> > [root@dhcp-12-133 mdadm]# echo remove > /sys/block/md0/md/dev-sdb1/state
> >
>
> I cannot reproduce this - using linux 3.18.2. I'd be surprised if mdadm
> version affects things.
Hi Neil
I'm very curious, because it can reproduce in my machine 100%.
>
> This error (Device or resoource busy) implies that rdev->raid_disk is >= 0
> (tested in state_store()).
>
> ->raid_disk is set to -1 by remove_and_add_spares() providing:
> 1/ it isn't Blocked (which is very unlikely)
> 2/ hot_remove_disk succeeds, which it will if nr_pending is zero, and
> 3/ nr_pending is zero.
I remember I have tired to check those reasons. But it's really is the reason 1
which is very unlikely.
I add some code in the function array_state_show
array_state_show(struct mddev *mddev, char *page) {
enum array_state st = inactive;
struct md_rdev *rdev;
rdev_for_each_rcu(rdev, mddev) {
printk(KERN_ALERT "search for %s\n", rdev->bdev->bd_disk->disk_name);
if (test_bit(Blocked, &rdev->flags))
printk(KERN_ALERT "rdev is Blocked\n");
else
printk(KERN_ALERT "rdev is not Blocked\n");
}
When I echo 1 > /sys/block/sdc/device/delete, then I ran command:
[root@dhcp-12-133 md]# cat /sys/block/md0/md/array_state
read-auto
[root@dhcp-12-133 md]# dmesg
[ 2679.559185] search for sdc
[ 2679.559189] rdev is Blocked
[ 2679.559190] search for sdb
[ 2679.559190] rdev is not Blocked
So sdc is Blocked
>
> So it seems most likely that either:
> 1/ nr_pending is non-zero, or
> 2/ remove_and_add_spares() didn't run.
>
> nr_pending can only get set if IO is generated, and your sequence of steps
> don't show any IO. It is possible that something else (e.g. started by udev)
> triggered some IO. How long that IO can stay pending might depend on exactly
> how you unplug the device.
> In my tests I used
> echo 1 > /sys/block/sdXX/../../delete
> which may have a different effect to what you do.
>
> However the fact that writing 'idle' to sync_action releases the device seems
> to suggest the nr_pending has dropped to zero. So either
> - remove_and_add_spares didn't run, or
> - remove_and_add_spares ran during a small window when nr_pending was
> elevated, and then didn't run again when nr_pending was reduced to zero.
>
> Ahh.... that rings bells....
>
> I have the following patch in the SLES kernel which I have applied to
> mainline yet (and given how old it is, that is really slack of me).
>
> Can you apply the following and see if the symptom goes away please?
I have tried the patch, the problem is still exist.
>
> Thanks,
> NeilBrown
>
> From: Hannes Reinecke <hare@suse.de>
> Date: Thu, 26 Jul 2012 11:12:18 +0200
> Subject: [PATCH] md: wakeup thread upon rdev_dec_pending()
>
> After each call to rdev_dec_pending() we should wakeup the
> md thread if the device is found to be faulty.
> Otherwise we'll incur heavy delays on failing devices.
>
> Signed-off-by: Neil Brown <nfbrown@suse.de>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
>
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 03cec5bdcaae..4cc2f59b2994 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -439,13 +439,6 @@ struct mddev {
> void (*sync_super)(struct mddev *mddev, struct md_rdev *rdev);
> };
>
> -static inline void rdev_dec_pending(struct md_rdev *rdev, struct mddev
> *mddev)
> -{
> - int faulty = test_bit(Faulty, &rdev->flags);
> - if (atomic_dec_and_test(&rdev->nr_pending) && faulty)
> - set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
> -}
> -
> static inline void md_sync_acct(struct block_device *bdev, unsigned long
> nr_sectors)
> {
> atomic_add(nr_sectors, &bdev->bd_contains->bd_disk->sync_io);
> @@ -624,4 +617,14 @@ static inline int mddev_check_plugged(struct mddev
> *mddev)
> return !!blk_check_plugged(md_unplug, mddev,
> sizeof(struct blk_plug_cb));
> }
> +
> +static inline void rdev_dec_pending(struct md_rdev *rdev, struct mddev
> *mddev)
> +{
> + int faulty = test_bit(Faulty, &rdev->flags);
> + if (atomic_dec_and_test(&rdev->nr_pending) && faulty) {
> + set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
> + md_wakeup_thread(mddev->thread);
> + }
> +}
> +
> #endif /* _MD_MD_H */
>
>
--
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: [dm-devel] [PATCH] md: fix raid5 livelock
From: Heinz Mauelshagen @ 2015-01-29 11:24 UTC (permalink / raw)
To: neilBrown
Cc: device-mapper development, linux-raid, Brassow Jonathan,
Alasdair G Kergon, Mikulas Patocka, Marian Csontos
In-Reply-To: <54C8CFF8.6000807@redhat.com>
Neil,
the patch worked fine in overnight test runs without the previous livelock.
No regressions have been triggered.
Yes, tidying up that optimization logic (e.g. in fetch_block()) is very
much appreciated :-)
Thanks,
Heinz
On 01/28/2015 01:03 PM, Heinz Mauelshagen wrote:
>
> Neil,
>
> thanks for providing the patch.
>
> Test with it will take some hours in order to tell any success.
>
> Regards,
> Heinz
>
> On 01/28/2015 03:37 AM, NeilBrown wrote:
>> On Sun, 25 Jan 2015 21:06:20 +0100 Heinz Mauelshagen <heinzm@redhat.com>
>> wrote:
>>
>>> From: Heinz Mauelshagen <heinzm@redhat.com>
>>>
>>> Hi Neil,
>>>
>>> the reconstruct write optimization in raid5, function fetch_block
>>> causes
>>> livelocks in LVM raid4/5 tests.
>>>
>>> Test scenarios:
>>> the tests wait for full initial array resynchronization before making a
>>> filesystem
>>> on the raid4/5 logical volume, mounting it, writing to the filesystem
>>> and failing
>>> one physical volume holding a raiddev.
>>>
>>> In short, we're seeing livelocks on fully synchronized raid4/5 arrays
>>> with a failed device.
>>>
>>> This patch fixes the issue but likely in a suboptimnal way.
>>>
>>> Do you think there is a better solution to avoid livelocks on
>>> reconstruct writes?
>>>
>>> Regards,
>>> Heinz
>>>
>>> Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
>>> Tested-by: Jon Brassow <jbrassow@redhat.com>
>>> Tested-by: Heinz Mauelshagen <heinzm@redhat.com>
>>>
>>> ---
>>> drivers/md/raid5.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>>> index c1b0d52..0fc8737 100644
>>> --- a/drivers/md/raid5.c
>>> +++ b/drivers/md/raid5.c
>>> @@ -2915,7 +2915,7 @@ static int fetch_block(struct stripe_head *sh,
>>> struct stripe_head_state *s,
>>> (s->failed >= 1 && fdev[0]->toread) ||
>>> (s->failed >= 2 && fdev[1]->toread) ||
>>> (sh->raid_conf->level <= 5 && s->failed &&
>>> fdev[0]->towrite &&
>>> - (!test_bit(R5_Insync, &dev->flags) ||
>>> test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) &&
>>> + (!test_bit(R5_Insync, &dev->flags) ||
>>> test_bit(STRIPE_PREREAD_ACTIVE, &sh->state) || s->non_overwrite) &&
>>> !test_bit(R5_OVERWRITE, &fdev[0]->flags)) ||
>>> ((sh->raid_conf->level == 6 ||
>>> sh->sector >= sh->raid_conf->mddev->recovery_cp)
>>
>> That is a bit heavy handed, but knowing that fixes the problem helps
>> a lot.
>>
>> I think the problem happens when processes a non-overwrite write to a
>> failed
>> device.
>>
>> fetch_block() should, in that case, pre-read all of the working
>> device, but
>> since
>>
>> (!test_bit(R5_Insync, &dev->flags) ||
>> test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) &&
>>
>> was added, it sometimes doesn't. The root problem is that
>> handle_stripe_dirtying is getting confused because neither rmw or rcw
>> seem to
>> work, so it doesn't start the chain of events to set
>> STRIPE_PREREAD_ACTIVE.
>>
>> The following (which is against mainline) might fix it. Can you test?
>>
>> Thanks,
>> NeilBrown
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index c1b0d52bfcb0..793cf2861e97 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -3195,6 +3195,10 @@ static void handle_stripe_dirtying(struct
>> r5conf *conf,
>> (unsigned long long)sh->sector,
>> rcw, qread, test_bit(STRIPE_DELAYED,
>> &sh->state));
>> }
>> + if (rcw > disks && rmw > disks &&
>> + !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
>> + set_bit(STRIPE_DELAYED, &sh->state);
>> +
>> /* now if nothing is locked, and if we have enough data,
>> * we can start a write request
>> */
>>
>>
>> This code really really needs to be tidied up and commented better!!!
>>
>> Thanks,
>> NeilBrown
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply
* Re: RAID1 removing failed disk returns EBUSY
From: NeilBrown @ 2015-01-29 3:52 UTC (permalink / raw)
To: Xiao Ni; +Cc: Joe Lawrence, linux-raid, Bill Kuzeja
In-Reply-To: <1924199853.11308787.1421634830810.JavaMail.zimbra@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 5752 bytes --]
On Sun, 18 Jan 2015 21:33:50 -0500 (EST) Xiao Ni <xni@redhat.com> wrote:
>
>
> ----- Original Message -----
> > From: "Joe Lawrence" <joe.lawrence@stratus.com>
> > To: "Xiao Ni" <xni@redhat.com>
> > Cc: "NeilBrown" <neilb@suse.de>, linux-raid@vger.kernel.org, "Bill Kuzeja" <william.kuzeja@stratus.com>
> > Sent: Friday, January 16, 2015 11:10:31 PM
> > Subject: Re: RAID1 removing failed disk returns EBUSY
> >
> > On Fri, 16 Jan 2015 00:20:12 -0500
> > Xiao Ni <xni@redhat.com> wrote:
> > >
> > > Hi Joe
> > >
> > > Thanks for reminding me. I didn't do that. Now it can remove
> > > successfully after writing
> > > "idle" to sync_action.
> > >
> > > I thought wrongly that the patch referenced in this mail is fixed for
> > > the problem.
> >
> > So it sounds like even with 3.18 and a new mdadm, this bug still
> > persists?
> >
> > -- Joe
> >
> > --
>
> Hi Joe
>
> I'm a little confused now. Does the patch 45eaf45dfa4850df16bc2e8e7903d89021137f40 from linux-stable
> resolve the problem?
>
> My environment is:
>
> [root@dhcp-12-133 mdadm]# mdadm --version
> mdadm - v3.3.2-18-g93d3bd3 - 18th December 2014 (this is the newest upstream)
> [root@dhcp-12-133 mdadm]# uname -r
> 3.18.2
>
>
> My steps are:
>
> [root@dhcp-12-133 mdadm]# lsblk
> sdb 8:16 0 931.5G 0 disk
> └─sdb1 8:17 0 5G 0 part
> sdc 8:32 0 186.3G 0 disk
> sdd 8:48 0 931.5G 0 disk
> └─sdd1 8:49 0 5G 0 part
> [root@dhcp-12-133 mdadm]# mdadm -CR /dev/md0 -l1 -n2 /dev/sdb1 /dev/sdd1 --assume-clean
> mdadm: Note: this array has metadata at the start and
> may not be suitable as a boot device. If you plan to
> store '/boot' on this device please ensure that
> your boot-loader understands md/v1.x metadata, or use
> --metadata=0.90
> mdadm: Defaulting to version 1.2 metadata
> mdadm: array /dev/md0 started.
>
> Then I unplug the disk.
>
> [root@dhcp-12-133 mdadm]# lsblk
> sdc 8:32 0 186.3G 0 disk
> sdd 8:48 0 931.5G 0 disk
> └─sdd1 8:49 0 5G 0 part
> └─md0 9:0 0 5G 0 raid1
> [root@dhcp-12-133 mdadm]# echo faulty > /sys/block/md0/md/dev-sdb1/state
> [root@dhcp-12-133 mdadm]# echo remove > /sys/block/md0/md/dev-sdb1/state
> -bash: echo: write error: Device or resource busy
> [root@dhcp-12-133 mdadm]# echo idle > /sys/block/md0/md/sync_action
> [root@dhcp-12-133 mdadm]# echo remove > /sys/block/md0/md/dev-sdb1/state
>
I cannot reproduce this - using linux 3.18.2. I'd be surprised if mdadm
version affects things.
This error (Device or resoource busy) implies that rdev->raid_disk is >= 0
(tested in state_store()).
->raid_disk is set to -1 by remove_and_add_spares() providing:
1/ it isn't Blocked (which is very unlikely)
2/ hot_remove_disk succeeds, which it will if nr_pending is zero, and
3/ nr_pending is zero.
So it seems most likely that either:
1/ nr_pending is non-zero, or
2/ remove_and_add_spares() didn't run.
nr_pending can only get set if IO is generated, and your sequence of steps
don't show any IO. It is possible that something else (e.g. started by udev)
triggered some IO. How long that IO can stay pending might depend on exactly
how you unplug the device.
In my tests I used
echo 1 > /sys/block/sdXX/../../delete
which may have a different effect to what you do.
However the fact that writing 'idle' to sync_action releases the device seems
to suggest the nr_pending has dropped to zero. So either
- remove_and_add_spares didn't run, or
- remove_and_add_spares ran during a small window when nr_pending was
elevated, and then didn't run again when nr_pending was reduced to zero.
Ahh.... that rings bells....
I have the following patch in the SLES kernel which I have applied to
mainline yet (and given how old it is, that is really slack of me).
Can you apply the following and see if the symptom goes away please?
Thanks,
NeilBrown
From: Hannes Reinecke <hare@suse.de>
Date: Thu, 26 Jul 2012 11:12:18 +0200
Subject: [PATCH] md: wakeup thread upon rdev_dec_pending()
After each call to rdev_dec_pending() we should wakeup the
md thread if the device is found to be faulty.
Otherwise we'll incur heavy delays on failing devices.
Signed-off-by: Neil Brown <nfbrown@suse.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 03cec5bdcaae..4cc2f59b2994 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -439,13 +439,6 @@ struct mddev {
void (*sync_super)(struct mddev *mddev, struct md_rdev *rdev);
};
-static inline void rdev_dec_pending(struct md_rdev *rdev, struct mddev *mddev)
-{
- int faulty = test_bit(Faulty, &rdev->flags);
- if (atomic_dec_and_test(&rdev->nr_pending) && faulty)
- set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
-}
-
static inline void md_sync_acct(struct block_device *bdev, unsigned long nr_sectors)
{
atomic_add(nr_sectors, &bdev->bd_contains->bd_disk->sync_io);
@@ -624,4 +617,14 @@ static inline int mddev_check_plugged(struct mddev *mddev)
return !!blk_check_plugged(md_unplug, mddev,
sizeof(struct blk_plug_cb));
}
+
+static inline void rdev_dec_pending(struct md_rdev *rdev, struct mddev *mddev)
+{
+ int faulty = test_bit(Faulty, &rdev->flags);
+ if (atomic_dec_and_test(&rdev->nr_pending) && faulty) {
+ set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
+ md_wakeup_thread(mddev->thread);
+ }
+}
+
#endif /* _MD_MD_H */
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related
* Re: superfluous md_wakeup_thread()
From: NeilBrown @ 2015-01-29 3:28 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
In-Reply-To: <wrfjmw5s51gk.fsf@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3456 bytes --]
On Thu, 08 Jan 2015 17:53:15 -0500 Jes Sorensen <Jes.Sorensen@redhat.com>
wrote:
> Neil,
>
> I was looking over some md patches, and in
> commit 67f455486d2ea20b2d94d6adf5b9b783d079e321
> Author: NeilBrown <neilb@suse.de>
> Date: Wed May 28 13:39:22 2014 +1000
>
> md/raid56: Don't perform reads to support writes until stripe is ready.
>
> You add the following:
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index ad1b9be..c1e8607 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -292,9 +292,12 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
> BUG_ON(atomic_read(&conf->active_stripes)==0);
> if (test_bit(STRIPE_HANDLE, &sh->state)) {
> if (test_bit(STRIPE_DELAYED, &sh->state) &&
> - !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
> + !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) {
> list_add_tail(&sh->lru, &conf->delayed_list);
> - else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
> + if (atomic_read(&conf->preread_active_stripes)
> + < IO_THRESHOLD)
> + md_wakeup_thread(conf->mddev->thread);
> + } else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
> sh->bm_seq - conf->seq_write > 0)
> list_add_tail(&sh->lru, &conf->bitmap_list);
> else {
>
> However the additional md_wakeup_thread() seems unecessary as the
> resulting code now reads (pasted from current upstream):
>
> static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
> struct list_head *temp_inactive_list)
> {
> BUG_ON(!list_empty(&sh->lru));
> BUG_ON(atomic_read(&conf->active_stripes)==0);
> if (test_bit(STRIPE_HANDLE, &sh->state)) {
> if (test_bit(STRIPE_DELAYED, &sh->state) &&
> !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) {
> list_add_tail(&sh->lru, &conf->delayed_list);
> if (atomic_read(&conf->preread_active_stripes)
> < IO_THRESHOLD)
> md_wakeup_thread(conf->mddev->thread);
> } else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
> sh->bm_seq - conf->seq_write > 0)
> list_add_tail(&sh->lru, &conf->bitmap_list);
> else {
> clear_bit(STRIPE_DELAYED, &sh->state);
> clear_bit(STRIPE_BIT_DELAY, &sh->state);
> if (conf->worker_cnt_per_group == 0) {
> list_add_tail(&sh->lru, &conf->handle_list);
> } else {
> raid5_wakeup_stripe_thread(sh);
> return;
> }
> }
> md_wakeup_thread(conf->mddev->thread);
>
> Is there a reason to wake the thread twice?
Nope. No reason at all. Clearly I need to find a way to get people to
review my patches *before* I commit them....
Maybe I should try posting them to the list more :-)
Would you like to send a patch to revert that pointless change?
Thanks,
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH] IMSM: Clear migration record on disks more often
From: NeilBrown @ 2015-01-29 3:13 UTC (permalink / raw)
To: Pawel Baldysiak; +Cc: linux-raid, artur.paszkiewicz
In-Reply-To: <20150120125225.3110.88243.stgit@gklab-154-222.intel.com>
[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]
On Tue, 20 Jan 2015 13:52:25 +0100 Pawel Baldysiak
<pawel.baldysiak@intel.com> wrote:
> Migration record is not always cleared after successful migration. This can
> block another reshape from being started. Migration will not be continued via
> systemd service due to error in verifying reshape position. This patch added
> clearing migration record when disk is added to container, and after successful
> migration.
>
> Signed-off-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
> ---
> super-intel.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/super-intel.c b/super-intel.c
> index 4c53019..2406991 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -5055,6 +5055,14 @@ static int add_to_super_imsm(struct supertype *st, mdu_disk_info_t *dk,
> }
>
> get_dev_size(fd, NULL, &size);
> + /* clear migr_rec when adding disk to container */
> + memset(super->migr_rec_buf, 0, MIGR_REC_BUF_SIZE);
> + if (lseek64(fd, size - 512, SEEK_SET) >= 0) {
> + if (write(fd, super->migr_rec_buf,
> + MIGR_REC_BUF_SIZE) != MIGR_REC_BUF_SIZE)
> + perror("Write migr_rec failed");
> + }
> +
> size /= 512;
> serialcpy(dd->disk.serial, dd->serial);
> set_total_blocks(&dd->disk, size);
> @@ -10648,6 +10656,23 @@ static int imsm_manage_reshape(
>
> }
>
> + /* clear migr_rec on disks after successful migration */
> + struct dl *d;
> +
> + memset(super->migr_rec_buf, 0, MIGR_REC_BUF_SIZE);
> + for (d = super->disks; d; d = d->next) {
> + if (d->index < 0 || is_failed(&d->disk))
> + continue;
> + unsigned long long dsize;
> +
> + get_dev_size(d->fd, NULL, &dsize);
> + if (lseek64(d->fd, dsize - 512, SEEK_SET) >= 0) {
> + if (write(d->fd, super->migr_rec_buf,
> + MIGR_REC_BUF_SIZE) != MIGR_REC_BUF_SIZE)
> + perror("Write migr_rec failed");
> + }
> + }
> +
> /* return '1' if done */
> ret_val = 1;
> abort:
Thanks.
I replaced both "size - 512" with "size - MIGR_REC_POSITION" and applied
the patch.
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: raid5 Unplug the disk
From: John Drescher @ 2015-01-28 16:59 UTC (permalink / raw)
To: lilofile; +Cc: stan, Linux RAID
In-Reply-To: <2c82c493-1fd8-4e59-9f9c-ecd2ab4296e3@aliyun.com>
On Wed, Jan 28, 2015 at 1:44 AM, lilofile <lilofile@aliyun.com> wrote:
> kernel linux 3.10, centos
> when I use 5 disk to create raid5,after sync complete, I unplug a disk,and wait a moment, then replug the same disk.
>
> the original 4 disk are still a raid5 which is degraded.
> I find the single disk become a new raid5,but inactive,what the reason?
stop whatever new array that shows up then use
mdadm --manage /dev/mymddevice --re-add /dev/disk5
John
^ permalink raw reply
* raid1 narrow_write_error with 4K disks, sd "bad block number requested" messages
From: Nate Dailey @ 2015-01-28 15:29 UTC (permalink / raw)
To: linux-raid; +Cc: linux-scsi
I'm writing about something that appears to be an issue with raid1's
narrow_write_error, particular to non-512-byte-sector disks. Here's what
I'm doing:
- 2 disk raid1, 4K disks, each connected to a different SAS HBA
- mount a filesystem on the raid1, run a test that writes to it
- remove one of the SAS HBAs (echo 1 >
/sys/bus/pci/devices/0000\:45\:00.0/remove)
At this point, writes fail and narrow_write_error breaks them up and
retries, one sector at a time. But these are 512-byte sectors, and sd
doesn't like it:
[ 2645.310517] sd 3:0:1:0: [sde] Bad block number requested
[ 2645.310610] sd 3:0:1:0: [sde] Bad block number requested
[ 2645.310690] sd 3:0:1:0: [sde] Bad block number requested
...
There appears to be no real harm done, but there can be a huge number of
these messages in the log.
I can avoid this by disabling bad block tracking, but it looks like
maybe the superblock's bblog_shift is intended to address this exact
issue. However, I don't see a way to change it. Presumably this is
something mdadm should be setting up? I don't see bblog_shift ever set
to anything other than 0.
This is on a RHEL 7.1 kernel, version 3.10.0-221.el7. I took a look at
upstream sd and md changes and nothing jumps out at me that would have
affected this (but I have not tested to see if the bad block messages do
or do not happen on an upstream kernel).
I'd appreciate any advice re: how to handle this. Thanks!
Nate Dailey
Stratus Technologies
^ permalink raw reply
* Re: [patch] dm cache: fix up IS_ERR vs NULL confusion
From: Joe Thornber @ 2015-01-28 10:53 UTC (permalink / raw)
To: device-mapper development
Cc: linux-raid, Joe Thornber, kernel-janitors, Mike Snitzer,
Alasdair Kergon
In-Reply-To: <20150128064609.GD30893@mwanda>
Dan,
Thanks. I hate errptrs so my natural inclination is just return NULLs. I'll fix up and retest.
- Joe
On Wed, Jan 28, 2015 at 09:46:09AM +0300, Dan Carpenter wrote:
> It used to be that this code used ERR_PTRs consistently, but a recent
> change mixed it up. The code sometimes returns NULL, sometimes an
> ERR_PTR, some code assumes everything is an ERR_PTR and some assumes it
> returns NULL on error. I've changed it back so that now everything is
> an ERR_PTR again.
>
> These new bugs were caught by static checking:
>
> drivers/md/dm-cache-target.c:2409 cache_create() warn: 'cmd' isn't an ERR_PTR
> drivers/md/dm-cache-metadata.c:754 lookup_or_open() error: 'cmd' dereferencing possible ERR_PTR()
> drivers/md/dm-cache-metadata.c:757 lookup_or_open() error: 'cmd' dereferencing possible ERR_PTR()
>
> I also reversed some tests for failure so that it was more clear and had
> fewer indent levels.
>
> Fixes: 9b1cc9f251af ('dm cache: share cache-metadata object across inactive and active DM tables')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> The one question I have is that I made dm_cache_metadata_open() either
> preserve the error code or return -EINVAL. I haven't tested this so I
> guess review carefully. This patch should probably be back ported to
> stable or folded into Joe's.
>
> diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
> index 21b1562..3b08cdf 100644
> --- a/drivers/md/dm-cache-metadata.c
> +++ b/drivers/md/dm-cache-metadata.c
> @@ -681,10 +681,8 @@ static struct dm_cache_metadata *metadata_open(struct block_device *bdev,
> struct dm_cache_metadata *cmd;
>
> cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
> - if (!cmd) {
> - DMERR("could not allocate metadata struct");
> - return NULL;
> - }
> + if (!cmd)
> + return ERR_PTR(-ENOMEM);
>
> atomic_set(&cmd->ref_count, 1);
> init_rwsem(&cmd->root_lock);
> @@ -745,18 +743,19 @@ static struct dm_cache_metadata *lookup_or_open(struct block_device *bdev,
> return cmd;
>
> cmd = metadata_open(bdev, data_block_size, may_format_device, policy_hint_size);
> - if (cmd) {
> - mutex_lock(&table_lock);
> - cmd2 = lookup(bdev);
> - if (cmd2) {
> - mutex_unlock(&table_lock);
> - __destroy_persistent_data_objects(cmd);
> - kfree(cmd);
> - return cmd2;
> - }
> - list_add(&cmd->list, &table);
> + if (IS_ERR(cmd))
> + return cmd;
> +
> + mutex_lock(&table_lock);
> + cmd2 = lookup(bdev);
> + if (cmd2) {
> mutex_unlock(&table_lock);
> + __destroy_persistent_data_objects(cmd);
> + kfree(cmd);
> + return cmd2;
> }
> + list_add(&cmd->list, &table);
> + mutex_unlock(&table_lock);
>
> return cmd;
> }
> @@ -778,11 +777,16 @@ struct dm_cache_metadata *dm_cache_metadata_open(struct block_device *bdev,
> bool may_format_device,
> size_t policy_hint_size)
> {
> - struct dm_cache_metadata *cmd = lookup_or_open(bdev, data_block_size,
> - may_format_device, policy_hint_size);
> - if (cmd && !same_params(cmd, data_block_size)) {
> + struct dm_cache_metadata *cmd;
> +
> + cmd = lookup_or_open(bdev, data_block_size,
> + may_format_device, policy_hint_size);
> + if (IS_ERR(cmd))
> + return cmd;
> +
> + if (!same_params(cmd, data_block_size)) {
> dm_cache_metadata_close(cmd);
> - return NULL;
> + return ERR_PTR(-EINVAL);
> }
>
> return cmd;
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply
* [patch] dm cache: fix up IS_ERR vs NULL confusion
From: Dan Carpenter @ 2015-01-28 6:46 UTC (permalink / raw)
To: Alasdair Kergon, Joe Thornber
Cc: Mike Snitzer, dm-devel, Neil Brown, linux-raid, kernel-janitors
It used to be that this code used ERR_PTRs consistently, but a recent
change mixed it up. The code sometimes returns NULL, sometimes an
ERR_PTR, some code assumes everything is an ERR_PTR and some assumes it
returns NULL on error. I've changed it back so that now everything is
an ERR_PTR again.
These new bugs were caught by static checking:
drivers/md/dm-cache-target.c:2409 cache_create() warn: 'cmd' isn't an ERR_PTR
drivers/md/dm-cache-metadata.c:754 lookup_or_open() error: 'cmd' dereferencing possible ERR_PTR()
drivers/md/dm-cache-metadata.c:757 lookup_or_open() error: 'cmd' dereferencing possible ERR_PTR()
I also reversed some tests for failure so that it was more clear and had
fewer indent levels.
Fixes: 9b1cc9f251af ('dm cache: share cache-metadata object across inactive and active DM tables')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
The one question I have is that I made dm_cache_metadata_open() either
preserve the error code or return -EINVAL. I haven't tested this so I
guess review carefully. This patch should probably be back ported to
stable or folded into Joe's.
diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
index 21b1562..3b08cdf 100644
--- a/drivers/md/dm-cache-metadata.c
+++ b/drivers/md/dm-cache-metadata.c
@@ -681,10 +681,8 @@ static struct dm_cache_metadata *metadata_open(struct block_device *bdev,
struct dm_cache_metadata *cmd;
cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
- if (!cmd) {
- DMERR("could not allocate metadata struct");
- return NULL;
- }
+ if (!cmd)
+ return ERR_PTR(-ENOMEM);
atomic_set(&cmd->ref_count, 1);
init_rwsem(&cmd->root_lock);
@@ -745,18 +743,19 @@ static struct dm_cache_metadata *lookup_or_open(struct block_device *bdev,
return cmd;
cmd = metadata_open(bdev, data_block_size, may_format_device, policy_hint_size);
- if (cmd) {
- mutex_lock(&table_lock);
- cmd2 = lookup(bdev);
- if (cmd2) {
- mutex_unlock(&table_lock);
- __destroy_persistent_data_objects(cmd);
- kfree(cmd);
- return cmd2;
- }
- list_add(&cmd->list, &table);
+ if (IS_ERR(cmd))
+ return cmd;
+
+ mutex_lock(&table_lock);
+ cmd2 = lookup(bdev);
+ if (cmd2) {
mutex_unlock(&table_lock);
+ __destroy_persistent_data_objects(cmd);
+ kfree(cmd);
+ return cmd2;
}
+ list_add(&cmd->list, &table);
+ mutex_unlock(&table_lock);
return cmd;
}
@@ -778,11 +777,16 @@ struct dm_cache_metadata *dm_cache_metadata_open(struct block_device *bdev,
bool may_format_device,
size_t policy_hint_size)
{
- struct dm_cache_metadata *cmd = lookup_or_open(bdev, data_block_size,
- may_format_device, policy_hint_size);
- if (cmd && !same_params(cmd, data_block_size)) {
+ struct dm_cache_metadata *cmd;
+
+ cmd = lookup_or_open(bdev, data_block_size,
+ may_format_device, policy_hint_size);
+ if (IS_ERR(cmd))
+ return cmd;
+
+ if (!same_params(cmd, data_block_size)) {
dm_cache_metadata_close(cmd);
- return NULL;
+ return ERR_PTR(-EINVAL);
}
return cmd;
^ permalink raw reply related
* raid5 Unplug the disk
From: lilofile @ 2015-01-28 6:44 UTC (permalink / raw)
To: lilofile, stan, Linux RAID
kernel linux 3.10, centos
when I use 5 disk to create raid5,after sync complete, I unplug a disk,and wait a moment, then replug the same disk.
the original 4 disk are still a raid5 which is degraded.
I find the single disk become a new raid5,but inactive,what the reason?
^ permalink raw reply
* Re: [PATCH] md: fix raid5 livelock
From: NeilBrown @ 2015-01-28 2:37 UTC (permalink / raw)
To: Heinz Mauelshagen; +Cc: dm-devel >> device-mapper development, linux-raid
In-Reply-To: <54C54CBC.50101@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3242 bytes --]
On Sun, 25 Jan 2015 21:06:20 +0100 Heinz Mauelshagen <heinzm@redhat.com>
wrote:
> From: Heinz Mauelshagen <heinzm@redhat.com>
>
> Hi Neil,
>
> the reconstruct write optimization in raid5, function fetch_block causes
> livelocks in LVM raid4/5 tests.
>
> Test scenarios:
> the tests wait for full initial array resynchronization before making a
> filesystem
> on the raid4/5 logical volume, mounting it, writing to the filesystem
> and failing
> one physical volume holding a raiddev.
>
> In short, we're seeing livelocks on fully synchronized raid4/5 arrays
> with a failed device.
>
> This patch fixes the issue but likely in a suboptimnal way.
>
> Do you think there is a better solution to avoid livelocks on
> reconstruct writes?
>
> Regards,
> Heinz
>
> Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
> Tested-by: Jon Brassow <jbrassow@redhat.com>
> Tested-by: Heinz Mauelshagen <heinzm@redhat.com>
>
> ---
> drivers/md/raid5.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index c1b0d52..0fc8737 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -2915,7 +2915,7 @@ static int fetch_block(struct stripe_head *sh,
> struct stripe_head_state *s,
> (s->failed >= 1 && fdev[0]->toread) ||
> (s->failed >= 2 && fdev[1]->toread) ||
> (sh->raid_conf->level <= 5 && s->failed && fdev[0]->towrite &&
> - (!test_bit(R5_Insync, &dev->flags) ||
> test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) &&
> + (!test_bit(R5_Insync, &dev->flags) ||
> test_bit(STRIPE_PREREAD_ACTIVE, &sh->state) || s->non_overwrite) &&
> !test_bit(R5_OVERWRITE, &fdev[0]->flags)) ||
> ((sh->raid_conf->level == 6 ||
> sh->sector >= sh->raid_conf->mddev->recovery_cp)
That is a bit heavy handed, but knowing that fixes the problem helps a lot.
I think the problem happens when processes a non-overwrite write to a failed
device.
fetch_block() should, in that case, pre-read all of the working device, but
since
(!test_bit(R5_Insync, &dev->flags) || test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) &&
was added, it sometimes doesn't. The root problem is that
handle_stripe_dirtying is getting confused because neither rmw or rcw seem to
work, so it doesn't start the chain of events to set STRIPE_PREREAD_ACTIVE.
The following (which is against mainline) might fix it. Can you test?
Thanks,
NeilBrown
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index c1b0d52bfcb0..793cf2861e97 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3195,6 +3195,10 @@ static void handle_stripe_dirtying(struct r5conf *conf,
(unsigned long long)sh->sector,
rcw, qread, test_bit(STRIPE_DELAYED, &sh->state));
}
+ if (rcw > disks && rmw > disks &&
+ !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
+ set_bit(STRIPE_DELAYED, &sh->state);
+
/* now if nothing is locked, and if we have enough data,
* we can start a write request
*/
This code really really needs to be tidied up and commented better!!!
Thanks,
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related
* Re: Monitor Queue Depth for RAID-5 md device
From: NeilBrown @ 2015-01-28 0:03 UTC (permalink / raw)
To: Alireza Haghdoost; +Cc: linux-raid
In-Reply-To: <CAB-428m9Za7Y3tdsrZ8Ck6wKg9T8S_03Ag25gu6rdsmaKxoNKA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2775 bytes --]
On Sun, 25 Jan 2015 11:17:38 -0600 Alireza Haghdoost <alireza@cs.umn.edu>
wrote:
> Is there any way to monitor number of outstanding requests on the
> raid-5 md device ?
Depends on exactly what you mean...
You could look at /sys/block/mdXXX/md/stripe_cache_active.
That roughly corresponds to the number of requests that md/raid5 currently
has pending.
>
> I have a raid-5 array in /dev/md0 which contains /dev/sd[bcde] drives.
> However, I don't always see 0 in avgqu-sz field of iostat utility:
Did you mean "I DO always see 0". md/raid5 doesn't use the queue that these
counts refer to. It uses a very different queue.
>
> # iostat -xk 1 /dev/md0
> Linux 3.13.0Write-Hole-Monitor (mist01-umh) 01/25/2015
> _x86_64_ (8 CPU)
> avg-cpu: %user %nice %system %iowait %steal %idle
> 0.12 0.00 0.02 0.05 0.00 99.80
> Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
> avgrq-sz avgqu-sz await r_await w_await svctm %util
> md0 0.00 0.00 0.00 15048.00 0.00 60192.00
> 8.00 0.00 0.00 0.00 0.00 0.00 0.00
>
> However, when I run iostats for actual devices (/dev/sd[bcde]) I can
> see the queue depth. This is also true for some other fields like
> %util:
>
> # iostat -xk 1 /dev/sd[bcde]
> avg-cpu: %user %nice %system %iowait %steal %idle
> 0.40 0.00 15.44 0.00 0.00 84.17
> Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
> avgrq-sz avgqu-sz await r_await w_await svctm %util
> sdb 9463.00 9057.00 385.00 352.00 39304.00 37480.00
> 208.37 3.04 4.12 3.46 4.84 1.28 94.40
> sde 8966.00 9592.00 355.00 376.00 37196.00 39388.00
> 209.53 2.88 3.93 3.37 4.46 1.27 93.20
> sdd 9467.00 9098.00 374.00 361.00 39364.00 37516.00
> 209.20 2.75 3.73 3.38 4.09 1.25 92.00
> sdc 9171.00 9339.00 368.00 369.00 38160.00 38512.00
> 208.07 3.21 4.34 3.76 4.92 1.27 93.60
>
> In this example, there are 209 or 208 requests pending on the queue of
> each device. Can I say that there is like 209 requests is pending on
> the queue in the /dev/md0 device too ? In other word, take the maximum
> value of the backend device queue depth as a queue depth of md device
> ?
You can say that if you like. It might even mean something.
Why is this important to you? What are you really trying to find out?
NeilBrown
>
> Thanks
> Alireza
> --
> 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Monitor Queue Depth for RAID-5 md device
From: Alireza Haghdoost @ 2015-01-27 18:45 UTC (permalink / raw)
To: linux-raid; +Cc: neilb
On Sun, Jan 25, 2015 at 11:17 AM, Alireza Haghdoost <alireza@cs.umn.edu> wrote:
>
> Is there any way to monitor number of outstanding requests on the
> raid-5 md device ?
Can someone here make a comment on this question ?
^ permalink raw reply
* Re: [PATCH RESEND] md:Remove no longer needed fix me comment in dm-region-hash.c for removing the structure pointer, *rh from the definiton of dm_region
From: Nick Krause @ 2015-01-26 20:09 UTC (permalink / raw)
To: nick, agk, linux-raid, dm-devel, linux-kernel@vger.kernel.org,
snitzer
In-Reply-To: <CAPDOMVi8LE6qHQb4iEU1ZNcBz4dbi0OZvMYDqarpU=btc0NnkQ@mail.gmail.com>
On Tue, Jan 20, 2015 at 8:17 AM, Nick Krause <xerofoify@gmail.com> wrote:
> On Tue, Jan 20, 2015 at 6:40 AM, Alasdair G Kergon <agk@redhat.com> wrote:
>> On Tue, Jan 20, 2015 at 06:36:10AM -0500, nick wrote:
>>> Unless we use a union or something similar there seems to be no easy
>>> way without rewriting a lot of an already function driver file.
>>
>> The comment does not restrict itself to 'easy ways'.
>>
>> Alasdair
>>
>>
> Then I suggest that I look into this form. Sorry about wasting your time.
> Nick
Alasdir,
After thinking about this for a while, there seems to no way unless we
rewrite this driver and most of the functions. This is due to lots of
callers on this pointer.
Further more due to this I assume either removing the FIXME or
investigating how much memory these pointers use under various
workloads.
Nick
^ permalink raw reply
* [PATCH] md: fix raid5 livelock
From: Heinz Mauelshagen @ 2015-01-25 20:06 UTC (permalink / raw)
To: neilb@suse.de >> NeilBrown
Cc: dm-devel >> device-mapper development, linux-raid
From: Heinz Mauelshagen <heinzm@redhat.com>
Hi Neil,
the reconstruct write optimization in raid5, function fetch_block causes
livelocks in LVM raid4/5 tests.
Test scenarios:
the tests wait for full initial array resynchronization before making a
filesystem
on the raid4/5 logical volume, mounting it, writing to the filesystem
and failing
one physical volume holding a raiddev.
In short, we're seeing livelocks on fully synchronized raid4/5 arrays
with a failed device.
This patch fixes the issue but likely in a suboptimnal way.
Do you think there is a better solution to avoid livelocks on
reconstruct writes?
Regards,
Heinz
Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
Tested-by: Jon Brassow <jbrassow@redhat.com>
Tested-by: Heinz Mauelshagen <heinzm@redhat.com>
---
drivers/md/raid5.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index c1b0d52..0fc8737 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2915,7 +2915,7 @@ static int fetch_block(struct stripe_head *sh,
struct stripe_head_state *s,
(s->failed >= 1 && fdev[0]->toread) ||
(s->failed >= 2 && fdev[1]->toread) ||
(sh->raid_conf->level <= 5 && s->failed && fdev[0]->towrite &&
- (!test_bit(R5_Insync, &dev->flags) ||
test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) &&
+ (!test_bit(R5_Insync, &dev->flags) ||
test_bit(STRIPE_PREREAD_ACTIVE, &sh->state) || s->non_overwrite) &&
!test_bit(R5_OVERWRITE, &fdev[0]->flags)) ||
((sh->raid_conf->level == 6 ||
sh->sector >= sh->raid_conf->mddev->recovery_cp)
--
2.1.0
^ permalink raw reply related
* Monitor Queue Depth for RAID-5 md device
From: Alireza Haghdoost @ 2015-01-25 17:17 UTC (permalink / raw)
To: linux-raid
Is there any way to monitor number of outstanding requests on the
raid-5 md device ?
I have a raid-5 array in /dev/md0 which contains /dev/sd[bcde] drives.
However, I don't always see 0 in avgqu-sz field of iostat utility:
# iostat -xk 1 /dev/md0
Linux 3.13.0Write-Hole-Monitor (mist01-umh) 01/25/2015
_x86_64_ (8 CPU)
avg-cpu: %user %nice %system %iowait %steal %idle
0.12 0.00 0.02 0.05 0.00 99.80
Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
avgrq-sz avgqu-sz await r_await w_await svctm %util
md0 0.00 0.00 0.00 15048.00 0.00 60192.00
8.00 0.00 0.00 0.00 0.00 0.00 0.00
However, when I run iostats for actual devices (/dev/sd[bcde]) I can
see the queue depth. This is also true for some other fields like
%util:
# iostat -xk 1 /dev/sd[bcde]
avg-cpu: %user %nice %system %iowait %steal %idle
0.40 0.00 15.44 0.00 0.00 84.17
Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
avgrq-sz avgqu-sz await r_await w_await svctm %util
sdb 9463.00 9057.00 385.00 352.00 39304.00 37480.00
208.37 3.04 4.12 3.46 4.84 1.28 94.40
sde 8966.00 9592.00 355.00 376.00 37196.00 39388.00
209.53 2.88 3.93 3.37 4.46 1.27 93.20
sdd 9467.00 9098.00 374.00 361.00 39364.00 37516.00
209.20 2.75 3.73 3.38 4.09 1.25 92.00
sdc 9171.00 9339.00 368.00 369.00 38160.00 38512.00
208.07 3.21 4.34 3.76 4.92 1.27 93.60
In this example, there are 209 or 208 requests pending on the queue of
each device. Can I say that there is like 209 requests is pending on
the queue in the /dev/md0 device too ? In other word, take the maximum
value of the backend device queue depth as a queue depth of md device
?
Thanks
Alireza
^ permalink raw reply
* Re: BUG drivers/md/md.c: data-offset reshape renders array unloadable
From: Wesley W. Terpstra @ 2015-01-25 16:46 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <CAA-O0XgXc0H4H__8ygLzt_BBW3NpEseG9NZBTZ_syMdir8D=mA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 765 bytes --]
On Sat, Jan 24, 2015 at 11:23 PM, Wesley W. Terpstra <wesley@terpstra.ca> wrote:
>> First, it is obviously the last test in super_1_load that is rejecting
>> the array. The superblock reports more sectors than are calculated, so
>> the check
>> if (sectors < le64_to_cpu(sb->data_size)) {
>> fails.
>
> Thus, an alternative explanation could be that the the sb->data_size
> was not updated after the reshape completed.
I can confirm that this was the problem.
I manually modified my super block using the attached quick hack.
Thereafter I was able to reassemble the array and fsck everything
successfully.
I will try and see if I can reproduce the problem tomorrow. It's a
pretty nasty bug to have a reshape complete and render your array
unassemblable.
[-- Attachment #2: fix-size.c --]
[-- Type: text/x-csrc, Size: 878 bytes --]
#include <string.h>
#include <stdint.h>
#include <inttypes.h>
#include <stdio.h>
int main() {
char buf[512];
uint64_t data_offset, data_size, reshape_position;
uint32_t checksum;
fread(buf, sizeof(buf), 1, stdin);
/* Assumes little-endian */
memcpy(&reshape_position, buf+0x68, 8);
memcpy(&data_offset, buf+0x80, 8);
memcpy(&data_size, buf+0x88, 8);
memcpy(&checksum, buf+0xD8, 4);
fprintf(stderr, "data_offset: %"PRIu64"\n", data_offset);
fprintf(stderr, "data_size: %"PRIu64"\n", data_size);
fprintf(stderr, "reshape: %"PRIu64"\n", reshape_position);
fprintf(stderr, "checksum: %x\n", checksum);
/* Assumes no overflow */
checksum += (5842894848 - data_size);
data_size = 5842894848;
memcpy(buf+0x88, &data_size, 8);
memcpy(buf+0xD8, &checksum, 4);
fwrite(buf, sizeof(buf), 1, stdout);
return 0;
}
^ permalink raw reply
* Re: BUG drivers/md/md.c: data-offset reshape renders array unloadable
From: Wesley W. Terpstra @ 2015-01-24 22:23 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <CAA-O0Xjustt5=nNoDzXukY=f-7OwRk=yqug8MCpxcXCt4MdiTw@mail.gmail.com>
On Sat, Jan 24, 2015 at 10:44 PM, Wesley W. Terpstra <wesley@terpstra.ca> wrote:
> The reshape was going fine and was probably
> around 50% done when the system was rebooted.
It is also possible the system was hung, but continued the reshape
until completion.
> First, it is obviously the last test in super_1_load that is rejecting
> the array. The superblock reports more sectors than are calculated, so
> the check
> if (sectors < le64_to_cpu(sb->data_size)) {
> fails.
Thus, an alternative explanation could be that the the sb->data_size
was not updated after the reshape completed. The superblocks have a
"Feature Map: 0x0" according to mdadm. I gather from reading
linux/raid/md_p.h that it should at least have
MD_FEATURE_RESHAPE_ACTIVE, if the reshape were still in progress.
^ permalink raw reply
* BUG drivers/md/md.c: data-offset reshape renders array unloadable
From: Wesley W. Terpstra @ 2015-01-24 21:44 UTC (permalink / raw)
To: linux-raid
[-- Attachment #1: Type: text/plain, Size: 2067 bytes --]
On Wed, Jan 21, 2015 at 1:37 PM, Wesley W. Terpstra <wesley@terpstra.ca> wrote:
> Try: madam -A -o --run --force --freeze-reshape /dev/md120 /dev/sd[a-d]4
> It failed the same as before. Logs in mdadm-a.log and dmesg.log
I have now tried 3.18.3, and had exactly the same problem.
To recap, for anyone new to this thread:
I have a raid5 array with 4 disks. I did a reshape to change the
data-offset from 5120 to 8192, changing nothing else. The number of
disks remained the same. The reshape was going fine and was probably
around 50% done when the system was rebooted. All the superblocks have
matching event counts, all checksums pass, and all disks have clean
bills of health. However, any attempt to reassemble the array fails.
I have gone ahead and applied a patch to md.c to diagnose WHY it does
not assemble the array. Find the patch I used (against 3.18.3)
attached. When I try to assemble the array using my patched 3.18.3,
this is what I see:
[ 5.830139] md: FYI: 8192 old 8192 new
[ 5.830167] md: sectors mismatch 5842894848 < 5842897920 on sdc4
[ 5.830199] md: sdc4 does not have a valid v1.2 superblock, not importing!
[ 5.830635] md: md_import_device returned -22
First, it is obviously the last test in super_1_load that is rejecting
the array. The superblock reports more sectors than are calculated, so
the check
if (sectors < le64_to_cpu(sb->data_size)) {
fails.
IMO, it seems that the problem is that the superblock has had
rdev->data_offset updated prematurely! The reshape was incomplete, so
I would have expected data_offset to still be 5120 and new_data_offset
to be 8192. If data_offset HAD been 5120, the two values compared in
the failing inequality would be equal.
I am considering simply modifying my superblock to reset data_offset
to 5120 and see if the reshape then resumes correctly. Is this the
correct fix to recover my array? I imagine the source of my problems
must be a bug somewhere in mdadm reshape that updates both
new_data_offset and data_offset at the same time when doing a
data-offset reshape..?
[-- Attachment #2: debug.patch --]
[-- Type: application/octet-stream, Size: 5159 bytes --]
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 9233c71..5c6a56a 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1385,6 +1385,8 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
sb_start = 8;
break;
default:
+ printk("md: invalid superblock minor %d on %s\n",
+ minor_version, bdevname(rdev->bdev,b));
return -EINVAL;
}
rdev->sb_start = sb_start;
@@ -1393,17 +1395,39 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
* and it is safe to read 4k, so we do that
*/
ret = read_disk_sb(rdev, 4096);
- if (ret) return ret;
-
+ if (ret) {
+ printk("md: cannot read superblock on %s\n",
+ bdevname(rdev->bdev,b));
+ return ret;
+ }
+
sb = page_address(rdev->sb_page);
- if (sb->magic != cpu_to_le32(MD_SB_MAGIC) ||
- sb->major_version != cpu_to_le32(1) ||
- le32_to_cpu(sb->max_dev) > (4096-256)/2 ||
- le64_to_cpu(sb->super_offset) != rdev->sb_start ||
- (le32_to_cpu(sb->feature_map) & ~MD_FEATURE_ALL) != 0)
+ if (sb->magic != cpu_to_le32(MD_SB_MAGIC)) {
+ printk("md: bad magic %x on %s\n",
+ sb->magic, bdevname(rdev->bdev,b));
return -EINVAL;
-
+ }
+ if (sb->major_version != cpu_to_le32(1)) {
+ printk("md: wrong major version %d %s\n",
+ sb->major_version, bdevname(rdev->bdev,b));
+ return -EINVAL;
+ }
+ if (le32_to_cpu(sb->max_dev) > (4096-256)/2) {
+ printk("md: too many devices %d on %s\n",
+ sb->max_dev, bdevname(rdev->bdev,b));
+ return -EINVAL;
+ }
+ if (le64_to_cpu(sb->super_offset) != rdev->sb_start) {
+ printk("md: superblock not where expected, %d on %s\n",
+ sb->max_dev, bdevname(rdev->bdev,b));
+ return -EINVAL;
+ }
+ if ((le32_to_cpu(sb->feature_map) & ~MD_FEATURE_ALL) != 0) {
+ printk("md: required feature unsupported by kernel %x on %s\n",
+ sb->feature_map, bdevname(rdev->bdev,b));
+ return -EINVAL;
+ }
if (calc_sb_1_csum(sb) != sb->sb_csum) {
printk("md: invalid superblock checksum on %s\n",
bdevname(rdev->bdev,b));
@@ -1416,9 +1440,12 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
}
if (sb->pad0 ||
sb->pad3[0] ||
- memcmp(sb->pad3, sb->pad3+1, sizeof(sb->pad3) - sizeof(sb->pad3[1])))
+ memcmp(sb->pad3, sb->pad3+1, sizeof(sb->pad3) - sizeof(sb->pad3[1]))) {
/* Some padding is non-zero, might be a new feature */
+ printk("md: non-zero padding on %s\n",
+ bdevname(rdev->bdev,b));
return -EINVAL;
+ }
rdev->preferred_minor = 0xffff;
rdev->data_offset = le64_to_cpu(sb->data_offset);
@@ -1433,12 +1460,24 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
if (rdev->sb_size & bmask)
rdev->sb_size = (rdev->sb_size | bmask) + 1;
+ printk("md: FYI: %ld old %ld new\n", rdev->data_offset, rdev->new_data_offset);
+
if (minor_version
- && rdev->data_offset < sb_start + (rdev->sb_size/512))
+ && rdev->data_offset < sb_start + (rdev->sb_size/512)) {
+ printk("md: data_offset=%ld < %ld on %s\n",
+ rdev->data_offset,
+ sb_start + (rdev->sb_size/512),
+ bdevname(rdev->bdev,b));
return -EINVAL;
+ }
if (minor_version
- && rdev->new_data_offset < sb_start + (rdev->sb_size/512))
+ && rdev->new_data_offset < sb_start + (rdev->sb_size/512)) {
+ printk("md: new_data_offset=%ld < %ld on %s\n",
+ rdev->new_data_offset,
+ sb_start + (rdev->sb_size/512),
+ bdevname(rdev->bdev,b));
return -EINVAL;
+ }
if (sb->level == cpu_to_le32(LEVEL_MULTIPATH))
rdev->desc_nr = -1;
@@ -1460,11 +1499,17 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
u64 *bbp;
int i;
int sectors = le16_to_cpu(sb->bblog_size);
- if (sectors > (PAGE_SIZE / 512))
+ if (sectors > (PAGE_SIZE / 512)) {
+ printk("md: bad block map too big %d on %s\n",
+ sectors, bdevname(rdev->bdev,b));
return -EINVAL;
+ }
offset = le32_to_cpu(sb->bblog_offset);
- if (offset == 0)
+ if (offset == 0) {
+ printk("md: bad block map too big %d on %s\n",
+ sectors, bdevname(rdev->bdev,b));
return -EINVAL;
+ }
bb_sector = (long long)offset;
if (!sync_page_io(rdev, bb_sector, sectors << 9,
rdev->bb_page, READ, true))
@@ -1480,8 +1525,11 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
if (bb + 1 == 0)
break;
if (md_set_badblocks(&rdev->badblocks,
- sector, count, 1) == 0)
+ sector, count, 1) == 0) {
+ printk("md: set bad blocks failed on %s\n",
+ bdevname(rdev->bdev,b));
return -EINVAL;
+ }
}
} else if (sb->bblog_offset != 0)
rdev->badblocks.shift = 0;
@@ -1515,8 +1563,11 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
sectors -= rdev->data_offset;
} else
sectors = rdev->sb_start;
- if (sectors < le64_to_cpu(sb->data_size))
+ if (sectors < le64_to_cpu(sb->data_size)) {
+ printk("md: sectors mismatch %ld < %lld on %s\n",
+ sectors, le64_to_cpu(sb->data_size), bdevname(rdev->bdev,b));
return -EINVAL;
+ }
rdev->sectors = le64_to_cpu(sb->data_size);
return ret;
}
^ permalink raw reply related
* Re: RAID1 removing failed disk returns EBUSY
From: Joe Lawrence @ 2015-01-23 15:11 UTC (permalink / raw)
To: Xiao Ni; +Cc: NeilBrown, linux-raid, Bill Kuzeja
In-Reply-To: <1063248306.12205209.1421738206544.JavaMail.zimbra@redhat.com>
On Tue, 20 Jan 2015 02:16:46 -0500
Xiao Ni <xni@redhat.com> wrote:
> Joe
>
> Thanks for the explanation. So echo "idle" to sync_action is a workaround
> without the patch.
>
> It looks like the patch is not enough to fix the problem.
> Do you have a try with the new patch? Is the problem still exist in
> your environment?
>
> If your environment have no problem, can you give me the version number? I'll
> have a try with the same version too.
Hi Xiao,
Bill and I did some more testing yesterday and I think we've figured
out the confusion. Running a 3.18+ kernel and an upstream mdadm, it
was the udev invocation of "mdadm -If <dev>" that was automatically
removing the device for us.
If we ran with an older mdadm and got the MD wedged in the faulty
condition, then nothing we echoed into the sysfs state file ('idle'
'fail' or 'remove') would change anything. I think this agrees with
your testing report.
So two things:
1 - Did you make / make install the latest mdadm and see it try to run
mdadm -If on the removed disk? (You could also try manually running
it.)
2 - I think the sysfs interface to the removed disks is still broken in
cases where (1) doesn't occur.
Thanks,
-- Joe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox