* [PATCH 4/6] Assemble array with write journal
From: Song Liu @ 2015-08-28 23:27 UTC (permalink / raw)
To: linux-raid; +Cc: shli, dan.j.williams, neilb, hch, Song Liu
In-Reply-To: <1440804426-1461372-1-git-send-email-songliubraving@fb.com>
Example output:
./mdadm --assemble /dev/md0 /dev/sd[c-f] /dev/sdb1
mdadm: /dev/md0 has been started with 4 drives and 1 journal.
mdadm checks superblock for journal devices. If the
array appears to have a journal device, but it is not given,
it will complain as
./mdadm --assemble /dev/md0 /dev/sd[c-f]
mdadm: Not safe to assemble with journal device missing, consider --force.
This can be overwritten with --force
./mdadm --assemble /dev/md0 /dev/sd[c-f] --force
mdadm: Force start with missing journal device...
mdadm: /dev/md0 has been started with 4 drives.
Signed-off-by: Shaohua Li <shli@fb.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
Assemble.c | 42 ++++++++++++++++++++++++++++++++++--------
mdadm.h | 3 +++
super1.c | 38 +++++++++++++++++++++++++++++++++-----
3 files changed, 70 insertions(+), 13 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index d9e9001..556a43b 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -735,7 +735,7 @@ static int load_devices(struct devs *devices, char *devmap,
i = devcnt;
else
i = devices[devcnt].i.disk.raid_disk;
- if (i+1 == 0) {
+ if (i+1 == 0 || i == MD_DISK_ROLE_JOURNAL) {
if (nextspare < content->array.raid_disks*2)
nextspare = content->array.raid_disks*2;
i = nextspare++;
@@ -944,6 +944,7 @@ static int start_array(int mdfd,
unsigned int okcnt,
unsigned int sparecnt,
unsigned int rebuilding_cnt,
+ unsigned int journalcnt,
struct context *c,
int clean, char *avail,
int start_partial_ok,
@@ -955,6 +956,22 @@ static int start_array(int mdfd,
int i;
unsigned int req_cnt;
+ if (st->ss->require_journal) {
+ rv = st->ss->require_journal(st);
+ if (rv == 2) {
+ pr_err("BUG: Superblock not loaded in Assemble.c:start_array\n");
+ return 1;
+ }
+
+ if (journalcnt == 0 && rv == 1) {
+ if (!(c->force)) {
+ pr_err("Not safe to assemble with journal device missing, consider --force.\n");
+ return 1;
+ } else
+ pr_err("Force start with missing journal device...\n");
+ }
+ }
+
rv = set_array_info(mdfd, st, content);
if (rv && !err_ok) {
pr_err("failed to set array info for %s: %s\n",
@@ -1032,7 +1049,8 @@ static int start_array(int mdfd,
if (content->array.level == LEVEL_CONTAINER) {
if (c->verbose >= 0) {
pr_err("Container %s has been assembled with %d drive%s",
- mddev, okcnt+sparecnt, okcnt+sparecnt==1?"":"s");
+ mddev, okcnt+sparecnt+journalcnt,
+ okcnt+sparecnt+journalcnt==1?"":"s");
if (okcnt < (unsigned)content->array.raid_disks)
fprintf(stderr, " (out of %d)",
content->array.raid_disks);
@@ -1118,6 +1136,8 @@ static int start_array(int mdfd,
fprintf(stderr, "%s %d rebuilding", sparecnt?",":" and", rebuilding_cnt);
if (sparecnt)
fprintf(stderr, " and %d spare%s", sparecnt, sparecnt==1?"":"s");
+ if (journalcnt == 1)
+ fprintf(stderr, " and 1 journal");
fprintf(stderr, ".\n");
}
if (content->reshape_active &&
@@ -1289,7 +1309,7 @@ int Assemble(struct supertype *st, char *mddev,
int *best = NULL; /* indexed by raid_disk */
int bestcnt = 0;
int devcnt;
- unsigned int okcnt, sparecnt, rebuilding_cnt, replcnt;
+ unsigned int okcnt, sparecnt, rebuilding_cnt, replcnt, journalcnt;
int i;
int was_forced = 0;
int most_recent = 0;
@@ -1530,6 +1550,7 @@ try_again:
okcnt = 0;
replcnt = 0;
sparecnt=0;
+ journalcnt=0;
rebuilding_cnt=0;
for (i=0; i< bestcnt; i++) {
int j = best[i];
@@ -1540,8 +1561,10 @@ try_again:
/* note: we ignore error flags in multipath arrays
* as they don't make sense
*/
- if (content->array.level != LEVEL_MULTIPATH)
- if (!(devices[j].i.disk.state & (1<<MD_DISK_ACTIVE))) {
+ if (content->array.level != LEVEL_MULTIPATH) {
+ if (devices[j].i.disk.state & (1<<MD_DISK_JOURNAL)) {
+ journalcnt++;
+ } else if (!(devices[j].i.disk.state & (1<<MD_DISK_ACTIVE))) {
if (!(devices[j].i.disk.state
& (1<<MD_DISK_FAULTY))) {
devices[j].uptodate = 1;
@@ -1549,6 +1572,7 @@ try_again:
}
continue;
}
+ }
/* If this device thinks that 'most_recent' has failed, then
* we must reject this device.
*/
@@ -1583,7 +1607,7 @@ try_again:
replcnt++;
} else
rebuilding_cnt++;
- } else
+ } else if (devices[j].i.disk.raid_disk != MD_DISK_ROLE_JOURNAL)
sparecnt++;
}
}
@@ -1647,7 +1671,9 @@ try_again:
int j = best[i];
unsigned int desired_state;
- if (i >= content->array.raid_disks * 2)
+ if (devices[j].i.disk.raid_disk == MD_DISK_ROLE_JOURNAL)
+ desired_state = (1<<MD_DISK_JOURNAL);
+ else if (i >= content->array.raid_disks * 2)
desired_state = 0;
else if (i & 1)
desired_state = (1<<MD_DISK_ACTIVE) | (1<<MD_DISK_REPLACEMENT);
@@ -1794,7 +1820,7 @@ try_again:
rv = start_array(mdfd, mddev, content,
st, ident, best, bestcnt,
chosen_drive, devices, okcnt, sparecnt,
- rebuilding_cnt,
+ rebuilding_cnt, journalcnt,
c,
clean, avail, start_partial_ok,
pre_exist != NULL,
diff --git a/mdadm.h b/mdadm.h
index 3cc1532..bc6680f 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -969,6 +969,9 @@ extern struct superswitch {
/* validate container after assemble */
int (*validate_container)(struct mdinfo *info);
+ /* whether the array require a journal device */
+ int (*require_journal)(struct supertype *st);
+
int swapuuid; /* true if uuid is bigending rather than hostendian */
int external;
const char *name; /* canonical metadata name */
diff --git a/super1.c b/super1.c
index 799c86c..810a323 100644
--- a/super1.c
+++ b/super1.c
@@ -138,6 +138,37 @@ struct misc_dev_info {
|MD_FEATURE_NEW_OFFSET \
|MD_FEATURE_JOURNAL \
)
+/* return value:
+ * 0, jouranl not required
+ * 1, journal required
+ * 2, no superblock loated (st->sb == NULL)
+ */
+static int require_journal1(struct supertype *st)
+{
+ struct mdp_superblock_1 *sb = st->sb;
+ int i;
+ if (sb)
+ for (i=0; i<MAX_DEVS; i++) {
+ if (MD_DISK_ROLE_JOURNAL == sb->dev_roles[i])
+ return 1;
+ }
+ else
+ return 2; /* no sb loaded */
+ return 0;
+}
+
+static int role_from_sb(struct mdp_superblock_1 *sb)
+{
+ unsigned int d;
+ int role;
+
+ d = __le32_to_cpu(sb->dev_number);
+ if (d < __le32_to_cpu(sb->max_dev))
+ role = __le16_to_cpu(sb->dev_roles[d]);
+ else
+ role = MD_DISK_ROLE_SPARE;
+ return role;
+}
/* return how many bytes are needed for bitmap, for cluster-md each node
* should have it's own bitmap */
@@ -480,11 +511,7 @@ static void examine_super1(struct supertype *st, char *homehost)
printf(")\n");
#endif
printf(" Device Role : ");
- d = __le32_to_cpu(sb->dev_number);
- if (d < __le32_to_cpu(sb->max_dev))
- role = __le16_to_cpu(sb->dev_roles[d]);
- else
- role = MD_DISK_ROLE_SPARE;
+ role = role_from_sb(sb);
if (role >= MD_DISK_ROLE_FAULTY)
printf("spare\n");
else if (role == MD_DISK_ROLE_JOURNAL)
@@ -2556,6 +2583,7 @@ struct superswitch super1 = {
.locate_bitmap = locate_bitmap1,
.write_bitmap = write_bitmap1,
.free_super = free_super1,
+ .require_journal = require_journal1,
#if __BYTE_ORDER == BIG_ENDIAN
.swapuuid = 0,
#else
--
1.8.1
^ permalink raw reply related
* [PATCH 5/6] Check write journal in incremental
From: Song Liu @ 2015-08-28 23:27 UTC (permalink / raw)
To: linux-raid; +Cc: shli, dan.j.williams, neilb, hch, Song Liu
In-Reply-To: <1440804426-1461372-1-git-send-email-songliubraving@fb.com>
If journal device is missing, do not start the array, and shows:
./mdadm -I /dev/sdf
mdadm: journal device is missing, not safe to start yet.
The array will be started when the journal device is attached with -I
./mdadm -I /dev/sdb1
mdadm: /dev/sdb1 attached to /dev/md/0_0, which has been started.
To force start without journal device:
./mdadm -I /dev/sdf --run
mdadm: Trying to run with missing journal device
mdadm: /dev/sdf attached to /dev/md/0_0, which has been started.
Signed-off-by: Shaohua Li <shli@fb.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
Incremental.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/Incremental.c b/Incremental.c
index 304cc6d..74905e3 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -35,7 +35,7 @@
static int count_active(struct supertype *st, struct mdinfo *sra,
int mdfd, char **availp,
- struct mdinfo *info);
+ struct mdinfo *info, int *journal_device_missing);
static void find_reject(int mdfd, struct supertype *st, struct mdinfo *sra,
int number, __u64 events, int verbose,
char *array_name);
@@ -104,6 +104,7 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
struct map_ent target_array;
int have_target;
char *devname = devlist->devname;
+ int journal_device_missing = 0;
struct createinfo *ci = conf_get_create_info();
@@ -518,7 +519,7 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
sysfs_free(sra);
sra = sysfs_read(mdfd, NULL, (GET_DEVS | GET_STATE |
GET_OFFSET | GET_SIZE));
- active_disks = count_active(st, sra, mdfd, &avail, &info);
+ active_disks = count_active(st, sra, mdfd, &avail, &info, &journal_device_missing);
if (enough(info.array.level, info.array.raid_disks,
info.array.layout, info.array.state & 1,
avail) == 0) {
@@ -548,10 +549,12 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
}
map_unlock(&map);
- if (c->runstop > 0 || active_disks >= info.array.working_disks) {
+ if (c->runstop > 0 || (!journal_device_missing && active_disks >= info.array.working_disks)) {
struct mdinfo *dsk;
/* Let's try to start it */
+ if (journal_device_missing)
+ pr_err("Trying to run with missing journal device\n");
if (info.reshape_active && !(info.reshape_active & RESHAPE_NO_BACKUP)) {
pr_err("%s: This array is being reshaped and cannot be started\n",
chosen_name);
@@ -618,6 +621,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
} else {
if (c->export) {
printf("MD_STARTED=unsafe\n");
+ } else if (journal_device_missing) {
+ pr_err("Journal device is missing, not safe to start yet.\n");
} else if (c->verbose >= 0)
pr_err("%s attached to %s, not enough to start safely.\n",
devname, chosen_name);
@@ -684,7 +689,8 @@ static void find_reject(int mdfd, struct supertype *st, struct mdinfo *sra,
static int count_active(struct supertype *st, struct mdinfo *sra,
int mdfd, char **availp,
- struct mdinfo *bestinfo)
+ struct mdinfo *bestinfo,
+ int *journal_device_missing)
{
/* count how many devices in sra think they are active */
struct mdinfo *d;
@@ -698,6 +704,8 @@ static int count_active(struct supertype *st, struct mdinfo *sra,
int devnum;
int b, i;
int raid_disks = 0;
+ int require_journal_dev = 0;
+ int has_journal_dev = 0;
if (!sra)
return 0;
@@ -718,8 +726,19 @@ static int count_active(struct supertype *st, struct mdinfo *sra,
close(dfd);
if (ok != 0)
continue;
+
+ if (st->ss->require_journal) {
+ require_journal_dev = st->ss->require_journal(st);
+ if (require_journal_dev == 2) {
+ pr_err("BUG: Superblock not loaded in Incremental.c:count_active\n");
+ return 0;
+ }
+ }
+
info.array.raid_disks = raid_disks;
st->ss->getinfo_super(st, &info, devmap + raid_disks * devnum);
+ if (info.disk.raid_disk == MD_DISK_ROLE_JOURNAL)
+ has_journal_dev = 1;
if (!avail) {
raid_disks = info.array.raid_disks;
avail = xcalloc(raid_disks, 1);
@@ -769,6 +788,10 @@ static int count_active(struct supertype *st, struct mdinfo *sra,
replcnt++;
st->ss->free_super(st);
}
+
+ if (require_journal_dev && !has_journal_dev)
+ *journal_device_missing = 1;
+
if (!avail)
return 0;
/* We need to reject any device that thinks the best device is
--
1.8.1
^ permalink raw reply related
* [PATCH 6/6] Add help message and man entry for --write-journal
From: Song Liu @ 2015-08-28 23:27 UTC (permalink / raw)
To: linux-raid; +Cc: shli, dan.j.williams, neilb, hch, Song Liu
In-Reply-To: <1440804426-1461372-1-git-send-email-songliubraving@fb.com>
Signed-off-by: Shaohua Li <shli@fb.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
ReadMe.c | 1 +
mdadm.8.in | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/ReadMe.c b/ReadMe.c
index 10921e3..fb5a671 100644
--- a/ReadMe.c
+++ b/ReadMe.c
@@ -376,6 +376,7 @@ char Help_create[] =
" --name= -N : Textual name for array - max 32 characters\n"
" --bitmap-chunk= : bitmap chunksize in Kilobytes.\n"
" --delay= -d : bitmap update delay in seconds.\n"
+" --write-journal= : Specify journal device for RAID-4/5/6 array\n"
"\n"
;
diff --git a/mdadm.8.in b/mdadm.8.in
index bf3e131..2844039 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -990,6 +990,12 @@ Only works when the array is for clustered environment. It specifies
the maximum number of nodes in the cluster that will use this device
simultaneously. If not specified, this defaults to 4.
+.TP
+.BR \-\-write-journal
+Specify journal device for the RAID-4/5/6 array. The journal device
+should be a SSD with reasonable lifetime.
+
+
.SH For assemble:
.TP
--
1.8.1
^ permalink raw reply related
* DO YOU NEED A LOAN? BUSINESS OR PERSONAL,IF YES APPLY NOW
From: online2130204 @ 2015-08-29 14:50 UTC (permalink / raw)
We Offer personal and business loan at 2% Interest Rate within 1 year to 10 years repayment duration. We offer loan to Individuals and Corporate Bodies. Our loan are well insured for maximum security is our priority.for more info contact us via email: mavinicfastloan@gmail.com
^ permalink raw reply
* Reply Back On-Time
From: Mr. Craig Chen @ 2015-09-01 2:20 UTC (permalink / raw)
I have a Business Proposal for you to handle with me, Get back to me for
more details on this project.
^ permalink raw reply
* Re-add of raid1 drive resulted in strange loss of data on Archlinux?
From: David C. Rankin @ 2015-09-01 5:00 UTC (permalink / raw)
To: mdraid
All,
Here is one I cannot explain. This is a follow-on from the thread "How best
to re-sync raid1 array? zero superblock on removed disk and let it rebuild?"
posted 8/28.
By way of brief background, after moving disks to a highpoint raid controller
to get around an on-board controller failure, all arrays were OK. On a
subsequent reboot it was as if not attempt was made to activate sda7 and the
root partition was operating in degraded mode on sdb7 alone. The consensus was
to fail, remove then --add the device and allow it to re-sync. All worked perfectly.
The problem: After brining sda7 back into the array, several routine updates
were done and the system was rebooted. The system found grub, began to boot,
then crashed "file not found /usr/lib/libkmod.so" -- Huh? (this is with mdadm
3.3.2-2)
Boot the fallback image - same result. Huh? - again. Checking files in
/usr/lib - sure enough there were a number of libraries that were '0' byte
files. (libkmod.so being one of them) Attempting to locate the packages they
belonged to also failed as the package manager had lost all reference to what
package owned the missing files?
Attempts to query the package manager database to simply list the files
associated with packages updated while operating in degraded mode showed the
packages queried to have no associated file. Big Huh?? For example, for those
familiar with Archlinux pacman package manager:
# pacman -Ql unixodbc
#
would list the package as providing no files. This was completely bewildering.
It was like all updates during degraded mode were lost leaving the disks after
re-sync not knowing where or what files were associated with which packages, and
showing all libraries updated in degraded mode as "empty"? (0 bytes).
I have no clue how this can happen. But thinking through the situation, the
only thing that made any sense would be if when re-adding sda7 to the md1 array,
the sync worked the wrong way, updating the good sdb7 to the state of the
re-added sda7 instead of vice-versa??? Is this even possible?
When sda7 was re-added to the system, it was fully allowed to sync before any
additional updates or reboots, so whatever took place that caused the issue,
took place during the re-add. After re-sync, I did scrub the array with ' echo
check > /sys/block/md1/md/sync_action', but I can't see how that would have
cause the loss? Further, when booting to the install media, and during all
subsequent reboots, md1 came up correctly with both sda7 and sdb7 active in the
array. So I'm stumped...
Have there ever been other similar reports? If so, can anyone suggest how
this could have happened? I would really like to avoid a repeat. (thankfully
forcing re-install of all packages updated during degraded mode fixed the most
of the missing files and libraries) There were some 440 libraries that were 0-bytes.
What say the experts? Any idea how something like this can occur? Any
suggestions as to what to check to attempt to confirm/rule-out what happened?
Thanks for any help you can provide. (all up and running well again)
Personalities : [raid1]
md1 : active raid1 sda7[2] sdb7[1]
52396032 blocks super 1.2 [2/2] [UU]
md3 : active raid1 sda6[0] sdb6[1]
1047552 blocks super 1.2 [2/2] [UU]
md0 : active raid1 sdb5[1] sda5[0]
204608 blocks super 1.2 [2/2] [UU]
md2 : active raid1 sdb8[1] sda8[0]
922944192 blocks super 1.2 [2/2] [UU]
bitmap: 0/7 pages [0KB], 65536KB chunk
unused devices: <none>
--
David C. Rankin, J.D.,P.E.
^ permalink raw reply
* Re: [PATCH 2/6] Show device as journal in --detail --examine
From: Christoph Hellwig @ 2015-09-02 6:53 UTC (permalink / raw)
To: Song Liu; +Cc: linux-raid, shli, dan.j.williams, neilb, hch
In-Reply-To: <1440804426-1461372-3-git-send-email-songliubraving@fb.com>
> #define MD_FEATURE_NEW_OFFSET 64 /* new_offset must be honoured */
> +#define MD_FEATURE_JOURNAL 512 /* support write journal */
The kernel patches are using 256 here, so your userspace won't work
with them but rather generate very unhelpful warnings:
[ 16.177710] md: sdc does not have a valid v1.2 superblock, not importing!
[ 16.178634] md: md_import_device returned -22
mdadm: ADD_NEW_DISK for /dev/sdc failed: Invalid argument
^ permalink raw reply
* RE: [PATCH 2/6] Show device as journal in --detail --examine
From: Song Liu @ 2015-09-02 7:04 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-raid@vger.kernel.org, Shaohua Li, dan.j.williams@intel.com,
neilb@suse.de
In-Reply-To: <20150902065303.GA20708@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Tuesday, September 1, 2015 11:53 PM
> To: Song Liu
> Cc: linux-raid@vger.kernel.org; Shaohua Li; dan.j.williams@intel.com;
> neilb@suse.de; hch@infradead.org
> Subject: Re: [PATCH 2/6] Show device as journal in --detail --examine
>
> > #define MD_FEATURE_NEW_OFFSET 64 /* new_offset must
> be honoured */
> > +#define MD_FEATURE_JOURNAL 512 /* support write
> journal */
>
> The kernel patches are using 256 here, so your userspace won't work with them
> but rather generate very unhelpful warnings:
>
> [ 16.177710] md: sdc does not have a valid v1.2 superblock, not importing!
> [ 16.178634] md: md_import_device returned -22
> mdadm: ADD_NEW_DISK for /dev/sdc failed: Invalid argument
The latest kernel patches (now on Neil's devel branch) is using the following. We made this change because 256 is used by MD_FEATURE_CLUSTERED.
#define MD_FEATURE_CLUSTERED 256 /* clustered MD */
#define MD_FEATURE_JOURNAL 512 /* support write cache */
^ permalink raw reply
* Re: [PATCH 2/6] Show device as journal in --detail --examine
From: Christoph Hellwig @ 2015-09-02 7:07 UTC (permalink / raw)
To: Song Liu
Cc: Christoph Hellwig, linux-raid@vger.kernel.org, Shaohua Li,
dan.j.williams@intel.com, neilb@suse.de
In-Reply-To: <C709E4D363AAB64590BFAC54D4C478AA0104B4C7DD@PRN-MBX02-4.TheFacebook.com>
Where did this change happen? The patches posted on Aug 13 still have
256 here.
^ permalink raw reply
* RE: [PATCH 2/6] Show device as journal in --detail --examine
From: Song Liu @ 2015-09-02 7:09 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-raid@vger.kernel.org, Shaohua Li, dan.j.williams@intel.com,
neilb@suse.de
In-Reply-To: <20150902070722.GA14882@infradead.org>
I believe Neil made this change when applying Shaohua's patch to the devel branch.
Thanks,
Song
> Where did this change happen? The patches posted on Aug 13 still have
> 256 here.
^ permalink raw reply
* Expanding RAID0
From: Alexander Afonyashin @ 2015-09-02 8:57 UTC (permalink / raw)
To: linux-raid
Hi,
I added 3rd drive to 2-disk raid0 (with some issues like mdadm hung
and all disks became spares) but now it looks like:
md3 : active raid4 sda4[0] sdc4[3] sdb4[1]
1304305152 blocks super 1.2 level 4, 512k chunk, algorithm 5 [4/3] [UUU_]
(/dev/sdc4 was added). I wonder what should be better to convert raid4
back to raid0 again:
- mdadm -G -l 0 -n 0 /dev/md3
or
- echo 0 > /sys/block/md3/md/level
Which command doesn't start 'sync' process again? (I suspect that
first one does)
Regards,
Alexander Afonyashin
^ permalink raw reply
* fix use after free in raid5-cache
From: Christoph Hellwig @ 2015-09-02 14:14 UTC (permalink / raw)
To: shli, neilb; +Cc: dan.j.williams, linux-raid, Kernel-team
Hi Shaohua, hi Neil,
this series fixes a use after free of the r5l_io_unit structure I ran into
while testing the caching code. The real fix is in patch 3, but other two
patches contain refactoring to enable the fix.
^ permalink raw reply
* [PATCH 1/3] raid5-cache: move functionality out of __r5l_set_io_unit_state
From: Christoph Hellwig @ 2015-09-02 14:14 UTC (permalink / raw)
To: shli, neilb; +Cc: dan.j.williams, linux-raid, Kernel-team
In-Reply-To: <1441203288-22515-1-git-send-email-hch@lst.de>
Just keep __r5l_set_io_unit_state as a small set the state wrapper,
and remove r5l_set_io_unit_state entirely after moving the real
functionality to the two callers that need it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/raid5-cache.c | 84 ++++++++++++++++++++++++------------------------
1 file changed, 42 insertions(+), 42 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 52feb90..9f984f8 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -213,58 +213,33 @@ static void r5l_compress_stripe_end_list(struct r5l_log *log)
}
static void r5l_wake_reclaim(struct r5l_log *log, sector_t space);
-static void __r5l_set_io_unit_state(struct r5l_io_unit *io,
- enum r5l_io_unit_state state)
-{
- struct r5l_log *log = io->log;
+static inline void __r5l_set_io_unit_state(struct r5l_io_unit *io,
+ enum r5l_io_unit_state state)
+{
if (WARN_ON(io->state >= state))
return;
io->state = state;
- if (state == IO_UNIT_IO_END)
- r5l_move_io_unit_list(&log->running_ios, &log->io_end_ios,
- IO_UNIT_IO_END);
- if (state == IO_UNIT_STRIPE_END) {
- struct r5l_io_unit *last;
- sector_t reclaimable_space;
-
- r5l_move_io_unit_list(&log->io_end_ios, &log->stripe_end_ios,
- IO_UNIT_STRIPE_END);
-
- last = list_last_entry(&log->stripe_end_ios,
- struct r5l_io_unit, log_sibling);
- reclaimable_space = r5l_ring_distance(log, log->last_checkpoint,
- last->log_end);
- if (reclaimable_space >= log->max_free_space)
- r5l_wake_reclaim(log, 0);
-
- r5l_compress_stripe_end_list(log);
- }
- wake_up(&io->wait_state);
-}
-
-static void r5l_set_io_unit_state(struct r5l_io_unit *io,
- enum r5l_io_unit_state state)
-{
- struct r5l_log *log = io->log;
- unsigned long flags;
-
- spin_lock_irqsave(&log->io_list_lock, flags);
- __r5l_set_io_unit_state(io, state);
- spin_unlock_irqrestore(&log->io_list_lock, flags);
}
static void r5l_log_endio(struct bio *bio, int error)
{
struct r5l_io_unit *io = bio->bi_private;
struct r5l_log *log = io->log;
+ unsigned long flags;
bio_put(bio);
if (!atomic_dec_and_test(&io->pending_io))
return;
- r5l_set_io_unit_state(io, IO_UNIT_IO_END);
+ spin_lock_irqsave(&log->io_list_lock, flags);
+ __r5l_set_io_unit_state(io, IO_UNIT_IO_END);
+ r5l_move_io_unit_list(&log->running_ios, &log->io_end_ios,
+ IO_UNIT_IO_END);
+ spin_unlock_irqrestore(&log->io_list_lock, flags);
+
+ wake_up(&io->wait_state);
md_wakeup_thread(log->rdev->mddev->thread);
}
@@ -273,6 +248,7 @@ static void r5l_submit_current_io(struct r5l_log *log)
struct r5l_io_unit *io = log->current_io;
struct r5l_meta_block *block;
struct bio *bio;
+ unsigned long flags;
u32 crc;
if (!io)
@@ -284,7 +260,10 @@ static void r5l_submit_current_io(struct r5l_log *log)
block->checksum = cpu_to_le32(crc);
log->current_io = NULL;
- r5l_set_io_unit_state(io, IO_UNIT_IO_START);
+
+ spin_lock_irqsave(&io->log->io_list_lock, flags);
+ __r5l_set_io_unit_state(io, IO_UNIT_IO_START);
+ spin_unlock_irqrestore(&io->log->io_list_lock, flags);
while ((bio = bio_list_pop(&io->bios))) {
/* all IO must start from rdev->data_offset */
@@ -540,18 +519,39 @@ static void r5l_run_no_space_stripes(struct r5l_log *log)
spin_unlock(&log->no_space_stripes_lock);
}
+static void __r5l_stripe_write_finished(struct r5l_io_unit *io)
+{
+ struct r5l_log *log = io->log;
+ struct r5l_io_unit *last;
+ sector_t reclaimable_space;
+ unsigned long flags;
+
+ spin_lock_irqsave(&log->io_list_lock, flags);
+ __r5l_set_io_unit_state(io, IO_UNIT_STRIPE_END);
+ r5l_move_io_unit_list(&log->io_end_ios, &log->stripe_end_ios,
+ IO_UNIT_STRIPE_END);
+ spin_unlock_irqrestore(&log->io_list_lock, flags);
+ wake_up(&io->wait_state);
+
+ last = list_last_entry(&log->stripe_end_ios,
+ struct r5l_io_unit, log_sibling);
+ reclaimable_space = r5l_ring_distance(log, log->last_checkpoint,
+ last->log_end);
+ if (reclaimable_space >= log->max_free_space)
+ r5l_wake_reclaim(log, 0);
+
+ r5l_compress_stripe_end_list(log);
+}
+
void r5l_stripe_write_finished(struct stripe_head *sh)
{
struct r5l_io_unit *io;
- /* Don't support stripe batch */
io = sh->log_io;
- if (!io)
- return;
sh->log_io = NULL;
- if (atomic_dec_and_test(&io->pending_stripe))
- r5l_set_io_unit_state(io, IO_UNIT_STRIPE_END);
+ if (io && atomic_dec_and_test(&io->pending_stripe))
+ __r5l_stripe_write_finished(io);
}
/*
--
1.9.1
^ permalink raw reply related
* [PATCH 2/3] raid5-cache: remove r5l_move_io_unit_list
From: Christoph Hellwig @ 2015-09-02 14:14 UTC (permalink / raw)
To: shli, neilb; +Cc: dan.j.williams, linux-raid, Kernel-team
In-Reply-To: <1441203288-22515-1-git-send-email-hch@lst.de>
We only move I/O unit structures from one list to another under the
io_list_lock, so there is no need to scan the whole list for other
elements of the same state - we only change the state for one of them
at a time.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/raid5-cache.c | 21 ++-------------------
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 9f984f8..8684100 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -168,21 +168,6 @@ static void r5l_free_io_unit(struct r5l_log *log, struct r5l_io_unit *io)
kmem_cache_free(log->io_kc, io);
}
-static void r5l_move_io_unit_list(struct list_head *from, struct list_head *to,
- enum r5l_io_unit_state state)
-{
- struct r5l_io_unit *io;
-
- while (!list_empty(from)) {
- io = list_first_entry(from, struct r5l_io_unit, log_sibling);
- /* don't change list order */
- if (io->state >= state)
- list_move_tail(&io->log_sibling, to);
- else
- break;
- }
-}
-
/*
* We don't want too many io_units reside in stripe_end_ios list, which will
* waste a lot of memory. So we try to remove some. But we must keep at least 2
@@ -235,8 +220,7 @@ static void r5l_log_endio(struct bio *bio, int error)
spin_lock_irqsave(&log->io_list_lock, flags);
__r5l_set_io_unit_state(io, IO_UNIT_IO_END);
- r5l_move_io_unit_list(&log->running_ios, &log->io_end_ios,
- IO_UNIT_IO_END);
+ list_move_tail(&io->log_sibling, &log->io_end_ios);
spin_unlock_irqrestore(&log->io_list_lock, flags);
wake_up(&io->wait_state);
@@ -528,8 +512,7 @@ static void __r5l_stripe_write_finished(struct r5l_io_unit *io)
spin_lock_irqsave(&log->io_list_lock, flags);
__r5l_set_io_unit_state(io, IO_UNIT_STRIPE_END);
- r5l_move_io_unit_list(&log->io_end_ios, &log->stripe_end_ios,
- IO_UNIT_STRIPE_END);
+ list_move_tail(&io->log_sibling, &log->stripe_end_ios);
spin_unlock_irqrestore(&log->io_list_lock, flags);
wake_up(&io->wait_state);
--
1.9.1
^ permalink raw reply related
* [PATCH 3/3] raid5-cache: fix __r5l_stripe_write_finished
From: Christoph Hellwig @ 2015-09-02 14:14 UTC (permalink / raw)
To: shli, neilb; +Cc: dan.j.williams, linux-raid, Kernel-team
In-Reply-To: <1441203288-22515-1-git-send-email-hch@lst.de>
The stripe end processing has two flaws:
- it walks the stripe_end_ios list for finding the last I/O unit
and collapsing unneded ones without taking the proper lock
- it may remove the currently completed io unit, which will cause
a use after free when calling wake_up on io->wait_state
Fix this by ensuring we hold io_list_lock over the whole function,
and rewrite the I/O unit colapsing algorithm to be smarted about
handling the stripe_end_ios list.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/raid5-cache.c | 33 +++++++++++++--------------------
1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 8684100..054bad4 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -174,27 +174,23 @@ static void r5l_free_io_unit(struct r5l_log *log, struct r5l_io_unit *io)
* io_units. The superblock must point to a valid meta, if it's the last meta,
* recovery can scan less
* */
-static void r5l_compress_stripe_end_list(struct r5l_log *log)
+static void r5l_compress_stripe_end_list(struct r5l_log *log,
+ struct r5l_io_unit *last)
{
- struct r5l_io_unit *first, *last, *io;
+ struct r5l_io_unit *first, *cur;
+
+ assert_spin_locked(&log->io_list_lock);
first = list_first_entry(&log->stripe_end_ios,
struct r5l_io_unit, log_sibling);
- last = list_last_entry(&log->stripe_end_ios,
- struct r5l_io_unit, log_sibling);
if (first == last)
return;
- list_del(&first->log_sibling);
- list_del(&last->log_sibling);
- while (!list_empty(&log->stripe_end_ios)) {
- io = list_first_entry(&log->stripe_end_ios,
- struct r5l_io_unit, log_sibling);
- list_del(&io->log_sibling);
- first->log_end = io->log_end;
- r5l_free_io_unit(log, io);
+
+ while ((cur = list_next_entry(first, log_sibling)) != last) {
+ list_del(&cur->log_sibling);
+ first->log_end = cur->log_end;
+ r5l_free_io_unit(log, cur);
}
- list_add_tail(&first->log_sibling, &log->stripe_end_ios);
- list_add_tail(&last->log_sibling, &log->stripe_end_ios);
}
static void r5l_wake_reclaim(struct r5l_log *log, sector_t space);
@@ -506,24 +502,21 @@ static void r5l_run_no_space_stripes(struct r5l_log *log)
static void __r5l_stripe_write_finished(struct r5l_io_unit *io)
{
struct r5l_log *log = io->log;
- struct r5l_io_unit *last;
sector_t reclaimable_space;
unsigned long flags;
spin_lock_irqsave(&log->io_list_lock, flags);
__r5l_set_io_unit_state(io, IO_UNIT_STRIPE_END);
list_move_tail(&io->log_sibling, &log->stripe_end_ios);
- spin_unlock_irqrestore(&log->io_list_lock, flags);
wake_up(&io->wait_state);
- last = list_last_entry(&log->stripe_end_ios,
- struct r5l_io_unit, log_sibling);
reclaimable_space = r5l_ring_distance(log, log->last_checkpoint,
- last->log_end);
+ io->log_end);
if (reclaimable_space >= log->max_free_space)
r5l_wake_reclaim(log, 0);
- r5l_compress_stripe_end_list(log);
+ r5l_compress_stripe_end_list(log, io);
+ spin_unlock_irqrestore(&log->io_list_lock, flags);
}
void r5l_stripe_write_finished(struct stripe_head *sh)
--
1.9.1
^ permalink raw reply related
* Re: fix use after free in raid5-cache
From: Shaohua Li @ 2015-09-02 16:55 UTC (permalink / raw)
To: Christoph Hellwig
Cc: neilb, dan.j.williams, linux-raid, Kernel-team, Song Liu
In-Reply-To: <1441203288-22515-1-git-send-email-hch@lst.de>
On Wed, Sep 02, 2015 at 04:14:45PM +0200, Christoph Hellwig wrote:
> Hi Shaohua, hi Neil,
>
> this series fixes a use after free of the r5l_io_unit structure I ran into
> while testing the caching code. The real fix is in patch 3, but other two
> patches contain refactoring to enable the fix.
Hi Christoph,
Thanks for looking at it. I had some patches hold on my side, which fix
the use after free issue too. I changed the io_unit list handling a
little bit. Specifically making r5l_flush_stripe_to_raid run flush in
asynchronous way and also fix io_unit free issue.
For this patch set, the 1st is a good cleanup. I think the 2 & 3 have
the same issue changing the list order. For example, io_unit A is
dispatched to log earlier than io_unit B, but io_unit B can finish
earlier than io_unit A. If we move io_unit B to io_end_ios first, and
there is a crash, the metadata of io_unit A could be corrupt, recovery
can't find io_unit B. Please see the comments at
r5l_flush_stripe_to_raid().
I can rebase the 1st against my patches, what do you think?
Thanks,
Shaohua
^ permalink raw reply
* Re: fix use after free in raid5-cache
From: Christoph Hellwig @ 2015-09-02 17:22 UTC (permalink / raw)
To: Shaohua Li; +Cc: neilb, dan.j.williams, linux-raid, Kernel-team, Song Liu
In-Reply-To: <20150902165522.GA2792971@devbig257.prn2.facebook.com>
Ok, I spotted the break there now. Feel free to rebase it and please
push your current tree out, as I have a bunch of other changes pending.
^ permalink raw reply
* [PATCH 0/8] raid5-cache fixes
From: Shaohua Li @ 2015-09-02 20:49 UTC (permalink / raw)
To: linux-raid; +Cc: Kernel-team, songliubraving, hch, dan.j.williams, neilb
Fix some bugs, improve performance and add trim/resync support.
Thanks,
Shaohua
Christoph Hellwig (1):
raid5-cache: move functionality out of __r5l_set_io_unit_state
Shaohua Li (7):
md: fix feature map check
raid5: fix build error
raid5-cache: switching to state machine for log disk cache flush
raid5-cache: fix a user-after-free bug
raid5-cache: optimize FLUSH IO with log enabled
md: skip resync for raid array with journal
raid5-cache: add trim support for log
drivers/md/md.c | 6 +-
drivers/md/md.h | 1 +
drivers/md/raid5-cache.c | 254 ++++++++++++++++++++++++++++-------------------
drivers/md/raid5.c | 17 +++-
drivers/md/raid5.h | 1 +
5 files changed, 175 insertions(+), 104 deletions(-)
--
1.8.1
^ permalink raw reply
* [PATCH 1/8] md: fix feature map check
From: Shaohua Li @ 2015-09-02 20:49 UTC (permalink / raw)
To: linux-raid; +Cc: Kernel-team, songliubraving, hch, dan.j.williams, neilb
In-Reply-To: <cover.1441221530.git.shli@fb.com>
the feature map is a __le32, should convert it to cpu endian.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Shaohua Li <shli@fb.com>
---
drivers/md/md.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 8e49ea3..b3f9eed 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1660,7 +1660,7 @@ static int super_1_validate(struct mddev *mddev, struct md_rdev *rdev)
set_bit(Faulty, &rdev->flags);
break;
case MD_DISK_ROLE_JOURNAL: /* journal device */
- if (!(sb->feature_map & MD_FEATURE_JOURNAL)) {
+ if (!(le32_to_cpu(sb->feature_map) & MD_FEATURE_JOURNAL)) {
/* journal device without journal feature */
printk(KERN_WARNING
"md: journal device provided without "
--
1.8.1
^ permalink raw reply related
* [PATCH 2/8] raid5: fix build error
From: Shaohua Li @ 2015-09-02 20:49 UTC (permalink / raw)
To: linux-raid; +Cc: Kernel-team, songliubraving, hch, dan.j.williams, neilb
In-Reply-To: <cover.1441221530.git.shli@fb.com>
xchg doesn't work with u64 in 32-bit, so change the data type to
'unsigned long'. In theory we could overflow here, but it's not worthy
worrying about.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Shaohua Li <shli@fb.com>
---
drivers/md/raid5-cache.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 52feb90..c85d72a 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -69,7 +69,7 @@ struct r5l_log {
struct kmem_cache *io_kc;
struct md_thread *reclaim_thread;
- sector_t reclaim_target; /* number of space that need to be reclaimed.
+ unsigned long reclaim_target; /* number of space that need to be reclaimed.
* if it's 0, reclaim spaces used by io_units
* which are in IO_UNIT_STRIPE_END state (eg,
* reclaim dones't wait for specific io_unit
@@ -701,13 +701,14 @@ static void r5l_reclaim_thread(struct md_thread *thread)
static void r5l_wake_reclaim(struct r5l_log *log, sector_t space)
{
- sector_t target;
+ unsigned long target;
+ unsigned long new = (unsigned long)space; /* overflow in theory */
do {
target = log->reclaim_target;
- if (space < target)
+ if (new < target)
return;
- } while (cmpxchg(&log->reclaim_target, target, space) != target);
+ } while (cmpxchg(&log->reclaim_target, target, new) != target);
md_wakeup_thread(log->reclaim_thread);
}
--
1.8.1
^ permalink raw reply related
* [PATCH 3/8] raid5-cache: switching to state machine for log disk cache flush
From: Shaohua Li @ 2015-09-02 20:49 UTC (permalink / raw)
To: linux-raid; +Cc: Kernel-team, songliubraving, hch, dan.j.williams, neilb
In-Reply-To: <cover.1441221530.git.shli@fb.com>
Before we write stripe data to raid disks, we must guarantee stripe data
is settled down in log disk. To do this, we flush log disk cache and
wait the flush finish. That wait introduces sleep time in raid5d thread
and impact performance. This patch moves the log disk cache flush
process to the stripe handling state machine, which can remove the wait
in raid5d.
Signed-off-by: Shaohua Li <shli@fb.com>
---
drivers/md/raid5-cache.c | 116 ++++++++++++++++++++++++++---------------------
drivers/md/raid5.c | 7 ++-
2 files changed, 71 insertions(+), 52 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index c85d72a..86b836c 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -61,6 +61,10 @@ struct r5l_log {
struct list_head io_end_ios; /* io_units which have been completely
* written to the log but not yet written
* to the RAID */
+ struct list_head flushing_ios; /* io_units which are waiting for log
+ * cache flush */
+ struct list_head flushed_ios; /* io_units which settle down in log disk */
+ struct bio flush_bio;
struct list_head stripe_end_ios; /* io_units which have been
* completely written to the RAID *
* but have not yet been considered *
@@ -113,8 +117,7 @@ enum r5l_io_unit_state {
IO_UNIT_IO_START = 1, /* io_unit bio start writting to log,
* don't accepting new bio */
IO_UNIT_IO_END = 2, /* io_unit bio finish writting to log */
- IO_UNIT_STRIPE_START = 3, /* stripes of io_unit are flushing to raid */
- IO_UNIT_STRIPE_END = 4, /* stripes data finished writting to raid */
+ IO_UNIT_STRIPE_END = 3, /* stripes data finished writting to raid */
};
static sector_t r5l_ring_add(struct r5l_log *log, sector_t start, sector_t inc)
@@ -228,7 +231,7 @@ static void __r5l_set_io_unit_state(struct r5l_io_unit *io,
struct r5l_io_unit *last;
sector_t reclaimable_space;
- r5l_move_io_unit_list(&log->io_end_ios, &log->stripe_end_ios,
+ r5l_move_io_unit_list(&log->flushed_ios, &log->stripe_end_ios,
IO_UNIT_STRIPE_END);
last = list_last_entry(&log->stripe_end_ios,
@@ -554,6 +557,28 @@ void r5l_stripe_write_finished(struct stripe_head *sh)
r5l_set_io_unit_state(io, IO_UNIT_STRIPE_END);
}
+static void r5l_log_flush_endio(struct bio *bio, int error)
+{
+ struct r5l_log *log = container_of(bio, struct r5l_log,
+ flush_bio);
+ unsigned long flags;
+ struct r5l_io_unit *io;
+ struct stripe_head *sh;
+
+ spin_lock_irqsave(&log->io_list_lock, flags);
+ list_for_each_entry(io, &log->flushing_ios, log_sibling) {
+ while (!list_empty(&io->stripe_list)) {
+ sh = list_first_entry(&io->stripe_list,
+ struct stripe_head, log_list);
+ list_del_init(&sh->log_list);
+ set_bit(STRIPE_HANDLE, &sh->state);
+ raid5_release_stripe(sh);
+ }
+ }
+ list_splice_tail_init(&log->flushing_ios, &log->flushed_ios);
+ spin_unlock_irqrestore(&log->io_list_lock, flags);
+}
+
/*
* Starting dispatch IO to raid.
* io_unit(meta) consists of a log. There is one situation we want to avoid. A
@@ -570,44 +595,31 @@ void r5l_stripe_write_finished(struct stripe_head *sh)
* */
void r5l_flush_stripe_to_raid(struct r5l_log *log)
{
- struct r5l_io_unit *io;
- struct stripe_head *sh;
- bool run_stripe;
-
+ bool do_flush;
if (!log)
return;
- spin_lock_irq(&log->io_list_lock);
- run_stripe = !list_empty(&log->io_end_ios);
- spin_unlock_irq(&log->io_list_lock);
-
- if (!run_stripe)
- return;
-
- blkdev_issue_flush(log->rdev->bdev, GFP_NOIO, NULL);
spin_lock_irq(&log->io_list_lock);
- list_for_each_entry(io, &log->io_end_ios, log_sibling) {
- if (io->state >= IO_UNIT_STRIPE_START)
- continue;
- __r5l_set_io_unit_state(io, IO_UNIT_STRIPE_START);
-
- while (!list_empty(&io->stripe_list)) {
- sh = list_first_entry(&io->stripe_list,
- struct stripe_head, log_list);
- list_del_init(&sh->log_list);
- set_bit(STRIPE_HANDLE, &sh->state);
- raid5_release_stripe(sh);
- }
+ /* flush bio is running */
+ if (!list_empty(&log->flushing_ios)) {
+ spin_unlock_irq(&log->io_list_lock);
+ return;
}
+ list_splice_tail_init(&log->io_end_ios, &log->flushing_ios);
+ do_flush = !list_empty(&log->flushing_ios);
spin_unlock_irq(&log->io_list_lock);
+
+ if (!do_flush)
+ return;
+ bio_reset(&log->flush_bio);
+ log->flush_bio.bi_bdev = log->rdev->bdev;
+ log->flush_bio.bi_end_io = r5l_log_flush_endio;
+ submit_bio(WRITE_FLUSH, &log->flush_bio);
}
static void r5l_kick_io_unit(struct r5l_log *log, struct r5l_io_unit *io)
{
- /* the log thread will write the io unit */
- wait_event(io->wait_state, io->state >= IO_UNIT_IO_END);
- if (io->state < IO_UNIT_STRIPE_START)
- r5l_flush_stripe_to_raid(log);
+ md_wakeup_thread(log->rdev->mddev->thread);
wait_event(io->wait_state, io->state >= IO_UNIT_STRIPE_END);
}
@@ -626,6 +638,8 @@ static void r5l_do_reclaim(struct r5l_log *log)
* shouldn't reuse space of an unreclaimable io_unit
* */
while (1) {
+ struct list_head *target_list = NULL;
+
while (!list_empty(&log->stripe_end_ios)) {
io = list_first_entry(&log->stripe_end_ios,
struct r5l_io_unit, log_sibling);
@@ -637,29 +651,26 @@ static void r5l_do_reclaim(struct r5l_log *log)
if (free >= reclaim_target ||
(list_empty(&log->running_ios) &&
list_empty(&log->io_end_ios) &&
- list_empty(&log->stripe_end_ios)))
+ list_empty(&log->flushing_ios) &&
+ list_empty(&log->flushed_ios)))
break;
/* Below waiting mostly happens when we shutdown the raid */
- if (!list_empty(&log->io_end_ios)) {
- io = list_first_entry(&log->io_end_ios,
- struct r5l_io_unit, log_sibling);
- spin_unlock_irq(&log->io_list_lock);
- /* nobody else can delete the io, we are safe */
- r5l_kick_io_unit(log, io);
- spin_lock_irq(&log->io_list_lock);
- continue;
- }
-
- if (!list_empty(&log->running_ios)) {
- io = list_first_entry(&log->running_ios,
- struct r5l_io_unit, log_sibling);
- spin_unlock_irq(&log->io_list_lock);
- /* nobody else can delete the io, we are safe */
- r5l_kick_io_unit(log, io);
- spin_lock_irq(&log->io_list_lock);
- continue;
- }
+ if (!list_empty(&log->flushed_ios))
+ target_list = &log->flushed_ios;
+ else if (!list_empty(&log->flushing_ios))
+ target_list = &log->flushing_ios;
+ else if (!list_empty(&log->io_end_ios))
+ target_list = &log->io_end_ios;
+ else if (!list_empty(&log->running_ios))
+ target_list = &log->running_ios;
+
+ io = list_first_entry(target_list,
+ struct r5l_io_unit, log_sibling);
+ spin_unlock_irq(&log->io_list_lock);
+ /* nobody else can delete the io, we are safe */
+ r5l_kick_io_unit(log, io);
+ spin_lock_irq(&log->io_list_lock);
}
spin_unlock_irq(&log->io_list_lock);
@@ -1048,6 +1059,9 @@ int r5l_init_log(struct r5conf *conf, struct md_rdev *rdev)
INIT_LIST_HEAD(&log->running_ios);
INIT_LIST_HEAD(&log->io_end_ios);
INIT_LIST_HEAD(&log->stripe_end_ios);
+ INIT_LIST_HEAD(&log->flushing_ios);
+ INIT_LIST_HEAD(&log->flushed_ios);
+ bio_init(&log->flush_bio);
log->io_kc = KMEM_CACHE(r5l_io_unit, 0);
if (!log->io_kc)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index cd24d58..b247997 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5758,8 +5758,12 @@ static int handle_active_stripes(struct r5conf *conf, int group,
for (i = 0; i < NR_STRIPE_HASH_LOCKS; i++)
if (!list_empty(temp_inactive_list + i))
break;
- if (i == NR_STRIPE_HASH_LOCKS)
+ if (i == NR_STRIPE_HASH_LOCKS) {
+ spin_unlock_irq(&conf->device_lock);
+ r5l_flush_stripe_to_raid(conf->log);
+ spin_lock_irq(&conf->device_lock);
return batch_size;
+ }
release_inactive = true;
}
spin_unlock_irq(&conf->device_lock);
@@ -5767,6 +5771,7 @@ static int handle_active_stripes(struct r5conf *conf, int group,
release_inactive_stripe_list(conf, temp_inactive_list,
NR_STRIPE_HASH_LOCKS);
+ r5l_flush_stripe_to_raid(conf->log);
if (release_inactive) {
spin_lock_irq(&conf->device_lock);
return 0;
--
1.8.1
^ permalink raw reply related
* [PATCH 4/8] raid5-cache: fix a user-after-free bug
From: Shaohua Li @ 2015-09-02 20:49 UTC (permalink / raw)
To: linux-raid; +Cc: Kernel-team, songliubraving, hch, dan.j.williams, neilb
In-Reply-To: <cover.1441221530.git.shli@fb.com>
r5l_compress_stripe_end_list() can free an io_unit. This breaks the
assumption only reclaimer can free io_unit. We can add a reference count
based io_unit free, but since only reclaim can wait io_unit becoming to
STRIPE_END state, we use a simple global wait queue here.
Signed-off-by: Shaohua Li <shli@fb.com>
---
drivers/md/raid5-cache.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 86b836c..2f5e2b8 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -78,6 +78,7 @@ struct r5l_log {
* which are in IO_UNIT_STRIPE_END state (eg,
* reclaim dones't wait for specific io_unit
* switching to IO_UNIT_STRIPE_END state) */
+ wait_queue_head_t iounit_wait;
struct list_head no_space_stripes; /* pending stripes, log has no space */
spinlock_t no_space_stripes_lock;
@@ -108,7 +109,6 @@ struct r5l_io_unit {
struct list_head stripe_list; /* stripes added to the io_unit */
int state;
- wait_queue_head_t wait_state;
};
/* r5l_io_unit state */
@@ -161,7 +161,6 @@ static struct r5l_io_unit *r5l_alloc_io_unit(struct r5l_log *log)
INIT_LIST_HEAD(&io->log_sibling);
INIT_LIST_HEAD(&io->stripe_list);
io->state = IO_UNIT_RUNNING;
- init_waitqueue_head(&io->wait_state);
return io;
}
@@ -242,8 +241,8 @@ static void __r5l_set_io_unit_state(struct r5l_io_unit *io,
r5l_wake_reclaim(log, 0);
r5l_compress_stripe_end_list(log);
+ wake_up(&log->iounit_wait);
}
- wake_up(&io->wait_state);
}
static void r5l_set_io_unit_state(struct r5l_io_unit *io,
@@ -617,10 +616,11 @@ void r5l_flush_stripe_to_raid(struct r5l_log *log)
submit_bio(WRITE_FLUSH, &log->flush_bio);
}
-static void r5l_kick_io_unit(struct r5l_log *log, struct r5l_io_unit *io)
+static void r5l_kick_io_unit(struct r5l_log *log)
{
md_wakeup_thread(log->rdev->mddev->thread);
- wait_event(io->wait_state, io->state >= IO_UNIT_STRIPE_END);
+ wait_event_lock_irq(log->iounit_wait, !list_empty(&log->stripe_end_ios),
+ log->io_list_lock);
}
static void r5l_write_super(struct r5l_log *log, sector_t cp);
@@ -665,12 +665,7 @@ static void r5l_do_reclaim(struct r5l_log *log)
else if (!list_empty(&log->running_ios))
target_list = &log->running_ios;
- io = list_first_entry(target_list,
- struct r5l_io_unit, log_sibling);
- spin_unlock_irq(&log->io_list_lock);
- /* nobody else can delete the io, we are safe */
- r5l_kick_io_unit(log, io);
- spin_lock_irq(&log->io_list_lock);
+ r5l_kick_io_unit(log);
}
spin_unlock_irq(&log->io_list_lock);
@@ -1071,6 +1066,7 @@ int r5l_init_log(struct r5conf *conf, struct md_rdev *rdev)
log->rdev->mddev, "reclaim");
if (!log->reclaim_thread)
goto reclaim_thread;
+ init_waitqueue_head(&log->iounit_wait);
INIT_LIST_HEAD(&log->no_space_stripes);
spin_lock_init(&log->no_space_stripes_lock);
--
1.8.1
^ permalink raw reply related
* [PATCH 5/8] raid5-cache: move functionality out of __r5l_set_io_unit_state
From: Shaohua Li @ 2015-09-02 20:49 UTC (permalink / raw)
To: linux-raid
Cc: Kernel-team, songliubraving, hch, dan.j.williams, neilb,
Christoph Hellwig
In-Reply-To: <cover.1441221530.git.shli@fb.com>
From: Christoph Hellwig <hch@lst.de>
Just keep __r5l_set_io_unit_state as a small set the state wrapper, and
remove r5l_set_io_unit_state entirely after moving the real
functionality to the two callers that need it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Shaohua Li <shli@fb.com>
---
drivers/md/raid5-cache.c | 81 +++++++++++++++++++++++-------------------------
1 file changed, 39 insertions(+), 42 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 2f5e2b8..5a216ab 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -214,59 +214,31 @@ static void r5l_compress_stripe_end_list(struct r5l_log *log)
list_add_tail(&last->log_sibling, &log->stripe_end_ios);
}
-static void r5l_wake_reclaim(struct r5l_log *log, sector_t space);
static void __r5l_set_io_unit_state(struct r5l_io_unit *io,
enum r5l_io_unit_state state)
{
- struct r5l_log *log = io->log;
-
if (WARN_ON(io->state >= state))
return;
io->state = state;
- if (state == IO_UNIT_IO_END)
- r5l_move_io_unit_list(&log->running_ios, &log->io_end_ios,
- IO_UNIT_IO_END);
- if (state == IO_UNIT_STRIPE_END) {
- struct r5l_io_unit *last;
- sector_t reclaimable_space;
-
- r5l_move_io_unit_list(&log->flushed_ios, &log->stripe_end_ios,
- IO_UNIT_STRIPE_END);
-
- last = list_last_entry(&log->stripe_end_ios,
- struct r5l_io_unit, log_sibling);
- reclaimable_space = r5l_ring_distance(log, log->last_checkpoint,
- last->log_end);
- if (reclaimable_space >= log->max_free_space)
- r5l_wake_reclaim(log, 0);
-
- r5l_compress_stripe_end_list(log);
- wake_up(&log->iounit_wait);
- }
}
-static void r5l_set_io_unit_state(struct r5l_io_unit *io,
- enum r5l_io_unit_state state)
-{
- struct r5l_log *log = io->log;
- unsigned long flags;
-
- spin_lock_irqsave(&log->io_list_lock, flags);
- __r5l_set_io_unit_state(io, state);
- spin_unlock_irqrestore(&log->io_list_lock, flags);
-}
-
-static void r5l_log_endio(struct bio *bio, int error)
+static inline void r5l_log_endio(struct bio *bio, int error)
{
struct r5l_io_unit *io = bio->bi_private;
struct r5l_log *log = io->log;
+ unsigned long flags;
bio_put(bio);
if (!atomic_dec_and_test(&io->pending_io))
return;
- r5l_set_io_unit_state(io, IO_UNIT_IO_END);
+ spin_lock_irqsave(&log->io_list_lock, flags);
+ __r5l_set_io_unit_state(io, IO_UNIT_IO_END);
+ r5l_move_io_unit_list(&log->running_ios, &log->io_end_ios,
+ IO_UNIT_IO_END);
+ spin_unlock_irqrestore(&log->io_list_lock, flags);
+
md_wakeup_thread(log->rdev->mddev->thread);
}
@@ -275,6 +247,7 @@ static void r5l_submit_current_io(struct r5l_log *log)
struct r5l_io_unit *io = log->current_io;
struct r5l_meta_block *block;
struct bio *bio;
+ unsigned long flags;
u32 crc;
if (!io)
@@ -286,7 +259,9 @@ static void r5l_submit_current_io(struct r5l_log *log)
block->checksum = cpu_to_le32(crc);
log->current_io = NULL;
- r5l_set_io_unit_state(io, IO_UNIT_IO_START);
+ spin_lock_irqsave(&log->io_list_lock, flags);
+ __r5l_set_io_unit_state(io, IO_UNIT_IO_START);
+ spin_unlock_irqrestore(&log->io_list_lock, flags);
while ((bio = bio_list_pop(&io->bios))) {
/* all IO must start from rdev->data_offset */
@@ -451,6 +426,7 @@ static void r5l_log_stripe(struct r5l_log *log, struct stripe_head *sh,
sh->log_io = io;
}
+static void r5l_wake_reclaim(struct r5l_log *log, sector_t space);
/*
* running in raid5d, where reclaim could wait for raid5d too (when it flushes
* data from log to raid disks), so we shouldn't wait for reclaim here
@@ -542,18 +518,39 @@ static void r5l_run_no_space_stripes(struct r5l_log *log)
spin_unlock(&log->no_space_stripes_lock);
}
+static void __r5l_stripe_write_finished(struct r5l_io_unit *io)
+{
+ struct r5l_log *log = io->log;
+ struct r5l_io_unit *last;
+ sector_t reclaimable_space;
+ unsigned long flags;
+
+ spin_lock_irqsave(&log->io_list_lock, flags);
+ __r5l_set_io_unit_state(io, IO_UNIT_STRIPE_END);
+ r5l_move_io_unit_list(&log->flushed_ios, &log->stripe_end_ios,
+ IO_UNIT_STRIPE_END);
+
+ last = list_last_entry(&log->stripe_end_ios,
+ struct r5l_io_unit, log_sibling);
+ reclaimable_space = r5l_ring_distance(log, log->last_checkpoint,
+ last->log_end);
+ if (reclaimable_space >= log->max_free_space)
+ r5l_wake_reclaim(log, 0);
+
+ r5l_compress_stripe_end_list(log);
+ spin_unlock_irqrestore(&log->io_list_lock, flags);
+ wake_up(&log->iounit_wait);
+}
+
void r5l_stripe_write_finished(struct stripe_head *sh)
{
struct r5l_io_unit *io;
- /* Don't support stripe batch */
io = sh->log_io;
- if (!io)
- return;
sh->log_io = NULL;
- if (atomic_dec_and_test(&io->pending_stripe))
- r5l_set_io_unit_state(io, IO_UNIT_STRIPE_END);
+ if (io && atomic_dec_and_test(&io->pending_stripe))
+ __r5l_stripe_write_finished(io);
}
static void r5l_log_flush_endio(struct bio *bio, int error)
--
1.8.1
^ permalink raw reply related
* [PATCH 6/8] raid5-cache: optimize FLUSH IO with log enabled
From: Shaohua Li @ 2015-09-02 20:49 UTC (permalink / raw)
To: linux-raid; +Cc: Kernel-team, songliubraving, hch, dan.j.williams, neilb
In-Reply-To: <cover.1441221530.git.shli@fb.com>
With log enabled, bio is written to raid disks after the bio is settled
down in log disk. The recovery guarantees we can recovery the bio data
from log disk, so we we skip FLUSH IO.
Signed-off-by: Shaohua Li <shli@fb.com>
---
drivers/md/raid5-cache.c | 18 ++++++++++++++++++
drivers/md/raid5.c | 10 ++++++++--
drivers/md/raid5.h | 1 +
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 5a216ab..27fb513 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -502,6 +502,24 @@ void r5l_write_stripe_run(struct r5l_log *log)
mutex_unlock(&log->io_mutex);
}
+int r5l_handle_flush_request(struct r5l_log *log, struct bio *bio)
+{
+ if (!log)
+ return -ENODEV;
+ /*
+ * we flush log disk cache first, then write stripe data to raid disks.
+ * So if bio is finished, the log disk cache is flushed already. The
+ * recovery guarantees we can recovery the bio from log disk, so we
+ * don't need to flush again
+ * */
+ if (bio->bi_iter.bi_size == 0) {
+ bio_endio(bio, 0);
+ return 0;
+ }
+ bio->bi_rw &= ~REQ_FLUSH;
+ return -EAGAIN;
+}
+
/* This will run after log space is reclaimed */
static void r5l_run_no_space_stripes(struct r5l_log *log)
{
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index b247997..394cdf8 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5166,8 +5166,14 @@ static void make_request(struct mddev *mddev, struct bio * bi)
bool do_prepare;
if (unlikely(bi->bi_rw & REQ_FLUSH)) {
- md_flush_request(mddev, bi);
- return;
+ int ret = r5l_handle_flush_request(conf->log, bi);
+ if (ret == 0)
+ return;
+ if (ret == -ENODEV) {
+ md_flush_request(mddev, bi);
+ return;
+ }
+ /* ret == -EAGAIN, fallback */
}
md_write_start(mddev, bi);
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 7ecd7d4..e6b9a40 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -629,4 +629,5 @@ extern int r5l_write_stripe(struct r5l_log *log, struct stripe_head *head_sh);
extern void r5l_write_stripe_run(struct r5l_log *log);
extern void r5l_flush_stripe_to_raid(struct r5l_log *log);
extern void r5l_stripe_write_finished(struct stripe_head *sh);
+extern int r5l_handle_flush_request(struct r5l_log *log, struct bio *bio);
#endif
--
1.8.1
^ permalink raw reply related
* [PATCH 7/8] md: skip resync for raid array with journal
From: Shaohua Li @ 2015-09-02 20:49 UTC (permalink / raw)
To: linux-raid; +Cc: Kernel-team, songliubraving, hch, dan.j.williams, neilb
In-Reply-To: <cover.1441221530.git.shli@fb.com>
If a raid array has journal, the journal can guarantee the consistency,
we can skip resync after a unclean shutdown. The exception is raid
creation or user initiated resync, which we still do a raid resync.
Signed-off-by: Shaohua Li <shli@fb.com>
---
drivers/md/md.c | 4 ++++
drivers/md/md.h | 1 +
2 files changed, 5 insertions(+)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index b3f9eed..95824fb 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1669,6 +1669,8 @@ static int super_1_validate(struct mddev *mddev, struct md_rdev *rdev)
}
set_bit(Journal, &rdev->flags);
rdev->journal_tail = le64_to_cpu(sb->journal_tail);
+ if (mddev->recovery_cp == MaxSector)
+ set_bit(MD_JOURNAL_CLEAN, &mddev->flags);
break;
default:
rdev->saved_raid_disk = role;
@@ -1711,6 +1713,8 @@ static void super_1_sync(struct mddev *mddev, struct md_rdev *rdev)
sb->events = cpu_to_le64(mddev->events);
if (mddev->in_sync)
sb->resync_offset = cpu_to_le64(mddev->recovery_cp);
+ else if (test_bit(MD_JOURNAL_CLEAN, &mddev->flags))
+ sb->resync_offset = cpu_to_le64(MaxSector);
else
sb->resync_offset = cpu_to_le64(0);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 226f4ba..0288a0b 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -236,6 +236,7 @@ struct mddev {
#define MD_STILL_CLOSED 4 /* If set, then array has not been opened since
* md_ioctl checked on it.
*/
+#define MD_JOURNAL_CLEAN 5 /* A raid with journal is already clean */
int suspended;
atomic_t active_io;
--
1.8.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox