Linux RAID subsystem development
 help / color / mirror / Atom feed
* mdadm --create parameter ordering
From: Steven Reddie @ 2015-07-13 10:26 UTC (permalink / raw)
  To: linux-raid

Hi,

Given the following command:

    mdadm -Cf /dev/md3 -e1.2 -n5 -l6 /dev/sdga5 /dev/sdgb5 /dev/sdgc5
/dev/sdgd5 /dev/sdge5 -u<uuid>

Is the order that the disks are specified on the command-line significant?

Somebody suggested this command to put the disks back in the correct
order (though I believe he was mistaken about the correct order). It
seems to me that if the order on the command-line is significant that
this would have caused parity to get recalculated (based on the
[incorrect] order of the disks) and the new parity to be written over
actual data blocks. I see no evidence that this has occurred, which
leads me to believe that the actual ordering within the array is
detected and everything proceeds as it should, in which case issuing
the command above likely was a very time-consuming no-op.

Also, it seems that I should be able to issue such a command without
the full scan of the disks. Is that what the "--run" option is for? If
so, does "--readonly" protect me from harm that improper use of
"--run" may cause?

Thanks,

Steven

^ permalink raw reply

* Re: Resurrecting a Dirty RAID-5
From: Adam Goryachev @ 2015-07-13  7:15 UTC (permalink / raw)
  To: Can Jeuleers, linux-raid
In-Reply-To: <55A35B4B.9020706@gmail.com>

On 13/07/15 16:31, Can Jeuleers wrote:
> On 13/07/15 00:33, Adam Goryachev wrote:
>> Also, for your raid1, never have a hot spare, just do this:
>> mdadm /dev/mdX --grow --raid-devices=3
>> Then you will always have all your data replicated on all three drives,
>> so again, no hot spare required.
> Never say never, as there are valid use cases for having hot spares in a
> RAID1 set.
>
> My own use case is that I want to be reasonably assured that my spare
> won't fail at around the same time as the active disks (due to having
> the same age and having been subjected to exactly the same workload).

Yes, this is true too... though with physical HDD, I would expect there 
are enough differences to mean that they will fail at different times, 
even SSD's should have enough variance.

Of course, the other option is to purchase the drives at different times 
(eg, one month apart) so they are also from different batches, as well 
as have a month or two difference in how they are used.

Of course, if both drives in the RAID1 fail at the exact same moment, 
how is a hot spare better than a three drive raid1? Wouldn't the chance 
of three drives failing at the same critical moment be less than the 
chance of two drives failing at the same time (or one drive to fail, and 
then the second to fail during the increased load of a resync)?

I really have no idea about the actual statistical numbers/chances, but 
sounds like a valid question to me...

PS, of course, you should never say never, so I do still agree with you, 
someone, somewhere might have a reason to do it differently. However, if 
they really do, then they should know better than me.

Regards,
Adam

-- 
Adam Goryachev
Website Managers
P: +61 2 8304 0000                    adam@websitemanagers.com.au
F: +61 2 8304 0001                     www.websitemanagers.com.au


^ permalink raw reply

* Re: Resurrecting a Dirty RAID-5
From: Can Jeuleers @ 2015-07-13  6:31 UTC (permalink / raw)
  To: linux-raid
In-Reply-To: <55A2EB25.10607@websitemanagers.com.au>

On 13/07/15 00:33, Adam Goryachev wrote:
> Also, for your raid1, never have a hot spare, just do this:
> mdadm /dev/mdX --grow --raid-devices=3
> Then you will always have all your data replicated on all three drives,
> so again, no hot spare required.

Never say never, as there are valid use cases for having hot spares in a
RAID1 set.

My own use case is that I want to be reasonably assured that my spare
won't fail at around the same time as the active disks (due to having
the same age and having been subjected to exactly the same workload).

^ permalink raw reply

* Re: Resurrecting a Dirty RAID-5
From: Adam Goryachev @ 2015-07-12 22:33 UTC (permalink / raw)
  To: jtroan, Mikael Abrahamsson; +Cc: linux-raid
In-Reply-To: <OF4E023A92.1624DC91-ON85257E80.0051B769-85257E80.005287C5@jt-sw.com>

On 13/07/15 01:01, jtroan@jt-sw.com wrote:
>> On Wed, 8 Jul 2015, jtroan@jt-sw.com wrote:
>>
>>> I'm also thinking about adding a fourth drive and try to config it as a
>>> hot-spare, giving me some extra margin for failure.
>> I would recommend you turn your raid5 into raid6 instead of having a
>> raid5+spare. This can be done with a fairly simple command, without
>> downtime.
>>
> I like the idea of using RAID-6 for / (for all the VMs under /var).  (I
> figure I'll probably still have to do a spare on my RAID-1 device
> for /boot .)
>
> What's the (mdadm?) command to convert an MD from RAID-5 to RAID-6?
>
I think the standard option would be to have a hot spare, and then 
something like:
mdadm /dev/mdX --grow --level=raid6

Also, for your raid1, never have a hot spare, just do this:
mdadm /dev/mdX --grow --raid-devices=3
Then you will always have all your data replicated on all three drives, 
so again, no hot spare required.

Also remember to duplicate the grub/whatever boot sector....

Regards,
Adam

-- 
Adam Goryachev Website Managers www.websitemanagers.com.au

^ permalink raw reply

* Re: mdadm-grow-continue service crashing (similiar to "raid5 reshape is stuck" thread from May)
From: Edward Kuns @ 2015-07-12 19:24 UTC (permalink / raw)
  To: Phil Turmel; +Cc: linux-raid
In-Reply-To: <55A26F65.5020703@turmel.org>

On Sun, Jul 12, 2015 at 8:45 AM, Phil Turmel <philip@turmel.org> wrote:
> Why were you using --grow for these operations only to reverse it?  This
> is dangerous if you have a layer or filesystem on your array that
> doesn't support shrinking.  None of the --grow operations were necessary
> in this sequence to achieve the end result of replacing disks.
[snip]
> At no point should you have changed the number of raid devices.
[snip]
> And for the still-running but suspect drive, the --replace operation
> would have been the right choice, again, after --add of a spare.

I didn't mention the steps I did to replace the failed drive because
that went flawlessly.  I did a fail and remove on it to be sure, but
got complaints that it was already failed/removed.  When I did an add
for the replacement drive, it came in and synced automatically.  I
only ran into trouble trying to replace the "not yet dead but suspect"
drive.  I was following examples on the Internet.  The example I was
following was a clearly a bad one.  The examples I found didn't
suggest the --replace option.  This is ultimately my fault for not
being familiar enough with this.  Now I know better.

FWIW, I had LVM on top of the raid5, with two partitions (/var and an
extra storage one) on the LVM.  (I think there is some spare space
too.)  The goal, of course, is being able to survive any single-drive
failure, which I did.

You said this is dangerous.  I went from 4->5 and then immediately
5->4 drives.  I didn't expand the LVM on the raid5, and the
replacement partition was a little bigger than the original.  Next
time, I'll use --replace, obviously.  I just want to understand why it
is dangerous.  As long as the replacement partition is as big as the
one it is replacing, isn't this just extra work, and more chance of
running into problems like the one I ran into?  But other than that,
it shouldn't risk the actual data stored on the RAID,should it?

> many modern distros delete /tmp on reboot and/or play
> games with namespaces to isolate different users' /tmp spaces.

So if the machine crashes during a rebuild, you may lose that backup
file, depending on the distro.  OK.  Is there a better solution to
this?  Unfortunately, at the time of the failure to shrink, the
rebuild that failed to start, stdout and stderr were not going to
/var/log/messages, so I have no idea what the complaint was at that
time.  Does this service send so much output to stdout/stderr that
it's useful to suppress it?  If I'd seen something in
/var/log/messages, it would have been more clear that there was a
service with a complaint that was the cause of the rebuild failing to
start.  I wouldn't have done as much thrashing trying to figure out
why.

> These are the only operations you should have done in the first place.
> Although I would have put the --add first, so the --fail operation would
> have triggered a rebuild onto the spare right away.

I did the fail/remove/add at the very end, after replacing the dead
drive, after finally completing the "don't do it this way again"
grow-to-5-then-shrink-to-4 process to replace the not-yet-dead drive.
After the shrink finally completed, the new 4th drive showed as a
spare and removed at the same time.  i.e., this dump from my first
EMail:

    Number   Major   Minor   RaidDevice State
       0       8        2        0    active sync   /dev/sda2
       1       8       17        1    active sync   /dev/sdb1
       5       8       33        2    active sync   /dev/sdc1
       6       0        0        6    removed

       6       8       49        -    spare   /dev/sdd1

Doing a fail, then remove, then add on that 4th partition (sdd1)
brought it back and it very quickly synced.  I did a forced fsck on
both partitions to be sure, and both were clean.

       Thanks

              Eddie

^ permalink raw reply

* Re: Resurrecting a Dirty RAID-5
From: jtroan @ 2015-07-12 15:01 UTC (permalink / raw)
  To: Mikael Abrahamsson; +Cc: linux-raid
In-Reply-To: <alpine.DEB.2.02.1507120756020.11810@uplift.swm.pp.se>

>
> On Wed, 8 Jul 2015, jtroan@jt-sw.com wrote:
>
> > I'm also thinking about adding a fourth drive and try to config it as a

> > hot-spare, giving me some extra margin for failure.
>
> I would recommend you turn your raid5 into raid6 instead of having a
> raid5+spare. This can be done with a fairly simple command, without
> downtime.
>

I like the idea of using RAID-6 for / (for all the VMs under /var).  (I
figure I'll probably still have to do a spare on my RAID-1 device
for /boot .)

What's the (mdadm?) command to convert an MD from RAID-5 to RAID-6?

Thanks.....

=======================================================================
John M. Troan  <jtroan@jt-sw.com>
Maintainer: Football Site @ JT-SW.com
  http://www.jt-sw.com/football
Chief of Computer Operations
  U.S.S. Kitty Hawk / NCC-1659
=======================================================================


^ permalink raw reply

* Re: mdadm-grow-continue service crashing (similiar to "raid5 reshape is stuck" thread from May)
From: Phil Turmel @ 2015-07-12 13:45 UTC (permalink / raw)
  To: Edward Kuns, linux-raid
In-Reply-To: <CACsGCySTbrEYddSNfi7+9KnxWeddkBGDbxTsBSJkkvGHOaJwKg@mail.gmail.com>

Hi Edward,

On 07/12/2015 02:02 AM, Edward Kuns wrote:

[trim /]

> The short version of the story is that I replaced the dead drive and
> let the raid5 partition rebuild.  Then I added a new drive and let the
> partition rebuild.  Then I removed the not-yet-dead drive and here is
> where I ran into the same problem as the other poster.  Basically, I
> did this to replace the still-working-but-suspect device, after the
> partition completed rebuilding when I replaced the actually-dead
> drive:
> 
> mdadm --manage /dev/md125 --add /dev/sdf1
> mdadm --grow --raid-devices=5 /dev/md125
> 
>  ... wait for the rebuild to complete
> 
> mdadm --fail /dev/md125 /dev/sdd2
> mdadm --remove /dev/md125 /dev/sdd2
> mdadm --grow --raid-devices=4 /dev/md125
> 
> mdadm: this change will reduce the size of the array.
>        use --grow --array-size first to truncate array.
>        e.g. mdadm --grow /dev/md125 --array-size 118964736
> 
> mdadm --grow /dev/md125 --array-size 118964736
> mdadm --grow --raid-devices=4 /dev/md125
> 
> ... this failed with a mysterious complaint about my first partition
> (Cannot set new_offset).  Research got me to try:
> 
> mdadm --grow --raid-devices=4 /dev/md125 --backup-file /root/md125.backup

Why were you using --grow for these operations only to reverse it?  This
is dangerous if you have a layer or filesystem on your array that
doesn't support shrinking.  None of the --grow operations were necessary
in this sequence to achieve the end result of replacing disks.

> .... here everything ground to a halt.  The reshape was at 0% and
> there was no disk activity.
> 
> The solution was to edit
> /lib/systemd/system/mdadm-grow-continue@.service to look like this (it
> was important that the backup file was placed in /tmp and not in /root
> or anywhere else.  SELinux allowed mdadm to create a file in /tmp by
> not anywhere else I tried):

I'm not an SELinux guy, so I can't help with the rest, but you should
know that many modern distros delete /tmp on reboot and/or play games
with namespaces to isolate different users' /tmp spaces.

[trim /]

> I did a fail, remove, and
> add on /dev/sdd1  and it very quickly synced and came into service.
> The command "mdadm --detail /dev/md125" now shows a happy raid5 with
> four partitions in it, all "active sync"

These are the only operations you should have done in the first place.
Although I would have put the --add first, so the --fail operation would
have triggered a rebuild onto the spare right away.  At no point should
you have changed the number of raid devices.

And for the still-running but suspect drive, the --replace operation
would have been the right choice, again, after --add of a spare.

HTH,

Phil

^ permalink raw reply

* mdadm-grow-continue service crashing (similiar to "raid5 reshape is stuck" thread from May)
From: Edward Kuns @ 2015-07-12  6:02 UTC (permalink / raw)
  To: linux-raid

I experienced a total drive failure.  Looking into it, I discovered
that the particular hard drive model that failed is a particularly bad
one.  So I replaced not only the failed drive, but another of the same
model.  In the process, I ran into a problem where on reboot the RAID
device was inactive.

