* [PATCH 04/17] fix variable offset when ':' is present in the device name
From: Song Liu @ 2015-12-02 0:25 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, dan.j.williams, shli, Dan Williams, Song Liu
In-Reply-To: <1449015933-255689-1-git-send-email-songliubraving@fb.com>
From: Dan Williams <djbw@fb.com>
Stopping at the first ':' precludes using device names like
/dev/disk/by-path/pci-0000:04:00.0-sas-0x500605b005846060:1:0-0x5000cca01a7c5605:10
so, check that the name is not ambiguous before chopping off the offset at the end
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
Create.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/Create.c b/Create.c
index 21d1374..2642b36 100644
--- a/Create.c
+++ b/Create.c
@@ -306,7 +306,13 @@ int Create(struct supertype *st, char *mddev,
continue;
}
if (data_offset == VARIABLE_OFFSET) {
- doff = strchr(dname, ':');
+ doff = strrchr(dname, ':');
+ if (stat(dname, &stb) == 0) {
+ pr_err("cannot determine if %s is a device name, or a device with a data-offset argument of '%s'\n",
+ dname, doff+1);
+ pr_err("check that you have specified a data-offset for all array members\n");
+ exit(2);
+ }
if (doff) {
*doff++ = 0;
dv->data_offset = parse_size(doff);
--
2.4.6
^ permalink raw reply related
* [PATCH 03/17] 'act_spare' should be the minimum requirement for dynamic domains
From: Song Liu @ 2015-12-02 0:25 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, dan.j.williams, shli, Dan Williams, Song Liu
In-Reply-To: <1449015933-255689-1-git-send-email-songliubraving@fb.com>
From: Dan Williams <djbw@fb.com>
Let bare devices that fit the domain become raid members. Rather than
require the tracking of 'failed slots', simply define a group of slots
for the domain and let any bare devices attached in those domains join
a compatible array.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
policy.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/policy.c b/policy.c
index 960a37e..6afafad 100644
--- a/policy.c
+++ b/policy.c
@@ -874,7 +874,7 @@ int write_rule(struct rule *rule, int fd, int force_part)
/* Generate single entry in udev rule basing on POLICY line found in config
* file. Take only those with paths, only first occurrence if paths are equal
- * and if actions supports handling of spares (>=act_spare_same_slot)
+ * and if actions supports handling of spares (>=act_spare)
*/
int generate_entries(int fd)
{
@@ -891,7 +891,7 @@ int generate_entries(int fd)
/* only policies with paths and with actions supporting
* bare disks are considered */
loop_value = find_rule(loop->rule, pol_act);
- if (!loop_value || map_act(loop_value) < act_spare_same_slot)
+ if (!loop_value || map_act(loop_value) < act_spare)
continue;
rule = find_path_rule(loop->rule);
if (!rule)
@@ -901,7 +901,7 @@ int generate_entries(int fd)
if (dup->type != rule_policy && loop->type != rule_part)
continue;
dup_value = find_rule(dup->rule, pol_act);
- if (!dup_value || map_act(dup_value) < act_spare_same_slot)
+ if (!dup_value || map_act(dup_value) < act_spare)
continue;
rule = find_path_rule(dup->rule);
if (!rule)
--
2.4.6
^ permalink raw reply related
* [PATCH 02/17] mdadm: allow a /dev/disk/by-slot as domain path descriptor
From: Song Liu @ 2015-12-02 0:25 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, dan.j.williams, shli, Dan Williams, Song Liu
In-Reply-To: <1449015933-255689-1-git-send-email-songliubraving@fb.com>
From: Dan Williams <djbw@fb.com>
In a POLICY line let 'slot=' statements define a domain by
/dev/disk/by-slot and take precedence over 'path=' (/dev/disk/by-path)
definitions.
Also support writing udev rules for slot paths.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
mdadm.h | 2 +-
policy.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 69 insertions(+), 15 deletions(-)
diff --git a/mdadm.h b/mdadm.h
index 5d5e97f..167ca0c 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1126,7 +1126,7 @@ struct pol_rule {
};
extern char rule_policy[], rule_part[];
-extern char rule_path[], rule_type[];
+extern char rule_path[], rule_slot[], rule_type[];
extern char type_part[], type_disk[];
extern void policyline(char *line, char *type);
diff --git a/policy.c b/policy.c
index 064d349..960a37e 100644
--- a/policy.c
+++ b/policy.c
@@ -189,16 +189,17 @@ struct dev_policy *pol_find(struct dev_policy *pol, char *name)
return pol;
}
-static char *disk_path(struct mdinfo *disk)
+static char *__disk_path(struct mdinfo *disk, const char *path_prefix, int null_ok)
{
struct stat stb;
int prefix_len;
DIR *by_path;
- char symlink[PATH_MAX] = "/dev/disk/by-path/";
+ char symlink[PATH_MAX];
char nm[PATH_MAX];
struct dirent *ent;
int rv;
+ strcpy(symlink, path_prefix);
by_path = opendir(symlink);
if (by_path) {
prefix_len = strlen(symlink);
@@ -219,6 +220,9 @@ static char *disk_path(struct mdinfo *disk)
}
closedir(by_path);
}
+ closedir(by_path);
+ if (null_ok)
+ return NULL;
/* A NULL path isn't really acceptable - use the devname.. */
sprintf(symlink, "/sys/dev/block/%d:%d", disk->disk.major, disk->disk.minor);
rv = readlink(symlink, nm, sizeof(nm)-1);
@@ -232,6 +236,15 @@ static char *disk_path(struct mdinfo *disk)
return xstrdup("unknown");
}
+static char *disk_path(struct mdinfo *disk)
+{
+ char *path = __disk_path(disk, "/dev/disk/by-slot/", 1);
+
+ if (!path)
+ return __disk_path(disk, "/dev/disk/by-path/", 0);
+ return path;
+}
+
char type_part[] = "part";
char type_disk[] = "disk";
static char *disk_type(struct mdinfo *disk)
@@ -246,6 +259,13 @@ static char *disk_type(struct mdinfo *disk)
return type_disk;
}
+static int rule_is_path(struct rule *rule)
+{
+ if (rule->name == rule_path || rule->name == rule_slot)
+ return 1;
+ return 0;
+}
+
static int pol_match(struct rule *rule, char *path, char *type)
{
/* check if this rule matches on path and type */
@@ -253,7 +273,7 @@ static int pol_match(struct rule *rule, char *path, char *type)
int typeok = 0;
while (rule) {
- if (rule->name == rule_path) {
+ if (rule_is_path(rule)) {
if (pathok == 0)
pathok = -1;
if (path && fnmatch(rule->value, path, 0) == 0)
@@ -403,6 +423,7 @@ void pol_add(struct dev_policy **pol,
/*
* disk_policy() gathers policy information for the
* disk described in the given mdinfo (disk.{major,minor}).
+ * prefer 'by-slot' over 'by-path'
*/
struct dev_policy *disk_policy(struct mdinfo *disk)
{
@@ -432,6 +453,7 @@ struct dev_policy *devid_policy(int dev)
*/
char rule_path[] = "path";
+char rule_slot[] = "slot";
char rule_type[] = "type";
char rule_policy[] = "policy";
@@ -470,7 +492,8 @@ void policyline(char *line, char *type)
pr->type = type;
pr->rule = NULL;
for (w = dl_next(line); w != line ; w = dl_next(w)) {
- if (try_rule(w, rule_path, &pr->rule))
+ if (try_rule(w, rule_slot, &pr->rule)
+ || try_rule(w, rule_path, &pr->rule))
config_rules_has_path = 1;
else if (! try_rule(w, rule_type, &pr->rule) &&
! try_rule(w, pol_metadata, &pr->rule) &&
@@ -793,31 +816,59 @@ char *find_rule(struct rule *rule, char *rule_type)
return NULL;
}
+/* prefer 'slot' rules over plain 'path' rules */
+static struct rule *find_path_rule(struct rule *rule)
+{
+ struct rule *path_rule = NULL;
+
+ while (rule) {
+ if (rule->name == rule_slot)
+ return rule;
+ if (!path_rule && rule->name == rule_path)
+ path_rule = rule;
+ rule = rule->next;
+ }
+
+ return path_rule;
+}
+
#define UDEV_RULE_FORMAT \
"ACTION==\"add\", SUBSYSTEM==\"block\", " \
-"ENV{DEVTYPE}==\"%s\", ENV{ID_PATH}==\"%s\", " \
+"ENV{DEVTYPE}==\"%s\", ENV{%s}==\"%s\", " \
"RUN+=\"" BINDIR "/mdadm --incremental $env{DEVNAME}\"\n"
#define UDEV_RULE_FORMAT_NOTYPE \
"ACTION==\"add\", SUBSYSTEM==\"block\", " \
-"ENV{ID_PATH}==\"%s\", " \
+"ENV{%s}==\"%s\", " \
"RUN+=\"" BINDIR "/mdadm --incremental $env{DEVNAME}\"\n"
+
+
/* Write rule in the rule file. Use format from UDEV_RULE_FORMAT */
int write_rule(struct rule *rule, int fd, int force_part)
{
char line[1024];
- char *pth = find_rule(rule, rule_path);
+ struct rule *path_rule = find_path_rule(rule);
char *typ = find_rule(rule, rule_type);
- if (!pth)
+ char *slot_path, *pth;
+
+ if (!path_rule)
return -1;
+ pth = path_rule->value;
+ if (path_rule->name == rule_slot)
+ slot_path = "ID_SLOT";
+ else
+ slot_path = "ID_PATH";
+
if (force_part)
typ = type_part;
if (typ)
- snprintf(line, sizeof(line) - 1, UDEV_RULE_FORMAT, typ, pth);
+ snprintf(line, sizeof(line) - 1, UDEV_RULE_FORMAT,
+ typ, slot_path, pth);
else
- snprintf(line, sizeof(line) - 1, UDEV_RULE_FORMAT_NOTYPE, pth);
+ snprintf(line, sizeof(line) - 1, UDEV_RULE_FORMAT_NOTYPE,
+ slot_path, pth);
return write(fd, line, strlen(line)) == (int)strlen(line);
}
@@ -829,6 +880,7 @@ int generate_entries(int fd)
{
struct pol_rule *loop, *dup;
char *loop_value, *dup_value;
+ struct rule *rule;
int duplicate;
for (loop = config_rules; loop; loop = loop->next) {
@@ -841,18 +893,20 @@ int generate_entries(int fd)
loop_value = find_rule(loop->rule, pol_act);
if (!loop_value || map_act(loop_value) < act_spare_same_slot)
continue;
- loop_value = find_rule(loop->rule, rule_path);
- if (!loop_value)
+ rule = find_path_rule(loop->rule);
+ if (!rule)
continue;
+ loop_value = rule->value;
for (dup = config_rules; dup != loop; dup = dup->next) {
if (dup->type != rule_policy && loop->type != rule_part)
continue;
dup_value = find_rule(dup->rule, pol_act);
if (!dup_value || map_act(dup_value) < act_spare_same_slot)
continue;
- dup_value = find_rule(dup->rule, rule_path);
- if (!dup_value)
+ rule = find_path_rule(dup->rule);
+ if (!rule)
continue;
+ dup_value = rule->value;
if (strcmp(loop_value, dup_value) == 0) {
duplicate = 1;
break;
--
2.4.6
^ permalink raw reply related
* [PATCH 01/17] udev rules and infrastructure for /dev/disk/by-slot
From: Song Liu @ 2015-12-02 0:25 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, dan.j.williams, shli, Dan Williams, Song Liu
In-Reply-To: <1449015933-255689-1-git-send-email-songliubraving@fb.com>
From: Dan Williams <djbw@fb.com>
/dev/disk/by-path maps the device by its sysfs path, but uses
data not necessarily guaranteed to be static. It also takes extra work
to map where the device is physically attached.
static data for creating the path name.
For example:
pci-0000:04:00.0-sas-0x500605b005846061:1:0-0x5000cca01a7c5606:0
phy_id------------------------------------^
end_device-sas_address----------------------^
...on mpt2sas 'phy_id' is a dynamically allocated number based on when
the port comes up. There is no differentiation based on where the end
device is attached as the end device sas address is the only indication
of the exit point from the topology.
In contrast:
enclosure-0x5f80f41f1d6a60ff-slot9
...is a static name for this physical location in an enclosure.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
Makefile | 3 +
ses_slot_id | 144 ++++++++++++++++++++++++++++++++++++++++++++++
udev-enclosure-slot.rules | 13 +++++
3 files changed, 160 insertions(+)
create mode 100755 ses_slot_id
create mode 100644 udev-enclosure-slot.rules
diff --git a/Makefile b/Makefile
index fde2e63..4eb2dad 100644
--- a/Makefile
+++ b/Makefile
@@ -110,6 +110,7 @@ LDLIBS=-ldl
INSTALL = /usr/bin/install
DESTDIR =
BINDIR = /sbin
+LIBDIR = /lib/mdadm
MANDIR = /usr/share/man
MAN4DIR = $(MANDIR)/man4
MAN5DIR = $(MANDIR)/man5
@@ -292,6 +293,8 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
$(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules
+ $(INSTALL) -D -m 655 ses_slot_id $(DESTDIR)$(LIBDIR)/ses_slot_id
+ $(INSTALL) -D -m 644 udev-enclosure-slot.rules $(DESTDIR)$(UDEVDIR)/rules.d/62-md-enclosure-slot.rules
@for file in 63-md-raid-arrays.rules 64-md-raid-assembly.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 ; \
diff --git a/ses_slot_id b/ses_slot_id
new file mode 100755
index 0000000..629f1d1
--- /dev/null
+++ b/ses_slot_id
@@ -0,0 +1,144 @@
+#!/bin/sh
+
+# lookup ses slot information for a given device a /dev/disk/by-slot to
+# supplement the not-always-persistent path in the sas case
+# based on the the original shell script implementation of path_id
+
+SYSFS=/sys
+RESULT=1
+TYPE=
+OPWD="`pwd`"
+full_sysfs_path=
+full_sysfs_device_path=
+
+if [ -z "$DEVPATH" -a -z "$1" ] ; then
+ exit 1
+fi
+
+if [ -z "$DEVPATH" ] ; then
+ case "$1" in
+ $SYSFS/*)
+ DEVPATH="${1#$SYSFS}"
+ ;;
+ *)
+ DEVPATH=$1
+ ;;
+ esac
+fi
+
+if [ ! -e $SYSFS$DEVPATH/dev ] ; then
+ exit 1
+fi
+
+case "$DEVPATH" in
+ /devices/*)
+ cd "$SYSFS$DEVPATH/subsystem";
+ TYPE="`pwd -P`"
+ cd "$OPWD"
+ TYPE="${TYPE##*/}"
+ ;;
+ /class/*)
+ TYPE="${DEVPATH#/class/}"
+ TYPE="${TYPE%%/*}"
+ ;;
+ /block/*)
+ TYPE=block
+ ;;
+ *)
+ exit 1
+ ;;
+esac
+
+handle_ses () {
+ : handle_ses $*
+ local DEV=$1
+
+ ses_dev=$(find $DEV -maxdepth 1 -name 'enclosure_device:*' -print -quit)
+ ses_dev=$(readlink -f "$ses_dev")
+ enc_dev="${ses_dev%/*}"
+
+ if [ -f "$ses_dev/slot" ]; then
+ read slot < "$ses_dev/slot"
+ fi
+
+ if [ -f "$enc_dev/id" ]; then
+ read enclosure_id < "$enc_dev/id"
+ fi
+
+ if [ -z "$slot" -o -z "$enclosure_id" ]; then
+ : no enclosure device
+ D=
+ RESULT=1
+ return
+ fi
+
+ d="enclosure-${enclosure_id}-slot${slot}"
+ D=
+ RESULT=0
+}
+
+handle_device () {
+ full_sysfs_path="$SYSFS$DEVPATH"
+ case "$DEVPATH" in
+ /devices/*)
+ # new sysfs layout
+ if [ -L $full_sysfs_path/subsystem ]; then
+ full_sysfs_path="${full_sysfs_path%/*}"
+ cd "$full_sysfs_path/subsystem";
+ subsys="`pwd -P`"
+ cd "$OPWD"
+ subsys="${subsys##*/}"
+ if [ "$subsys" = "block" ]; then
+ # parent is "block", it's a partition, move one up
+ full_sysfs_path="${full_sysfs_path%/*}"
+ fi
+ cd $full_sysfs_path
+ fi
+ ;;
+ *)
+ # old sysfs layout
+ if [ ! -L $full_sysfs_path/device ] ; then
+ if [ -f $full_sysfs_path/range ] ; then return ; fi
+ full_sysfs_path="${full_sysfs_path%/*}"
+ : full_sysfs_path "$full_sysfs_path"
+ if [ ! -L $full_sysfs_path/device -o ! -f $full_sysfs_path/dev ] ; then
+ return
+ fi
+ fi
+ cd $full_sysfs_path/device
+ ;;
+ esac
+ full_sysfs_device_path="`pwd -P`"
+ cd "$OPWD"
+ D=$full_sysfs_device_path
+ while [ ! -z "$D" ] ; do
+ case "$D" in
+ */end_device-[0-9]*:[0-9]*:[0-9]*/*)
+ handle_ses "$D"
+ if [ $RESULT = 0 ]; then
+ found_ses="yes"
+ fi
+ ;;
+ */devices)
+ D=
+ ;;
+ *)
+ : not handled
+ RESULT=1
+ return
+ ;;
+ esac
+ done
+}
+
+case "$TYPE" in
+ block)
+ handle_device
+ echo "ID_SLOT='$d'"
+ ;;
+ *)
+ RESULT=1
+ ;;
+esac
+
+exit $RESULT
diff --git a/udev-enclosure-slot.rules b/udev-enclosure-slot.rules
new file mode 100644
index 0000000..75c6376
--- /dev/null
+++ b/udev-enclosure-slot.rules
@@ -0,0 +1,13 @@
+# do not edit this file, it will be overwritten on update
+
+SUBSYSTEM!="block", GOTO="ses_slot_end"
+
+ACTION!="add|change", GOTO="ses_slot_end"
+KERNEL!="sd*", GOTO="ses_slot_end"
+
+# by-slot (parent device slot)
+ENV{DEVTYPE}=="disk", IMPORT{program}="/lib/mdadm/ses_slot_id %p"
+ENV{DEVTYPE}=="disk", ENV{ID_SLOT}=="?*", SYMLINK+="disk/by-slot/$env{ID_SLOT}"
+ENV{DEVTYPE}=="partition", ENV{ID_SLOT}=="?*", SYMLINK+="disk/by-slot/$env{ID_SLOT}-part%n"
+
+LABEL="ses_slot_end"
--
2.4.6
^ permalink raw reply related
* [PATCH 00/17] mdadm: SCSI enclosure based array
From: Song Liu @ 2015-12-02 0:25 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, dan.j.williams, shli, Song Liu
These patches by Dan Williams enable creating RAID array based on
SCSI enclosures.
mdadm gets information about SCSI enclosure through ses module in
kernel, and creates array for HDDs in an enclosure.
New udev rules are added to make drive swap in the enclosure based
array easier.
Typical array create flow looks like:
/* examine enclosures in the system */
mdadm --detail-platform --brief --enclosure > /etc/mdadm.conf
/* create policy for enclosure0 and enclosure1 */
echo "POLICY domain=domain0 action=spare slot=enclosure0" >> /etc/mdadm.conf
echo "POLICY domain=domain1 action=spare slot=enclosure1" >> /etc/mdadm.conf
/* add md-ses program */
echo "PROGRAM /usr/share/mdadm/md-ses" >> /etc/mdadm.conf
/* create md0 and md1 */
mdadm --create /dev/md0 enclosure0 -n 15 -l 6 -e 1.2 -c 1024 --run
mdadm --create /dev/md1 enclosure1 -n 15 -l 6 -e 1.2 -c 1024 --run
/* add array to mdadm.conf */
mdadm -Eb enclosure0 enclosure1 >> /etc/mdadm.conf
/* create udev rules that add drives to array */
mdadm --udev-rules --enclosure > /lib/udev/rules.d/65-md-raid-hotswap.rules
Dan Williams (15):
udev rules and infrastructure for /dev/disk/by-slot
mdadm: allow a /dev/disk/by-slot as domain path descriptor
'act_spare' should be the minimum requirement for dynamic domains
fix variable offset when ':' is present in the device name
imsm: quiet detail platform
ses: workaround sysfs deprecated
prepare for enclosure platform details
enclosure detection/enumeration
Add --brief support to --enclosure
config: ENCLOSURE keyword and enclosure device list expansion
policy: enable enclosure names for domain definitions
invoke hot-add policy on "change" events
Toggle enclosure leds in response to raid events
scsi mode sense/select support
SCSIMODE: configuration file support for mode page settings.
Song Liu (2):
Clear MBR in when Kill superblock
In generated udev rules, skip RAID members
Create.c | 10 +-
Detail.c | 63 +++----
Incremental.c | 9 +
Kill.c | 22 +++
Makefile | 31 +++-
Monitor.c | 3 +
ReadMe.c | 2 +
config.c | 281 +++++++++++++++++++++++++++-
contrib/md-ses | 310 +++++++++++++++++++++++++++++++
enclosure.c | 355 ++++++++++++++++++++++++++++++++++++
forward-scsi-uevents | 14 ++
mdadm.c | 99 +++++++---
mdadm.h | 31 +++-
policy.c | 116 +++++++++---
ses_slot_id | 144 +++++++++++++++
sg_io.c | 170 ++++++++++++++++-
sg_io.h | 27 +++
super-intel.c | 29 ++-
sysfs.c | 11 +-
udev-enclosure-slot.rules.in | 11 ++
udev-workaround-sysfs-deprecated.in | 2 +
util.c | 7 +
22 files changed, 1648 insertions(+), 99 deletions(-)
create mode 100755 contrib/md-ses
create mode 100644 enclosure.c
create mode 100755 forward-scsi-uevents
create mode 100755 ses_slot_id
create mode 100644 sg_io.h
create mode 100644 udev-enclosure-slot.rules.in
create mode 100644 udev-workaround-sysfs-deprecated.in
--
2.4.6
^ permalink raw reply
* RAID 5,6 sequential writing seems slower in newer kernels
From: Dallas Clement @ 2015-12-01 23:02 UTC (permalink / raw)
To: linux-raid
Hi,
I have a NAS system with 12 spinning disks that has been running with
the 2.6.39.4 kernel. It has a 4 core xeon processor (E31275 @
3.40GHz), with 8 GB of RAM. The 12 disks in my RAID array are Hitachi
4 TB, 7200 RPM SATA drives. The filesystem is XFS.
Recently I have been evaluating RAID performance on newer kernels 3.10
and 4.2. I have observed that with the same settings, I am seeing
much slower RAID 5 and 6 sequential write speeds with newer kernels
compared to what I was seeing with the 2.6.39.4 kernel. However, the
4.2 kernel has much better read speeds for both sequential and random
patterns. I understand that there have been many improvements to RAID
5 and 6 in the 4.1 kernel. I definitely am seeing improvement with
reads but not writes.
If I observe disk and array throughput with iostat, the individual
disk utilization and wMB/s is much lower in the newer kernels. With
the older 2.6.39.4 kernel, disk utilization seems to stay above 80%
with wMB/s around 74 MB/s, whereas the newer kernel disk utilization
seems to vary between 20-70% with wMB/s around 9-38 MB/s. CPU iowait
gets up to about 10% much of the time. These Hitachi disks are
capable of sustaining around 170 MB/s, which is just about what I see
when doing sequential writes to all 12 disks concurrently in a JBOD
configuration, i.e no RAID. The iowait for 12 disks of JBOD gets up
to about 97% - which makes the system very unresponsive.
One other observation is that RAID 0 sequential write speeds in newer
kernels are only slightly less than what I was seeing in 2.6.39.4.
I am frankly surprised at these results. Perhaps there is some
configuration or tunable settings that have changed since the 2.6
kernel that I am unaware of that affect RAID 5, 6 performance. Please
comment if you have any ideas which might explain what I am seeing.
Thanks,
Dalla
^ permalink raw reply
* Re: [PATCH v2 3/3] md: convert to use the generic badblocks code
From: Verma, Vishal L @ 2015-12-01 19:52 UTC (permalink / raw)
To: shli@kernel.org
Cc: linux-raid@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-nvdimm@lists.01.org, linux-block@vger.kernel.org,
neilb@suse.com, axboe@fb.com, jmoyer@redhat.com
In-Reply-To: <20151201185518.GA13926@kernel.org>
On Tue, 2015-12-01 at 10:55 -0800, Shaohua Li wrote:
> On Wed, Nov 25, 2015 at 11:43:33AM -0700, Vishal Verma wrote:
> > Retain badblocks as part of rdev, but use the accessor functions
> > from
> > include/linux/badblocks for all manipulation.
> >
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > ---
> > drivers/md/md.c | 507 +++----------------------------------------
> > -------------
> > drivers/md/md.h | 40 +----
> > 2 files changed, 23 insertions(+), 524 deletions(-)
> >
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index c702de1..63eab20 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -34,6 +34,7 @@
> >
> > #include <linux/kthread.h>
> > #include <linux/blkdev.h>
> > +#include <linux/badblocks.h>
> > #include <linux/sysctl.h>
> > #include <linux/seq_file.h>
> > #include <linux/fs.h>
> > @@ -707,8 +708,7 @@ void md_rdev_clear(struct md_rdev *rdev)
> > put_page(rdev->bb_page);
> > rdev->bb_page = NULL;
> > }
> > - kfree(rdev->badblocks.page);
> > - rdev->badblocks.page = NULL;
> > + badblocks_free(&rdev->badblocks);
> > }
>
> why does rdev have extra badblocks? the gendisk already had one.
rdev originally had badblocks, and this path set adds badblocks to
gendisk. It does appear that md's badblock tracking will be a bit
redundant if/once gendisk has badblocks support - see the discussion
here:
https://lists.01.org/pipermail/linux-nvdimm/2015-November/002980.html
-Vishal
^ permalink raw reply
* Re: [PATCH v2 3/3] md: convert to use the generic badblocks code
From: Shaohua Li @ 2015-12-01 18:55 UTC (permalink / raw)
To: Vishal Verma
Cc: linux-nvdimm, linux-block, linux-raid, linux-scsi, Jens Axboe,
NeilBrown, Jeff Moyer
In-Reply-To: <1448477013-9174-4-git-send-email-vishal.l.verma@intel.com>
On Wed, Nov 25, 2015 at 11:43:33AM -0700, Vishal Verma wrote:
> Retain badblocks as part of rdev, but use the accessor functions from
> include/linux/badblocks for all manipulation.
>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
> drivers/md/md.c | 507 +++-----------------------------------------------------
> drivers/md/md.h | 40 +----
> 2 files changed, 23 insertions(+), 524 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index c702de1..63eab20 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -34,6 +34,7 @@
>
> #include <linux/kthread.h>
> #include <linux/blkdev.h>
> +#include <linux/badblocks.h>
> #include <linux/sysctl.h>
> #include <linux/seq_file.h>
> #include <linux/fs.h>
> @@ -707,8 +708,7 @@ void md_rdev_clear(struct md_rdev *rdev)
> put_page(rdev->bb_page);
> rdev->bb_page = NULL;
> }
> - kfree(rdev->badblocks.page);
> - rdev->badblocks.page = NULL;
> + badblocks_free(&rdev->badblocks);
> }
why does rdev have extra badblocks? the gendisk already had one.
^ permalink raw reply
* [PATCH 4/4] mdadm: do not display bitmap info if it is cleared
From: Guoqing Jiang @ 2015-12-01 16:30 UTC (permalink / raw)
To: neilb, linux-raid; +Cc: rgoldwyn, Guoqing Jiang
In-Reply-To: <1448987412-3932-1-git-send-email-gqjiang@suse.com>
"mdadm -X DISK" is used to report information about a bitmap
file, it is better to not display all the related infos if
bitmap is cleared with "--bitmap=none" under grow mode.
To do that, the locate_bitmap is changed a little to have a
return value based on MD_FEATURE_BITMAP_OFFSET.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
bitmap.c | 8 ++++++--
mdadm.h | 2 +-
super0.c | 7 ++++---
super1.c | 12 +++++++++---
4 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/bitmap.c b/bitmap.c
index 803eda3..dab674b 100644
--- a/bitmap.c
+++ b/bitmap.c
@@ -221,8 +221,12 @@ int bitmap_file_open(char *filename, struct supertype **stp)
pr_err("No bitmap possible with %s metadata\n",
st->ss->name);
return -1;
- } else
- st->ss->locate_bitmap(st, fd);
+ } else {
+ if (st->ss->locate_bitmap(st, fd)) {
+ pr_err("%s doesn't have bitmap\n", filename);
+ fd = -1;
+ }
+ }
*stp = st;
} else {
diff --git a/mdadm.h b/mdadm.h
index 5d5e97f..aad0fa8 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -873,7 +873,7 @@ extern struct superswitch {
/* Seek 'fd' to start of write-intent-bitmap. Must be an
* md-native format bitmap
*/
- void (*locate_bitmap)(struct supertype *st, int fd);
+ int (*locate_bitmap)(struct supertype *st, int fd);
/* if add_internal_bitmap succeeded for existing array, this
* writes it out.
*/
diff --git a/super0.c b/super0.c
index 6ad9d39..7f80014 100644
--- a/super0.c
+++ b/super0.c
@@ -1155,16 +1155,16 @@ static int add_internal_bitmap0(struct supertype *st, int *chunkp,
return 1;
}
-static void locate_bitmap0(struct supertype *st, int fd)
+static int locate_bitmap0(struct supertype *st, int fd)
{
unsigned long long dsize;
unsigned long long offset;
if (!get_dev_size(fd, NULL, &dsize))
- return;
+ return -1;
if (dsize < MD_RESERVED_SECTORS*512)
- return;
+ return -1;
offset = MD_NEW_SIZE_SECTORS(dsize>>9);
@@ -1173,6 +1173,7 @@ static void locate_bitmap0(struct supertype *st, int fd)
offset += MD_SB_BYTES;
lseek64(fd, offset, 0);
+ return 0;
}
static int write_bitmap0(struct supertype *st, int fd, enum bitmap_update update)
diff --git a/super1.c b/super1.c
index 062d9e7..7a1156d 100644
--- a/super1.c
+++ b/super1.c
@@ -1520,7 +1520,7 @@ static int add_to_super1(struct supertype *st, mdu_disk_info_t *dk,
}
#endif
-static void locate_bitmap1(struct supertype *st, int fd);
+static int locate_bitmap1(struct supertype *st, int fd);
static int store_super1(struct supertype *st, int fd)
{
@@ -2304,24 +2304,30 @@ add_internal_bitmap1(struct supertype *st,
return 1;
}
-static void locate_bitmap1(struct supertype *st, int fd)
+static int locate_bitmap1(struct supertype *st, int fd)
{
unsigned long long offset;
struct mdp_superblock_1 *sb;
int mustfree = 0;
+ int ret;
if (!st->sb) {
if (st->ss->load_super(st, fd, NULL))
- return; /* no error I hope... */
+ return -1; /* no error I hope... */
mustfree = 1;
}
sb = st->sb;
+ if ((__le32_to_cpu(sb->feature_map) & MD_FEATURE_BITMAP_OFFSET))
+ ret = 0;
+ else
+ ret = -1;
offset = __le64_to_cpu(sb->super_offset);
offset += (int32_t) __le32_to_cpu(sb->bitmap_offset);
if (mustfree)
free(sb);
lseek64(fd, offset<<9, 0);
+ return ret;
}
static int write_bitmap1(struct supertype *st, int fd, enum bitmap_update update)
--
2.1.4
^ permalink raw reply related
* [PATCH 3/4] mdadm: don't show cluster name once the bitmap is cleared
From: Guoqing Jiang @ 2015-12-01 16:30 UTC (permalink / raw)
To: neilb, linux-raid; +Cc: rgoldwyn, Guoqing Jiang
In-Reply-To: <1448987412-3932-1-git-send-email-gqjiang@suse.com>
Don't show cluster name if bitmap is cleared.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
super1.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/super1.c b/super1.c
index 1735c2d..062d9e7 100644
--- a/super1.c
+++ b/super1.c
@@ -324,7 +324,7 @@ static void examine_super1(struct supertype *st, char *homehost)
strncmp(sb->set_name, homehost, l) == 0)
printf(" (local to host %s)", homehost);
printf("\n");
- if (bms->nodes > 0)
+ if (bms->nodes > 0 && (__le32_to_cpu(sb->feature_map) & MD_FEATURE_BITMAP_OFFSET))
printf(" Cluster Name : %-64s\n", bms->cluster_name);
atime = __le64_to_cpu(sb->ctime) & 0xFFFFFFFFFFULL;
printf(" Creation Time : %.24s\n", ctime(&atime));
@@ -780,7 +780,7 @@ static void detail_super1(struct supertype *st, char *homehost)
sb->set_name[l] == ':' &&
strncmp(sb->set_name, homehost, l) == 0)
printf(" (local to host %s)", homehost);
- if (bms->nodes > 0)
+ if (bms->nodes > 0 && (__le32_to_cpu(sb->feature_map) & MD_FEATURE_BITMAP_OFFSET))
printf("\n Cluster Name : %-64s", bms->cluster_name);
printf("\n UUID : ");
for (i=0; i<16; i++) {
--
2.1.4
^ permalink raw reply related
* [PATCH 2/4] mdadm: output info more precisely when change bitmap to none
From: Guoqing Jiang @ 2015-12-01 16:30 UTC (permalink / raw)
To: neilb, linux-raid; +Cc: rgoldwyn, Guoqing Jiang
In-Reply-To: <1448987412-3932-1-git-send-email-gqjiang@suse.com>
WHen change bitmap to none, the infos could be more accurate
based on existed bitmap type.
And s->bitmap_file is passed from cmd "--bitmap=TYPE", so
remove s->bitmap_file from err info since it should means
change the bitmap to one type failed rather than the type is
already presented.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
Grow.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/Grow.c b/Grow.c
index 80d7b22..30ff7f3 100644
--- a/Grow.c
+++ b/Grow.c
@@ -328,12 +328,15 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
if (strcmp(s->bitmap_file, "none")==0) {
array.state &= ~(1<<MD_SB_BITMAP_PRESENT);
if (ioctl(fd, SET_ARRAY_INFO, &array)!= 0) {
- pr_err("failed to remove internal bitmap.\n");
+ if (array.state & (1<<MD_SB_CLUSTERED))
+ pr_err("failed to remove clustered bitmap.\n");
+ else
+ pr_err("failed to remove internal bitmap.\n");
return 1;
}
return 0;
}
- pr_err("%s bitmap already present on %s\n", s->bitmap_file, devname);
+ pr_err("bitmap already present on %s\n", devname);
return 1;
}
--
2.1.4
^ permalink raw reply related
* [PATCH 1/4] mdadm: let cluster raid could also add disk within incremental mode
From: Guoqing Jiang @ 2015-12-01 16:30 UTC (permalink / raw)
To: neilb, linux-raid; +Cc: rgoldwyn, Guoqing Jiang
For cluster raid, the disc.state need to be changed accordingly under
incremental mode.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
Incremental.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Incremental.c b/Incremental.c
index 781d27d..7afa7d7 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -444,6 +444,10 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
/* add disk needs to know about containers */
if (st->ss->external)
sra->array.level = LEVEL_CONTAINER;
+
+ if (info.array.state & (1 << MD_SB_CLUSTERED))
+ info.disk.state |= (1 << MD_DISK_CLUSTER_ADD);
+
err = add_disk(mdfd, st, sra, &info);
if (err < 0 && errno == EBUSY) {
/* could be another device present with the same
--
2.1.4
^ permalink raw reply related
* [mdadm PATCH] Grow raid disks without backup file fail
From: Xiao Ni @ 2015-12-01 7:42 UTC (permalink / raw)
To: linux-raid
Hi Neil
When add one new disk to a raid5 and try to grow the number members,
it's failed to start reshape.
Because before echo "reshape" to sync_action, raid5d will miss the
chance to clear MD_RECOVERY_NEEDED. The method is as same as
start_reshape.
Signed-off-by : Xiao Ni (xni@redhat.com)
---
Grow.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/Grow.c b/Grow.c
index 80d7b22..a5bed3c 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2796,6 +2796,7 @@ static int reshape_array(char *container, int fd, char *devname,
int done;
struct mdinfo *sra = NULL;
char buf[20];
+ int cnt = 5;
/* when reshaping a RAID0, the component_size might be zero.
* So try to fix that up.
@@ -3114,7 +3115,12 @@ static int reshape_array(char *container, int fd, char *devname,
if (impose_reshape(sra, info, st, fd, restart,
devname, container, &reshape) < 0)
goto release;
- if (sysfs_set_str(sra, NULL, "sync_action", "reshape") < 0) {
+ do {
+ err = sysfs_set_str(sra, NULL, "sync_action", "reshape");
+ if (err)
+ sleep(1);
+ } while (err && errno == EBUSY && cnt-- > 0);
+ if (err < 0) {
struct mdinfo *sd;
if (errno != EINVAL) {
pr_err("Failed to initiate reshape!\n");
--
2.4.3
^ permalink raw reply related
* Re: [PATCH V3 2/2] md-cluster: Protect communication with mutexes
From: Guoqing Jiang @ 2015-11-30 15:09 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, rgoldwyn
In-Reply-To: <87mvtw5mm6.fsf@notabene.neil.brown.name>
Hi Neil,
On 11/30/2015 08:58 AM, NeilBrown wrote:
> On Mon, Nov 30 2015, Guoqing Jiang wrote:
>
>> +#define MD_CLUSTER_SEND_LOCK 4
>> +/* If cluster operations must lock the communication channel,
>> + * so as to perform extra operations (and no other operation
>> + * is allowed on the MD, such as adding a disk. Token needs
>> + * to be locked and held until the operation completes with
>> + * a md_update_sb(), which would eventually release the lock.
>> + */
> This comment has unbalanced parentheses. How did it even compile :-)
Oops, thanks for catch that, how careless I am.
> But there is something else that isn't as clear as it could be....
>
>>
>> @@ -970,14 +1019,18 @@ static int add_new_disk(struct mddev *mddev, struct md_rdev *rdev)
>> ret = -ENOENT;
>> if (ret)
>> unlock_comm(cinfo);
>> - else
>> + else {
>> dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_CR);
>> + set_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
>> + wake_up(&cinfo->wait);
>> + }
> This code suggests that something has caused a metadata update to be
> triggered. i.e. something set MD_CHANGE_DEVS or similar.
> That is presumably add_bound_rdev() - which hasn't yet been called, but
> while soon after in add_new_disk().
>
> This feels a little bit fragile. The new code in md-cluster is only
> correct if code in add_new_disk does a particular thing. I guess I'd at
> least like to see a comment in add_new_disk() in md-cluster explaining
> what will cause MD_CLUSTE_SEND_LOCKED_ALREADY to eventually be cleared.
> Maybe it could also schedule the MD_CHANGE_DEVS to be completely certain
> that will happen, but that probably isn't necessary.
>
> So I've applied these for now, but if you could fix one comment and add
> another that would help.
What about the following changes? Thanks a lot.
[PATCH] md-cluster: comments update for MD_CLUSTER_SEND_LOCKED_ALREADY
1. fix unbalanced parentheses.
2. add more description about that MD_CLUSTER_SEND_LOCKED_ALREADY
will be cleared after set it in add_new_disk.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
drivers/md/md-cluster.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index ad3ec7d..68b4866 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -53,9 +53,9 @@ struct resync_info {
* accomodate lock and hold. See next comment.
*/
#define MD_CLUSTER_SEND_LOCK 4
-/* If cluster operations must lock the communication channel,
- * so as to perform extra operations (and no other operation
- * is allowed on the MD, such as adding a disk. Token needs
+/* If cluster operations (such as adding a disk) must lock
+ * the communication channel, so as to perform extra operations
+ * and no other operation is allowed on the MD. Token needs
* to be locked and held until the operation completes with
* a md_update_sb(), which would eventually release the lock.
*/
@@ -1021,6 +1021,16 @@ static int add_new_disk(struct mddev *mddev,
struct md_rdev *rdev)
unlock_comm(cinfo);
else {
dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_CR);
+ /* Since MD_CHANGE_DEVS will be set in add_bound_rdev which
+ * will run soon after add_new_disk, the path will be
invoked:
+ * md_wakeup_thread(mddev->thread) -> conf->thread
(raid1d)
+ * -> md_check_recovery -> md_update_sb
+ * -> metadata_update_start/finish
+ * MD_CLUSTER_SEND_LOCKED_ALREADY will be cleared
eventually.
+ *
+ * For other failure cases, metadata_update_cancel and
+ * add_new_disk_cancel also clear below bit as well.
+ */
set_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
wake_up(&cinfo->wait);
}
--
2.1.4
Guoqing
^ permalink raw reply related
* Re: [PATCH 2/3] md-cluster: introduce clear_clusterinfo_from_sb
From: Guoqing Jiang @ 2015-11-30 1:45 UTC (permalink / raw)
To: NeilBrown; +Cc: rgoldwyn, linux-raid
In-Reply-To: <87h9k45m6n.fsf@notabene.neil.brown.name>
NeilBrown wrote:
> On Fri, Nov 27 2015, Guoqing Jiang wrote:
>
>
>> Hi Neil,
>>
>> On 11/27/2015 01:42 PM, NeilBrown wrote:
>>
>>> On Fri, Nov 20 2015, Guoqing Jiang <gqjiang@suse.com> wrote:
>>>
>>>
>>>> To change the bitmap from clustered to none, we also
>>>> need to clean related info about cluster from sb, such
>>>> as version, nodes and cluster_name.
>>>>
>>> I've applied the other two (though I removed the __func__ and __LINE__
>>> From error messages - they are just noise: the source of the message can
>>> easily be found with out them - I should remove all the others).
>>>
>>> However I haven't applied this because there doesn't seem to be any
>>> point.
>>> Once your told that main md superblock that there is no bitmap, it
>>> doesn't really matter what data is in the superblock of that
>>> non-existent bitmap - it will never be looked at. Will it?
>>>
>> mdadm could still get nodes and cluster_name after change
>> bitmap to none, so some cmds like "mdadm -X /dev/sda" could
>> still see clustered info, it seems a little confused for user.
>>
>
> In that case we should fix mdadm to not report a bitmap if
> MD_FEATURE_BITMAP_OFFSET isn't set (though maybe still report it if
> --force is given).
>
> i.e. if this is an issue, it is not specific to clustered-bitmaps.
>
>
Thanks for the comment, I will modify mdadm for the case.
Regards,
Guoqing
^ permalink raw reply
* Re: [PATCH 2/3] md-cluster: introduce clear_clusterinfo_from_sb
From: NeilBrown @ 2015-11-30 1:08 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: rgoldwyn, linux-raid
In-Reply-To: <5657F227.3030202@suse.com>
[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]
On Fri, Nov 27 2015, Guoqing Jiang wrote:
> Hi Neil,
>
> On 11/27/2015 01:42 PM, NeilBrown wrote:
>> On Fri, Nov 20 2015, Guoqing Jiang <gqjiang@suse.com> wrote:
>>
>>> To change the bitmap from clustered to none, we also
>>> need to clean related info about cluster from sb, such
>>> as version, nodes and cluster_name.
>> I've applied the other two (though I removed the __func__ and __LINE__
>> From error messages - they are just noise: the source of the message can
>> easily be found with out them - I should remove all the others).
>>
>> However I haven't applied this because there doesn't seem to be any
>> point.
>> Once your told that main md superblock that there is no bitmap, it
>> doesn't really matter what data is in the superblock of that
>> non-existent bitmap - it will never be looked at. Will it?
> mdadm could still get nodes and cluster_name after change
> bitmap to none, so some cmds like "mdadm -X /dev/sda" could
> still see clustered info, it seems a little confused for user.
In that case we should fix mdadm to not report a bitmap if
MD_FEATURE_BITMAP_OFFSET isn't set (though maybe still report it if
--force is given).
i.e. if this is an issue, it is not specific to clustered-bitmaps.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH V3 2/2] md-cluster: Protect communication with mutexes
From: NeilBrown @ 2015-11-30 0:58 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1448818989-8280-1-git-send-email-gqjiang@suse.com>
[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]
On Mon, Nov 30 2015, Guoqing Jiang wrote:
> +#define MD_CLUSTER_SEND_LOCK 4
> +/* If cluster operations must lock the communication channel,
> + * so as to perform extra operations (and no other operation
> + * is allowed on the MD, such as adding a disk. Token needs
> + * to be locked and held until the operation completes with
> + * a md_update_sb(), which would eventually release the lock.
> + */
This comment has unbalanced parentheses. How did it even compile :-)
But there is something else that isn't as clear as it could be....
>
> @@ -970,14 +1019,18 @@ static int add_new_disk(struct mddev *mddev, struct md_rdev *rdev)
> ret = -ENOENT;
> if (ret)
> unlock_comm(cinfo);
> - else
> + else {
> dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_CR);
> + set_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
> + wake_up(&cinfo->wait);
> + }
This code suggests that something has caused a metadata update to be
triggered. i.e. something set MD_CHANGE_DEVS or similar.
That is presumably add_bound_rdev() - which hasn't yet been called, but
while soon after in add_new_disk().
This feels a little bit fragile. The new code in md-cluster is only
correct if code in add_new_disk does a particular thing. I guess I'd at
least like to see a comment in add_new_disk() in md-cluster explaining
what will cause MD_CLUSTE_SEND_LOCKED_ALREADY to eventually be cleared.
Maybe it could also schedule the MD_CHANGE_DEVS to be completely certain
that will happen, but that probably isn't necessary.
So I've applied these for now, but if you could fix one comment and add
another that would help.
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* [PATCH V3 2/2] md-cluster: Protect communication with mutexes
From: Guoqing Jiang @ 2015-11-29 17:43 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1448787432-4138-2-git-send-email-gqjiang@suse.com>
Communication can happen through multiple threads. It is possible that
one thread steps over another threads sequence. So, we use mutexes to
protect both the send and receive sequences.
Send communication is locked through state bit, MD_CLUSTER_SEND_LOCK.
Communication is locked with bit manipulation in order to allow
"lock and hold" for the add operation. In case of an add operation,
if the lock is held, MD_CLUSTER_SEND_LOCKED_ALREADY is set.
When md_update_sb() calls metadata_update_start(), it checks
(in a single statement to avoid races), if the communication
is already locked. If yes, it merely returns zero, else it
locks the token lockresource.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
v3 change: add missed mutex_unlock in recv_daemon
v2 changes:
1. replace send_mutex with wait_queue.
2. change MD_CLUSTER_COMM_LOCKED to MD_CLUSTER_SEND_LOCKED_ALREADY,
and set it later with add_new_disk.
3. add a new lock_token and change lock_comm to make it better
suit for the specific cases of locking communication.
drivers/md/md-cluster.c | 73 ++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 63 insertions(+), 10 deletions(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index b659ef7..ad3ec7d 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -48,12 +48,26 @@ struct resync_info {
#define MD_CLUSTER_SUSPEND_READ_BALANCING 2
#define MD_CLUSTER_BEGIN_JOIN_CLUSTER 3
+/* Lock the send communication. This is done through
+ * bit manipulation as opposed to a mutex in order to
+ * accomodate lock and hold. See next comment.
+ */
+#define MD_CLUSTER_SEND_LOCK 4
+/* If cluster operations must lock the communication channel,
+ * so as to perform extra operations (and no other operation
+ * is allowed on the MD, such as adding a disk. Token needs
+ * to be locked and held until the operation completes with
+ * a md_update_sb(), which would eventually release the lock.
+ */
+#define MD_CLUSTER_SEND_LOCKED_ALREADY 5
+
struct md_cluster_info {
/* dlm lock space and resources for clustered raid. */
dlm_lockspace_t *lockspace;
int slot_number;
struct completion completion;
+ struct mutex recv_mutex;
struct dlm_lock_resource *bitmap_lockres;
struct dlm_lock_resource **other_bitmap_lockres;
struct dlm_lock_resource *resync_lockres;
@@ -68,6 +82,7 @@ struct md_cluster_info {
struct dlm_lock_resource *no_new_dev_lockres;
struct md_thread *recv_thread;
struct completion newdisk_completion;
+ wait_queue_head_t wait;
unsigned long state;
};
@@ -508,9 +523,11 @@ static void recv_daemon(struct md_thread *thread)
struct cluster_msg msg;
int ret;
+ mutex_lock(&cinfo->recv_mutex);
/*get CR on Message*/
if (dlm_lock_sync(message_lockres, DLM_LOCK_CR)) {
pr_err("md/raid1:failed to get CR on MESSAGE\n");
+ mutex_unlock(&cinfo->recv_mutex);
return;
}
@@ -534,33 +551,45 @@ static void recv_daemon(struct md_thread *thread)
ret = dlm_unlock_sync(message_lockres);
if (unlikely(ret != 0))
pr_info("unlock msg failed return %d\n", ret);
+ mutex_unlock(&cinfo->recv_mutex);
}
-/* lock_comm()
+/* lock_token()
* Takes the lock on the TOKEN lock resource so no other
* node can communicate while the operation is underway.
- * If called again, and the TOKEN lock is alread in EX mode
- * return success. However, care must be taken that unlock_comm()
- * is called only once.
*/
-static int lock_comm(struct md_cluster_info *cinfo)
+static int lock_token(struct md_cluster_info *cinfo)
{
int error;
- if (cinfo->token_lockres->mode == DLM_LOCK_EX)
- return 0;
-
error = dlm_lock_sync(cinfo->token_lockres, DLM_LOCK_EX);
if (error)
pr_err("md-cluster(%s:%d): failed to get EX on TOKEN (%d)\n",
__func__, __LINE__, error);
+
+ /* Lock the receive sequence */
+ mutex_lock(&cinfo->recv_mutex);
return error;
}
+/* lock_comm()
+ * Sets the MD_CLUSTER_SEND_LOCK bit to lock the send channel.
+ */
+static int lock_comm(struct md_cluster_info *cinfo)
+{
+ wait_event(cinfo->wait,
+ !test_and_set_bit(MD_CLUSTER_SEND_LOCK, &cinfo->state));
+
+ return lock_token(cinfo);
+}
+
static void unlock_comm(struct md_cluster_info *cinfo)
{
WARN_ON(cinfo->token_lockres->mode != DLM_LOCK_EX);
+ mutex_unlock(&cinfo->recv_mutex);
dlm_unlock_sync(cinfo->token_lockres);
+ clear_bit(MD_CLUSTER_SEND_LOCK, &cinfo->state);
+ wake_up(&cinfo->wait);
}
/* __sendmsg()
@@ -713,6 +742,8 @@ static int join(struct mddev *mddev, int nodes)
spin_lock_init(&cinfo->suspend_lock);
init_completion(&cinfo->completion);
set_bit(MD_CLUSTER_BEGIN_JOIN_CLUSTER, &cinfo->state);
+ init_waitqueue_head(&cinfo->wait);
+ mutex_init(&cinfo->recv_mutex);
mddev->cluster_info = cinfo;
@@ -843,9 +874,25 @@ static int slot_number(struct mddev *mddev)
return cinfo->slot_number - 1;
}
+/*
+ * Check if the communication is already locked, else lock the communication
+ * channel.
+ * If it is already locked, token is in EX mode, and hence lock_token()
+ * should not be called.
+ */
static int metadata_update_start(struct mddev *mddev)
{
- return lock_comm(mddev->cluster_info);
+ struct md_cluster_info *cinfo = mddev->cluster_info;
+
+ wait_event(cinfo->wait,
+ !test_and_set_bit(MD_CLUSTER_SEND_LOCK, &cinfo->state) ||
+ test_and_clear_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state));
+
+ /* If token is already locked, return 0 */
+ if (cinfo->token_lockres->mode == DLM_LOCK_EX)
+ return 0;
+
+ return lock_token(cinfo);
}
static int metadata_update_finish(struct mddev *mddev)
@@ -870,6 +917,7 @@ static int metadata_update_finish(struct mddev *mddev)
ret = __sendmsg(cinfo, &cmsg);
} else
pr_warn("md-cluster: No good device id found to send\n");
+ clear_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
unlock_comm(cinfo);
return ret;
}
@@ -877,6 +925,7 @@ static int metadata_update_finish(struct mddev *mddev)
static void metadata_update_cancel(struct mddev *mddev)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
+ clear_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
unlock_comm(cinfo);
}
@@ -970,14 +1019,18 @@ static int add_new_disk(struct mddev *mddev, struct md_rdev *rdev)
ret = -ENOENT;
if (ret)
unlock_comm(cinfo);
- else
+ else {
dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_CR);
+ set_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
+ wake_up(&cinfo->wait);
+ }
return ret;
}
static void add_new_disk_cancel(struct mddev *mddev)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
+ clear_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
unlock_comm(cinfo);
}
--
2.1.4
^ permalink raw reply related
* [PATCH] dm: constify crypt_iv_operations structures
From: Julia Lawall @ 2015-11-29 13:09 UTC (permalink / raw)
To: Alasdair Kergon
Cc: kernel-janitors, Mike Snitzer, dm-devel, Neil Brown, linux-raid,
linux-kernel
The crypt_iv_operations are never modified, so declare them
as const.
Done with the help of Coccinelle.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/md/dm-crypt.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 3147c8d..c7058fc 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -141,7 +141,7 @@ struct crypt_config {
char *cipher;
char *cipher_string;
- struct crypt_iv_operations *iv_gen_ops;
+ const struct crypt_iv_operations *iv_gen_ops;
union {
struct iv_essiv_private essiv;
struct iv_benbi_private benbi;
@@ -756,15 +756,15 @@ static int crypt_iv_tcw_post(struct crypt_config *cc, u8 *iv,
return r;
}
-static struct crypt_iv_operations crypt_iv_plain_ops = {
+static const struct crypt_iv_operations crypt_iv_plain_ops = {
.generator = crypt_iv_plain_gen
};
-static struct crypt_iv_operations crypt_iv_plain64_ops = {
+static const struct crypt_iv_operations crypt_iv_plain64_ops = {
.generator = crypt_iv_plain64_gen
};
-static struct crypt_iv_operations crypt_iv_essiv_ops = {
+static const struct crypt_iv_operations crypt_iv_essiv_ops = {
.ctr = crypt_iv_essiv_ctr,
.dtr = crypt_iv_essiv_dtr,
.init = crypt_iv_essiv_init,
@@ -772,17 +772,17 @@ static struct crypt_iv_operations crypt_iv_essiv_ops = {
.generator = crypt_iv_essiv_gen
};
-static struct crypt_iv_operations crypt_iv_benbi_ops = {
+static const struct crypt_iv_operations crypt_iv_benbi_ops = {
.ctr = crypt_iv_benbi_ctr,
.dtr = crypt_iv_benbi_dtr,
.generator = crypt_iv_benbi_gen
};
-static struct crypt_iv_operations crypt_iv_null_ops = {
+static const struct crypt_iv_operations crypt_iv_null_ops = {
.generator = crypt_iv_null_gen
};
-static struct crypt_iv_operations crypt_iv_lmk_ops = {
+static const struct crypt_iv_operations crypt_iv_lmk_ops = {
.ctr = crypt_iv_lmk_ctr,
.dtr = crypt_iv_lmk_dtr,
.init = crypt_iv_lmk_init,
@@ -791,7 +791,7 @@ static struct crypt_iv_operations crypt_iv_lmk_ops = {
.post = crypt_iv_lmk_post
};
-static struct crypt_iv_operations crypt_iv_tcw_ops = {
+static const struct crypt_iv_operations crypt_iv_tcw_ops = {
.ctr = crypt_iv_tcw_ctr,
.dtr = crypt_iv_tcw_dtr,
.init = crypt_iv_tcw_init,
^ permalink raw reply related
* [PATCH Resend V2 2/2] md-cluster: Protect communication with mutexes
From: Guoqing Jiang @ 2015-11-29 8:57 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1448787432-4138-1-git-send-email-gqjiang@suse.com>
Communication can happen through multiple threads. It is possible that
one thread steps over another threads sequence. So, we use mutexes to
protect both the send and receive sequences.
Send communication is locked through state bit, MD_CLUSTER_SEND_LOCK.
Communication is locked with bit manipulation in order to allow
"lock and hold" for the add operation. In case of an add operation,
if the lock is held, MD_CLUSTER_SEND_LOCKED_ALREADY is set.
When md_update_sb() calls metadata_update_start(), it checks
(in a single statement to avoid races), if the communication
is already locked. If yes, it merely returns zero, else it
locks the token lockresource.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
v2 changes:
1. replace send_mutex with wait_queue.
2. change MD_CLUSTER_COMM_LOCKED to MD_CLUSTER_SEND_LOCKED_ALREADY,
and set it later with add_new_disk.
3. add a new lock_token and change lock_comm to make it better
suit for the specific cases of locking communication.
drivers/md/md-cluster.c | 72 ++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 62 insertions(+), 10 deletions(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index b659ef7..c37132a 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -48,12 +48,26 @@ struct resync_info {
#define MD_CLUSTER_SUSPEND_READ_BALANCING 2
#define MD_CLUSTER_BEGIN_JOIN_CLUSTER 3
+/* Lock the send communication. This is done through
+ * bit manipulation as opposed to a mutex in order to
+ * accomodate lock and hold. See next comment.
+ */
+#define MD_CLUSTER_SEND_LOCK 4
+/* If cluster operations must lock the communication channel,
+ * so as to perform extra operations (and no other operation
+ * is allowed on the MD, such as adding a disk. Token needs
+ * to be locked and held until the operation completes with
+ * a md_update_sb(), which would eventually release the lock.
+ */
+#define MD_CLUSTER_SEND_LOCKED_ALREADY 5
+
struct md_cluster_info {
/* dlm lock space and resources for clustered raid. */
dlm_lockspace_t *lockspace;
int slot_number;
struct completion completion;
+ struct mutex recv_mutex;
struct dlm_lock_resource *bitmap_lockres;
struct dlm_lock_resource **other_bitmap_lockres;
struct dlm_lock_resource *resync_lockres;
@@ -68,6 +82,7 @@ struct md_cluster_info {
struct dlm_lock_resource *no_new_dev_lockres;
struct md_thread *recv_thread;
struct completion newdisk_completion;
+ wait_queue_head_t wait;
unsigned long state;
};
@@ -508,6 +523,7 @@ static void recv_daemon(struct md_thread *thread)
struct cluster_msg msg;
int ret;
+ mutex_lock(&cinfo->recv_mutex);
/*get CR on Message*/
if (dlm_lock_sync(message_lockres, DLM_LOCK_CR)) {
pr_err("md/raid1:failed to get CR on MESSAGE\n");
@@ -534,33 +550,45 @@ static void recv_daemon(struct md_thread *thread)
ret = dlm_unlock_sync(message_lockres);
if (unlikely(ret != 0))
pr_info("unlock msg failed return %d\n", ret);
+ mutex_unlock(&cinfo->recv_mutex);
}
-/* lock_comm()
+/* lock_token()
* Takes the lock on the TOKEN lock resource so no other
* node can communicate while the operation is underway.
- * If called again, and the TOKEN lock is alread in EX mode
- * return success. However, care must be taken that unlock_comm()
- * is called only once.
*/
-static int lock_comm(struct md_cluster_info *cinfo)
+static int lock_token(struct md_cluster_info *cinfo)
{
int error;
- if (cinfo->token_lockres->mode == DLM_LOCK_EX)
- return 0;
-
error = dlm_lock_sync(cinfo->token_lockres, DLM_LOCK_EX);
if (error)
pr_err("md-cluster(%s:%d): failed to get EX on TOKEN (%d)\n",
__func__, __LINE__, error);
+
+ /* Lock the receive sequence */
+ mutex_lock(&cinfo->recv_mutex);
return error;
}
+/* lock_comm()
+ * Sets the MD_CLUSTER_SEND_LOCK bit to lock the send channel.
+ */
+static int lock_comm(struct md_cluster_info *cinfo)
+{
+ wait_event(cinfo->wait,
+ !test_and_set_bit(MD_CLUSTER_SEND_LOCK, &cinfo->state));
+
+ return lock_token(cinfo);
+}
+
static void unlock_comm(struct md_cluster_info *cinfo)
{
WARN_ON(cinfo->token_lockres->mode != DLM_LOCK_EX);
+ mutex_unlock(&cinfo->recv_mutex);
dlm_unlock_sync(cinfo->token_lockres);
+ clear_bit(MD_CLUSTER_SEND_LOCK, &cinfo->state);
+ wake_up(&cinfo->wait);
}
/* __sendmsg()
@@ -713,6 +741,8 @@ static int join(struct mddev *mddev, int nodes)
spin_lock_init(&cinfo->suspend_lock);
init_completion(&cinfo->completion);
set_bit(MD_CLUSTER_BEGIN_JOIN_CLUSTER, &cinfo->state);
+ init_waitqueue_head(&cinfo->wait);
+ mutex_init(&cinfo->recv_mutex);
mddev->cluster_info = cinfo;
@@ -843,9 +873,25 @@ static int slot_number(struct mddev *mddev)
return cinfo->slot_number - 1;
}
+/*
+ * Check if the communication is already locked, else lock the communication
+ * channel.
+ * If it is already locked, token is in EX mode, and hence lock_token()
+ * should not be called.
+ */
static int metadata_update_start(struct mddev *mddev)
{
- return lock_comm(mddev->cluster_info);
+ struct md_cluster_info *cinfo = mddev->cluster_info;
+
+ wait_event(cinfo->wait,
+ !test_and_set_bit(MD_CLUSTER_SEND_LOCK, &cinfo->state) ||
+ test_and_clear_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state));
+
+ /* If token is already locked, return 0 */
+ if (cinfo->token_lockres->mode == DLM_LOCK_EX)
+ return 0;
+
+ return lock_token(cinfo);
}
static int metadata_update_finish(struct mddev *mddev)
@@ -870,6 +916,7 @@ static int metadata_update_finish(struct mddev *mddev)
ret = __sendmsg(cinfo, &cmsg);
} else
pr_warn("md-cluster: No good device id found to send\n");
+ clear_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
unlock_comm(cinfo);
return ret;
}
@@ -877,6 +924,7 @@ static int metadata_update_finish(struct mddev *mddev)
static void metadata_update_cancel(struct mddev *mddev)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
+ clear_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
unlock_comm(cinfo);
}
@@ -970,14 +1018,18 @@ static int add_new_disk(struct mddev *mddev, struct md_rdev *rdev)
ret = -ENOENT;
if (ret)
unlock_comm(cinfo);
- else
+ else {
dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_CR);
+ set_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
+ wake_up(&cinfo->wait);
+ }
return ret;
}
static void add_new_disk_cancel(struct mddev *mddev)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
+ clear_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
unlock_comm(cinfo);
}
--
2.1.4
^ permalink raw reply related
* [PATCH Resend V2 1/2] md-cluster: Defer MD reloading to mddev->thread
From: Guoqing Jiang @ 2015-11-29 8:57 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, rgoldwyn
Reloading of superblock must be performed under reconfig_mutex. However,
this cannot be done with md_reload_sb because it would deadlock with
the message DLM lock. So, we defer it in md_check_recovery() which is
executed by mddev->thread.
This introduces a new flag, MD_RELOAD_SB, which if set, will reload the
superblock. And good_device_nr is also added to 'struct mddev' which is
used to get the num of the good device within cluster raid.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
v2 changes: remove atomic and add good_device_nr to struct mddev
drivers/md/md-cluster.c | 4 +++-
drivers/md/md.c | 4 ++++
drivers/md/md.h | 4 ++++
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index db9375f..b659ef7 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -432,8 +432,10 @@ static void process_add_new_disk(struct mddev *mddev, struct cluster_msg *cmsg)
static void process_metadata_update(struct mddev *mddev, struct cluster_msg *msg)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
- md_reload_sb(mddev, le32_to_cpu(msg->raid_slot));
+ mddev->good_device_nr = le32_to_cpu(msg->raid_slot);
+ set_bit(MD_RELOAD_SB, &mddev->flags);
dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_CR);
+ md_wakeup_thread(mddev->thread);
}
static void process_remove_disk(struct mddev *mddev, struct cluster_msg *msg)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 642c87c..5ee18a0 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8273,6 +8273,7 @@ void md_check_recovery(struct mddev *mddev)
(mddev->flags & MD_UPDATE_SB_FLAGS & ~ (1<<MD_CHANGE_PENDING)) ||
test_bit(MD_RECOVERY_NEEDED, &mddev->recovery) ||
test_bit(MD_RECOVERY_DONE, &mddev->recovery) ||
+ test_bit(MD_RELOAD_SB, &mddev->flags) ||
(mddev->external == 0 && mddev->safemode == 1) ||
(mddev->safemode == 2 && ! atomic_read(&mddev->writes_pending)
&& !mddev->in_sync && mddev->recovery_cp == MaxSector)
@@ -8321,6 +8322,9 @@ void md_check_recovery(struct mddev *mddev)
rdev->raid_disk < 0)
md_kick_rdev_from_array(rdev);
}
+
+ if (test_and_clear_bit(MD_RELOAD_SB, &mddev->flags))
+ md_reload_sb(mddev, mddev->good_device_nr);
}
if (!mddev->external) {
diff --git a/drivers/md/md.h b/drivers/md/md.h
index f5b9aad..7a6da5c 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -235,6 +235,9 @@ struct mddev {
*/
#define MD_JOURNAL_CLEAN 5 /* A raid with journal is already clean */
#define MD_HAS_JOURNAL 6 /* The raid array has journal feature set */
+#define MD_RELOAD_SB 7 /* Reload the superblock because another node
+ * updated it.
+ */
int suspended;
atomic_t active_io;
@@ -465,6 +468,7 @@ struct mddev {
struct work_struct event_work; /* used by dm to report failure event */
void (*sync_super)(struct mddev *mddev, struct md_rdev *rdev);
struct md_cluster_info *cluster_info;
+ unsigned int good_device_nr; /* good device num within cluster raid */
};
static inline int __must_check mddev_lock(struct mddev *mddev)
--
2.1.4
^ permalink raw reply related
* Re: [PATCH Resend V2 2/2] md-cluster: Protect communication with mutexes (fwd)
From: Julia Lawall @ 2015-11-29 6:39 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: neilb, linux-raid, rgoldwyn, kbuild-all
Looks suspicious. Please check.
julia
---------- Forwarded message ----------
Date: Sun, 29 Nov 2015 09:17:12 +0800
From: kbuild test robot <fengguang.wu@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH Resend V2 2/2] md-cluster: Protect communication with
mutexes
CC: kbuild-all@01.org
In-Reply-To: <1448787432-4138-2-git-send-email-gqjiang@suse.com>
TO: Guoqing Jiang <gqjiang@suse.com>
CC: neilb@suse.com
CC: linux-raid@vger.kernel.org, rgoldwyn@suse.com
Hi Guoqing,
[auto build test WARNING on: md/for-next]
[also build test WARNING on: next-20151127]
[cannot apply to: v4.4-rc2]
url: https://github.com/0day-ci/linux/commits/Guoqing-Jiang/md-cluster-Defer-MD-reloading-to-mddev-thread/20151129-085944
base: git://neil.brown.name/md for-next
:::::: branch date: 17 minutes ago
:::::: commit date: 17 minutes ago
>> drivers/md/md-cluster.c:530:2-8: preceding lock on line 526
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 7cd1e0cb4a84f8dd6ab7bfdb4a2e1d4eda66ad6d
vim +530 drivers/md/md-cluster.c
4664680c Goldwyn Rodrigues 2014-06-07 520 struct md_cluster_info *cinfo = thread->mddev->cluster_info;
4664680c Goldwyn Rodrigues 2014-06-07 521 struct dlm_lock_resource *ack_lockres = cinfo->ack_lockres;
4664680c Goldwyn Rodrigues 2014-06-07 522 struct dlm_lock_resource *message_lockres = cinfo->message_lockres;
4664680c Goldwyn Rodrigues 2014-06-07 523 struct cluster_msg msg;
b5ef5678 Guoqing Jiang 2015-07-10 524 int ret;
4664680c Goldwyn Rodrigues 2014-06-07 525
7cd1e0cb Guoqing Jiang 2015-11-29 @526 mutex_lock(&cinfo->recv_mutex);
4664680c Goldwyn Rodrigues 2014-06-07 527 /*get CR on Message*/
4664680c Goldwyn Rodrigues 2014-06-07 528 if (dlm_lock_sync(message_lockres, DLM_LOCK_CR)) {
4664680c Goldwyn Rodrigues 2014-06-07 529 pr_err("md/raid1:failed to get CR on MESSAGE\n");
4664680c Goldwyn Rodrigues 2014-06-07 @530 return;
4664680c Goldwyn Rodrigues 2014-06-07 531 }
4664680c Goldwyn Rodrigues 2014-06-07 532
4664680c Goldwyn Rodrigues 2014-06-07 533 /* read lvb and wake up thread to process this message_lockres */
:::::: The code at line 530 was first introduced by commit
:::::: 4664680c389828928efc61ce3d2cf2c65ad35c97 Communication Framework: Receiving
:::::: TO: Goldwyn Rodrigues <rgoldwyn@suse.com>
:::::: CC: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [dm-devel] [PATCH] dm-ioctl: fix 4-characters indentations
From: Minfei Huang @ 2015-11-28 8:38 UTC (permalink / raw)
To: device-mapper development
Cc: Alasdair Kergon, Mike Snitzer, Neil Brown, linux-raid,
Geliang Tang, linux-kernel
In-Reply-To: <1f27d82f27c62841bcb9858f90ad27fd0dce78e3.1448455317.git.geliangtang@163.com>
Hi.
IMHO, this sort of defect shouldn't be fixed like this. It should be
fixed with other bugs, if necessary.
Once this patch is applied, it will break up the 'git blame'.
Thanks
Minfei
On 11/25/15 at 08:45pm, Geliang Tang wrote:
> Change 4-characters indentations to 8-characters.
>
> Signed-off-by: Geliang Tang <geliangtang@163.com>
> ---
> drivers/md/dm-ioctl.c | 58 +++++++++++++++++++++++++--------------------------
> 1 file changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index 80a4395..39aae6b 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -45,10 +45,10 @@ struct dm_table {
> };
>
> struct vers_iter {
> - size_t param_size;
> - struct dm_target_versions *vers, *old_vers;
> - char *end;
> - uint32_t flags;
> + size_t param_size;
> + struct dm_target_versions *vers, *old_vers;
> + char *end;
> + uint32_t flags;
> };
>
>
> @@ -557,36 +557,36 @@ static int list_devices(struct dm_ioctl *param, size_t param_size)
>
> static void list_version_get_needed(struct target_type *tt, void *needed_param)
> {
> - size_t *needed = needed_param;
> + size_t *needed = needed_param;
>
> - *needed += sizeof(struct dm_target_versions);
> - *needed += strlen(tt->name);
> - *needed += ALIGN_MASK;
> + *needed += sizeof(struct dm_target_versions);
> + *needed += strlen(tt->name);
> + *needed += ALIGN_MASK;
> }
>
> static void list_version_get_info(struct target_type *tt, void *param)
> {
> - struct vers_iter *info = param;
> -
> - /* Check space - it might have changed since the first iteration */
> - if ((char *)info->vers + sizeof(tt->version) + strlen(tt->name) + 1 >
> - info->end) {
> -
> - info->flags = DM_BUFFER_FULL_FLAG;
> - return;
> - }
> -
> - if (info->old_vers)
> - info->old_vers->next = (uint32_t) ((void *)info->vers -
> - (void *)info->old_vers);
> - info->vers->version[0] = tt->version[0];
> - info->vers->version[1] = tt->version[1];
> - info->vers->version[2] = tt->version[2];
> - info->vers->next = 0;
> - strcpy(info->vers->name, tt->name);
> -
> - info->old_vers = info->vers;
> - info->vers = align_ptr(((void *) ++info->vers) + strlen(tt->name) + 1);
> + struct vers_iter *info = param;
> +
> + /* Check space - it might have changed since the first iteration */
> + if ((char *)info->vers + sizeof(tt->version) + strlen(tt->name) + 1 >
> + info->end) {
> +
> + info->flags = DM_BUFFER_FULL_FLAG;
> + return;
> + }
> +
> + if (info->old_vers)
> + info->old_vers->next = (uint32_t) ((void *)info->vers -
> + (void *)info->old_vers);
> + info->vers->version[0] = tt->version[0];
> + info->vers->version[1] = tt->version[1];
> + info->vers->version[2] = tt->version[2];
> + info->vers->next = 0;
> + strcpy(info->vers->name, tt->name);
> +
> + info->old_vers = info->vers;
> + info->vers = align_ptr(((void *) ++info->vers) + strlen(tt->name) + 1);
> }
>
> static int list_versions(struct dm_ioctl *param, size_t param_size)
> --
> 2.5.0
>
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply
* Reply Me For Details
From: HS09912HH3 @ 2015-11-28 4:13 UTC (permalink / raw)
Good day,
I wish to contact you personally for an important proposal that might be
of interest to you. I am sending this mail just to know if this email address
is functional.
I have something absolutely essential to discuss with you. Contact me for
details through my private email: shu60w@qq.com
Regards,
shu60w@qq.com
^ permalink raw reply
* [md:for-next 7/8] drivers/md/md.c:6594:6: error: expected ';' before 'rv'
From: kbuild test robot @ 2015-11-27 6:19 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: kbuild-all, linux-raid, NeilBrown
[-- Attachment #1: Type: text/plain, Size: 1196 bytes --]
tree: git://neil.brown.name/md for-next
head: e4b9fcde6f296ce022bde147a4de29bd8be541dc
commit: 0a50cdf5058bd85f236e57aa2d00373751254dd9 [7/8] md-cluster: append some actions when change bitmap from clustered to none
config: x86_64-lkp (attached as .config)
reproduce:
git checkout 0a50cdf5058bd85f236e57aa2d00373751254dd9
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/md/md.c: In function 'update_array_info':
>> drivers/md/md.c:6594:6: error: expected ';' before 'rv'
rv = -EPERM;
^
vim +6594 drivers/md/md.c
6588 }
6589 if (mddev->bitmap_info.nodes) {
6590 /* hold PW on all the bitmap lock */
6591 if (md_cluster_ops->lock_all_bitmaps(mddev) <= 0) {
6592 printk("md: can't change bitmap to none since the"
6593 " array is in use by more than one node\n")
> 6594 rv = -EPERM;
6595 md_cluster_ops->unlock_all_bitmaps(mddev);
6596 goto err;
6597 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 22071 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox