* [PATCH v2 2/2] md/r5cache: fix io_unit handling in r5l_log_endio()
From: Song Liu @ 2017-07-31 21:52 UTC (permalink / raw)
To: linux-raid
Cc: shli, neilb, kernel-team, dan.j.williams, hch, jes.sorensen,
Song Liu
In-Reply-To: <20170731215227.2409587-1-songliubraving@fb.com>
In r5l_log_endio(), once log->io_list_lock is released, the io unit
may be accessed (or even freed) by other threads. Current code
doesn't handle the io_unit properly, which leads to potential race
conditions.
This patch solves this race condition by:
1. Add a pending_stripe count flush_payload. Note that multiple
flush_payloads are counted as only one pending_stripe. Flag
has_flush_payload is added to show whether the io unit has
flush_payload;
2. In r5l_log_endio(), check flags has_null_flush and
has_flush_payload with log->io_list_lock held. After the lock
is released, this IO unit is only accessed when we know the
pending_stripe counter cannot be zeroed by other threas.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
drivers/md/raid5-cache.c | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 98b5aa9..76c236c 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -239,6 +239,7 @@ struct r5l_io_unit {
unsigned int has_flush:1; /* include flush request */
unsigned int has_fua:1; /* include fua request */
unsigned int has_null_flush:1; /* include empty flush request */
+ unsigned int has_flush_payload:1; /* include flush payload */
/*
* io isn't sent yet, flush/fua request can only be submitted till it's
* the first IO in running_ios list
@@ -571,6 +572,8 @@ static void r5l_log_endio(struct bio *bio)
struct r5l_io_unit *io_deferred;
struct r5l_log *log = io->log;
unsigned long flags;
+ bool has_null_flush;
+ bool has_flush_payload;
if (bio->bi_status)
md_error(log->rdev->mddev, log->rdev);
@@ -580,6 +583,19 @@ static void r5l_log_endio(struct bio *bio)
spin_lock_irqsave(&log->io_list_lock, flags);
__r5l_set_io_unit_state(io, IO_UNIT_IO_END);
+
+ /*
+ * if the io doesn't not have null_flush or flush payload,
+ * it is not safe to access it after releasing io_list_lock.
+ * Therefore, it is necessary to check the condition with
+ * the lock held.
+ */
+ has_null_flush = io->has_null_flush;
+ has_flush_payload = io->has_flush_payload;
+ /* finish flush only io_unit and PAYLOAD_FLUSH only io_unit */
+ if (atomic_read(&io->pending_stripe) == 0)
+ __r5l_set_io_unit_state(io, IO_UNIT_STRIPE_END);
+
if (log->need_cache_flush && !list_empty(&io->stripe_list))
r5l_move_to_end_ios(log);
else
@@ -600,18 +616,20 @@ static void r5l_log_endio(struct bio *bio)
if (log->need_cache_flush)
md_wakeup_thread(log->rdev->mddev->thread);
- if (io->has_null_flush) {
+ if (has_null_flush) {
struct bio *bi;
WARN_ON(bio_list_empty(&io->flush_barriers));
while ((bi = bio_list_pop(&io->flush_barriers)) != NULL) {
bio_endio(bi);
- atomic_dec(&io->pending_stripe);
+ if (atomic_dec_and_test(&io->pending_stripe)) {
+ __r5l_stripe_write_finished(io);
+ return;
}
}
-
- /* finish flush only io_unit and PAYLOAD_FLUSH only io_unit */
- if (atomic_read(&io->pending_stripe) == 0)
+ }
+ if (has_flush_payload)
+ if (atomic_dec_and_test(&io->pending_stripe))
__r5l_stripe_write_finished(io);
}
@@ -881,6 +899,11 @@ static void r5l_append_flush_payload(struct r5l_log *log, sector_t sect)
payload->size = cpu_to_le32(sizeof(__le64));
payload->flush_stripes[0] = cpu_to_le64(sect);
io->meta_offset += meta_size;
+ /* multiple flush payloads count as one pending_stripe */
+ if (!io->has_flush_payload) {
+ io->has_flush_payload = 1;
+ atomic_inc(&io->pending_stripe);
+ }
mutex_unlock(&log->io_mutex);
}
--
2.9.3
^ permalink raw reply related
* (unknown),
From: xa0ajutor @ 2017-08-01 1:35 UTC (permalink / raw)
To: linux-raid
[-- Attachment #1: EMAIL_2558300_linux-raid.zip --]
[-- Type: application/zip, Size: 2603 bytes --]
^ permalink raw reply
* [PATCH] udev rules: introduce rules for cluster-md to confirm device
From: Guoqing Jiang @ 2017-08-01 3:15 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, neilb, Guoqing Jiang
For cluster scenario, add device is different with native raid.
When a node issues adds a device, then the node will broadcast
a message with UUID to other nodes in the cluster. If receiving
node can find the device with the specific UUID, it must confirm
the device, otherwise reports it is missing.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
Makefile | 6 ++++--
udev-md-clustered-confirm-device.rules | 20 ++++++++++++++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
create mode 100644 udev-md-clustered-confirm-device.rules
diff --git a/Makefile b/Makefile
index 021d3ad..0977f27 100644
--- a/Makefile
+++ b/Makefile
@@ -256,8 +256,10 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
$(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
$(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
-install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules
- @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
+install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules \
+ udev-md-clustered-confirm-device.rules
+ @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules \
+ 69-md-clustered-confirm-device.rules ; \
do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp.1 && \
$(ECHO) $(INSTALL) -D -m 644 udev-$${file#??-} $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
$(INSTALL) -D -m 644 .install.tmp.1 $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
diff --git a/udev-md-clustered-confirm-device.rules b/udev-md-clustered-confirm-device.rules
new file mode 100644
index 0000000..d763e27
--- /dev/null
+++ b/udev-md-clustered-confirm-device.rules
@@ -0,0 +1,20 @@
+# do not edit this file, it will be overwritten on update
+
+SUBSYSTEM!="block", GOTO="clustermd_end"
+
+# handle md arrays
+KERNEL!="md*", GOTO="clustermd_end"
+ENV{DEVTYPE}!="disk", GOTO="clustermd_end"
+
+# Based on the received UUID, node confirms the device if
+# it is found by blkid, otherwise the node reports it is
+# missing.
+ACTION=="change", ENV{EVENT}=="ADD_DEVICE", ENV{DEVICE_UUID}=="?*", ENV{RAID_DISK}=="?*", \
+PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", RESULT!="", \
+RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:$result"
+
+ACTION=="change", ENV{EVENT}=="ADD_DEVICE", ENV{DEVICE_UUID}=="?*", ENV{RAID_DISK}=="?*", \
+PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", RESULT=="", \
+RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:missing"
+
+LABEL="clustermd_end"
--
2.10.0
^ permalink raw reply related
* Re: [PATCH] udev rules: introduce rules for cluster-md to confirm device
From: NeilBrown @ 2017-08-01 4:40 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Guoqing Jiang
In-Reply-To: <20170801031504.8452-1-gqjiang@suse.com>
[-- Attachment #1: Type: text/plain, Size: 3275 bytes --]
On Tue, Aug 01 2017, Guoqing Jiang wrote:
> For cluster scenario, add device is different with native raid.
> When a node issues adds a device, then the node will broadcast
> a message with UUID to other nodes in the cluster. If receiving
> node can find the device with the specific UUID, it must confirm
> the device, otherwise reports it is missing.
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
> Makefile | 6 ++++--
> udev-md-clustered-confirm-device.rules | 20 ++++++++++++++++++++
> 2 files changed, 24 insertions(+), 2 deletions(-)
> create mode 100644 udev-md-clustered-confirm-device.rules
>
> diff --git a/Makefile b/Makefile
> index 021d3ad..0977f27 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -256,8 +256,10 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
> $(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
> $(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
>
> -install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules
> - @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
> +install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules \
> + udev-md-clustered-confirm-device.rules
> + @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules \
> + 69-md-clustered-confirm-device.rules ; \
> do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp.1 && \
> $(ECHO) $(INSTALL) -D -m 644 udev-$${file#??-} $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
> $(INSTALL) -D -m 644 .install.tmp.1 $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
> diff --git a/udev-md-clustered-confirm-device.rules b/udev-md-clustered-confirm-device.rules
> new file mode 100644
> index 0000000..d763e27
> --- /dev/null
> +++ b/udev-md-clustered-confirm-device.rules
> @@ -0,0 +1,20 @@
> +# do not edit this file, it will be overwritten on update
> +
> +SUBSYSTEM!="block", GOTO="clustermd_end"
> +
> +# handle md arrays
> +KERNEL!="md*", GOTO="clustermd_end"
> +ENV{DEVTYPE}!="disk", GOTO="clustermd_end"
> +
> +# Based on the received UUID, node confirms the device if
> +# it is found by blkid, otherwise the node reports it is
> +# missing.
> +ACTION=="change", ENV{EVENT}=="ADD_DEVICE", ENV{DEVICE_UUID}=="?*", ENV{RAID_DISK}=="?*", \
> +PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", RESULT!="", \
> +RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:$result"
> +
> +ACTION=="change", ENV{EVENT}=="ADD_DEVICE", ENV{DEVICE_UUID}=="?*", ENV{RAID_DISK}=="?*", \
> +PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", RESULT=="", \
> +RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:missing"
> +
> +LABEL="clustermd_end"
> --
> 2.10.0
This can run "blkid" twice, which seems wasteful.
I think it would be best to skip to the end if ACTION != "change" and if
event != ADD_DEVICE etc.
Then
PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", ENV{".mdadm.blkid"} = "$result"
Then have two rules which tests ENV{".mdadm.blkid"} and run one of the
two mdadm commands.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH] udev rules: introduce rules for cluster-md to confirm device
From: Guoqing Jiang @ 2017-08-01 8:05 UTC (permalink / raw)
To: NeilBrown, linux-raid; +Cc: jes.sorensen
In-Reply-To: <87o9rzvr2r.fsf@notabene.neil.brown.name>
On 08/01/2017 12:40 PM, NeilBrown wrote:
> On Tue, Aug 01 2017, Guoqing Jiang wrote:
>
>> For cluster scenario, add device is different with native raid.
>> When a node issues adds a device, then the node will broadcast
>> a message with UUID to other nodes in the cluster. If receiving
>> node can find the device with the specific UUID, it must confirm
>> the device, otherwise reports it is missing.
>>
>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>> ---
>> Makefile | 6 ++++--
>> udev-md-clustered-confirm-device.rules | 20 ++++++++++++++++++++
>> 2 files changed, 24 insertions(+), 2 deletions(-)
>> create mode 100644 udev-md-clustered-confirm-device.rules
>>
>> diff --git a/Makefile b/Makefile
>> index 021d3ad..0977f27 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -256,8 +256,10 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
>> $(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
>> $(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
>>
>> -install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules
>> - @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
>> +install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules \
>> + udev-md-clustered-confirm-device.rules
>> + @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules \
>> + 69-md-clustered-confirm-device.rules ; \
>> do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp.1 && \
>> $(ECHO) $(INSTALL) -D -m 644 udev-$${file#??-} $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
>> $(INSTALL) -D -m 644 .install.tmp.1 $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
>> diff --git a/udev-md-clustered-confirm-device.rules b/udev-md-clustered-confirm-device.rules
>> new file mode 100644
>> index 0000000..d763e27
>> --- /dev/null
>> +++ b/udev-md-clustered-confirm-device.rules
>> @@ -0,0 +1,20 @@
>> +# do not edit this file, it will be overwritten on update
>> +
>> +SUBSYSTEM!="block", GOTO="clustermd_end"
>> +
>> +# handle md arrays
>> +KERNEL!="md*", GOTO="clustermd_end"
>> +ENV{DEVTYPE}!="disk", GOTO="clustermd_end"
>> +
>> +# Based on the received UUID, node confirms the device if
>> +# it is found by blkid, otherwise the node reports it is
>> +# missing.
>> +ACTION=="change", ENV{EVENT}=="ADD_DEVICE", ENV{DEVICE_UUID}=="?*", ENV{RAID_DISK}=="?*", \
>> +PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", RESULT!="", \
>> +RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:$result"
>> +
>> +ACTION=="change", ENV{EVENT}=="ADD_DEVICE", ENV{DEVICE_UUID}=="?*", ENV{RAID_DISK}=="?*", \
>> +PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", RESULT=="", \
>> +RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:missing"
>> +
>> +LABEL="clustermd_end"
>> --
>> 2.10.0
> This can run "blkid" twice, which seems wasteful.
> I think it would be best to skip to the end if ACTION != "change" and if
> event != ADD_DEVICE etc.
> Then
> PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", ENV{".mdadm.blkid"} = "$result"
>
> Then have two rules which tests ENV{".mdadm.blkid"} and run one of the
> two mdadm commands.
Thanks for the suggestion! I will send v2 soon.
Cheers,
Guoqing
^ permalink raw reply
* [PATCH V2] udev rules: introduce rules for cluster-md to confirm device
From: Guoqing Jiang @ 2017-08-01 8:19 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, neilb, Guoqing Jiang
In-Reply-To: <20170801031504.8452-1-gqjiang@suse.com>
For cluster scenario, add device is different with native raid.
When a node issues adds a device, then the node will broadcast
a message with UUID to other nodes in the cluster. If receiving
node can find the device with the specific UUID, it must confirm
the device, otherwise reports it is missing.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
Makefile | 6 ++++--
udev-md-clustered-confirm-device.rules | 20 ++++++++++++++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
create mode 100644 udev-md-clustered-confirm-device.rules
diff --git a/Makefile b/Makefile
index 021d3ad..0977f27 100644
--- a/Makefile
+++ b/Makefile
@@ -256,8 +256,10 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
$(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
$(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
-install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules
- @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
+install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules \
+ udev-md-clustered-confirm-device.rules
+ @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules \
+ 69-md-clustered-confirm-device.rules ; \
do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp.1 && \
$(ECHO) $(INSTALL) -D -m 644 udev-$${file#??-} $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
$(INSTALL) -D -m 644 .install.tmp.1 $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
diff --git a/udev-md-clustered-confirm-device.rules b/udev-md-clustered-confirm-device.rules
new file mode 100644
index 0000000..08ac74a
--- /dev/null
+++ b/udev-md-clustered-confirm-device.rules
@@ -0,0 +1,20 @@
+# do not edit this file, it will be overwritten on update
+
+SUBSYSTEM!="block", GOTO="clustermd_end"
+
+# handle md arrays
+KERNEL!="md*", GOTO="clustermd_end"
+ENV{DEVTYPE}!="disk", GOTO="clustermd_end"
+ACTION!="change", GOTO="clustermd_end"
+ENV{EVENT}!="ADD_DEVICE", GOTO="clustermd_end"
+
+# Based on the received UUID, node confirms the device if
+# it is found by blkid, otherwise the node reports it is
+# missing.
+ACTION=="change", ENV{EVENT}=="ADD_DEVICE", ENV{DEVICE_UUID}=="?*", ENV{RAID_DISK}=="?*", \
+PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", ENV{device} = "$result"
+
+ENV{device}!="", RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:$env{device}"
+ENV{device}=="", RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:missing"
+
+LABEL="clustermd_end"
--
2.10.0
^ permalink raw reply related
* Re: Recovery after accidental raid5 superblock rewrite
From: Phil Turmel @ 2017-08-01 14:01 UTC (permalink / raw)
To: Paul Tonelli, Andreas Klauer; +Cc: linux-raid
In-Reply-To: <e0c059b0-2d68-92bf-3eb9-e2827602061c@tonel.li>
On 07/31/2017 03:57 PM, Paul Tonelli wrote:
> Hello (again)
>
> Sorry to resuscitate this topic back from the dead, but I have again the
> same issue.
>
> TL;DR
>
> The difference compared to last time:
> - I created a clean raid a few days back
> - the data is completely backed up and available
> - I can actually access the data
>
> but I still have no clue about what went wrong.
>
> 1) I created the raid:
>
> sudo mdadm --create --verbose /dev/md0 --level=5 --raid-devices=3
> /dev/sdb /dev/sdc /dev/sdd
>
> without lvm this time, juste a single ext4 partition
>
> mkfs.ext4 /dev/md0
> mount /dev/md0 /srv/data
>
> I copied 3Tb on it
>
> I just rebooted the machine
>
> 2) and (again) no md0 assembled at boot:
>
> mdadm -E /dev/sd[bcd]
> /dev/sdb:
> MBR Magic : aa55
> Partition[0] : 4294967295 sectors at 1 (type ee)
> /dev/sdc:
> MBR Magic : aa55
> Partition[0] : 4294967295 sectors at 1 (type ee)
> /dev/sdd:
> MBR Magic : aa55
> Partition[0] : 4294967295 sectors at 1 (type ee)
You have partition tables on these drives, but you are using the entire
drives when you create your arrays. You need to zero the first 4k of
each drive to kill off the incorrect partition tables.
You might also have a GPT partition table backup at the end of the disk
that needs to die as well.
Phil
^ permalink raw reply
* (unknown),
From: Angela H. Whiteman @ 2017-08-01 14:53 UTC (permalink / raw)
There's an Unclaimed Inheritance with your Last Name. Reply to; abailey456789@gmail.com<mailto:abailey456789@gmail.com> with your Full Names.
^ permalink raw reply
* [PATCH v3] md/r5cache: fix io_unit handling in r5l_log_endio()
From: Song Liu @ 2017-08-01 20:53 UTC (permalink / raw)
To: linux-raid
Cc: shli, neilb, kernel-team, dan.j.williams, hch, jes.sorensen,
Song Liu
In r5l_log_endio(), once log->io_list_lock is released, the io unit
may be accessed (or even freed) by other threads. Current code
doesn't handle the io_unit properly, which leads to potential race
conditions.
This patch solves this race condition by:
1. Add a pending_stripe count flush_payload. Note that multiple
flush_payloads are counted as only one pending_stripe. Flag
has_flush_payload is added to show whether the io unit has
flush_payload;
2. In r5l_log_endio(), check flags has_null_flush and
has_flush_payload with log->io_list_lock held. After the lock
is released, this IO unit is only accessed when we know the
pending_stripe counter cannot be zeroed by other threas.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
drivers/md/raid5-cache.c | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index ce98414..eedea35 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -239,6 +239,7 @@ struct r5l_io_unit {
unsigned int has_flush:1; /* include flush request */
unsigned int has_fua:1; /* include fua request */
unsigned int has_null_flush:1; /* include empty flush request */
+ unsigned int has_flush_payload:1; /* include flush payload */
/*
* io isn't sent yet, flush/fua request can only be submitted till it's
* the first IO in running_ios list
@@ -571,6 +572,8 @@ static void r5l_log_endio(struct bio *bio)
struct r5l_io_unit *io_deferred;
struct r5l_log *log = io->log;
unsigned long flags;
+ bool has_null_flush;
+ bool has_flush_payload;
if (bio->bi_status)
md_error(log->rdev->mddev, log->rdev);
@@ -580,6 +583,19 @@ static void r5l_log_endio(struct bio *bio)
spin_lock_irqsave(&log->io_list_lock, flags);
__r5l_set_io_unit_state(io, IO_UNIT_IO_END);
+
+ /*
+ * if the io doesn't not have null_flush or flush payload,
+ * it is not safe to access it after releasing io_list_lock.
+ * Therefore, it is necessary to check the condition with
+ * the lock held.
+ */
+ has_null_flush = io->has_null_flush;
+ has_flush_payload = io->has_flush_payload;
+ /* finish flush only io_unit and PAYLOAD_FLUSH only io_unit */
+ if (atomic_read(&io->pending_stripe) == 0)
+ __r5l_set_io_unit_state(io, IO_UNIT_STRIPE_END);
+
if (log->need_cache_flush && !list_empty(&io->stripe_list))
r5l_move_to_end_ios(log);
else
@@ -600,18 +616,20 @@ static void r5l_log_endio(struct bio *bio)
if (log->need_cache_flush)
md_wakeup_thread(log->rdev->mddev->thread);
- if (io->has_null_flush) {
+ if (has_null_flush) {
struct bio *bi;
WARN_ON(bio_list_empty(&io->flush_barriers));
while ((bi = bio_list_pop(&io->flush_barriers)) != NULL) {
bio_endio(bi);
- atomic_dec(&io->pending_stripe);
+ if (atomic_dec_and_test(&io->pending_stripe)) {
+ __r5l_stripe_write_finished(io);
+ return;
}
}
-
- /* finish flush only io_unit and PAYLOAD_FLUSH only io_unit */
- if (atomic_read(&io->pending_stripe) == 0)
+ }
+ if (has_flush_payload)
+ if (atomic_dec_and_test(&io->pending_stripe))
__r5l_stripe_write_finished(io);
}
@@ -881,6 +899,11 @@ static void r5l_append_flush_payload(struct r5l_log *log, sector_t sect)
payload->size = cpu_to_le32(sizeof(__le64));
payload->flush_stripes[0] = cpu_to_le64(sect);
io->meta_offset += meta_size;
+ /* multiple flush payloads count as one pending_stripe */
+ if (!io->has_flush_payload) {
+ io->has_flush_payload = 1;
+ atomic_inc(&io->pending_stripe);
+ }
mutex_unlock(&log->io_mutex);
}
--
2.9.3
^ permalink raw reply related
* Re: [PATCH V2] udev rules: introduce rules for cluster-md to confirm device
From: NeilBrown @ 2017-08-01 21:15 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Guoqing Jiang
In-Reply-To: <20170801081956.29777-1-gqjiang@suse.com>
[-- Attachment #1: Type: text/plain, Size: 3358 bytes --]
On Tue, Aug 01 2017, Guoqing Jiang wrote:
> For cluster scenario, add device is different with native raid.
> When a node issues adds a device, then the node will broadcast
> a message with UUID to other nodes in the cluster. If receiving
> node can find the device with the specific UUID, it must confirm
> the device, otherwise reports it is missing.
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
> Makefile | 6 ++++--
> udev-md-clustered-confirm-device.rules | 20 ++++++++++++++++++++
> 2 files changed, 24 insertions(+), 2 deletions(-)
> create mode 100644 udev-md-clustered-confirm-device.rules
>
> diff --git a/Makefile b/Makefile
> index 021d3ad..0977f27 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -256,8 +256,10 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
> $(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
> $(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
>
> -install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules
> - @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
> +install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules \
> + udev-md-clustered-confirm-device.rules
> + @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules \
> + 69-md-clustered-confirm-device.rules ; \
> do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp.1 && \
> $(ECHO) $(INSTALL) -D -m 644 udev-$${file#??-} $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
> $(INSTALL) -D -m 644 .install.tmp.1 $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
> diff --git a/udev-md-clustered-confirm-device.rules b/udev-md-clustered-confirm-device.rules
> new file mode 100644
> index 0000000..08ac74a
> --- /dev/null
> +++ b/udev-md-clustered-confirm-device.rules
> @@ -0,0 +1,20 @@
> +# do not edit this file, it will be overwritten on update
> +
> +SUBSYSTEM!="block", GOTO="clustermd_end"
> +
> +# handle md arrays
> +KERNEL!="md*", GOTO="clustermd_end"
> +ENV{DEVTYPE}!="disk", GOTO="clustermd_end"
> +ACTION!="change", GOTO="clustermd_end"
> +ENV{EVENT}!="ADD_DEVICE", GOTO="clustermd_end"
Move these here is good.
> +
> +# Based on the received UUID, node confirms the device if
> +# it is found by blkid, otherwise the node reports it is
> +# missing.
> +ACTION=="change", ENV{EVENT}=="ADD_DEVICE", ENV{DEVICE_UUID}=="?*", ENV{RAID_DISK}=="?*", \
leaving them here as well is careless. Maybe move the tests on
DEVICE_UUID and RAID_DISK up to the "goto" section too.
> +PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", ENV{device} = "$result"
I don't think using a generic environment name like "device" is a good
idea.
This will get included in the udev database.
At least include "md" or "mdadm" in the name so people know what it
means.
In my example I put a "." at the start. This cause the name to be
excluded from the udev database.
NeilBrown
> +
> +ENV{device}!="", RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:$env{device}"
> +ENV{device}=="", RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:missing"
> +
> +LABEL="clustermd_end"
> --
> 2.10.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH V2] udev rules: introduce rules for cluster-md to confirm device
From: Guoqing Jiang @ 2017-08-02 1:53 UTC (permalink / raw)
To: NeilBrown, linux-raid; +Cc: jes.sorensen
In-Reply-To: <87bmnzugzk.fsf@notabene.neil.brown.name>
On 08/02/2017 05:15 AM, NeilBrown wrote:
> On Tue, Aug 01 2017, Guoqing Jiang wrote:
>
>> For cluster scenario, add device is different with native raid.
>> When a node issues adds a device, then the node will broadcast
>> a message with UUID to other nodes in the cluster. If receiving
>> node can find the device with the specific UUID, it must confirm
>> the device, otherwise reports it is missing.
>>
>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>> ---
>> Makefile | 6 ++++--
>> udev-md-clustered-confirm-device.rules | 20 ++++++++++++++++++++
>> 2 files changed, 24 insertions(+), 2 deletions(-)
>> create mode 100644 udev-md-clustered-confirm-device.rules
>>
>> diff --git a/Makefile b/Makefile
>> index 021d3ad..0977f27 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -256,8 +256,10 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
>> $(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
>> $(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
>>
>> -install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules
>> - @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
>> +install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules \
>> + udev-md-clustered-confirm-device.rules
>> + @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules \
>> + 69-md-clustered-confirm-device.rules ; \
>> do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp.1 && \
>> $(ECHO) $(INSTALL) -D -m 644 udev-$${file#??-} $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
>> $(INSTALL) -D -m 644 .install.tmp.1 $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
>> diff --git a/udev-md-clustered-confirm-device.rules b/udev-md-clustered-confirm-device.rules
>> new file mode 100644
>> index 0000000..08ac74a
>> --- /dev/null
>> +++ b/udev-md-clustered-confirm-device.rules
>> @@ -0,0 +1,20 @@
>> +# do not edit this file, it will be overwritten on update
>> +
>> +SUBSYSTEM!="block", GOTO="clustermd_end"
>> +
>> +# handle md arrays
>> +KERNEL!="md*", GOTO="clustermd_end"
>> +ENV{DEVTYPE}!="disk", GOTO="clustermd_end"
>> +ACTION!="change", GOTO="clustermd_end"
>> +ENV{EVENT}!="ADD_DEVICE", GOTO="clustermd_end"
> Move these here is good.
>
>
>> +
>> +# Based on the received UUID, node confirms the device if
>> +# it is found by blkid, otherwise the node reports it is
>> +# missing.
>> +ACTION=="change", ENV{EVENT}=="ADD_DEVICE", ENV{DEVICE_UUID}=="?*", ENV{RAID_DISK}=="?*", \
> leaving them here as well is careless. Maybe move the tests on
> DEVICE_UUID and RAID_DISK up to the "goto" section too.
Ok, I will move them above.
ENV{DEVICE_UUID}!="?*", GOTO="clustermd_end"
ENV{RAID_DISK}!="?*", GOTO="clustermd_end"
>> +PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", ENV{device} = "$result"
> I don't think using a generic environment name like "device" is a good
> idea.
> This will get included in the udev database.
Hmm, yes, I can see it by run "udevadm info -e", not good ...
> At least include "md" or "mdadm" in the name so people know what it
> means.
> In my example I put a "." at the start. This cause the name to be
> excluded from the udev database.
Thanks for detailed explanation, I didn't know that.
Best Regards,
Guoqing
^ permalink raw reply
* [PATCH v3] udev rules: introduce rules for cluster-md to confirm device
From: Guoqing Jiang @ 2017-08-02 2:05 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, neilb, Guoqing Jiang
In-Reply-To: <20170801031504.8452-1-gqjiang@suse.com>
For cluster scenario, add device is different with native raid.
When a node issues adds a device, then the node will broadcast
a message with UUID to other nodes in the cluster. If receiving
node can find the device with the specific UUID, it must confirm
the device, otherwise reports it is missing.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
Makefile | 6 ++++--
udev-md-clustered-confirm-device.rules | 21 +++++++++++++++++++++
2 files changed, 25 insertions(+), 2 deletions(-)
create mode 100644 udev-md-clustered-confirm-device.rules
diff --git a/Makefile b/Makefile
index 021d3ad..0977f27 100644
--- a/Makefile
+++ b/Makefile
@@ -256,8 +256,10 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
$(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
$(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
-install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules
- @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
+install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules \
+ udev-md-clustered-confirm-device.rules
+ @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules \
+ 69-md-clustered-confirm-device.rules ; \
do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp.1 && \
$(ECHO) $(INSTALL) -D -m 644 udev-$${file#??-} $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
$(INSTALL) -D -m 644 .install.tmp.1 $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
diff --git a/udev-md-clustered-confirm-device.rules b/udev-md-clustered-confirm-device.rules
new file mode 100644
index 0000000..3e5381e
--- /dev/null
+++ b/udev-md-clustered-confirm-device.rules
@@ -0,0 +1,21 @@
+# do not edit this file, it will be overwritten on update
+
+SUBSYSTEM!="block", GOTO="clustermd_end"
+
+# handle md arrays
+KERNEL!="md*", GOTO="clustermd_end"
+ENV{DEVTYPE}!="disk", GOTO="clustermd_end"
+ACTION!="change", GOTO="clustermd_end"
+ENV{EVENT}!="ADD_DEVICE", GOTO="clustermd_end"
+ENV{DEVICE_UUID}!="?*", GOTO="clustermd_end"
+ENV{RAID_DISK}!="?*", GOTO="clustermd_end"
+
+# Based on the received UUID, node confirms the device if
+# it is found by blkid, otherwise the node reports it is
+# missing.
+PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", ENV{.md.newdevice} = "$result"
+
+ENV{.md.newdevice}!="", RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:$env{.md.newdevice}"
+ENV{.md.newdevice}=="", RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:missing"
+
+LABEL="clustermd_end"
--
2.10.0
^ permalink raw reply related
* Re: Extremely important and Urgent
From: Singer Valve @ 2017-08-02 3:46 UTC (permalink / raw)
To: Recipients
I am getting in touch with you regarding an extremely important and urgent matter.
If you would oblige me the opportunity, I shall provide you with details upon your response.
Faithfully,
Ms. Singer Valve
^ permalink raw reply
* [PATCH] md/bitmap: preserve whole sb_page when initializing bitmap
From: Song Liu @ 2017-08-02 6:11 UTC (permalink / raw)
To: linux-raid, shli
Cc: Song Liu, shli, neilb, kernel-team, dan.j.williams, hch,
jes.sorensen
When bitmap_resize() is used to initialize the bitmap, it is
necessary to preserve the whole page of sb_page instead of just
the header (bitmap_super_t). This is because the sb_page may
contain bitmap read from the disks (initialized by mdadm).
Note that, this code path is only triggered with certain
combinations of parameters. One example is when raid456 array
is created with write journal
Signed-off-by: Song Liu <songliubraving@fb.com>
---
drivers/md/bitmap.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index 40f3cd7..93dd809 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -2118,7 +2118,7 @@ int bitmap_resize(struct bitmap *bitmap, sector_t blocks,
if (store.sb_page && bitmap->storage.sb_page)
memcpy(page_address(store.sb_page),
page_address(bitmap->storage.sb_page),
- sizeof(bitmap_super_t));
+ init ? PAGE_SIZE : sizeof(bitmap_super_t));
bitmap_file_unmap(&bitmap->storage);
bitmap->storage = store;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v4] mdadm/r5cache: allow adding journal to array without journal
From: Jes Sorensen @ 2017-08-02 13:25 UTC (permalink / raw)
To: Song Liu; +Cc: linux-raid, shli, neilb, Kernel Team, schmidt
In-Reply-To: <wrfjzig5fetg.fsf@gmail.com>
On 03/28/2017 02:15 PM, jes.sorensen@gmail.com wrote:
> Song Liu <songliubraving@fb.com> writes:
>> Currently, --add-journal can be only used to recreate broken journal
>> for arrays with journal since creation. As the kernel code getting
>> more mature, this constraint is no longer necessary.
>>
>> This patch allows --add-journal to add journal to array without
>> journal.
>>
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> ---
>> Manage.c | 6 ------
>> mdadm.8.in | 5 ++---
>> 2 files changed, 2 insertions(+), 9 deletions(-)
>
> Applied!
It was pointed out to me that this patch never showed up in git.
After digging through my git tree a bit, I see no sign of having applied
it :( Well it's in there now, apologies.
Kudos to Christian Schmidt for pointing this out.
Cheers,
Jes
^ permalink raw reply
* Re: [PATCH v3] udev rules: introduce rules for cluster-md to confirm device
From: Jes Sorensen @ 2017-08-02 13:31 UTC (permalink / raw)
To: Guoqing Jiang, linux-raid; +Cc: neilb
In-Reply-To: <20170802020541.3412-1-gqjiang@suse.com>
On 08/01/2017 10:05 PM, Guoqing Jiang wrote:
> For cluster scenario, add device is different with native raid.
> When a node issues adds a device, then the node will broadcast
> a message with UUID to other nodes in the cluster. If receiving
> node can find the device with the specific UUID, it must confirm
> the device, otherwise reports it is missing.
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
> Makefile | 6 ++++--
> udev-md-clustered-confirm-device.rules | 21 +++++++++++++++++++++
> 2 files changed, 25 insertions(+), 2 deletions(-)
> create mode 100644 udev-md-clustered-confirm-device.rules
Looks reasonable to me - and thanks Neil for the previous reviews.
Applied!
Jes
^ permalink raw reply
* Re: [PATCH v3] udev rules: introduce rules for cluster-md to confirm device
From: John Stoffel @ 2017-08-03 1:43 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <20170802020541.3412-1-gqjiang@suse.com>
>>>>> "Guoqing" == Guoqing Jiang <gqjiang@suse.com> writes:
Guoqing> For cluster scenario, add device is different with native raid.
Guoqing> When a node issues adds a device, then the node will broadcast
Guoqing> a message with UUID to other nodes in the cluster. If receiving
Guoqing> node can find the device with the specific UUID, it must confirm
Guoqing> the device, otherwise reports it is missing.
Guoqing> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
Guoqing> ---
Guoqing> Makefile | 6 ++++--
Guoqing> udev-md-clustered-confirm-device.rules | 21 +++++++++++++++++++++
Guoqing> 2 files changed, 25 insertions(+), 2 deletions(-)
Guoqing> create mode 100644 udev-md-clustered-confirm-device.rules
Guoqing> diff --git a/Makefile b/Makefile
Guoqing> index 021d3ad..0977f27 100644
Guoqing> --- a/Makefile
Guoqing> +++ b/Makefile
Guoqing> @@ -256,8 +256,10 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
Guoqing> $(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
Guoqing> $(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
Guoqing> -install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules
Guoqing> - @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
Guoqing> +install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules \
Guoqing> + udev-md-clustered-confirm-device.rules
You have udev-md-clustered-.... here, but down below you use
clustermd-end in your names. Which I find slightly annoying because
they're different. Just me being annoying here. :-) I didn't check
if the other raid rules have this issue as well.
Guoqing> + @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules \
Guoqing> + 69-md-clustered-confirm-device.rules ; \
Guoqing> do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp.1 && \
Guoqing> $(ECHO) $(INSTALL) -D -m 644 udev-$${file#??-} $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
Guoqing> $(INSTALL) -D -m 644 .install.tmp.1 $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
Guoqing> diff --git a/udev-md-clustered-confirm-device.rules b/udev-md-clustered-confirm-device.rules
Guoqing> new file mode 100644
Guoqing> index 0000000..3e5381e
Guoqing> --- /dev/null
Guoqing> +++ b/udev-md-clustered-confirm-device.rules
Guoqing> @@ -0,0 +1,21 @@
Guoqing> +# do not edit this file, it will be overwritten on update
Guoqing> +
Guoqing> +SUBSYSTEM!="block", GOTO="clustermd_end"
Guoqing> +
Guoqing> +# handle md arrays
Guoqing> +KERNEL!="md*", GOTO="clustermd_end"
Guoqing> +ENV{DEVTYPE}!="disk", GOTO="clustermd_end"
Guoqing> +ACTION!="change", GOTO="clustermd_end"
Guoqing> +ENV{EVENT}!="ADD_DEVICE", GOTO="clustermd_end"
Guoqing> +ENV{DEVICE_UUID}!="?*", GOTO="clustermd_end"
Guoqing> +ENV{RAID_DISK}!="?*", GOTO="clustermd_end"
Guoqing> +
Guoqing> +# Based on the received UUID, node confirms the device if
Guoqing> +# it is found by blkid, otherwise the node reports it is
Guoqing> +# missing.
Guoqing> +PROGRAM="BINDIR/blkid -o device -t UUID_SUB=$env{DEVICE_UUID}", ENV{.md.newdevice} = "$result"
Guoqing> +
Guoqing> +ENV{.md.newdevice}!="", RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:$env{.md.newdevice}"
Guoqing> +ENV{.md.newdevice}=="", RUN+="BINDIR/mdadm --manage $env{DEVNAME} --cluster-confirm $env{RAID_DISK}:missing"
Guoqing> +
Guoqing> +LABEL="clustermd_end"
Guoqing> --
Guoqing> 2.10.0
Guoqing> --
Guoqing> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
Guoqing> the body of a message to majordomo@vger.kernel.org
Guoqing> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3] udev rules: introduce rules for cluster-md to confirm device
From: Guoqing Jiang @ 2017-08-03 1:57 UTC (permalink / raw)
To: John Stoffel; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <22914.32725.983017.495207@quad.stoffel.home>
On 08/03/2017 09:43 AM, John Stoffel wrote:
>>>>>> "Guoqing" == Guoqing Jiang <gqjiang@suse.com> writes:
> Guoqing> For cluster scenario, add device is different with native raid.
> Guoqing> When a node issues adds a device, then the node will broadcast
> Guoqing> a message with UUID to other nodes in the cluster. If receiving
> Guoqing> node can find the device with the specific UUID, it must confirm
> Guoqing> the device, otherwise reports it is missing.
>
> Guoqing> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> Guoqing> ---
> Guoqing> Makefile | 6 ++++--
> Guoqing> udev-md-clustered-confirm-device.rules | 21 +++++++++++++++++++++
> Guoqing> 2 files changed, 25 insertions(+), 2 deletions(-)
> Guoqing> create mode 100644 udev-md-clustered-confirm-device.rules
>
> Guoqing> diff --git a/Makefile b/Makefile
> Guoqing> index 021d3ad..0977f27 100644
> Guoqing> --- a/Makefile
> Guoqing> +++ b/Makefile
> Guoqing> @@ -256,8 +256,10 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
> Guoqing> $(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
> Guoqing> $(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
>
> Guoqing> -install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules
> Guoqing> - @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
> Guoqing> +install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules \
> Guoqing> + udev-md-clustered-confirm-device.rules
>
>
> You have udev-md-clustered-.... here, but down below you use
> clustermd-end in your names. Which I find slightly annoying because
> they're different. Just me being annoying here. :-) I didn't check
> if the other raid rules have this issue as well.
>
Well, I named it as "udev-md-clustered-*" to align with the name of
existed udev rules,
it also means the array is clustered. And clustermd is a
convenient/short name to use.
But not a big deal, I am open to the name if you have better idea, :-) .
Thanks,
Guoqing
^ permalink raw reply
* Re: [PATCH v3] udev rules: introduce rules for cluster-md to confirm device
From: John Stoffel @ 2017-08-03 2:50 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: John Stoffel, linux-raid, jes.sorensen, neilb
In-Reply-To: <59828314.3050602@suse.com>
>>>>> "Guoqing" == Guoqing Jiang <gqjiang@suse.com> writes:
Guoqing> On 08/03/2017 09:43 AM, John Stoffel wrote:
>>>>>>> "Guoqing" == Guoqing Jiang <gqjiang@suse.com> writes:
Guoqing> For cluster scenario, add device is different with native raid.
Guoqing> When a node issues adds a device, then the node will broadcast
Guoqing> a message with UUID to other nodes in the cluster. If receiving
Guoqing> node can find the device with the specific UUID, it must confirm
Guoqing> the device, otherwise reports it is missing.
>>
Guoqing> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
Guoqing> ---
Guoqing> Makefile | 6 ++++--
Guoqing> udev-md-clustered-confirm-device.rules | 21 +++++++++++++++++++++
Guoqing> 2 files changed, 25 insertions(+), 2 deletions(-)
Guoqing> create mode 100644 udev-md-clustered-confirm-device.rules
>>
Guoqing> diff --git a/Makefile b/Makefile
Guoqing> index 021d3ad..0977f27 100644
Guoqing> --- a/Makefile
Guoqing> +++ b/Makefile
Guoqing> @@ -256,8 +256,10 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
Guoqing> $(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
Guoqing> $(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
>>
Guoqing> -install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules
Guoqing> - @for file in 01-md-raid-creating.rules 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
Guoqing> +install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules udev-md-raid-creating.rules \
Guoqing> + udev-md-clustered-confirm-device.rules
>>
>>
>> You have udev-md-clustered-.... here, but down below you use
>> clustermd-end in your names. Which I find slightly annoying because
>> they're different. Just me being annoying here. :-) I didn't check
>> if the other raid rules have this issue as well.
>>
Guoqing> Well, I named it as "udev-md-clustered-*" to align with the
Guoqing> name of existed udev rules, it also means the array is
Guoqing> clustered. And clustermd is a convenient/short name to use.
I think mdcluster is better than clustermd, but that's only because
it's the same order as udev-md-clustered-* but we're both just
bike shedding here. :-)
Guoqing> But not a big deal, I am open to the name if you have better
Guoqing> idea, :-) .
Not really. Appreciate your nice reply to my semi-inane comment.
^ permalink raw reply
* [PATCH] mdadm: allow to assemble /dev/md_xxx named arrays.
From: jcejka @ 2017-08-03 16:32 UTC (permalink / raw)
To: linux-raid; +Cc: Josef Cejka
From: Josef Cejka <jcejka@suse.com>
Allow to assemble arrays using /dev/md_xxx name format
both from cmdline like mdadm -A /dev/md_xxx
or using ASSEMBLE directive in mdadm.conf
Signed-off-by: Josef Cejka <jcejka@suse.com>
---
config.c | 5 +++--
mdopen.c | 26 ++++++++++++++++----------
2 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/config.c b/config.c
index 48e0278..113ef01 100644
--- a/config.c
+++ b/config.c
@@ -389,6 +389,7 @@ void arrayline(char *line)
* /dev/md/{anything}
* /dev/mdNN
* /dev/md_dNN
+ * /dev/md_XXX
* <ignore>
* or anything that doesn't start '/' or '<'
*/
@@ -397,8 +398,8 @@ void arrayline(char *line)
(w[0] != '/' && w[0] != '<') ||
(strncmp(w, "/dev/md", 7) == 0 &&
is_number(w + 7)) ||
- (strncmp(w, "/dev/md_d", 9) == 0 &&
- is_number(w + 9))) {
+ (strncmp(w, "/dev/md_", 8) == 0 &&
+ strlen(w) > 8)) {
/* This is acceptable */;
if (mis.devname)
pr_err("only give one device per ARRAY line: %s and %s\n",
diff --git a/mdopen.c b/mdopen.c
index c4f1c12..0f3a244 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -174,19 +174,25 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
num = strtoul(e, NULL, 10);
strcpy(cname, dev+5);
cname[e-(dev+5)] = 0;
- /* name *must* be mdXX or md_dXX in this context */
+ /* name is not mdXX or md_dXX */
if (num < 0 ||
(strcmp(cname, "md") != 0 && strcmp(cname, "md_d") != 0)) {
- pr_err("%s is an invalid name for an md device. Try /dev/md/%s\n",
- dev, dev+5);
- return -1;
+ if (strncmp(cname, "md_", 3) != 0) {
+ pr_err("%s is an invalid name for an md device. Try /dev/md/%s\n",
+ dev, dev+5);
+ return -1;
+ }
+ /* /dev/md_<string> format */
+ strcpy(cname, dev+8);
+ num = -1;
+ } else {
+ if (strcmp(cname, "md") == 0)
+ use_mdp = 0;
+ else
+ use_mdp = 1;
+ /* recreate name: /dev/md/0 or /dev/md/d0 */
+ sprintf(cname, "%s%d", use_mdp?"d":"", num);
}
- if (strcmp(cname, "md") == 0)
- use_mdp = 0;
- else
- use_mdp = 1;
- /* recreate name: /dev/md/0 or /dev/md/d0 */
- sprintf(cname, "%s%d", use_mdp?"d":"", num);
} else
strcpy(cname, dev);
--
2.12.3
^ permalink raw reply related
* [PATCH] mdopen: prevent named arrays devices from buffer overflow
From: jcejka @ 2017-08-03 16:48 UTC (permalink / raw)
To: linux-raid; +Cc: Josef Cejka
From: Josef Cejka <jcejka@suse.com>
Device names md_XXX for named arrays must fit into 32 bytes
and longer strings provided by user should be rejected. Now they
corrupt the stack (overwrite following devname[] buffer) and
(if not detected) create arrays using old create_on_open
mechanism because write to new_array fails with E2BIG.
Reproducer:
mdadm -A /dev/md/abcdefghijklmnopqrstuvwxyz123 --uuid=...
Signed-off-by: Josef Cejka <jcejka@suse.com>
---
mdopen.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/mdopen.c b/mdopen.c
index 0f3a244..fd8a1db 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -314,7 +314,10 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
if (num < 0 && cname && ci->names) {
int fd;
int n = -1;
- sprintf(devnm, "md_%s", cname);
+ if (snprintf(devnm, sizeof(devnm), "md_%s", cname) >= sizeof(devnm)) {
+ pr_err("Device name md_%s must be shorter than %d bytes.\n", cname, sizeof(devnm));
+ return -1;
+ }
if (block_udev)
udev_block(devnm);
fd = open("/sys/module/md_mod/parameters/new_array", O_WRONLY);
@@ -364,7 +367,10 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
udev_block(devnm);
}
- sprintf(devname, "/dev/%s", devnm);
+ if (snprintf(devname, sizeof(devname), "/dev/%s", devnm) >= sizeof(devname)) {
+ pr_err("Device path /dev/%s must be shorter than %d bytes.\n", devnm, sizeof(devname));
+ return -1;
+ }
if (dev && dev[0] == '/')
strcpy(chosen, dev);
--
2.12.3
^ permalink raw reply related
* [PATCH v5] md/r5cache: fix io_unit handling in r5l_log_endio()
From: Song Liu @ 2017-08-03 17:03 UTC (permalink / raw)
To: linux-raid, shli
Cc: Song Liu, shli, neilb, kernel-team, dan.j.williams, hch,
jes.sorensen
In r5l_log_endio(), once log->io_list_lock is released, the io unit
may be accessed (or even freed) by other threads. Current code
doesn't handle the io_unit properly, which leads to potential race
conditions.
This patch solves this race condition by:
1. Add a pending_stripe count flush_payload. Multiple flush_payloads
are counted as only one pending_stripe. Flag has_flush_payload is
added to show whether the io unit has flush_payload;
2. In r5l_log_endio(), check flags has_null_flush and
has_flush_payload with log->io_list_lock held. After the lock
is released, this IO unit is only accessed when we know the
pending_stripe counter cannot be zeroed by other threads.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
drivers/md/raid5-cache.c | 40 +++++++++++++++++++++++++++++++---------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index ce98414..2dcbafa 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -236,9 +236,10 @@ struct r5l_io_unit {
bool need_split_bio;
struct bio *split_bio;
- unsigned int has_flush:1; /* include flush request */
- unsigned int has_fua:1; /* include fua request */
- unsigned int has_null_flush:1; /* include empty flush request */
+ unsigned int has_flush:1; /* include flush request */
+ unsigned int has_fua:1; /* include fua request */
+ unsigned int has_null_flush:1; /* include null flush request */
+ unsigned int has_flush_payload:1; /* include flush payload */
/*
* io isn't sent yet, flush/fua request can only be submitted till it's
* the first IO in running_ios list
@@ -571,6 +572,8 @@ static void r5l_log_endio(struct bio *bio)
struct r5l_io_unit *io_deferred;
struct r5l_log *log = io->log;
unsigned long flags;
+ bool has_null_flush;
+ bool has_flush_payload;
if (bio->bi_status)
md_error(log->rdev->mddev, log->rdev);
@@ -580,6 +583,16 @@ static void r5l_log_endio(struct bio *bio)
spin_lock_irqsave(&log->io_list_lock, flags);
__r5l_set_io_unit_state(io, IO_UNIT_IO_END);
+
+ /*
+ * if the io doesn't not have null_flush or flush payload,
+ * it is not safe to access it after releasing io_list_lock.
+ * Therefore, it is necessary to check the condition with
+ * the lock held.
+ */
+ has_null_flush = io->has_null_flush;
+ has_flush_payload = io->has_flush_payload;
+
if (log->need_cache_flush && !list_empty(&io->stripe_list))
r5l_move_to_end_ios(log);
else
@@ -600,19 +613,23 @@ static void r5l_log_endio(struct bio *bio)
if (log->need_cache_flush)
md_wakeup_thread(log->rdev->mddev->thread);
- if (io->has_null_flush) {
+ /* finish flush only io_unit and PAYLOAD_FLUSH only io_unit */
+ if (has_null_flush) {
struct bio *bi;
WARN_ON(bio_list_empty(&io->flush_barriers));
while ((bi = bio_list_pop(&io->flush_barriers)) != NULL) {
bio_endio(bi);
- atomic_dec(&io->pending_stripe);
+ if (atomic_dec_and_test(&io->pending_stripe)) {
+ __r5l_stripe_write_finished(io);
+ return;
+ }
}
}
-
- /* finish flush only io_unit and PAYLOAD_FLUSH only io_unit */
- if (atomic_read(&io->pending_stripe) == 0)
- __r5l_stripe_write_finished(io);
+ /* decrease pending_stripe for flush payload */
+ if (has_flush_payload)
+ if (atomic_dec_and_test(&io->pending_stripe))
+ __r5l_stripe_write_finished(io);
}
static void r5l_do_submit_io(struct r5l_log *log, struct r5l_io_unit *io)
@@ -881,6 +898,11 @@ static void r5l_append_flush_payload(struct r5l_log *log, sector_t sect)
payload->size = cpu_to_le32(sizeof(__le64));
payload->flush_stripes[0] = cpu_to_le64(sect);
io->meta_offset += meta_size;
+ /* multiple flush payloads count as one pending_stripe */
+ if (!io->has_flush_payload) {
+ io->has_flush_payload = 1;
+ atomic_inc(&io->pending_stripe);
+ }
mutex_unlock(&log->io_mutex);
}
--
2.9.3
^ permalink raw reply related
* [PATCH 0/4] Assorted mdadm patches.
From: NeilBrown @ 2017-08-04 5:30 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
One real bugfix - that a customer might have hit.
Another real bugfix, which proves no-one is using that feature.
And a couple of minor consistency improvements.
Can you guess which is which :-?
NeilBrown
---
NeilBrown (4):
Error messages should end with a newline character.
Use correct syntax for passing DEVLINKS to mdadm from udev
super1: only set clustered flag when bitmap is present
Don't use exit(ERANGE)
Build.c | 4 ++--
Grow.c | 4 ++--
Manage.c | 2 +-
mdadm.8.in | 2 +-
mdadm.c | 2 +-
mdopen.c | 2 +-
super1.c | 4 ++--
udev-md-raid-assembly.rules | 2 +-
8 files changed, 11 insertions(+), 11 deletions(-)
--
Signature
^ permalink raw reply
* [PATCH 1/4] Error messages should end with a newline character.
From: NeilBrown @ 2017-08-04 5:30 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
In-Reply-To: <150182450644.25561.9683614144764209335.stgit@noble>
Add "\n" to the end of error messages which don't already
have one. Also spell "opened" correctly.
Signed-off-by: NeilBrown <neilb@suse.com>
---
Build.c | 4 ++--
Grow.c | 4 ++--
Manage.c | 2 +-
mdopen.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Build.c b/Build.c
index 70ba06843796..962c2e3776b9 100644
--- a/Build.c
+++ b/Build.c
@@ -181,7 +181,7 @@ int Build(char *mddev, struct mddev_dev *devlist,
int major = BITMAP_MAJOR_HI;
#if 0
if (s->bitmap_chunk == UnSet) {
- pr_err("%s cannot be openned.", s->bitmap_file);
+ pr_err("%s cannot be opened.\n", s->bitmap_file);
goto abort;
}
#endif
@@ -193,7 +193,7 @@ int Build(char *mddev, struct mddev_dev *devlist,
}
bitmap_fd = open(s->bitmap_file, O_RDWR);
if (bitmap_fd < 0) {
- pr_err("%s cannot be openned.", s->bitmap_file);
+ pr_err("%s cannot be opened.\n", s->bitmap_file);
goto abort;
}
}
diff --git a/Grow.c b/Grow.c
index 4ecb1d8449ae..9305db6b6df4 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3183,7 +3183,7 @@ static int reshape_array(char *container, int fd, char *devname,
if (info2) {
if (sysfs_init(info2, fd, st->devnm)) {
- pr_err("unable to initialize sysfs for %s",
+ pr_err("unable to initialize sysfs for %s\n",
st->devnm);
free(info2);
goto release;
@@ -5127,7 +5127,7 @@ int Grow_continue_command(char *devname, int fd,
}
if (sysfs_init(content, fd2, mdstat->devnm)) {
- pr_err("Unable to initialize sysfs for %s, Grow cannot continue",
+ pr_err("Unable to initialize sysfs for %s, Grow cannot continue.\n",
mdstat->devnm);
ret_val = 1;
close(fd2);
diff --git a/Manage.c b/Manage.c
index 04b9398c2e4f..6b6112f7abc9 100644
--- a/Manage.c
+++ b/Manage.c
@@ -1423,7 +1423,7 @@ int Manage_subdevs(char *devname, int fd,
}
add_devlist = conf_get_devs();
if (add_devlist == NULL) {
- pr_err("no devices to scan for missing members.");
+ pr_err("no devices to scan for missing members.\n");
continue;
}
for (dp = &add_devlist; *dp; dp = & (*dp)->next)
diff --git a/mdopen.c b/mdopen.c
index c4f1c12c2dcd..3c0052f2db23 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -198,7 +198,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
return -1;
}
if (cname[0] == 0) {
- pr_err("%s is an invalid name for an md device (empty!).", dev);
+ pr_err("%s is an invalid name for an md device (empty!).\n", dev);
return -1;
}
if (num < 0) {
^ permalink raw reply related
* [PATCH 2/4] Use correct syntax for passing DEVLINKS to mdadm from udev
From: NeilBrown @ 2017-08-04 5:30 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
In-Reply-To: <150182450644.25561.9683614144764209335.stgit@noble>
${DEVLINKS}
is not valid udev syntax, and is passed through uninterpreted.
$env{DEVLINKS}
or
%e{DEVLINKS}
is correct.
Signed-off-by: NeilBrown <neilb@suse.com>
---
mdadm.8.in | 2 +-
udev-md-raid-assembly.rules | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/mdadm.8.in b/mdadm.8.in
index ecfe9da8d474..d8c2dfca6040 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -2948,7 +2948,7 @@ This is the only context where the aliases are used. They are
usually provided by a
.I udev
rules mentioning
-.BR ${DEVLINKS} .
+.BR $env{DEVLINKS} .
.IP +
Does the device have a valid md superblock? If a specific metadata
diff --git a/udev-md-raid-assembly.rules b/udev-md-raid-assembly.rules
index 8ca232a44b1f..9f055ed022d5 100644
--- a/udev-md-raid-assembly.rules
+++ b/udev-md-raid-assembly.rules
@@ -30,7 +30,7 @@ LABEL="md_inc"
# remember you can limit what gets auto/incrementally assembled by
# mdadm.conf(5)'s 'AUTO' and selectively whitelist using 'ARRAY'
-ACTION=="add|change", IMPORT{program}="BINDIR/mdadm --incremental --export $devnode --offroot ${DEVLINKS}"
+ACTION=="add|change", IMPORT{program}="BINDIR/mdadm --incremental --export $devnode --offroot $env{DEVLINKS}"
ACTION=="add|change", ENV{MD_STARTED}=="*unsafe*", ENV{MD_FOREIGN}=="no", ENV{SYSTEMD_WANTS}+="mdadm-last-resort@$env{MD_DEVICE}.timer"
ACTION=="remove", ENV{ID_PATH}=="?*", RUN+="BINDIR/mdadm -If $name --path $env{ID_PATH}"
ACTION=="remove", ENV{ID_PATH}!="?*", RUN+="BINDIR/mdadm -If $name"
^ permalink raw reply related
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