I finally found a solution to my problem in the earlier thread "raid5
reshape is stuck" that started on 15 May.  By the way, I am on Fedora
21

> rpm -q mdadm
mdadm-3.3.2-1.fc21.x86_64

> uname -srvmpio
Linux 4.0.4-202.fc21.x86_64 #1 SMP Wed May 27 22:28:42 UTC 2015 x86_64
x86_64 x86_64 GNU/Linux

The short version of the story is that I replaced the dead drive and
let the raid5 partition rebuild.  Then I added a new drive and let the
partition rebuild.  Then I removed the not-yet-dead drive and here is
where I ran into the same problem as the other poster.  Basically, I
did this to replace the still-working-but-suspect device, after the
partition completed rebuilding when I replaced the actually-dead
drive:

mdadm --manage /dev/md125 --add /dev/sdf1
mdadm --grow --raid-devices=5 /dev/md125

 ... wait for the rebuild to complete

mdadm --fail /dev/md125 /dev/sdd2
mdadm --remove /dev/md125 /dev/sdd2
mdadm --grow --raid-devices=4 /dev/md125

mdadm: this change will reduce the size of the array.
       use --grow --array-size first to truncate array.
       e.g. mdadm --grow /dev/md125 --array-size 118964736

mdadm --grow /dev/md125 --array-size 118964736
mdadm --grow --raid-devices=4 /dev/md125

... this failed with a mysterious complaint about my first partition
(Cannot set new_offset).  Research got me to try:

mdadm --grow --raid-devices=4 /dev/md125 --backup-file /root/md125.backup

.... here everything ground to a halt.  The reshape was at 0% and
there was no disk activity.

The solution was to edit
/lib/systemd/system/mdadm-grow-continue@.service to look like this (it
was important that the backup file was placed in /tmp and not in /root
or anywhere else.  SELinux allowed mdadm to create a file in /tmp by
not anywhere else I tried):

#  This file is part of mdadm.
#
#  mdadm is free software; you can redistribute it and/or modify it
#  under the terms of the GNU General Public License as published by
#  the Free Software Foundation; either version 2 of the License, or
#  (at your option) any later version.

[Unit]
Description=Manage MD Reshape on /dev/%I
DefaultDependencies=no

[Service]
ExecStart=/usr/sbin/mdadm --grow --continue /dev/%I
--backup-file=/tmp/raid-backup-file
StandardInput=null
#StandardOutput=null
#StandardError=null
KillMode=none

I had to comment out the standard out and error lines to see why the
service was failing.  I was pulling out my hair.  The raid device
failed to initialize, so my computer dumped me into runlevel 1.

When the process finished after the above fix, I ended up in a weird state:

    Number   Major   Minor   RaidDevice State
       0       8        2        0    active sync   /dev/sda2
       1       8       17        1    active sync   /dev/sdb1
       5       8       33        2    active sync   /dev/sdc1
       6       0        0        6    removed

       6       8       49        -    spare   /dev/sdd1

but that is probably as a result of what I tried to bring it back.  I
could "stop" the raid and manually recreate it and the filesystems on
it were fine.  But it wouldn't come up without me doing that.  I'm
going to try to fail and re-add that disk again and see if it works
now that it was able to complete a sync.  I did a fail, remove, and
add on /dev/sdd1  and it very quickly synced and came into service.
The command "mdadm --detail /dev/md125" now shows a happy raid5 with
four partitions in it, all "active sync"  So all I had to do was add
the --backup-file to the command to "grow" down to 4 devices, and also
to mdadm-grow-continue@.service.

I thought I'd let you know, in particular, that adding
--backup-file=/tmp/raid-backup-file to the service file worked to get
the process unstuck, and that due to SELinux it must be in tmp.  Also,
should the "Cannot set new_offset" complaint maybe suggest trying
again with a backup file?

                 Eddie

^ permalink raw reply

* Re: Resurrecting a Dirty RAID-5
From: Mikael Abrahamsson @ 2015-07-12  5:57 UTC (permalink / raw)
  To: jtroan; +Cc: linux-raid
In-Reply-To: <OF71F1F6CD.C562AA57-ON85257E7D.000FAD79-85257E7D.0010311B@jt-sw.com>

On Wed, 8 Jul 2015, jtroan@jt-sw.com wrote:

> I'm also thinking about adding a fourth drive and try to config it as a 
> hot-spare, giving me some extra margin for failure.

I would recommend you turn your raid5 into raid6 instead of having a 
raid5+spare. This can be done with a fairly simple command, without 
downtime.

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: Shaohua Li @ 2015-07-10 17:48 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150710164209.5928d762@noble>

On Fri, Jul 10, 2015 at 04:42:09PM +1000, NeilBrown wrote:
> On Thu, 9 Jul 2015 22:18:15 -0700 Shaohua Li <shli@fb.com> wrote:
> 
> > On Fri, Jul 10, 2015 at 03:10:44PM +1000, NeilBrown wrote:
> > > On Thu, 9 Jul 2015 21:52:43 -0700 Shaohua Li <shli@fb.com> wrote:
> > > 
> > > > On Fri, Jul 10, 2015 at 02:36:56PM +1000, NeilBrown wrote:
> > > > > On Thu, 9 Jul 2015 21:08:49 -0700 Shaohua Li <shli@fb.com> wrote:
> > > > > 
> > > 
> > > > > There is also the issue of what action commits a previous transaction.
> > > > > I'm not sure what you had.  I'm suggesting that each metadata block
> > > > > commits previous transactions.  Is that a close-enough match to what
> > > > > you had?
> > > > 
> > > > What did you mean about a transaction? In my implementation, metadata
> > > > block and followed stripe data/parity consist of an io unit. io units can
> > > > be finished out of order. but if io unit has flush request (the data has
> > > > flush/flush bio or metadata is a flush block), the io unit can only
> > > > start after all previous io units and disk cache flush finish. Such io
> > > > unit is strictly ordered. The log patch describes this behavior. Does it
> > > > match?
> > > 
> > > Yes, a "transaction" is an "io unit".  The flushing is the same.
> > > I just couldn't remember how, when reading the log on restart, you
> > > determined if a given "io unit" was reliably consistent, or whether it
> > > should be ignored (having possibly only partially been written).
> > 
> > The metadata block has a checksum for data of the block. data/parity has
> > checksum stored in metadata block. This way we can know if metadata and
> > data is consistent.
> > 
> 
> OK .. though I'm not totally sold on the value of checksums.  When a
> checksum doesn't match, that means something.  When a checksum does
> match, it could just be a co-incidence.
> I'd rather have a process that made checksums unnecessary, and only use
> the checksums as a double-check.

