* Re: Fault tolerance in RAID0 with badblocks
From: Wols Lists @ 2017-05-04 13:44 UTC (permalink / raw)
To: Ravi (Tom) Hale, linux-raid
In-Reply-To: <03294ec0-2df0-8c1c-dd98-2e9e5efb6f4f@hale.ee>
On 04/05/17 11:04, Ravi (Tom) Hale wrote:
> Since btrfs doesn't support badblocks, this btrfs mailing list post[1]
> suggested to use mdadm RAID0 3.1+.
Having read the email you linked to, I don't think mdadm will be any
help at all ...
>
> Is there a way of having blocks from a spare device automatically
> replacing bad blocks when they are next written to (like SMART does for
> HDDs)?
What quite do you mean?
>
> Or would mdadm be able to add a "badblocks layer" to btrfs in some other
> way?
No. With modern hard drives, no filesystem should pay any attention to
badblocks - it's all handled in the drive firmware. Badblocks is an
unfortunate legacy from the past when drives really were CHS, and the
layer above needed some way of knowing which blocks were bad and should
be avoided. mdadm has had a lot of grief with its handling of badblocks,
and getting drives confused, and it's all totally unnecessary anyway.
Let the drive worry about what blocks are bad. One major point behind
LBA is it hides the actual disk layout from the computer, and allows the
drive to relocate blocks that aren't working properly. Let it do its job.
If you want to use raid, don't bother with 0. Use mdadm and raid 5 or 6
to combine your drives, and create a btrfs filesystem on top. (Don't
bother with raid1 - that part of btrfs apparently works well, so use the
filesystem variant, not an external one.)
>
> My use case is mining storj - I don't mind some data loss.
Using a badblock list will have no impact on this whatsoever.
>
> [1] https://www.spinics.net/lists/linux-btrfs/msg40909.html
>
Cheers,
Wol
^ permalink raw reply
* Re: mdadm/mdmon Name vs argv[0]
From: Pawel Baldysiak @ 2017-05-04 13:40 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
In-Reply-To: <a61470a2-2c3b-3f4a-5712-dfb87f3acad8@gmail.com>
On 05/02/2017 04:19 PM, Jes Sorensen wrote:
> Hi Pawel,
>
> In the below commit you changed the handling of how mdadm/mdmon handles
> printing the name of the running process.
>
> However, if people for some reason decided to change the names of
> mdadm/mdmon this would no longer be correct.
>
> Do you remember if there is any reason why you didn't use argv[0] for this?
>
> commit d56dd607ba433d9334f0fb4114fe081742ae4361
> Author: Pawel Baldysiak <pawel.baldysiak@intel.com>
> Date: Wed Feb 11 22:25:03 2015 +0100
>
> Change way of printing name of a process
>
> Cheers,
> Jes
Hi Jes,
We did not consider this option once we were implementing the change.
Also, I was not considering that someone would like to change the
mdadm/mdmon names :)
I see one potential issue with usage of argv[0]:
If you look into mdmon's code (start of the main()),
you will see that first char is replaced with '@'.
Thanks
Pawel
^ permalink raw reply
* Re: [dm-devel] [PATCH v6 0/3] dm: boot a mapped device without an initramfs
From: Alasdair G Kergon @ 2017-05-04 13:20 UTC (permalink / raw)
To: Enric Balletbo Serra
Cc: Kees Cook, Will Drewry, Guenter Roeck, Mike Snitzer,
linux-doc@vger.kernel.org, David Zeuthen, LKML, linux-raid,
dm-devel, Enric Balletbo i Serra, Shaohua Li, Alasdair Kergon
In-Reply-To: <20170504121452.GF4943@agk-dp.fab.redhat.com>
Some more thoughts with your example, dmsetup might look like:
# dmsetup create --bootformat "lroot:uuid,rw,0 2097152 linear 8:2 0, \
2097152 2097152 linear 8:3 0, 4194304 2097152 linear 8:4 0"
- also supporting creating multiple devices if the semi-colon is used
- colon to separate name from uuid, like we already do major:minor
- colon to separate other flags from rw if we need them in future
- splitting first on a unescaped semi-colons, then on the first two
unescaped commas, and then on unescaped commas and then unescaped spaces
within the table
- backslash escapes the following character so it is never a treated
as a separator
- lroot\:uuid\;\\\ \"\, would be a device with no uuid and the name
lroot:uuid;\ ", (on a non-udev system without name mangling)
# dmsetup ls --bootformat lroot
dm="lroot:uuid,rw,0 2097152 linear 8:2 0, 2097152 2097152 \
linear 8:3 0, 4194304 2097152 linear 8:4 0"
# dmsetup ls --bootformat
(all devices on one output line)
While the code also supports devices in the /dev/sda2 format for
convenience, please use the preferred 8:2 format in any implementation
and documented examples (to avoid the unnecessary dependency on /dev and
its dependencies).
Or with some alternative name for the option
--boot[format|param]
--short[format]
--kernelparam
--condensed
other suggestions?
dmsetup create --condensed
dmsetup ls --condensed
Alasdair
^ permalink raw reply
* Re: [mdadm PATCH] Create: move STOP_ARRAY to abort_locked
From: Zhilong Liu @ 2017-05-04 12:20 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid
In-Reply-To: <1493190229-14329-1-git-send-email-zlliu@suse.com>
Hi Jes,
apply for review, this is a bug I ever encountered.
On 04/26/2017 03:03 PM, Zhilong Liu wrote:
> The sysfs entry and devnm would be created once create_mddev()
> performed successfully, but the creating isn't completed here,
> move STOP_ARRAY to abort_locked, the purpose is to cleanup the
> partially created array.
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
> Create.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/Create.c b/Create.c
> index 6ca0924..fe0ab7e 100644
> --- a/Create.c
> +++ b/Create.c
> @@ -904,10 +904,8 @@ int Create(struct supertype *st, char *mddev,
> remove_partitions(fd);
> if (st->ss->add_to_super(st, &inf->disk,
> fd, dv->devname,
> - dv->data_offset)) {
> - ioctl(mdfd, STOP_ARRAY, NULL);
> + dv->data_offset))
> goto abort_locked;
> - }
> st->ss->getinfo_super(st, inf, NULL);
> safe_mode_delay = inf->safe_mode_delay;
>
> @@ -1008,7 +1006,6 @@ int Create(struct supertype *st, char *mddev,
> sysfs_set_safemode(&info, safe_mode_delay);
> if (err) {
> pr_err("failed to activate array.\n");
> - ioctl(mdfd, STOP_ARRAY, NULL);
> goto abort;
> }
> } else if (c->readonly &&
> @@ -1018,7 +1015,6 @@ int Create(struct supertype *st, char *mddev,
> "array_state", "readonly") < 0) {
> pr_err("Failed to start array: %s\n",
> strerror(errno));
> - ioctl(mdfd, STOP_ARRAY, NULL);
> goto abort;
> }
> } else {
> @@ -1030,7 +1026,6 @@ int Create(struct supertype *st, char *mddev,
> if (info.array.chunk_size & (info.array.chunk_size-1)) {
> cont_err("Problem may be that chunk size is not a power of 2\n");
> }
> - ioctl(mdfd, STOP_ARRAY, NULL);
> goto abort;
> }
> /* if start_ro module parameter is set, array is
> @@ -1061,7 +1056,9 @@ int Create(struct supertype *st, char *mddev,
> map_remove(&map, fd2devnm(mdfd));
> map_unlock(&map);
>
> - if (mdfd >= 0)
> + if (mdfd >= 0) {
> + ioctl(mdfd, STOP_ARRAY, NULL);
> close(mdfd);
> + }
> return 1;
> }
^ permalink raw reply
* [PATCH v1 2/2] mdadm/util: unify stat checking blkdev into function
From: Zhilong Liu @ 2017-05-04 12:16 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
In-Reply-To: <20170504121622.4014-1-zlliu@suse.com>
declare function stat_is_blkdev() to integrate repeated stat
checking blkdev operations, it returns 'true/1' when it is a
block device, and returns 'false/0' when it isn't.
The devname is necessary parameter, *rdev is optional, parse
the pointer of dev_t *rdev, if valid, assigned device number
to dev_t *rdev, if NULL, ignores.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
Assemble.c | 7 ++-----
Build.c | 25 ++++---------------------
Incremental.c | 21 ++++-----------------
Manage.c | 11 +----------
Monitor.c | 16 ++++------------
mdadm.h | 1 +
super-ddf.c | 10 ++++------
super-intel.c | 10 ++++------
util.c | 17 +++++++++++++++++
9 files changed, 41 insertions(+), 77 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index 9d0a89f..30d5838 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -512,15 +512,12 @@ static int select_devices(struct mddev_dev *devlist,
/* Now reject spares that don't match domains of identified members */
for (tmpdev = devlist; tmpdev; tmpdev = tmpdev->next) {
- struct stat stb;
if (tmpdev->used != 3)
continue;
- if (stat(tmpdev->devname, &stb)< 0) {
- pr_err("fstat failed for %s: %s\n",
- tmpdev->devname, strerror(errno));
+ if (!stat_is_blkdev(tmpdev->devname, &rdev)) {
tmpdev->used = 2;
} else {
- struct dev_policy *pol = devid_policy(stb.st_rdev);
+ struct dev_policy *pol = devid_policy(rdev);
int dt = domain_test(domains, pol, NULL);
if (inargv && dt != 0)
/* take this spare as domains match
diff --git a/Build.c b/Build.c
index 2d84b96..ad59867 100644
--- a/Build.c
+++ b/Build.c
@@ -41,7 +41,6 @@ int Build(char *mddev, struct mddev_dev *devlist,
* cc = chunk size factor: 0==4k, 1==8k etc.
*/
int i;
- struct stat stb;
dev_t rdev;
int subdevs = 0, missing_disks = 0;
struct mddev_dev *dv;
@@ -65,16 +64,8 @@ int Build(char *mddev, struct mddev_dev *devlist,
missing_disks++;
continue;
}
- if (stat(dv->devname, &stb)) {
- pr_err("Cannot find %s: %s\n",
- dv->devname, strerror(errno));
- return 1;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- pr_err("%s is not a block device.\n",
- dv->devname);
+ if (!stat_is_blkdev(dv->devname, NULL))
return 1;
- }
}
if (s->raiddisks != subdevs) {
@@ -162,16 +153,8 @@ int Build(char *mddev, struct mddev_dev *devlist,
if (strcmp("missing", dv->devname) == 0)
continue;
- if (stat(dv->devname, &stb)) {
- pr_err("Weird: %s has disappeared.\n",
- dv->devname);
+ if (!stat_is_blkdev(dv->devname, &rdev))
goto abort;
- }
- if ((stb.st_mode & S_IFMT)!= S_IFBLK) {
- pr_err("Weird: %s is no longer a block device.\n",
- dv->devname);
- goto abort;
- }
fd = open(dv->devname, O_RDONLY|O_EXCL);
if (fd < 0) {
pr_err("Cannot open %s: %s\n",
@@ -187,8 +170,8 @@ int Build(char *mddev, struct mddev_dev *devlist,
disk.state = (1<<MD_DISK_SYNC) | (1<<MD_DISK_ACTIVE);
if (dv->writemostly == FlagSet)
disk.state |= 1<<MD_DISK_WRITEMOSTLY;
- disk.major = major(stb.st_rdev);
- disk.minor = minor(stb.st_rdev);
+ disk.major = major(rdev);
+ disk.minor = minor(rdev);
if (ioctl(mdfd, ADD_NEW_DISK, &disk)) {
pr_err("ADD_NEW_DISK failed for %s: %s\n",
dv->devname, strerror(errno));
diff --git a/Incremental.c b/Incremental.c
index 11a34e7..97b2e99 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -86,8 +86,7 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
* - if number of OK devices match expected, or -R and there are enough,
* start the array (auto-readonly).
*/
- struct stat stb;
- dev_t rdev;
+ dev_t rdev, rdev2;
struct mdinfo info, dinfo;
struct mdinfo *sra = NULL, *d;
struct mddev_ident *match;
@@ -108,18 +107,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
struct createinfo *ci = conf_get_create_info();
- if (stat(devname, &stb) < 0) {
- if (c->verbose >= 0)
- pr_err("stat failed for %s: %s.\n",
- devname, strerror(errno));
- return rv;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- if (c->verbose >= 0)
- pr_err("%s is not a block device.\n",
- devname);
+ if (!stat_is_blkdev(devname, &rdev))
return rv;
- }
dfd = dev_open(devname, O_RDONLY);
if (dfd < 0) {
if (c->verbose >= 0)
@@ -158,10 +147,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
if (!devlist) {
devlist = conf_get_devs();
for (;devlist; devlist = devlist->next) {
- struct stat st2;
- if (stat(devlist->devname, &st2) == 0 &&
- (st2.st_mode & S_IFMT) == S_IFBLK &&
- st2.st_rdev == stb.st_rdev)
+ if (stat_is_blkdev(devlist->devname, &rdev2) &&
+ rdev2 == rdev)
break;
}
}
diff --git a/Manage.c b/Manage.c
index af55266..2c82326 100644
--- a/Manage.c
+++ b/Manage.c
@@ -1510,24 +1510,16 @@ int Manage_subdevs(char *devname, int fd,
*/
rdev = makedev(mj, mn);
} else {
- struct stat stb;
tfd = dev_open(dv->devname, O_RDONLY);
if (tfd >= 0) {
fstat_is_blkdev(tfd, dv->devname, &rdev);
close(tfd);
} else {
int open_err = errno;
- if (stat(dv->devname, &stb) != 0) {
- pr_err("Cannot find %s: %s\n",
- dv->devname, strerror(errno));
- goto abort;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
+ if (!stat_is_blkdev(dv->devname, &rdev))
if (dv->disposition == 'M')
/* non-fatal. Also improbable */
continue;
- pr_err("%s is not a block device.\n",
- dv->devname);
goto abort;
}
if (dv->disposition == 'r')
@@ -1544,7 +1536,6 @@ int Manage_subdevs(char *devname, int fd,
goto abort;
}
}
- rdev = stb.st_rdev;
}
switch(dv->disposition){
default:
diff --git a/Monitor.c b/Monitor.c
index 1f15377..e2b36ff 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -993,23 +993,13 @@ static void link_containers_with_subarrays(struct state *list)
/* Not really Monitor but ... */
int Wait(char *dev)
{
- struct stat stb;
char devnm[32];
- char *tmp;
int rv = 1;
int frozen_remaining = 3;
- if (stat(dev, &stb) != 0) {
- pr_err("Cannot find %s: %s\n", dev,
- strerror(errno));
+ if (!stat_is_blkdev(dev, NULL))
return 2;
- }
- tmp = stat2devnm(&stb);
- if (!tmp) {
- pr_err("%s is not a block device.\n", dev);
- return 2;
- }
- strcpy(devnm, tmp);
+ strcpy(devnm, dev);
while(1) {
struct mdstat_ent *ms = mdstat_read(1, 0);
@@ -1068,6 +1058,8 @@ int WaitClean(char *dev, int sock, int verbose)
int rv = 1;
char devnm[32];
+ if (!stat_is_blkdev(dev, NULL))
+ return 2;
fd = open(dev, O_RDONLY);
if (fd < 0) {
if (verbose)
diff --git a/mdadm.h b/mdadm.h
index 4adb840..a92feb2 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1435,6 +1435,7 @@ extern int check_partitions(int fd, char *dname,
unsigned long long freesize,
unsigned long long size);
extern int fstat_is_blkdev(int fd, char *devname, dev_t *rdev);
+extern int stat_is_blkdev(char *devname, dev_t *rdev);
extern int get_mdp_major(void);
extern int get_maj_min(char *dev, int *major, int *minor);
diff --git a/super-ddf.c b/super-ddf.c
index 796eaa5..9c82f4f 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -3490,7 +3490,7 @@ static int validate_geometry_ddf_bvd(struct supertype *st,
char *dev, unsigned long long *freesize,
int verbose)
{
- struct stat stb;
+ dev_t rdev;
struct ddf_super *ddf = st->sb;
struct dl *dl;
unsigned long long maxsize;
@@ -3526,13 +3526,11 @@ static int validate_geometry_ddf_bvd(struct supertype *st,
return 1;
}
/* This device must be a member of the set */
- if (stat(dev, &stb) < 0)
- return 0;
- if ((S_IFMT & stb.st_mode) != S_IFBLK)
+ if (!stat_is_blkdev(dev, NULL))
return 0;
for (dl = ddf->dlist ; dl ; dl = dl->next) {
- if (dl->major == (int)major(stb.st_rdev) &&
- dl->minor == (int)minor(stb.st_rdev))
+ if (dl->major == (int)major(rdev) &&
+ dl->minor == (int)minor(rdev))
break;
}
if (!dl) {
diff --git a/super-intel.c b/super-intel.c
index c4196ea..e13c940 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6855,7 +6855,7 @@ static int validate_geometry_imsm_volume(struct supertype *st, int level,
unsigned long long *freesize,
int verbose)
{
- struct stat stb;
+ dev_t rdev;
struct intel_super *super = st->sb;
struct imsm_super *mpb;
struct dl *dl;
@@ -6920,13 +6920,11 @@ static int validate_geometry_imsm_volume(struct supertype *st, int level,
}
/* This device must be a member of the set */
- if (stat(dev, &stb) < 0)
- return 0;
- if ((S_IFMT & stb.st_mode) != S_IFBLK)
+ if (!stat_is_blkdev(dev, &rdev))
return 0;
for (dl = super->disks ; dl ; dl = dl->next) {
- if (dl->major == (int)major(stb.st_rdev) &&
- dl->minor == (int)minor(stb.st_rdev))
+ if (dl->major == (int)major(rdev) &&
+ dl->minor == (int)minor(rdev))
break;
}
if (!dl) {
diff --git a/util.c b/util.c
index a92faf8..11ff2cc 100644
--- a/util.c
+++ b/util.c
@@ -747,6 +747,23 @@ int fstat_is_blkdev(int fd, char *devname, dev_t *rdev)
return 1;
}
+int stat_is_blkdev(char *devname, dev_t *rdev)
+{
+ struct stat stb;
+
+ if (stat(devname, &stb) != 0) {
+ pr_err("stat failed for %s: %s\n", devname, strerror(errno));
+ return 0;
+ }
+ if ((S_IFMT & stb.st_mode) != S_IFBLK) {
+ pr_err("%s is not a block device.\n", devname);
+ return 0;
+ }
+ if (rdev)
+ *rdev = stb.st_rdev;
+ return 1;
+}
+
int ask(char *mesg)
{
char *add = "";
--
2.10.2
^ permalink raw reply related
* [PATCH v1 1/2] mdadm/util: unify fstat checking blkdev into function
From: Zhilong Liu @ 2017-05-04 12:16 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
In-Reply-To: <20170504121622.4014-1-zlliu@suse.com>
declare function fstat_is_blkdev() to integrate repeated fstat
checking block device operations, it returns true/1 when it is
a block device, and returns false/0 when it isn't.
The fd and devname are necessary parameters, *rdev is optional,
parse the pointer of dev_t *rdev, if valid, assigned the device
number to dev_t *rdev, if NULL, ignores.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
Assemble.c | 19 +++++++------------
Build.c | 5 +++--
Create.c | 23 ++++++++++-------------
Grow.c | 10 ++++------
Incremental.c | 33 ++++++++++++---------------------
Manage.c | 2 +-
bitmap.c | 13 ++++---------
mdadm.h | 1 +
super-intel.c | 13 +++----------
util.c | 17 +++++++++++++++++
10 files changed, 62 insertions(+), 74 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index a9442c8..9d0a89f 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -149,6 +149,7 @@ static int select_devices(struct mddev_dev *devlist,
struct mdinfo *content = NULL;
int report_mismatch = ((inargv && c->verbose >= 0) || c->verbose > 0);
struct domainlist *domains = NULL;
+ dev_t rdev;
tmpdev = devlist; num_devs = 0;
while (tmpdev) {
@@ -169,7 +170,6 @@ static int select_devices(struct mddev_dev *devlist,
tmpdev = tmpdev ? tmpdev->next : NULL) {
char *devname = tmpdev->devname;
int dfd;
- struct stat stb;
struct supertype *tst;
struct dev_policy *pol = NULL;
int found_container = 0;
@@ -204,14 +204,7 @@ static int select_devices(struct mddev_dev *devlist,
pr_err("cannot open device %s: %s\n",
devname, strerror(errno));
tmpdev->used = 2;
- } else if (fstat(dfd, &stb)< 0) {
- /* Impossible! */
- pr_err("fstat failed for %s: %s\n",
- devname, strerror(errno));
- tmpdev->used = 2;
- } else if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- pr_err("%s is not a block device.\n",
- devname);
+ } else if (!fstat_is_blkdev(dfd, devname, &rdev)) {
tmpdev->used = 2;
} else if (must_be_container(dfd)) {
if (st) {
@@ -234,7 +227,8 @@ static int select_devices(struct mddev_dev *devlist,
devname);
tmpdev->used = 2;
} else if (auto_assem &&
- !conf_test_metadata(tst->ss->name, (pol = devid_policy(stb.st_rdev)),
+ !conf_test_metadata(tst->ss->name,
+ (pol = devid_policy(rdev)),
tst->ss->match_home(tst, c->homehost) == 1)) {
if (report_mismatch)
pr_err("%s has metadata type %s for which auto-assembly is disabled\n",
@@ -261,7 +255,8 @@ static int select_devices(struct mddev_dev *devlist,
tst->ss->name, devname);
tmpdev->used = 2;
} else if (auto_assem && st == NULL &&
- !conf_test_metadata(tst->ss->name, (pol = devid_policy(stb.st_rdev)),
+ !conf_test_metadata(tst->ss->name,
+ (pol = devid_policy(rdev)),
tst->ss->match_home(tst, c->homehost) == 1)) {
if (report_mismatch)
pr_err("%s has metadata type %s for which auto-assembly is disabled\n",
@@ -484,7 +479,7 @@ static int select_devices(struct mddev_dev *devlist,
/* Collect domain information from members only */
if (tmpdev && tmpdev->used == 1) {
if (!pol)
- pol = devid_policy(stb.st_rdev);
+ pol = devid_policy(rdev);
domain_merge(&domains, pol, tst?tst->ss->name:NULL);
}
dev_policy_free(pol);
diff --git a/Build.c b/Build.c
index 665d906..2d84b96 100644
--- a/Build.c
+++ b/Build.c
@@ -42,6 +42,7 @@ int Build(char *mddev, struct mddev_dev *devlist,
*/
int i;
struct stat stb;
+ dev_t rdev;
int subdevs = 0, missing_disks = 0;
struct mddev_dev *dv;
int bitmap_fd;
@@ -126,8 +127,8 @@ int Build(char *mddev, struct mddev_dev *devlist,
array.nr_disks = s->raiddisks;
array.raid_disks = s->raiddisks;
array.md_minor = 0;
- if (fstat(mdfd, &stb) == 0)
- array.md_minor = minor(stb.st_rdev);
+ if (fstat_is_blkdev(mdfd, mddev, &rdev))
+ array.md_minor = minor(rdev);
array.not_persistent = 1;
array.state = 0; /* not clean, but no errors */
if (s->assume_clean)
diff --git a/Create.c b/Create.c
index df1bc20..239545f 100644
--- a/Create.c
+++ b/Create.c
@@ -89,8 +89,8 @@ int Create(struct supertype *st, char *mddev,
char *maxdisc = NULL;
int dnum, raid_disk_num;
struct mddev_dev *dv;
+ dev_t rdev;
int fail = 0, warn = 0;
- struct stat stb;
int first_missing = subdevs * 2;
int second_missing = subdevs * 2;
int missing_disks = 0;
@@ -325,11 +325,8 @@ int Create(struct supertype *st, char *mddev,
dname, strerror(errno));
exit(2);
}
- if (fstat(dfd, &stb) != 0 ||
- (stb.st_mode & S_IFMT) != S_IFBLK) {
+ if (!fstat_is_blkdev(dfd, dname, NULL)) {
close(dfd);
- pr_err("%s is not a block device\n",
- dname);
exit(2);
}
close(dfd);
@@ -641,8 +638,8 @@ int Create(struct supertype *st, char *mddev,
* with, but it chooses to trust me instead. Sigh
*/
info.array.md_minor = 0;
- if (fstat(mdfd, &stb) == 0)
- info.array.md_minor = minor(stb.st_rdev);
+ if (fstat_is_blkdev(mdfd, mddev, &rdev))
+ info.array.md_minor = minor(rdev);
info.array.not_persistent = 0;
if (((s->level == 4 || s->level == 5) &&
@@ -841,7 +838,6 @@ int Create(struct supertype *st, char *mddev,
for (dnum = 0, raid_disk_num = 0, dv = devlist; dv;
dv = (dv->next) ? (dv->next) : moved_disk, dnum++) {
int fd;
- struct stat stb2;
struct mdinfo *inf = &infos[dnum];
if (dnum >= total_slots)
@@ -897,9 +893,10 @@ int Create(struct supertype *st, char *mddev,
dv->devname);
goto abort_locked;
}
- fstat(fd, &stb2);
- inf->disk.major = major(stb2.st_rdev);
- inf->disk.minor = minor(stb2.st_rdev);
+ if (!fstat_is_blkdev(fd, dv->devname, &rdev))
+ return 1;
+ inf->disk.major = major(rdev);
+ inf->disk.minor = minor(rdev);
}
if (fd >= 0)
remove_partitions(fd);
@@ -920,8 +917,8 @@ int Create(struct supertype *st, char *mddev,
if (!have_container) {
/* getinfo_super might have lost these ... */
- inf->disk.major = major(stb2.st_rdev);
- inf->disk.minor = minor(stb2.st_rdev);
+ inf->disk.major = major(rdev);
+ inf->disk.minor = minor(rdev);
}
break;
case 2:
diff --git a/Grow.c b/Grow.c
index f4bd301..a527436 100644
--- a/Grow.c
+++ b/Grow.c
@@ -109,7 +109,7 @@ int Grow_Add_device(char *devname, int fd, char *newdev)
*/
struct mdinfo info;
- struct stat stb;
+ dev_t rdev;
int nfd, fd2;
int d, nd;
struct supertype *st = NULL;
@@ -145,9 +145,7 @@ int Grow_Add_device(char *devname, int fd, char *newdev)
free(st);
return 1;
}
- fstat(nfd, &stb);
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- pr_err("%s is not a block device!\n", newdev);
+ if (!fstat_is_blkdev(nfd, newdev, &rdev)) {
close(nfd);
free(st);
return 1;
@@ -198,8 +196,8 @@ int Grow_Add_device(char *devname, int fd, char *newdev)
*/
info.disk.number = d;
- info.disk.major = major(stb.st_rdev);
- info.disk.minor = minor(stb.st_rdev);
+ info.disk.major = major(rdev);
+ info.disk.minor = minor(rdev);
info.disk.raid_disk = d;
info.disk.state = (1 << MD_DISK_SYNC) | (1 << MD_DISK_ACTIVE);
st->ss->update_super(st, &info, "linear-grow-new", newdev,
diff --git a/Incremental.c b/Incremental.c
index 8909f2f..11a34e7 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -87,6 +87,7 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
* start the array (auto-readonly).
*/
struct stat stb;
+ dev_t rdev;
struct mdinfo info, dinfo;
struct mdinfo *sra = NULL, *d;
struct mddev_ident *match;
@@ -174,21 +175,11 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
/* 2/ Find metadata, reject if none appropriate (check
* version/name from args) */
- if (fstat(dfd, &stb) < 0) {
- if (c->verbose >= 0)
- pr_err("fstat failed for %s: %s.\n",
- devname, strerror(errno));
- goto out;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- if (c->verbose >= 0)
- pr_err("%s is not a block device.\n",
- devname);
+ if (!fstat_is_blkdev(dfd, devname, &rdev))
goto out;
- }
- dinfo.disk.major = major(stb.st_rdev);
- dinfo.disk.minor = minor(stb.st_rdev);
+ dinfo.disk.major = major(rdev);
+ dinfo.disk.minor = minor(rdev);
policy = disk_policy(&dinfo);
have_target = policy_check_path(&dinfo, &target_array);
@@ -339,8 +330,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
}
dinfo = info;
- dinfo.disk.major = major(stb.st_rdev);
- dinfo.disk.minor = minor(stb.st_rdev);
+ dinfo.disk.major = major(rdev);
+ dinfo.disk.minor = minor(rdev);
if (add_disk(mdfd, st, &info, &dinfo) != 0) {
pr_err("failed to add %s to new array %s: %s.\n",
devname, chosen_name, strerror(errno));
@@ -441,8 +432,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
goto out_unlock;
}
}
- info.disk.major = major(stb.st_rdev);
- info.disk.minor = minor(stb.st_rdev);
+ info.disk.major = major(rdev);
+ info.disk.minor = minor(rdev);
/* add disk needs to know about containers */
if (st->ss->external)
sra->array.level = LEVEL_CONTAINER;
@@ -863,12 +854,12 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
* Return 0 on success, or some exit code on failure, probably 1.
*/
int rv = 1;
- struct stat stb;
+ dev_t rdev;
struct map_ent *mp, *map = NULL;
struct mdinfo *chosen = NULL;
int dfd = *dfdp;
- if (fstat(dfd, &stb) != 0)
+ if (!fstat_is_blkdev(dfd, devname, &rdev))
return 1;
/*
@@ -1038,8 +1029,8 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
devlist.writemostly = FlagDefault;
devlist.failfast = FlagDefault;
devlist.devname = chosen_devname;
- sprintf(chosen_devname, "%d:%d", major(stb.st_rdev),
- minor(stb.st_rdev));
+ sprintf(chosen_devname, "%d:%d", major(rdev),
+ minor(rdev));
devlist.disposition = 'a';
close(dfd);
*dfdp = -1;
diff --git a/Manage.c b/Manage.c
index 230309b..af55266 100644
--- a/Manage.c
+++ b/Manage.c
@@ -1513,7 +1513,7 @@ int Manage_subdevs(char *devname, int fd,
struct stat stb;
tfd = dev_open(dv->devname, O_RDONLY);
if (tfd >= 0) {
- fstat(tfd, &stb);
+ fstat_is_blkdev(tfd, dv->devname, &rdev);
close(tfd);
} else {
int open_err = errno;
diff --git a/bitmap.c b/bitmap.c
index 16a6b73..3653660 100644
--- a/bitmap.c
+++ b/bitmap.c
@@ -183,7 +183,6 @@ static int
bitmap_file_open(char *filename, struct supertype **stp, int node_num)
{
int fd;
- struct stat stb;
struct supertype *st = *stp;
fd = open(filename, O_RDONLY|O_DIRECT);
@@ -193,14 +192,7 @@ bitmap_file_open(char *filename, struct supertype **stp, int node_num)
return -1;
}
- if (fstat(fd, &stb) < 0) {
- pr_err("failed to determine bitmap file/device type: %s\n",
- strerror(errno));
- close(fd);
- return -1;
- }
-
- if ((stb.st_mode & S_IFMT) == S_IFBLK) {
+ if (fstat_is_blkdev(fd, filename, NULL)) {
/* block device, so we are probably after an internal bitmap */
if (!st)
st = guess_super(fd);
@@ -221,6 +213,9 @@ bitmap_file_open(char *filename, struct supertype **stp, int node_num)
}
*stp = st;
+ } else {
+ close(fd);
+ return -1;
}
return fd;
diff --git a/mdadm.h b/mdadm.h
index 07ee963..4adb840 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1434,6 +1434,7 @@ extern int check_raid(int fd, char *name);
extern int check_partitions(int fd, char *dname,
unsigned long long freesize,
unsigned long long size);
+extern int fstat_is_blkdev(int fd, char *devname, dev_t *rdev);
extern int get_mdp_major(void);
extern int get_maj_min(char *dev, int *major, int *minor);
diff --git a/super-intel.c b/super-intel.c
index 36f77d3..c4196ea 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6562,7 +6562,7 @@ count_volumes_list(struct md_list *devlist, char *homehost,
for (tmpdev = devlist; tmpdev; tmpdev = tmpdev->next) {
char *devname = tmpdev->devname;
- struct stat stb;
+ dev_t rdev;
struct supertype *tst;
int dfd;
if (tmpdev->used > 1)
@@ -6578,14 +6578,7 @@ count_volumes_list(struct md_list *devlist, char *homehost,
dprintf("cannot open device %s: %s\n",
devname, strerror(errno));
tmpdev->used = 2;
- } else if (fstat(dfd, &stb)< 0) {
- /* Impossible! */
- dprintf("fstat failed for %s: %s\n",
- devname, strerror(errno));
- tmpdev->used = 2;
- } else if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- dprintf("%s is not a block device.\n",
- devname);
+ } else if (!fstat_is_blkdev(dfd, devname, &rdev)) {
tmpdev->used = 2;
} else if (must_be_container(dfd)) {
struct supertype *cst;
@@ -6607,7 +6600,7 @@ count_volumes_list(struct md_list *devlist, char *homehost,
if (cst)
cst->ss->free_super(cst);
} else {
- tmpdev->st_rdev = stb.st_rdev;
+ tmpdev->st_rdev = rdev;
if (tst->ss->load_super(tst,dfd, NULL)) {
dprintf("no RAID superblock on %s\n",
devname);
diff --git a/util.c b/util.c
index c7585ac..a92faf8 100644
--- a/util.c
+++ b/util.c
@@ -730,6 +730,23 @@ int check_raid(int fd, char *name)
return 1;
}
+int fstat_is_blkdev(int fd, char *devname, dev_t *rdev)
+{
+ struct stat stb;
+
+ if (fstat(fd, &stb) != 0) {
+ pr_err("fstat failed for %s: %s\n", devname, strerror(errno));
+ return 0;
+ }
+ if ((S_IFMT & stb.st_mode) != S_IFBLK) {
+ pr_err("%s is not a block device.\n", devname);
+ return 0;
+ }
+ if (rdev)
+ *rdev = stb.st_rdev;
+ return 1;
+}
+
int ask(char *mesg)
{
char *add = "";
--
2.10.2
^ permalink raw reply related
* [PATCH v1 0/2] unify stat and fstat checking blkdev into functions
From: Zhilong Liu @ 2017-05-04 12:16 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
Hi, Jes;
This two patches declared two functions named fstat_is_blkdev()
and stat_is_blkdev() to integrate repeated checking block device
operations, decreased the repeated codes via to utility function.
And fix a bug in WaitClean(), it forgot to check block device.
This patchset depends on patchset
"[mdadm PATCH 0/2] unify stat and fstat into functions"
Changes:
- modified the return value of them, return true/1 if it is a
block device, return false/0 when it isn't.
- rename the functions and make their name shorter.
Thanks,
Zhilong
Zhilong Liu (2):
util: unify fstat checking blkdev into function
util: unify stat checking blkdev into function
Assemble.c | 26 +++++++++-----------------
Build.c | 30 +++++++-----------------------
Create.c | 23 ++++++++++-------------
Grow.c | 10 ++++------
Incremental.c | 52 +++++++++++++++-------------------------------------
Manage.c | 13 ++-----------
Monitor.c | 16 ++++------------
bitmap.c | 13 ++++---------
mdadm.h | 2 ++
super-ddf.c | 10 ++++------
super-intel.c | 23 +++++++----------------
util.c | 34 ++++++++++++++++++++++++++++++++++
12 files changed, 102 insertions(+), 150 deletions(-)
--
2.10.2
^ permalink raw reply
* Re: [PATCH v6 0/3] dm: boot a mapped device without an initramfs
From: Alasdair G Kergon @ 2017-05-04 12:14 UTC (permalink / raw)
To: Enric Balletbo Serra
Cc: Will Drewry, Kees Cook, Mike Snitzer, linux-doc@vger.kernel.org,
David Zeuthen, LKML, linux-raid, dm-devel, Guenter Roeck,
Enric Balletbo i Serra, Shaohua Li, Alasdair Kergon
In-Reply-To: <CAFqH_51YLoYrNcw+Fb99LbSY63wVWg+MW7nBYQqWjV6qfOAmNQ@mail.gmail.com>
On Thu, May 04, 2017 at 01:18:41PM +0200, Enric Balletbo Serra wrote:
> I'm wondering if a command line like this would be acceptable.
1) Make sure the implementation continues to support backslash quoting
so that any characters you introduce with special meanings (comma,
semi-colon, double-quote in that example) can still be used if required.
2) "none" is of course a valid uuid:) More comma-separation or
re-ordering, perhaps?
3) Whatever final format is agreed here should be supported by dmsetup
as well, so you can both supply the format to dmsetup and ask dmsetup
to display your existing devices in this format. Choose a format
that makes this easy.
Alasdair
^ permalink raw reply
* Re: [dm-devel] [PATCH v6 0/3] dm: boot a mapped device without an initramfs
From: Enric Balletbo Serra @ 2017-05-04 11:18 UTC (permalink / raw)
To: Kees Cook
Cc: Enric Balletbo i Serra, Will Drewry, Guenter Roeck, Mike Snitzer,
linux-doc@vger.kernel.org, David Zeuthen, LKML, linux-raid,
dm-devel, Shaohua Li, Alasdair Kergon
In-Reply-To: <CAGXu5jLD+0POz1sopyZNNqDAiq2MZ5ZYdjs43jX5OnWxkZt1Lg@mail.gmail.com>
Mike,
2017-04-18 19:37 GMT+02:00 Kees Cook <keescook@chromium.org>:
> On Tue, Apr 18, 2017 at 9:42 AM, Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
>> Hello,
>>
>> Some of these patches were send few years back, I saw that first
>> version was send to this list in 2010, and after version 4 did not
>> land [1]. Some days ago I resend the patches [2] and few hours later I
>> noticed that one year ago was send a v5 version [3] and I was not aware.
>>
>> There was some discussion about v5 and during the discussion Mike Snitzer
>> proposed that at least a change of the syntax is required, we're really
>> interested on see this upstream as is extensively used in ChromeOS based
>> devices so I'm wondering if we can restart the discussion and hopefully
>> we will be able to do the modifications needed.
>>
>> So my first question is, apart of the change of the syntax, what more
>> should be changed?
>
> AFAIK, this was the main change needed. Change the syntax and plumb
> into the ioctl interface. The discussion ended with Mike being open to
> the idea, and for me to go work on it. I haven't had time to work on
> it, though, so it has continued to be a locally carried patch:
> https://www.redhat.com/archives/dm-devel/2016-February/msg00199.html
>
From your email:
> >> > 2) If you are able to adequately justify the need for dm=:
> >> > I'd much rather the dm= kernel commandline be a simple series of
> >> > comma-delimited dmsetup-like commands.
> >> >
> >> > You'd handle each command with extremely basic parsing:
> >> > <dm_ioctl_cmd> <args> [, <dm_ioctl_cmd> <args>]
> >> > (inventing a special token to denote <newline>, to support tables with
> >> > multiple entries, rather than relying on commas and counts, etc)
> >>
I'm wondering if a command line like this would be acceptable.
Format is:
dm="<dev_name> <uuid> <mode>, <table>[, <table>][; <dev_name>
<uuid> <mode>, <table>[, <table>]][; ... ]"
where:
<dev_name> ::= The device name
<uuid> ::= xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx | "none"
<mode> ::= "ro" | "rw"
<table> ::= <start sector> <end sector> <target name>
<target parmaters>
An example of booting to a linear array:
dm="lroot none rw, 0 2097152 linear /dev/sda2 0, 2097152 2097152
linear /dev/sda3 0, 4194304 2097152 linear /dev/sda4 0"
Equivalent dmsetup command:
echo -e "0 2097152 linear /dev/sda2 0"\\n"2097152 2097152 linear
/dev/sda3 0"\\n"4194304 2097152 linear /dev/sda4 0" | sudo dmsetup
create lroot
An example of multiple device-mappers, with the dm="..." contents shown
here split on multiple lines for readability:
vroot none ro,
0 2097152 verity 1 /dev/sdb2 /dev/sdb3 4096 4096 262144 1 sha256 \
289b52edac1ac4f4c32c8f765795615a85d4daa454677d21a6d8767c4627dc48 \
632d7fe427a23a8e88493c553298a779997478a143d86da5d56a65db8a1f2a38;
vram none rw,
0 32768 linear 1:0 0,
32768 32768 linear 1:1 0
Thanks,
Enric
> More recently David Zeuthen has been poking at this code, so I've
> included him on CC here, in case there are new developments.
>
> -Kees
>
>>
>> Thanks for your help,
>> Enric
>>
>> [1] Patchwork links:
>> https://patchwork.kernel.org/patch/104857/
>> https://patchwork.kernel.org/patch/104856/
>> https://patchwork.kernel.org/patch/104858/
>>
>> [2] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1375276.html
>>
>> [3] https://www.redhat.com/archives/dm-devel/2016-February/msg00112.html
>>
>>
>> Brian Norris (1):
>> dm: make some mapped_device functions available
>>
>> Will Drewry (2):
>> dm: export a table+mapped device to the ioctl interface
>> init: add support to directly boot to a mapped device
>>
>> Documentation/admin-guide/kernel-parameters.rst | 1 +
>> Documentation/admin-guide/kernel-parameters.txt | 3 +
>> Documentation/device-mapper/boot.txt | 65 ++++
>> drivers/md/dm-ioctl.c | 36 ++
>> drivers/md/dm.h | 8 -
>> include/linux/device-mapper.h | 19 +
>> init/Makefile | 1 +
>> init/do_mounts.c | 1 +
>> init/do_mounts.h | 10 +
>> init/do_mounts_dm.c | 448 ++++++++++++++++++++++++
>> 10 files changed, 584 insertions(+), 8 deletions(-)
>> create mode 100644 Documentation/device-mapper/boot.txt
>> create mode 100644 init/do_mounts_dm.c
>>
>> --
>> 2.9.3
>>
>
>
>
> --
> Kees Cook
> Pixel Security
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply
* Re: [mdadm PATCH] Create: tell udev md device is not ready when first created.
From: Peter Rajnoha @ 2017-05-04 10:58 UTC (permalink / raw)
To: NeilBrown, Jes Sorensen; +Cc: linux-raid, dm-devel
In-Reply-To: <87h919ruj5.fsf@notabene.neil.brown.name>
On 04/28/2017 07:05 AM, NeilBrown wrote:
>
> When an array is created the content is not initialized,
> so it could have remnants of an old filesystem or md array
> etc on it.
> udev will see this and might try to activate it, which is almost
> certainly not what is wanted.
>
> So create a mechanism for mdadm to communicate with udev to tell
> it that the device isn't ready. This mechanism is the existance
> of a file /run/mdadm/created-mdXXX where mdXXX is the md device name.
>
> When creating an array, mdadm will create the file.
> A new udev rule file, 01-md-raid-creating.rules, will detect the
> precense of thst file and set ENV{SYSTEMD_READY}="0".
> This is fairly uniformly used to suppress actions based on the
> contents of the device.
>
...
> diff --git a/udev-md-raid-creating.rules b/udev-md-raid-creating.rules
> new file mode 100644
> index 000000000000..2be466bcefd2
> --- /dev/null
> +++ b/udev-md-raid-creating.rules
> @@ -0,0 +1,7 @@
> +# do not edit this file, it will be overwritten on update
> +# While mdadm is creating an array, it creates a file
> +# /run/mdadm/creating-mdXXX. If that file exists, then
> +# the array is not "ready" and we should make sure the
> +# content is ignored.
> +
> +KERNEL=="md*", TEST="/run/mdadm/creating-$kernel", ENV{SYSTEMD_READY}="0"
>
This should be TEST=="/run/mdadm/creating-$kernel" (double equals sign,
otherwise it's ignored by udev as incorrect rule).
--
Peter
^ permalink raw reply
* Fault tolerance in RAID0 with badblocks
From: Ravi (Tom) Hale @ 2017-05-04 10:04 UTC (permalink / raw)
To: linux-raid
Since btrfs doesn't support badblocks, this btrfs mailing list post[1]
suggested to use mdadm RAID0 3.1+.
Is there a way of having blocks from a spare device automatically
replacing bad blocks when they are next written to (like SMART does for
HDDs)?
Or would mdadm be able to add a "badblocks layer" to btrfs in some other
way?
My use case is mining storj - I don't mind some data loss.
[1] https://www.spinics.net/lists/linux-btrfs/msg40909.html
--
Cheers,
Tom Hale
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Roman Mamedov @ 2017-05-04 8:06 UTC (permalink / raw)
To: Andreas Klauer; +Cc: Shaohua Li, linux-raid, jes.sorensen, neilb
In-Reply-To: <20170504075551.GA3929@metamorpher.de>
On Thu, 4 May 2017 09:55:51 +0200
Andreas Klauer <Andreas.Klauer@metamorpher.de> wrote:
> For example? I was under the impression that pretty much all of them do.
> Even the ones that don't advertize it returned zero after trim for me.
> [Sometimes you get original data but that's Linux; gone after drop_caches]
> (Of course, I don't have access to that many different models of SSD...)
>
> But what do they actually return then? Original data?
Consult Wikipedia at least, if not the original ATA standards documents:
https://en.wikipedia.org/wiki/Trim_(computing)#ATA
Can return "undefined", "something that is the same every time" or "zeroes".
Apparently the OS can query which is to be expected with a particular device,
but firstly, you can't necessarily trust that 100%, and secondly, that's
operating on a level lower (ATA) than where md is.
--
With respect,
Roman
^ permalink raw reply
* Fwd: Re: Fwd: Please Help! Degraded Raid6
From: Bernd @ 2017-05-04 7:58 UTC (permalink / raw)
To: linux-raid; +Cc: Andreas.Klauer
In-Reply-To: <05DCE993-EEA8-420F-BB41-63612E361394@gmx.de>
-------- Ursprüngliche Nachricht --------
Von: Bernd <berndman80@gmx.de>
Gesendet: 3. Mai 2017 00:53:04 MESZ
An: Andreas Klauer <Andreas.Klauer@metamorpher.de>
Betreff: Re: Fwd: Please Help! Degraded Raid6
Hello, do this help?
berndman@berndman:~$ sudo mdadm --examine /dev/sd[a-z]1
[sudo] password for berndman:
/dev/sda1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : 529d3611:a91ea3d1:5a643bf9:2c66715c
Update Time : Fri Mar 31 14:56:50 2017
Checksum : c781f805 - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 2
Array State : AAAAAAA. ('A' == active, '.' == missing)
/dev/sdb1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : bb3dc649:52726bf3:5512b2c1:f0a7605e
Update Time : Fri Mar 31 14:56:50 2017
Checksum : ec3fdbec - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 6
Array State : AAAAAAA. ('A' == active, '.' == missing)
/dev/sdc1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : cb730c6a:eb14ee35:4f1b3480:63e037c8
Update Time : Fri Mar 31 14:56:50 2017
Checksum : 7761f5e9 - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 5
Array State : AAAAAAA. ('A' == active, '.' == missing)
/dev/sdd1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : 61ef95f8:b5d2477b:e4a454f1:34f2b27b
Update Time : Fri Mar 31 14:56:50 2017
Checksum : 6fe0caac - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 1
Array State : AAAAAAA. ('A' == active, '.' == missing)
/dev/sde1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : 7c16aad2:99309271:a8805e9a:f8be7e51
Update Time : Fri Mar 31 14:56:50 2017
Checksum : bf14f834 - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 3
Array State : AAAAAAA. ('A' == active, '.' == missing)
/dev/sdf1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : 5c3a8490:3b33d565:4fb9c177:859e3155
Update Time : Fri Mar 31 14:56:50 2017
Checksum : 524836e7 - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 0
Array State : AAAAAAA. ('A' == active, '.' == missing)
mdadm: No md superblock detected on /dev/sdg1.
/dev/sdh1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x2
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
Recovery Offset : 0 sectors
State : clean
Device UUID : 2283bdef:86a7c96b:d8ef0a1b:3651dd62
Update Time : Tue Sep 29 15:46:56 2015
Checksum : 65a01e2f - correct
Events : 843
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 7
Array State : AAAAAAAA ('A' == active, '.' == missing)
/dev/sdi1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : bade4a58:bce5a0a2:121368d5:f6ad5857
Update Time : Fri Mar 31 14:55:06 2017
Checksum : b6a7f695 - correct
Events : 1685
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 4
Array State : AAAAAAA. ('A' == active, '.' == missing)
berndman@berndman:~$
Am 3. Mai 2017 00:01:54 MESZ schrieb Andreas Klauer <Andreas.Klauer@metamorpher.de>:
>On Tue, May 02, 2017 at 11:11:10PM +0200, Bernd wrote:
>> berndman@berndman:~$ sudo mdadm --examine /dev/sd[a-z]
>
>You're using the wrong devices in your commands
>so the output is not the least bit interesting.
>
>> /dev/sda:
>> MBR Magic : aa55
>> Partition[0] : 3907027120 sectors at 2048 (type fd)
>> /dev/sdb:
>> MBR Magic : aa55
>> Partition[0] : 3907027120 sectors at 2048 (type fd)
>
>> /dev/sdh:
>> Magic : a92b4efc
>> Version : 1.2
>> Feature Map : 0x2
>> Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
>
>Well except for this but need all of them.
>
>> Update Time : Fri Jul 10 22:32:34 2015
>
>I hope it's not yet another case of a drive dropped out back
>in 2015 and you never doing anything about it in two years...
>Monitoring and instant email notification, is it that difficult?
>
>Regards
>Andreas Klauer
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Andreas Klauer @ 2017-05-04 7:55 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <20170504022258.eov6xh2zwtbfvcch@kernel.org>
On Wed, May 03, 2017 at 07:22:58PM -0700, Shaohua Li wrote:
>
> Unfortunately not all SSD return zero after trim.
>
For example? I was under the impression that pretty much all of them do.
Even the ones that don't advertize it returned zero after trim for me.
[Sometimes you get original data but that's Linux; gone after drop_caches]
(Of course, I don't have access to that many different models of SSD...)
But what do they actually return then? Original data? This might seem
fine at first but it can't be the case, right? I mean, even if the SSD
does not erase a trimmed block right away, sooner or later it would,
in order to re-use it for new data and wear leveling. If it's never
re-used what is the point of trimming it in the first place?
So it seems to me even if an SSD attempted to keep returning original data
for a while, sooner or later it would be gone and if that happens randomly,
as far as your RAID is concerned you will be in mismatch hell anyways.
So after a full trim, you can skip the initial sync either way.
Mismatches might also be an issue for SSD that do return zero after trim,
but have different erase block sizes / trim boundaries / partition offsets.
Or does read zero after trim actually mean it works at 4K page resolution?
Never mind, then. :-)
Regards
Andreas Klauer
^ permalink raw reply
* Re: DM_INTEGRITY Kconfig help (was: Re: dm: add integrity target)
From: Geert Uytterhoeven @ 2017-05-04 7:39 UTC (permalink / raw)
To: Milan Broz
Cc: Mikulas Patocka, Mike Snitzer, Linux Kernel Mailing List,
linux-raid
In-Reply-To: <6988fc7b-1ab4-4c7a-c7c9-121271ddd970@gmail.com>
Hi Milan,
On Thu, May 4, 2017 at 9:31 AM, Milan Broz <gmazyland@gmail.com> wrote:
> On 05/04/2017 08:03 AM, Geert Uytterhoeven wrote:
>>> +config DM_INTEGRITY
>>> + tristate "Integrity target"
>>> + depends on BLK_DEV_DM
>>> + select BLK_DEV_INTEGRITY
>>> + select DM_BUFIO
>>> + select CRYPTO
>>> + select ASYNC_XOR
>>> + ---help---
>>> + This is the integrity target.
>>
>> Which is...?
>>
>> Can you please extend the help message for the uneducated?
>
> Yes, sorry, this is our oversight.
>
> Mike already promised that this text will be extended in the next patchset.
Thanks, good to know this is planned.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: RAID creation resync behaviors
From: David Brown @ 2017-05-04 7:37 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <20170504015454.d4obiuume6e3yrdv@kernel.org>
On 04/05/17 03:54, Shaohua Li wrote:
> On Wed, May 03, 2017 at 11:06:01PM +0200, David Brown wrote:
>> On 03/05/17 22:27, Shaohua Li wrote:
>>> Hi,
>>>
>>> Currently we have different resync behaviors in array creation.
>>>
>>> - raid1: copy data from disk 0 to disk 1 (overwrite)
>>> - raid10: read both disks, compare and write if there is difference (compare-write)
>>> - raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
>>> - raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
>>>
>>> Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
>>> if user already does a trim before creation, the unncessary write could make
>>> SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
>>> detects the disks are SSD? Surely sometimes compare-write is slower than
>>> overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
>>> before creation sounds reasonable too.
>>>
>>
>> When doing the first sync, md tracks how far its sync has got, keeping a
>> record in the metadata in case it has to be restarted (such as due to a
>> reboot while syncing). Why not simply /not/ sync stripes until you first
>> write to them? It may be that a counter of synced stripes is not enough,
>> and you need a bitmap (like the write intent bitmap), but it would reduce
>> the creation sync time to 0 and avoid any writes at all.
>
> For raid 4/5/6, this means we always must do a full stripe write for any normal
> write if it hits a range not synced. This would harm the performance of the
> norma write.
Agreed. The unused sectors could be set to 0, rather than read from the
disks - that would reduce the latency and be friendly to high-end SSDs
with compression (zero blocks compress quite well!).
> For raid1/10, this sounds more appealing. But since each bit in
> the bitmap will stand for a range. If only part of the range is written by
> normal IO, we have two choices. sync the range immediately and clear the bit,
> this sync will impact normal IO. Don't do the sync immediately, but since the
> bit is set (which means the range isn't synced), read IO can only access the
> first disk, which is harmful too.
>
This could be done in a more sophisticated manner. (Yes, I appreciate
that "sophisticated" or "complex" are a serious disadvantage - I'm just
throwing up ideas that could be considered.)
Divide the array into "sync blocks", each covering a range of stripes,
with a bitmap of three states - unused, partially synced, fully synced.
All blocks start off unused. If a write is made to a previously unused
block, that block becomes partially synced, and the write has to be done
as a full stripe write. For a partially synced block, keep a list of
ranges of synced stripes (a list will normally be smaller than a bitmap
here). Whenever there are partially synced blocks in the array, have a
low priority process (like the normal array creation sync process, or
rebuild processes) sync the stripes until the block is finished as a
fully synced block.
That should let you delay the time-consuming and write intensive
creation sync until you actually need to sync the blocks, without /too/
much overhead in metadata or in delays when using the disk.
I have another couple of questions that might be relevant, but I am
really not sure about the correct answers.
First, if you have a stripe that you know is unused - it has not been
written to since the array was created - could the raid layer safely
return all zeros if an attempt was made to read the stripe?
Second, when syncing an unused stripe (such as during creation), rather
than reading the old data and copying it or generating parities, could
we simply write all zeros to all the blocks in the stripes? For many
SSDs, this is very efficient.
Best regards,
David
^ permalink raw reply
* Re: DM_INTEGRITY Kconfig help (was: Re: dm: add integrity target)
From: Milan Broz @ 2017-05-04 7:31 UTC (permalink / raw)
To: Geert Uytterhoeven, Mikulas Patocka, Mike Snitzer
Cc: Linux Kernel Mailing List, linux-raid
In-Reply-To: <CAMuHMdWafrUq0tbHsOS2Gm579Amv0LpYAqk=Kqg4t8fqgbWDgw@mail.gmail.com>
On 05/04/2017 08:03 AM, Geert Uytterhoeven wrote:
>> +config DM_INTEGRITY
>> + tristate "Integrity target"
>> + depends on BLK_DEV_DM
>> + select BLK_DEV_INTEGRITY
>> + select DM_BUFIO
>> + select CRYPTO
>> + select ASYNC_XOR
>> + ---help---
>> + This is the integrity target.
>
> Which is...?
>
> Can you please extend the help message for the uneducated?
Yes, sorry, this is our oversight.
Mike already promised that this text will be extended in the next patchset.
For now you can find the description in documentation
Documentation/device-mapper/dm-integrity.txt
"The dm-integrity target emulates a block device that has additional
per-sector tags that can be used for storing integrity information."
(The major designed use case is for providing metadata space for tags
for authenticated encryption in dm-crypt but it can be used also as
a standalone target.)
Thanks,
Milan
^ permalink raw reply
* DM_INTEGRITY Kconfig help (was: Re: dm: add integrity target)
From: Geert Uytterhoeven @ 2017-05-04 6:03 UTC (permalink / raw)
To: Mikulas Patocka, Milan Broz, Mike Snitzer
Cc: Linux Kernel Mailing List, linux-raid
On Wed, May 3, 2017 at 7:43 PM, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> Web: https://git.kernel.org/torvalds/c/7eada909bfd7ac90a4522e56aa3179d1fd68cd14
> Commit: 7eada909bfd7ac90a4522e56aa3179d1fd68cd14
> Parent: 400a0befc96240f7bb2a53b9622deffd55d385fe
> Refname: refs/heads/master
> Author: Mikulas Patocka <mpatocka@redhat.com>
> AuthorDate: Wed Jan 4 20:23:53 2017 +0100
> Committer: Mike Snitzer <snitzer@redhat.com>
> CommitDate: Fri Mar 24 15:49:07 2017 -0400
>
> dm: add integrity target
>
> The dm-integrity target emulates a block device that has additional
> per-sector tags that can be used for storing integrity information.
>
> A general problem with storing integrity tags with every sector is that
> writing the sector and the integrity tag must be atomic - i.e. in case of
> crash, either both sector and integrity tag or none of them is written.
>
> To guarantee write atomicity the dm-integrity target uses a journal. It
> writes sector data and integrity tags into a journal, commits the journal
> and then copies the data and integrity tags to their respective location.
>
> The dm-integrity target can be used with the dm-crypt target - in this
> situation the dm-crypt target creates the integrity data and passes them
> to the dm-integrity target via bio_integrity_payload attached to the bio.
> In this mode, the dm-crypt and dm-integrity targets provide authenticated
> disk encryption - if the attacker modifies the encrypted device, an I/O
> error is returned instead of random data.
>
> The dm-integrity target can also be used as a standalone target, in this
> mode it calculates and verifies the integrity tag internally. In this
> mode, the dm-integrity target can be used to detect silent data
> corruption on the disk or in the I/O path.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Signed-off-by: Milan Broz <gmazyland@gmail.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -500,4 +500,14 @@ config DM_LOG_WRITES
>
> If unsure, say N.
>
> +config DM_INTEGRITY
> + tristate "Integrity target"
> + depends on BLK_DEV_DM
> + select BLK_DEV_INTEGRITY
> + select DM_BUFIO
> + select CRYPTO
> + select ASYNC_XOR
> + ---help---
> + This is the integrity target.
Which is...?
Can you please extend the help message for the uneducated?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Shaohua Li @ 2017-05-04 2:22 UTC (permalink / raw)
To: Andreas Klauer; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <20170503235856.GA9698@metamorpher.de>
On Thu, May 04, 2017 at 01:58:56AM +0200, Andreas Klauer wrote:
> On Wed, May 03, 2017 at 01:27:48PM -0700, Shaohua Li wrote:
> > Write whole disk is very unfriendly for SSD, because it reduces lifetime.
> > And if user already does a trim before creation, the unncessary write
> > could make SSD slower in the future.
>
> I'm not a kernel developer so maybe I shouldn't reply. Feel free to ignore.
>
> I don't see this as a big issue, whoever uses SSD will likely also fstrim,
> so all SSD will know about free blocks regardless how the drive was added
> to the RAID.
>
> You don't resync everyday and once populated with data you just can't help
> but have many writes when adding / replacing drives. No way around it.
I agree fstrim doesn't make issue smaller. But there are still extra writes,
which if we can avoid in an easy way, we should do it.
> > An option to let mdadm trim SSD before creation sounds reasonable too.
>
> This is my personal opinion but - there is way too much trim in Linux.
>
> On HDD if you did a botched mkfs on the wrong device you still had a chance
> to recover data, with SSD it's all gone in an eyeblink, because mkfs.ext4
> and other programs unfortunately do trim without asking. Lots of people
> come to this list only after already playing with mdadm --create and if
> mdadm simply started trimming SSDs too, then all would be lost.
> LVM has these nice metadata backups but they're rendered useless
> if lvm.conf has issue_discards set to 1. Etc...
Totally understand the concerns. I think a new option is required for this and
it should not be default.
> And it's entirely superfluous, there was a big hullabaloo when SSD were
> new, everyone was concerned about how quickly they'd die when written to,
> but tests show their endurance is considerably greater than advertized.
> A single RAID resync won't put a dent in even a consumer's SSD lifetime.
From my experience, if my filesystem under a SSD is nearly full, I found the
system is unstable in at least one type of SSD. Fully write a SSD not just
reduces the lifetime, it also makes firmware of SSD has higher chance to fail.
> At the same time you have two utilities blkdiscard and fstrim so anyone
> who desires to trim can already easily do so with little effort. For SSD
> that return zero after TRIM you can already create like this:
>
> blkdiscard device1
> blkdiscard device2
> blkdiscard device3
> echo 3 > /proc/sys/vm/drop_caches # optional: Linux caches trimmed data
> mdadm --create --assume-clean /dev/md ... device1 device2 device3
Unfortunately not all SSD return zero after trim.
> If you wanted mdadm to do that directly, how about a mdadm --create --trim
> which implies assume-clean? But in my opinion it should not happen unasked.
> If it was up to me I'd even add a prompt asking to confirm dataloss...
>
> As for overwrite vs. compare-write, I don't know if it's possible or
> how painful it would be to implement but could you start out comparing,
> continue while the data actually matches, but switch to presumably much
> faster overwrite mode once there are sufficient mismatches? Perhaps with a
> fallback option so it can go back to compare later if data starts to match.
>
> So kind of a smart-compare-overwrite mode which would go something like:
>
> Compare. Match.
> Compare. Match.
> Compare. Mismatch. Overwrite.
> Compare. Mismatch. Overwrite x2.
> Compare. Mismatch. Overwrite x4.
> Compare. Match.
> Compare. Mismatch. Overwrite x8.
> Compare. Mismatch. Overwrite x16.
>
> Perhaps cap the overwrite multiplier at a certain point...
>
> Maybe a silly idea, I don't know.
This certainly is an interesting idea. Not sure if we should put complex
heuristics into kernel side though. If there is easy approach in mdadm side, it
definitely will be preferred.
Thanks,
Shaohua
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Shaohua Li @ 2017-05-04 2:04 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <87inlhpgzu.fsf@notabene.neil.brown.name>
On Thu, May 04, 2017 at 11:07:01AM +1000, Neil Brown wrote:
> On Wed, May 03 2017, Shaohua Li wrote:
>
> > Hi,
> >
> > Currently we have different resync behaviors in array creation.
> >
> > - raid1: copy data from disk 0 to disk 1 (overwrite)
> > - raid10: read both disks, compare and write if there is difference (compare-write)
> > - raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
> > - raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
>
> The approach taken for raid1 and raid4/5 provides the fastest sync for
> an array built on uninitialised spinning devices.
> RAID6 could use the same approach but would involve more CPU and so
> the original author of the RAID6 code (hpa) chose to go for the low-CPU
> cost option. I don't know if tests were done, or if they would still be
> valid on new hardware.
> The raid10 approach comes from "it is too hard to optimize in general
> because different RAID10 layouts have different trade-offs, so just
> take the easy way out."
ok, thanks for the explanation!
> >
> > Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
> > if user already does a trim before creation, the unncessary write could make
> > SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
> > detects the disks are SSD? Surely sometimes compare-write is slower than
> > overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
> > before creation sounds reasonable too.
>
> An option to ask mdadm to trim the data space and then --assume-clean
> certainly sounds reasonable.
This doesn't work well. read returns 0 for trimmed data space in some SSDs, but
not all. If not, we will have trouble.
> One possible approach would be to use compare-write until some
> threshold of writes were crossed, then switch to over-write. That could
> work well for RAID1, but could be awkward to manage for RAID5.
> Possibly mdadm could read the first few megas of each device in RAID5
> and try to guess if many writes will be needed. If they will, the
> current approach is best. If not, assemble the array so that
> compare-write is used.
I think this makes sense if we do trim first, assume in most SSDs read return 0
for trimmed space. Maybe trim first, and check if read returns 0. If returns 0,
do compare-write (even assume-clean), otherwise overwrite.
> I'm in favour of providing options and making the defaults "not
> terrible". I think they currently are "not terrible", but maybe they
> can be better in some cases.
Agree, more options are required.
Thanks,
Shaohua
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Shaohua Li @ 2017-05-04 1:54 UTC (permalink / raw)
To: David Brown; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <fe3c30e4-b0dd-639e-7db7-344aa93ce198@hesbynett.no>
On Wed, May 03, 2017 at 11:06:01PM +0200, David Brown wrote:
> On 03/05/17 22:27, Shaohua Li wrote:
> > Hi,
> >
> > Currently we have different resync behaviors in array creation.
> >
> > - raid1: copy data from disk 0 to disk 1 (overwrite)
> > - raid10: read both disks, compare and write if there is difference (compare-write)
> > - raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
> > - raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
> >
> > Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
> > if user already does a trim before creation, the unncessary write could make
> > SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
> > detects the disks are SSD? Surely sometimes compare-write is slower than
> > overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
> > before creation sounds reasonable too.
> >
>
> When doing the first sync, md tracks how far its sync has got, keeping a
> record in the metadata in case it has to be restarted (such as due to a
> reboot while syncing). Why not simply /not/ sync stripes until you first
> write to them? It may be that a counter of synced stripes is not enough,
> and you need a bitmap (like the write intent bitmap), but it would reduce
> the creation sync time to 0 and avoid any writes at all.
For raid 4/5/6, this means we always must do a full stripe write for any normal
write if it hits a range not synced. This would harm the performance of the
norma write. For raid1/10, this sounds more appealing. But since each bit in
the bitmap will stand for a range. If only part of the range is written by
normal IO, we have two choices. sync the range immediately and clear the bit,
this sync will impact normal IO. Don't do the sync immediately, but since the
bit is set (which means the range isn't synced), read IO can only access the
first disk, which is harmful too.
Thanks,
Shaohua
^ permalink raw reply
* Re: RAID creation resync behaviors
From: NeilBrown @ 2017-05-04 1:07 UTC (permalink / raw)
To: Shaohua Li, linux-raid; +Cc: jes.sorensen, neilb
In-Reply-To: <20170503202748.7r243wj5h4polt6y@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2120 bytes --]
On Wed, May 03 2017, Shaohua Li wrote:
> Hi,
>
> Currently we have different resync behaviors in array creation.
>
> - raid1: copy data from disk 0 to disk 1 (overwrite)
> - raid10: read both disks, compare and write if there is difference (compare-write)
> - raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
> - raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
The approach taken for raid1 and raid4/5 provides the fastest sync for
an array built on uninitialised spinning devices.
RAID6 could use the same approach but would involve more CPU and so
the original author of the RAID6 code (hpa) chose to go for the low-CPU
cost option. I don't know if tests were done, or if they would still be
valid on new hardware.
The raid10 approach comes from "it is too hard to optimize in general
because different RAID10 layouts have different trade-offs, so just
take the easy way out."
>
> Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
> if user already does a trim before creation, the unncessary write could make
> SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
> detects the disks are SSD? Surely sometimes compare-write is slower than
> overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
> before creation sounds reasonable too.
An option to ask mdadm to trim the data space and then --assume-clean
certainly sounds reasonable.
One possible approach would be to use compare-write until some
threshold of writes were crossed, then switch to over-write. That could
work well for RAID1, but could be awkward to manage for RAID5.
Possibly mdadm could read the first few megas of each device in RAID5
and try to guess if many writes will be needed. If they will, the
current approach is best. If not, assemble the array so that
compare-write is used.
I'm in favour of providing options and making the defaults "not
terrible". I think they currently are "not terrible", but maybe they
can be better in some cases.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Andreas Klauer @ 2017-05-03 23:58 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <20170503202748.7r243wj5h4polt6y@kernel.org>
On Wed, May 03, 2017 at 01:27:48PM -0700, Shaohua Li wrote:
> Write whole disk is very unfriendly for SSD, because it reduces lifetime.
> And if user already does a trim before creation, the unncessary write
> could make SSD slower in the future.
I'm not a kernel developer so maybe I shouldn't reply. Feel free to ignore.
I don't see this as a big issue, whoever uses SSD will likely also fstrim,
so all SSD will know about free blocks regardless how the drive was added
to the RAID.
You don't resync everyday and once populated with data you just can't help
but have many writes when adding / replacing drives. No way around it.
> An option to let mdadm trim SSD before creation sounds reasonable too.
This is my personal opinion but - there is way too much trim in Linux.
On HDD if you did a botched mkfs on the wrong device you still had a chance
to recover data, with SSD it's all gone in an eyeblink, because mkfs.ext4
and other programs unfortunately do trim without asking. Lots of people
come to this list only after already playing with mdadm --create and if
mdadm simply started trimming SSDs too, then all would be lost.
LVM has these nice metadata backups but they're rendered useless
if lvm.conf has issue_discards set to 1. Etc...
And it's entirely superfluous, there was a big hullabaloo when SSD were
new, everyone was concerned about how quickly they'd die when written to,
but tests show their endurance is considerably greater than advertized.
A single RAID resync won't put a dent in even a consumer's SSD lifetime.
At the same time you have two utilities blkdiscard and fstrim so anyone
who desires to trim can already easily do so with little effort. For SSD
that return zero after TRIM you can already create like this:
blkdiscard device1
blkdiscard device2
blkdiscard device3
echo 3 > /proc/sys/vm/drop_caches # optional: Linux caches trimmed data
mdadm --create --assume-clean /dev/md ... device1 device2 device3
If you wanted mdadm to do that directly, how about a mdadm --create --trim
which implies assume-clean? But in my opinion it should not happen unasked.
If it was up to me I'd even add a prompt asking to confirm dataloss...
As for overwrite vs. compare-write, I don't know if it's possible or
how painful it would be to implement but could you start out comparing,
continue while the data actually matches, but switch to presumably much
faster overwrite mode once there are sufficient mismatches? Perhaps with a
fallback option so it can go back to compare later if data starts to match.
So kind of a smart-compare-overwrite mode which would go something like:
Compare. Match.
Compare. Match.
Compare. Mismatch. Overwrite.
Compare. Mismatch. Overwrite x2.
Compare. Mismatch. Overwrite x4.
Compare. Match.
Compare. Mismatch. Overwrite x8.
Compare. Mismatch. Overwrite x16.
Perhaps cap the overwrite multiplier at a certain point...
Maybe a silly idea, I don't know.
Regards
Andreas Klauer
^ permalink raw reply
* Re: RAID creation resync behaviors
From: David Brown @ 2017-05-03 21:06 UTC (permalink / raw)
To: Shaohua Li, linux-raid; +Cc: jes.sorensen, neilb
In-Reply-To: <20170503202748.7r243wj5h4polt6y@kernel.org>
On 03/05/17 22:27, Shaohua Li wrote:
> Hi,
>
> Currently we have different resync behaviors in array creation.
>
> - raid1: copy data from disk 0 to disk 1 (overwrite)
> - raid10: read both disks, compare and write if there is difference (compare-write)
> - raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
> - raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
>
> Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
> if user already does a trim before creation, the unncessary write could make
> SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
> detects the disks are SSD? Surely sometimes compare-write is slower than
> overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
> before creation sounds reasonable too.
>
When doing the first sync, md tracks how far its sync has got, keeping a
record in the metadata in case it has to be restarted (such as due to a
reboot while syncing). Why not simply /not/ sync stripes until you
first write to them? It may be that a counter of synced stripes is not
enough, and you need a bitmap (like the write intent bitmap), but it
would reduce the creation sync time to 0 and avoid any writes at all.
David
^ permalink raw reply
* RAID creation resync behaviors
From: Shaohua Li @ 2017-05-03 20:27 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, neilb
Hi,
Currently we have different resync behaviors in array creation.
- raid1: copy data from disk 0 to disk 1 (overwrite)
- raid10: read both disks, compare and write if there is difference (compare-write)
- raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
- raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
if user already does a trim before creation, the unncessary write could make
SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
detects the disks are SSD? Surely sometimes compare-write is slower than
overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
before creation sounds reasonable too.
Thanks,
Shaohua
^ 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