We could do something like: write metadata/data, wait, write another
metadata. the second metadata indicates the first is in disk. But this
can impact performance very much. I think checksum should be fine. It
might be just a coninsidence, but the rate should extremely low. jbd2 is
using checksum too now.

Thanks,
Shaohua

^ permalink raw reply

* [PATCH 12/12] md-cluster: handle error situations more precisely in lockres_init
From: Guoqing Jiang @ 2015-07-10  9:01 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid
In-Reply-To: <1436518883-12783-1-git-send-email-gqjiang@suse.com>

In lockres_init, it's better to distinguish different err conditions.

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 drivers/md/md-cluster.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index c35a03a..54d225c 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -131,14 +131,14 @@ static struct dlm_lock_resource *lockres_init(struct mddev *mddev,
 	res->name = kzalloc(namelen + 1, GFP_KERNEL);
 	if (!res->name) {
 		pr_err("md-cluster: Unable to allocate resource name for resource %s\n", name);
-		goto out_err;
+		goto out_err_name;
 	}
 	strlcpy(res->name, name, namelen + 1);
 	if (with_lvb) {
 		res->lksb.sb_lvbptr = kzalloc(LVB_SIZE, GFP_KERNEL);
 		if (!res->lksb.sb_lvbptr) {
 			pr_err("md-cluster: Unable to allocate LVB for resource %s\n", name);
-			goto out_err;
+			goto out_err_lvb;
 		}
 		res->flags = DLM_LKF_VALBLK;
 	}
@@ -159,7 +159,9 @@ static struct dlm_lock_resource *lockres_init(struct mddev *mddev,
 	return res;
 out_err:
 	kfree(res->lksb.sb_lvbptr);
+out_err_lvb:
 	kfree(res->name);
+out_err_name:
 	kfree(res);
 	return NULL;
 }
@@ -627,7 +629,6 @@ static int gather_all_resync_info(struct mddev *mddev, int total_slots)
 	char str[64];
 	sector_t lo, hi;
 
-
 	for (i = 0; i < total_slots; i++) {
 		memset(str, '\0', 64);
 		snprintf(str, 64, "bitmap%04d", i);
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 11/12] md-cluster: Read the disk bitmap sb and check if it needs recovery
From: Guoqing Jiang @ 2015-07-10  9:01 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid
In-Reply-To: <1436518883-12783-1-git-send-email-gqjiang@suse.com>

In gather_all_resync_info, we need to read the disk bitmap sb and
check if it needs recovery.

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 drivers/md/md-cluster.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 29f65e2..c35a03a 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -625,6 +625,7 @@ static int gather_all_resync_info(struct mddev *mddev, int total_slots)
 	struct dlm_lock_resource *bm_lockres;
 	struct suspend_info *s;
 	char str[64];
+	sector_t lo, hi;
 
 
 	for (i = 0; i < total_slots; i++) {
@@ -659,7 +660,20 @@ static int gather_all_resync_info(struct mddev *mddev, int total_slots)
 			lockres_free(bm_lockres);
 			goto out;
 		}
-		/* TODO: Read the disk bitmap sb and check if it needs recovery */
+
+		/* Read the disk bitmap sb and check if it needs recovery */
+		ret = bitmap_copy_from_slot(mddev, i, &lo, &hi, false);
+		if (ret) {
+			pr_warn("md-cluster: Could not gather bitmaps from slot %d", i);
+			lockres_free(bm_lockres);
+			continue;
+		}
+		if ((hi > 0) && (lo < mddev->recovery_cp)) {
+			set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
+			mddev->recovery_cp = lo;
+			md_check_recovery(mddev);
+		}
+
 		dlm_unlock_sync(bm_lockres);
 		lockres_free(bm_lockres);
 	}
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 10/12] md-cluster: only call complete(&cinfo->completion) when node join cluster
From: Guoqing Jiang @ 2015-07-10  9:01 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid
In-Reply-To: <1436518883-12783-1-git-send-email-gqjiang@suse.com>

Introduce MD_CLUSTER_BEGIN_JOIN_CLUSTER flag to make sure
complete(&cinfo->completion) is only be invoked when node
join cluster. Otherwise node failure could also call the
complete, and it doesn't make sense to do it.

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 drivers/md/md-cluster.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 411b430..29f65e2 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -45,6 +45,7 @@ struct resync_info {
 /* md_cluster_info flags */
 #define		MD_CLUSTER_WAITING_FOR_NEWDISK		1
 #define		MD_CLUSTER_SUSPEND_READ_BALANCING	2
+#define		MD_CLUSTER_BEGIN_JOIN_CLUSTER		3
 
 
 struct md_cluster_info {
@@ -320,10 +321,17 @@ static void recover_done(void *arg, struct dlm_slot *slots,
 	struct md_cluster_info *cinfo = mddev->cluster_info;
 
 	cinfo->slot_number = our_slot;
-	complete(&cinfo->completion);
+	/* completion is only need to be complete when node join cluster,
+	 * it doesn't need to run during another node's failure */
+	if (test_bit(MD_CLUSTER_BEGIN_JOIN_CLUSTER, &cinfo->state)) {
+		complete(&cinfo->completion);
+		clear_bit(MD_CLUSTER_BEGIN_JOIN_CLUSTER, &cinfo->state);
+	}
 	clear_bit(MD_CLUSTER_SUSPEND_READ_BALANCING, &cinfo->state);
 }
 
+/* the ops is called when node join the cluster, and do lock recovery
+ * if node failure occurs */
 static const struct dlm_lockspace_ops md_ls_ops = {
 	.recover_prep = recover_prep,
 	.recover_slot = recover_slot,
@@ -675,6 +683,7 @@ static int join(struct mddev *mddev, int nodes)
 	INIT_LIST_HEAD(&cinfo->suspend_list);
 	spin_lock_init(&cinfo->suspend_lock);
 	init_completion(&cinfo->completion);
+	set_bit(MD_CLUSTER_BEGIN_JOIN_CLUSTER, &cinfo->state);
 
 	mutex_init(&cinfo->sb_mutex);
 	mddev->cluster_info = cinfo;
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 09/12] md-cluster: add missed lockres_free
From: Guoqing Jiang @ 2015-07-10  9:01 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid
In-Reply-To: <1436518883-12783-1-git-send-email-gqjiang@suse.com>

We also need to free the lock resource before goto out.

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 drivers/md/md-cluster.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 057a973..411b430 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -647,8 +647,10 @@ static int gather_all_resync_info(struct mddev *mddev, int total_slots)
 			lockres_free(bm_lockres);
 			continue;
 		}
-		if (ret)
+		if (ret) {
+			lockres_free(bm_lockres);
 			goto out;
+		}
 		/* TODO: Read the disk bitmap sb and check if it needs recovery */
 		dlm_unlock_sync(bm_lockres);
 		lockres_free(bm_lockres);
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 08/12] md-cluster: remove the unused sb_lock
From: Guoqing Jiang @ 2015-07-10  9:01 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid
In-Reply-To: <1436518883-12783-1-git-send-email-gqjiang@suse.com>

The sb_lock is not used anywhere, so let's remove it.

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 drivers/md/md-cluster.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 6f1ea3c..057a973 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -52,7 +52,6 @@ struct md_cluster_info {
 	dlm_lockspace_t *lockspace;
 	int slot_number;
 	struct completion completion;
-	struct dlm_lock_resource *sb_lock;
 	struct mutex sb_mutex;
 	struct dlm_lock_resource *bitmap_lockres;
 	struct list_head suspend_list;
@@ -692,12 +691,6 @@ static int join(struct mddev *mddev, int nodes)
 		ret = -ERANGE;
 		goto err;
 	}
-	cinfo->sb_lock = lockres_init(mddev, "cmd-super",
-					NULL, 0);
-	if (!cinfo->sb_lock) {
-		ret = -ENOMEM;
-		goto err;
-	}
 	/* Initiate the communication resources */
 	ret = -ENOMEM;
 	cinfo->recv_thread = md_register_thread(recv_daemon, mddev, "cluster_recv");
@@ -749,7 +742,6 @@ err:
 	lockres_free(cinfo->ack_lockres);
 	lockres_free(cinfo->no_new_dev_lockres);
 	lockres_free(cinfo->bitmap_lockres);
-	lockres_free(cinfo->sb_lock);
 	if (cinfo->lockspace)
 		dlm_release_lockspace(cinfo->lockspace, 2);
 	mddev->cluster_info = NULL;
@@ -770,7 +762,6 @@ static int leave(struct mddev *mddev)
 	lockres_free(cinfo->token_lockres);
 	lockres_free(cinfo->ack_lockres);
 	lockres_free(cinfo->no_new_dev_lockres);
-	lockres_free(cinfo->sb_lock);
 	lockres_free(cinfo->bitmap_lockres);
 	dlm_release_lockspace(cinfo->lockspace, 2);
 	return 0;
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 07/12] md-cluster: init suspend_list and suspend_lock early in join
From: Guoqing Jiang @ 2015-07-10  9:01 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid
In-Reply-To: <1436518883-12783-1-git-send-email-gqjiang@suse.com>

If the node just join the cluster, and receive the msg from other nodes
before init suspend_list, it will cause kernel crash due to NULL pointer
dereference, so move the initializations early to fix the bug.

md-cluster: Joined cluster 3578507b-e0cb-6d4f-6322-696cd7b1b10c slot 3
BUG: unable to handle kernel NULL pointer dereference at           (null)
... ... ...
Call Trace:
[<ffffffffa0444924>] process_recvd_msg+0x2e4/0x330 [md_cluster]
[<ffffffffa0444a06>] recv_daemon+0x96/0x170 [md_cluster]
[<ffffffffa045189d>] md_thread+0x11d/0x170 [md_mod]
[<ffffffff810768c4>] kthread+0xb4/0xc0
[<ffffffff8151927c>] ret_from_fork+0x7c/0xb0
... ... ...
RIP  [<ffffffffa0443581>] __remove_suspend_info+0x11/0xa0 [md_cluster]

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 drivers/md/md-cluster.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index b80a689..6f1ea3c 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -671,6 +671,8 @@ static int join(struct mddev *mddev, int nodes)
 	if (!cinfo)
 		return -ENOMEM;
 
+	INIT_LIST_HEAD(&cinfo->suspend_list);
+	spin_lock_init(&cinfo->suspend_lock);
 	init_completion(&cinfo->completion);
 
 	mutex_init(&cinfo->sb_mutex);
@@ -736,9 +738,6 @@ static int join(struct mddev *mddev, int nodes)
 		goto err;
 	}
 
-	INIT_LIST_HEAD(&cinfo->suspend_list);
-	spin_lock_init(&cinfo->suspend_lock);
-
 	ret = gather_all_resync_info(mddev, nodes);
 	if (ret)
 		goto err;
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 06/12] md-cluster: add the error check if failed to get dlm lock
From: Guoqing Jiang @ 2015-07-10  9:01 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid
In-Reply-To: <1436518883-12783-1-git-send-email-gqjiang@suse.com>

In complicated cluster environment, it is possible that the
dlm lock couldn't be get/convert on purpose, the related err
info is added for better debug potential issue.

For lockres_free, if the lock is blocking by a lock request or
conversion request, then dlm_unlock just put it back to grant
queue, so need to ensure the lock is free finally.

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 drivers/md/md-cluster.c | 41 +++++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 2a57f19..b80a689 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -166,10 +166,24 @@ out_err:
 
 static void lockres_free(struct dlm_lock_resource *res)
 {
+	int ret;
+
 	if (!res)
 		return;
 
-	dlm_unlock(res->ls, res->lksb.sb_lkid, 0, &res->lksb, res);
+	/* cancel a lock request or a conversion request that is blocked */
+	res->flags |= DLM_LKF_CANCEL;
+retry:
+	ret = dlm_unlock(res->ls, res->lksb.sb_lkid, 0, &res->lksb, res);
+	if (unlikely(ret != 0)) {
+		pr_info("%s: failed to unlock %s return %d\n", __func__, res->name, ret);
+
+		/* if a lock conversion is cancelled, then the lock is put
+		 * back to grant queue, need to ensure it is unlocked */
+		if (ret == -DLM_ECANCEL)
+			goto retry;
+	}
+	res->flags &= ~DLM_LKF_CANCEL;
 	wait_for_completion(&res->completion);
 
 	kfree(res->name);
@@ -474,6 +488,7 @@ static void recv_daemon(struct md_thread *thread)
 	struct dlm_lock_resource *ack_lockres = cinfo->ack_lockres;
 	struct dlm_lock_resource *message_lockres = cinfo->message_lockres;
 	struct cluster_msg msg;
+	int ret;
 
 	/*get CR on Message*/
 	if (dlm_lock_sync(message_lockres, DLM_LOCK_CR)) {
@@ -486,13 +501,21 @@ static void recv_daemon(struct md_thread *thread)
 	process_recvd_msg(thread->mddev, &msg);
 
 	/*release CR on ack_lockres*/
-	dlm_unlock_sync(ack_lockres);
+	ret = dlm_unlock_sync(ack_lockres);
+	if (unlikely(ret != 0))
+		pr_info("unlock ack failed return %d\n", ret);
 	/*up-convert to PR on message_lockres*/
-	dlm_lock_sync(message_lockres, DLM_LOCK_PR);
+	ret = dlm_lock_sync(message_lockres, DLM_LOCK_PR);
+	if (unlikely(ret != 0))
+		pr_info("lock PR on msg failed return %d\n", ret);
 	/*get CR on ack_lockres again*/
-	dlm_lock_sync(ack_lockres, DLM_LOCK_CR);
+	ret = dlm_lock_sync(ack_lockres, DLM_LOCK_CR);
+	if (unlikely(ret != 0))
+		pr_info("lock CR on ack failed return %d\n", ret);
 	/*release CR on message_lockres*/
-	dlm_unlock_sync(message_lockres);
+	ret = dlm_unlock_sync(message_lockres);
+	if (unlikely(ret != 0))
+		pr_info("unlock msg failed return %d\n", ret);
 }
 
 /* lock_comm()
@@ -567,7 +590,13 @@ static int __sendmsg(struct md_cluster_info *cinfo, struct cluster_msg *cmsg)
 	}
 
 failed_ack:
-	dlm_unlock_sync(cinfo->message_lockres);
+	error = dlm_unlock_sync(cinfo->message_lockres);
+	if (unlikely(error != 0)) {
+		pr_err("md-cluster: failed convert to NL on MESSAGE(%d)\n",
+			error);
+		/* in case the message can't be released due to some reason */
+		goto failed_ack;
+	}
 failed_message:
 	return error;
 }
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 05/12] md-cluster: init completion within lockres_init
From: Guoqing Jiang @ 2015-07-10  9:01 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid
In-Reply-To: <1436518883-12783-1-git-send-email-gqjiang@suse.com>

We should init completion within lockres_init, otherwise
completion could be initialized more than one time during
it's life cycle.

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 drivers/md/md-cluster.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 85b7836..2a57f19 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -100,7 +100,6 @@ static int dlm_lock_sync(struct dlm_lock_resource *res, int mode)
 {
 	int ret = 0;
 
-	init_completion(&res->completion);
 	ret = dlm_lock(res->ls, mode, &res->lksb,
 			res->flags, res->name, strlen(res->name),
 			0, sync_ast, res, res->bast);
@@ -125,6 +124,7 @@ static struct dlm_lock_resource *lockres_init(struct mddev *mddev,
 	res = kzalloc(sizeof(struct dlm_lock_resource), GFP_KERNEL);
 	if (!res)
 		return NULL;
+	init_completion(&res->completion);
 	res->ls = cinfo->lockspace;
 	res->mddev = mddev;
 	namelen = strlen(name);
@@ -169,7 +169,6 @@ static void lockres_free(struct dlm_lock_resource *res)
 	if (!res)
 		return;
 
-	init_completion(&res->completion);
 	dlm_unlock(res->ls, res->lksb.sb_lkid, 0, &res->lksb, res);
 	wait_for_completion(&res->completion);
 
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 04/12] md-cluster: fix deadlock issue on message lock
From: Guoqing Jiang @ 2015-07-10  9:01 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid
In-Reply-To: <1436518453-12660-1-git-send-email-gqjiang@suse.com>

There is problem with previous communication mechanism, and we got below
deadlock scenario with cluster which has 3 nodes.

	Sender                	    Receiver        		Receiver

	token(EX)
       message(EX)
      writes message
   downconverts message(CR)
      requests ack(EX)
		                  get message(CR)            gets message(CR)
                		  reads message                reads message
		               requests EX on message    requests EX on message

To fix this problem, we do the following changes:

1. the sender downconverts MESSAGE to CW rather than CR.
2. and the receiver request PR lock not EX lock on message.

And in case we failed to down-convert EX to CW on message, it is better to
unlock message otherthan still hold the lock.

Signed-off-by: Lidong Zhong <ldzhong@suse.com>
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 Documentation/md-cluster.txt |  4 ++--
 drivers/md/md-cluster.c      | 14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/md-cluster.txt b/Documentation/md-cluster.txt
index de1af7d..1b79436 100644
--- a/Documentation/md-cluster.txt
+++ b/Documentation/md-cluster.txt
@@ -91,7 +91,7 @@ The algorithm is:
     this message inappropriate or redundant.
 
  3. sender write LVB.
-    sender down-convert MESSAGE from EX to CR
+    sender down-convert MESSAGE from EX to CW
     sender try to get EX of ACK
     [ wait until all receiver has *processed* the MESSAGE ]
 
@@ -112,7 +112,7 @@ The algorithm is:
     sender down-convert ACK from EX to CR
     sender release MESSAGE
     sender release TOKEN
-                               receiver upconvert to EX of MESSAGE
+                               receiver upconvert to PR of MESSAGE
                                receiver get CR of ACK
                                receiver release MESSAGE
 
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 47199ad..85b7836 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -488,8 +488,8 @@ static void recv_daemon(struct md_thread *thread)
 
 	/*release CR on ack_lockres*/
 	dlm_unlock_sync(ack_lockres);
-	/*up-convert to EX on message_lockres*/
-	dlm_lock_sync(message_lockres, DLM_LOCK_EX);
+	/*up-convert to PR on message_lockres*/
+	dlm_lock_sync(message_lockres, DLM_LOCK_PR);
 	/*get CR on ack_lockres again*/
 	dlm_lock_sync(ack_lockres, DLM_LOCK_CR);
 	/*release CR on message_lockres*/
@@ -522,7 +522,7 @@ static void unlock_comm(struct md_cluster_info *cinfo)
  * The function:
  * 1. Grabs the message lockresource in EX mode
  * 2. Copies the message to the message LVB
- * 3. Downconverts message lockresource to CR
+ * 3. Downconverts message lockresource to CW
  * 4. Upconverts ack lock resource from CR to EX. This forces the BAST on other nodes
  *    and the other nodes read the message. The thread will wait here until all other
  *    nodes have released ack lock resource.
@@ -543,12 +543,12 @@ static int __sendmsg(struct md_cluster_info *cinfo, struct cluster_msg *cmsg)
 
 	memcpy(cinfo->message_lockres->lksb.sb_lvbptr, (void *)cmsg,
 			sizeof(struct cluster_msg));
-	/*down-convert EX to CR on Message*/
-	error = dlm_lock_sync(cinfo->message_lockres, DLM_LOCK_CR);
+	/*down-convert EX to CW on Message*/
+	error = dlm_lock_sync(cinfo->message_lockres, DLM_LOCK_CW);
 	if (error) {
-		pr_err("md-cluster: failed to convert EX to CR on MESSAGE(%d)\n",
+		pr_err("md-cluster: failed to convert EX to CW on MESSAGE(%d)\n",
 				error);
-		goto failed_message;
+		goto failed_ack;
 	}
 
 	/*up-convert CR to EX on Ack*/
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 03/12] md-cluster: transfer the resync ownership to another node
From: Guoqing Jiang @ 2015-07-10  8:54 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid, Guoqing Jiang
In-Reply-To: <1436518453-12660-1-git-send-email-gqjiang@suse.com>

When node A stop an array while the array is doing resync, we need
let another node B to take over the resync task.

To achieve the goal, we need the A send an explicit BITMAP_NEEDS_SYNC
message to the cluster. And the node B which received that message will
invoke __recover_slot to do resync.

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 drivers/md/md-cluster.c | 15 +++++++++++++++
 drivers/md/md.c         |  6 +++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 24caabe..47199ad 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -75,6 +75,7 @@ enum msg_type {
 	NEWDISK,
 	REMOVE,
 	RE_ADD,
+	BITMAP_NEEDS_SYNC,
 };
 
 struct cluster_msg {
@@ -454,6 +455,11 @@ static void process_recvd_msg(struct mddev *mddev, struct cluster_msg *msg)
 			__func__, __LINE__, msg->slot);
 		process_readd_disk(mddev, msg);
 		break;
+	case BITMAP_NEEDS_SYNC:
+		pr_info("%s: %d Received BITMAP_NEEDS_SYNC from %d\n",
+			__func__, __LINE__, msg->slot);
+		__recover_slot(mddev, msg->slot);
+		break;
 	default:
 		pr_warn("%s:%d Received unknown message from %d\n",
 			__func__, __LINE__, msg->slot);
@@ -814,8 +820,17 @@ static int resync_start(struct mddev *mddev, sector_t lo, sector_t hi)
 
 static void resync_finish(struct mddev *mddev)
 {
+	struct md_cluster_info *cinfo = mddev->cluster_info;
+	struct cluster_msg cmsg;
+	int slot = cinfo->slot_number - 1;
+
 	pr_info("%s:%d\n", __func__, __LINE__);
 	resync_send(mddev, RESYNCING, 0, 0);
+	if (test_bit(MD_RECOVERY_INTR, &mddev->recovery)) {
+		cmsg.type = cpu_to_le32(BITMAP_NEEDS_SYNC);
+		cmsg.slot = cpu_to_le32(slot);
+		sendmsg(cinfo, &cmsg);
+	}
 }
 
 static int area_resyncing(struct mddev *mddev, int direction,
diff --git a/drivers/md/md.c b/drivers/md/md.c
index df92d30..7d05dff 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7931,9 +7931,6 @@ void md_do_sync(struct md_thread *thread)
 	/* tell personality that we are finished */
 	mddev->pers->sync_request(mddev, max_sectors, &skipped);
 
-	if (mddev_is_clustered(mddev))
-		md_cluster_ops->resync_finish(mddev);
-
 	if (!test_bit(MD_RECOVERY_CHECK, &mddev->recovery) &&
 	    mddev->curr_resync > 2) {
 		if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery)) {
@@ -7967,6 +7964,9 @@ void md_do_sync(struct md_thread *thread)
 		}
 	}
  skip:
+	if (mddev_is_clustered(mddev))
+		md_cluster_ops->resync_finish(mddev);
+
 	set_bit(MD_CHANGE_DEVS, &mddev->flags);
 
 	spin_lock(&mddev->lock);
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 02/12] md-cluster: split recover_slot for future code reuse
From: Guoqing Jiang @ 2015-07-10  8:54 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid, Guoqing Jiang
In-Reply-To: <1436518453-12660-1-git-send-email-gqjiang@suse.com>

Make recover_slot as a wraper to __recover_slot, since the
logic of __recover_slot could be reuse for the condition
when other nodes need to take over the resync job.

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 drivers/md/md-cluster.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 85ef5c5..24caabe 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -269,16 +269,11 @@ static void recover_prep(void *arg)
 	set_bit(MD_CLUSTER_SUSPEND_READ_BALANCING, &cinfo->state);
 }
 
-static void recover_slot(void *arg, struct dlm_slot *slot)
+static void __recover_slot(struct mddev *mddev, int slot)
 {
-	struct mddev *mddev = arg;
 	struct md_cluster_info *cinfo = mddev->cluster_info;
 
-	pr_info("md-cluster: %s Node %d/%d down. My slot: %d. Initiating recovery.\n",
-			mddev->bitmap_info.cluster_name,
-			slot->nodeid, slot->slot,
-			cinfo->slot_number);
-	set_bit(slot->slot - 1, &cinfo->recovery_map);
+	set_bit(slot, &cinfo->recovery_map);
 	if (!cinfo->recovery_thread) {
 		cinfo->recovery_thread = md_register_thread(recover_bitmaps,
 				mddev, "recover");
@@ -290,6 +285,20 @@ static void recover_slot(void *arg, struct dlm_slot *slot)
 	md_wakeup_thread(cinfo->recovery_thread);
 }
 
+static void recover_slot(void *arg, struct dlm_slot *slot)
+{
+	struct mddev *mddev = arg;
+	struct md_cluster_info *cinfo = mddev->cluster_info;
+
+	pr_info("md-cluster: %s Node %d/%d down. My slot: %d. Initiating recovery.\n",
+			mddev->bitmap_info.cluster_name,
+			slot->nodeid, slot->slot,
+			cinfo->slot_number);
+	/* deduct one since dlm slot starts from one while the num of
+	 * cluster-md begins with 0 */
+	__recover_slot(mddev, slot->slot - 1);
+}
+
 static void recover_done(void *arg, struct dlm_slot *slots,
 		int num_slots, int our_slot,
 		uint32_t generation)
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 01/12] md-cluster: use %pU to print UUIDs
From: Guoqing Jiang @ 2015-07-10  8:54 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid, Guoqing Jiang
In-Reply-To: <1436518453-12660-1-git-send-email-gqjiang@suse.com>

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 drivers/md/md-cluster.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 0072190..85ef5c5 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -177,18 +177,6 @@ static void lockres_free(struct dlm_lock_resource *res)
 	kfree(res);
 }
 
-static char *pretty_uuid(char *dest, char *src)
-{
-	int i, len = 0;
-
-	for (i = 0; i < 16; i++) {
-		if (i == 4 || i == 6 || i == 8 || i == 10)
-			len += sprintf(dest + len, "-");
-		len += sprintf(dest + len, "%02x", (__u8)src[i]);
-	}
-	return dest;
-}
-
 static void add_resync_info(struct mddev *mddev, struct dlm_lock_resource *lockres,
 		sector_t lo, sector_t hi)
 {
@@ -388,7 +376,7 @@ static void process_add_new_disk(struct mddev *mddev, struct cluster_msg *cmsg)
 	int len;
 
 	len = snprintf(disk_uuid, 64, "DEVICE_UUID=");
-	pretty_uuid(disk_uuid + len, cmsg->uuid);
+	sprintf(disk_uuid + len, "%pU", cmsg->uuid);
 	snprintf(raid_slot, 16, "RAID_DISK=%d", cmsg->raid_slot);
 	pr_info("%s:%d Sending kobject change with %s and %s\n", __func__, __LINE__, disk_uuid, raid_slot);
 	init_completion(&cinfo->newdisk_completion);
@@ -646,7 +634,7 @@ static int join(struct mddev *mddev, int nodes)
 	mddev->cluster_info = cinfo;
 
 	memset(str, 0, 64);
-	pretty_uuid(str, mddev->uuid);
+	sprintf(str, "%pU", mddev->uuid);
 	ret = dlm_new_lockspace(str, mddev->bitmap_info.cluster_name,
 				DLM_LSFL_FS, LVB_SIZE,
 				&md_ls_ops, mddev, &ops_rv, &cinfo->lockspace);
-- 
1.7.12.4


^ permalink raw reply related

* [PATCH 00/12] md-cluster: code improvement, fixs and new feature
From: Guoqing Jiang @ 2015-07-10  8:54 UTC (permalink / raw)
  To: neilb; +Cc: rgoldwyn, linux-raid, Guoqing Jiang

This series mostly do the following jobs:

1. code improvement
	0001-md-cluster-use-pU-to-print-UUIDs.patch
	0006-md-cluster-add-the-error-check-if-failed-to-get-dlm-.patch
	0008-md-cluster-remove-the-unused-sb_lock.patch
	0009-md-cluster-add-missed-lockres_free.patch
	0010-md-cluster-only-call-complete-cinfo-completion-when-.patch
	0011-md-cluster-Read-the-disk-bitmap-sb-and-check-if-it-n.patch
	0012-md-cluster-handle-error-situations-more-precisely-in.patch
2. fixs some dlm issues and crash problem
	0004-md-cluster-fix-deadlock-issue-on-message-lock.patch
	0005-md-cluster-init-completion-within-lockres_init.patch
	0007-md-cluster-init-suspend_list-and-suspend_lock-early-.patch
3. new feature - When node A stop an array while the array is doing resync,
we need let another node B to take over the resync task.
	0002-md-cluster-split-recover_slot-for-future-code-reuse.patch
	0003-md-cluster-transfer-the-resync-ownership-to-another-.patch

Guoqing Jiang (12):
  md-cluster: use %pU to print UUIDs
  md-cluster: split recover_slot for future code reuse
  md-cluster: transfer the resync ownership to another node
  md-cluster: fix deadlock issue on message lock
  md-cluster: init completion within lockres_init
  md-cluster: add the error check if failed to get dlm lock
  md-cluster: init suspend_list and suspend_lock early in join
  md-cluster: remove the unused sb_lock
  md-cluster: add missed lockres_free
  md-cluster: only call complete(&cinfo->completion) when node join
    cluster
  md-cluster: Read the disk bitmap sb and check if it needs recovery
  md-cluster: handle error situations more precisely in lockres_init

 Documentation/md-cluster.txt |   4 +-
 drivers/md/md-cluster.c      | 162 +++++++++++++++++++++++++++++--------------
 drivers/md/md.c              |   6 +-
 3 files changed, 114 insertions(+), 58 deletions(-)

-- 
1.7.12.4


^ permalink raw reply

* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: NeilBrown @ 2015-07-10  6:42 UTC (permalink / raw)
  To: Shaohua Li; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150710051815.GA1902680@devbig257.prn2.facebook.com>

On Thu, 9 Jul 2015 22:18:15 -0700 Shaohua Li <shli@fb.com> wrote:

> On Fri, Jul 10, 2015 at 03:10:44PM +1000, NeilBrown wrote:
> > On Thu, 9 Jul 2015 21:52:43 -0700 Shaohua Li <shli@fb.com> wrote:
> > 
> > > On Fri, Jul 10, 2015 at 02:36:56PM +1000, NeilBrown wrote:
> > > > On Thu, 9 Jul 2015 21:08:49 -0700 Shaohua Li <shli@fb.com> wrote:
> > > > 
> > 
> > > > There is also the issue of what action commits a previous transaction.
> > > > I'm not sure what you had.  I'm suggesting that each metadata block
> > > > commits previous transactions.  Is that a close-enough match to what
> > > > you had?
> > > 
> > > What did you mean about a transaction? In my implementation, metadata
> > > block and followed stripe data/parity consist of an io unit. io units can
> > > be finished out of order. but if io unit has flush request (the data has
> > > flush/flush bio or metadata is a flush block), the io unit can only
> > > start after all previous io units and disk cache flush finish. Such io
> > > unit is strictly ordered. The log patch describes this behavior. Does it
> > > match?
> > 
> > Yes, a "transaction" is an "io unit".  The flushing is the same.
> > I just couldn't remember how, when reading the log on restart, you
> > determined if a given "io unit" was reliably consistent, or whether it
> > should be ignored (having possibly only partially been written).
> 
> The metadata block has a checksum for data of the block. data/parity has
> checksum stored in metadata block. This way we can know if metadata and
> data is consistent.
> 

OK .. though I'm not totally sold on the value of checksums.  When a
checksum doesn't match, that means something.  When a checksum does
match, it could just be a co-incidence.
I'd rather have a process that made checksums unnecessary, and only use
the checksums as a double-check.

NeilBrown

^ permalink raw reply

* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: Shaohua Li @ 2015-07-10  5:18 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150710151044.396f9645@noble>

On Fri, Jul 10, 2015 at 03:10:44PM +1000, NeilBrown wrote:
> On Thu, 9 Jul 2015 21:52:43 -0700 Shaohua Li <shli@fb.com> wrote:
> 
> > On Fri, Jul 10, 2015 at 02:36:56PM +1000, NeilBrown wrote:
> > > On Thu, 9 Jul 2015 21:08:49 -0700 Shaohua Li <shli@fb.com> wrote:
> > > 
> 
> > > There is also the issue of what action commits a previous transaction.
> > > I'm not sure what you had.  I'm suggesting that each metadata block
> > > commits previous transactions.  Is that a close-enough match to what
> > > you had?
> > 
> > What did you mean about a transaction? In my implementation, metadata
> > block and followed stripe data/parity consist of an io unit. io units can
> > be finished out of order. but if io unit has flush request (the data has
> > flush/flush bio or metadata is a flush block), the io unit can only
> > start after all previous io units and disk cache flush finish. Such io
> > unit is strictly ordered. The log patch describes this behavior. Does it
> > match?
> 
> Yes, a "transaction" is an "io unit".  The flushing is the same.
> I just couldn't remember how, when reading the log on restart, you
> determined if a given "io unit" was reliably consistent, or whether it
> should be ignored (having possibly only partially been written).

The metadata block has a checksum for data of the block. data/parity has
checksum stored in metadata block. This way we can know if metadata and
data is consistent.

Thanks,
Shaohua

^ 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