* Off-Topic Write cache disabling?
From: Weedy @ 2015-02-03 17:45 UTC (permalink / raw)
To: linux-raid
Is there a kernel option or sysfs toggle that disables write caching?
Or forces the kernel to commit everything constantly.
------
I don't really want to join another ML, especially a higher traffic
one just to ask this when it only bugs me sometimes. But I'll shut up
if this is unwanted.
------
I use a similar kernel config with respect to selected options on all
my systems but this only effect my laptop.
On any of my personal system or system I have remote access too
nr_dirty drifts up and down and nr_writeback stays around 0 (assuming
the system isn't working hard).
On my laptop both nr_dirty and nr_writeback stay at 0. I can make them
go up to 10ish if I untar something but then almost immediately go
back to 0. If I didn't know better I would swear dirty_*_centisecs or
something was set to a near instant commit interval but I haven't
found evidence of that. The hard drive light blinks almost constantly
once a second, even if I'm at a X login screen.
As I said this doesn't bug me most of the time but if I let my FF
session get too large or start multiple VMs anything that might make
me swap a little, the machine pretty much dies from IOWAIT. Which I'm
guessing is because it's trying to flush (syncfs?) imediately and
constantly.
You guys spend all day in the IO subsystem, any idea where I can keep
looking? It has persisted across reboots and kernel updates.
^ permalink raw reply
* Re: Off-Topic Write cache disabling?
From: Roger Heflin @ 2015-02-03 18:49 UTC (permalink / raw)
To: Weedy; +Cc: Linux RAID
In-Reply-To: <CAFE24U1G9+bqTzK4eD-cenyc5CGQQdrcu7S9Ks4CsHPS9wJ4zA@mail.gmail.com>
In general if you completely disable write caching things will likely
be unusable. When I have benchmarked things attempting to turn off
write cache performance has been mostly unusable.
I usually change /etc/sysctl.conf and change these entries to control
the amount of dirty cache.
vm.dirty_background_bytes = 25000000
vm.dirty_bytes = 50000000
That says max dirty allowed is 50mb, and it syncs down to 25mb when it syncs.
If you want to tighten things up I would lower those numbers down to
say 10mb/5mb and that should limit how much you can have in the dirty
cache.
You can obviously go lower, but I would expect issues if you get too
low, but I don't know what too low is.
On Tue, Feb 3, 2015 at 11:45 AM, Weedy <weedy2887@gmail.com> wrote:
> Is there a kernel option or sysfs toggle that disables write caching?
> Or forces the kernel to commit everything constantly.
>
> ------
> I don't really want to join another ML, especially a higher traffic
> one just to ask this when it only bugs me sometimes. But I'll shut up
> if this is unwanted.
> ------
>
> I use a similar kernel config with respect to selected options on all
> my systems but this only effect my laptop.
> On any of my personal system or system I have remote access too
> nr_dirty drifts up and down and nr_writeback stays around 0 (assuming
> the system isn't working hard).
> On my laptop both nr_dirty and nr_writeback stay at 0. I can make them
> go up to 10ish if I untar something but then almost immediately go
> back to 0. If I didn't know better I would swear dirty_*_centisecs or
> something was set to a near instant commit interval but I haven't
> found evidence of that. The hard drive light blinks almost constantly
> once a second, even if I'm at a X login screen.
> As I said this doesn't bug me most of the time but if I let my FF
> session get too large or start multiple VMs anything that might make
> me swap a little, the machine pretty much dies from IOWAIT. Which I'm
> guessing is because it's trying to flush (syncfs?) imediately and
> constantly.
>
> You guys spend all day in the IO subsystem, any idea where I can keep
> looking? It has persisted across reboots and kernel updates.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Off-Topic Write cache disabling?
From: Alireza Haghdoost @ 2015-02-03 19:16 UTC (permalink / raw)
To: Roger Heflin; +Cc: Weedy, Linux RAID
In-Reply-To: <CAAMCDedvF+Nv9LYqM7Ddgg2xMQKtgQDntOsG0krxWCV4H+zQmA@mail.gmail.com>
> and it syncs down to 25mb when it syncs.
What do you mean by "syncs down to 25mb" ?
^ permalink raw reply
* [GIT PULL REQUEST] 2 fixes for md in 3.19
From: NeilBrown @ 2015-02-03 21:35 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux RAID, lkml, Fengguang Wu, Mikulas Patocka,
Heinz Mauelshagen, Alasdair G Kergon, Peter Zijlstra
[-- Attachment #1: Type: text/plain, Size: 905 bytes --]
The following changes since commit 59343cd7c4809cf7598789e1cd14563780ae4239:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2015-01-27 13:55:36 -0800)
are available in the git repository at:
git://neil.brown.name/md/ tags/md/3.19-fixes
for you to fetch changes up to d95901433436aeb921eac58bfd8a2aa77f110384:
md/bitmap: fix a might_sleep() warning. (2015-02-02 17:08:03 +1100)
----------------------------------------------------------------
Two fixes for md
1/ Another live lock, needs backporting
2/ work-around false positive with new warnings.
----------------------------------------------------------------
NeilBrown (2):
md/raid5: fix another livelock caused by non-aligned writes.
md/bitmap: fix a might_sleep() warning.
drivers/md/bitmap.c | 13 +++++++++++++
drivers/md/raid5.c | 5 +++++
2 files changed, 18 insertions(+)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* [md PATCH 00/26] Pending md patches.
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
I probably don't post my pending patches as often as I
should ... I think I got out of the habit somewhere there.
So I'll try to do better.
Following are a collection of patches that I hope to submit
for 3.20. There is lots of clean-up and some re-arranging
of locking to make sure that accesses to /proc/mdstat and
most accesses to sysfs files never block.
Review and testing most welcome.
All of these are in my "for-next" branch, and so are in
linux-next.
Thanks,
NeilBrown
---
Hannes Reinecke (1):
md: wakeup thread upon rdev_dec_pending()
Jes Sorensen (1):
md: do_release_stripe(): No need to call md_wakeup_thread() twice
NeilBrown (24):
md/raid5: separate large if clause out of fetch_block().
md/raid5: separate out the easy conditions in need_this_block.
md/raid5: need_this_block: start simplifying the last two conditions.
md/raid5: need_this_block: tidy/fix last condition.
md: rename mddev->write_lock to mddev->lock
md: make ->congested robust against personality changes.
md: make merge_bvec_fn more robust in face of personality changes.
md/linear: remove rcu protections in favour of suspend/resume
md: split detach operation out from ->stop.
md: rename ->stop to ->free
md: level_store: group all important changes into one place.
md: protect ->pers changes with mddev->lock
md/bitmap: protect clearing of ->bitmap by mddev->lock
md: remove need for mddev_lock() in md_seq_show()
md/raid5: use ->lock to protect accessing raid5 sysfs attributes.
md: remove mddev_lock() from md_attr_show()
md: remove mddev_lock from rdev_attr_show()
md: remove unnecessary 'buf' from get_bitmap_file.
md: tidy up set_bitmap_file
md: move GET_BITMAP_FILE ioctl out from mddev_lock.
md: minor cleanup in safe_delay_store.
md: use mddev->lock to protect updates to resync_{min,max}.
md: move mddev_lock and related to md.h
md: make reconfig_mutex optional for writes to md sysfs files.
drivers/md/bitmap.c | 15 +
drivers/md/dm-raid.c | 8
drivers/md/faulty.c | 8
drivers/md/linear.c | 67 +---
drivers/md/md.c | 814 ++++++++++++++++++++++++++++++------------------
drivers/md/md.h | 57 +++
drivers/md/multipath.c | 22 -
drivers/md/raid0.c | 29 +-
drivers/md/raid1.c | 52 +--
drivers/md/raid1.h | 3
drivers/md/raid10.c | 37 --
drivers/md/raid10.h | 3
drivers/md/raid5.c | 334 ++++++++++++--------
drivers/md/raid5.h | 1
14 files changed, 848 insertions(+), 602 deletions(-)
--
Signature
^ permalink raw reply
* [md PATCH 01/26] md: do_release_stripe(): No need to call md_wakeup_thread() twice
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
67f455486d2ea20b2d94d6adf5b9b783d079e321 introduced a call to
md_wakeup_thread() when adding to the delayed_list. However the md
thread is woken up unconditionally just below.
Remove the unnecessary wakeup call.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/raid5.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index b98765f6f77f..274db1834d43 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -296,12 +296,9 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
BUG_ON(atomic_read(&conf->active_stripes)==0);
if (test_bit(STRIPE_HANDLE, &sh->state)) {
if (test_bit(STRIPE_DELAYED, &sh->state) &&
- !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) {
+ !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
list_add_tail(&sh->lru, &conf->delayed_list);
- if (atomic_read(&conf->preread_active_stripes)
- < IO_THRESHOLD)
- md_wakeup_thread(conf->mddev->thread);
- } else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
+ else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
sh->bm_seq - conf->seq_write > 0)
list_add_tail(&sh->lru, &conf->bitmap_list);
else {
^ permalink raw reply related
* [md PATCH 02/26] md/raid5: separate large if clause out of fetch_block().
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
fetch_block() has a very large and hard to read 'if' condition.
Separate it into its own function so that it can be
made more readable.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/raid5.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 274db1834d43..5a980fe7c982 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2895,14 +2895,14 @@ static int want_replace(struct stripe_head *sh, int disk_idx)
* Returns 1 when no more member devices need to be checked, otherwise returns
* 0 to tell the loop in handle_stripe_fill to continue
*/
-static int fetch_block(struct stripe_head *sh, struct stripe_head_state *s,
- int disk_idx, int disks)
+
+static int need_this_block(struct stripe_head *sh, struct stripe_head_state *s,
+ int disk_idx, int disks)
{
struct r5dev *dev = &sh->dev[disk_idx];
struct r5dev *fdev[2] = { &sh->dev[s->failed_num[0]],
&sh->dev[s->failed_num[1]] };
- /* is the data in this block needed, and can we get it? */
if (!test_bit(R5_LOCKED, &dev->flags) &&
!test_bit(R5_UPTODATE, &dev->flags) &&
(dev->toread ||
@@ -2919,7 +2919,18 @@ static int fetch_block(struct stripe_head *sh, struct stripe_head_state *s,
&& s->failed && s->to_write &&
(s->to_write - s->non_overwrite <
sh->raid_conf->raid_disks - sh->raid_conf->max_degraded) &&
- (!test_bit(R5_Insync, &dev->flags) || test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))))) {
+ (!test_bit(R5_Insync, &dev->flags) || test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)))))
+ return 1;
+ return 0;
+}
+
+static int fetch_block(struct stripe_head *sh, struct stripe_head_state *s,
+ int disk_idx, int disks)
+{
+ struct r5dev *dev = &sh->dev[disk_idx];
+
+ /* is the data in this block needed, and can we get it? */
+ if (need_this_block(sh, s, disk_idx, disks)) {
/* we would like to get this block, possibly by computing it,
* otherwise read it if the backing disk is insync
*/
^ permalink raw reply related
* [md PATCH 03/26] md/raid5: separate out the easy conditions in need_this_block.
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
Some of the conditions in need_this_block have very straight
forward motivation. Separate those out and document them.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/raid5.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 5a980fe7c982..21c1e796a0d6 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2903,14 +2903,34 @@ static int need_this_block(struct stripe_head *sh, struct stripe_head_state *s,
struct r5dev *fdev[2] = { &sh->dev[s->failed_num[0]],
&sh->dev[s->failed_num[1]] };
- if (!test_bit(R5_LOCKED, &dev->flags) &&
- !test_bit(R5_UPTODATE, &dev->flags) &&
- (dev->toread ||
- (dev->towrite && !test_bit(R5_OVERWRITE, &dev->flags)) ||
- s->syncing || s->expanding ||
- (s->replacing && want_replace(sh, disk_idx)) ||
- (s->failed >= 1 && fdev[0]->toread) ||
- (s->failed >= 2 && fdev[1]->toread) ||
+
+ if (test_bit(R5_LOCKED, &dev->flags) ||
+ test_bit(R5_UPTODATE, &dev->flags))
+ /* No point reading this as we already have it or have
+ * decided to get it.
+ */
+ return 0;
+
+ if (dev->toread ||
+ (dev->towrite && !test_bit(R5_OVERWRITE, &dev->flags)))
+ /* We need this block to directly satisfy a request */
+ return 1;
+
+ if (s->syncing || s->expanding ||
+ (s->replacing && want_replace(sh, disk_idx)))
+ /* When syncing, or expanding we read everything.
+ * When replacing, we need the replaced block.
+ */
+ return 1;
+
+ if ((s->failed >= 1 && fdev[0]->toread) ||
+ (s->failed >= 2 && fdev[1]->toread))
+ /* If we want to read from a failed device, then
+ * we need to actually read every other device.
+ */
+ return 1;
+
+ if (
(sh->raid_conf->level <= 5 && s->failed && fdev[0]->towrite &&
(!test_bit(R5_Insync, &dev->flags) || test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) &&
!test_bit(R5_OVERWRITE, &fdev[0]->flags)) ||
@@ -2919,7 +2939,7 @@ static int need_this_block(struct stripe_head *sh, struct stripe_head_state *s,
&& s->failed && s->to_write &&
(s->to_write - s->non_overwrite <
sh->raid_conf->raid_disks - sh->raid_conf->max_degraded) &&
- (!test_bit(R5_Insync, &dev->flags) || test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)))))
+ (!test_bit(R5_Insync, &dev->flags) || test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))))
return 1;
return 0;
}
^ permalink raw reply related
* [md PATCH 04/26] md/raid5: need_this_block: start simplifying the last two conditions.
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
Both the last two cases are only relevant if something has failed and
something needs to be written (but not over-written), and if it is OK
to pre-read blocks at this point. So factor out those tests and
explain them.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/raid5.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 21c1e796a0d6..bb42551c1a42 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2930,16 +2930,34 @@ static int need_this_block(struct stripe_head *sh, struct stripe_head_state *s,
*/
return 1;
+ /* Sometimes neither read-modify-write nor reconstruct-write
+ * cycles can work. In those cases we read every block we
+ * can. Then the parity-update is certain to have enough to
+ * work with.
+ * This can only be a problem when we need to write something,
+ * and some device has failed. If either of those tests
+ * fail we need look no further.
+ */
+ if (!s->failed || !s->to_write)
+ return 0;
+
+ if (test_bit(R5_Insync, &dev->flags) &&
+ !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
+ /* Pre-reads at not permitted until after short delay
+ * to gather multiple requests. However if this
+ * device is no Insync, the block could only be be computed
+ * and there is no need to delay that.
+ */
+ return 0;
if (
- (sh->raid_conf->level <= 5 && s->failed && fdev[0]->towrite &&
- (!test_bit(R5_Insync, &dev->flags) || test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) &&
+ (sh->raid_conf->level <= 5 && fdev[0]->towrite &&
!test_bit(R5_OVERWRITE, &fdev[0]->flags)) ||
((sh->raid_conf->level == 6 ||
sh->sector >= sh->raid_conf->mddev->recovery_cp)
- && s->failed && s->to_write &&
+ &&
(s->to_write - s->non_overwrite <
- sh->raid_conf->raid_disks - sh->raid_conf->max_degraded) &&
- (!test_bit(R5_Insync, &dev->flags) || test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))))
+ sh->raid_conf->raid_disks - sh->raid_conf->max_degraded)
+ ))
return 1;
return 0;
}
^ permalink raw reply related
* [md PATCH 05/26] md/raid5: need_this_block: tidy/fix last condition.
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
That last condition is unclear and over cautious.
There are two related issues here.
If a partial write is destined for a missing device, then
either RMW or RCW can work. We must read all the available
block. Only then can the missing blocks be calculated, and
then the parity update performed.
If RMW is not an option, then there is a complication even
without partial writes. If we would need to read a missing
device to perform the reconstruction, then we must first read every
block so the missing device data can be computed.
This is the case for RAID6 (Which currently does not support
RMW) and for times when we don't trust the parity (after a crash)
and so are in the process of resyncing it.
So make these two cases more clear and separate, and perform
the relevant tests more thoroughly.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/raid5.c | 42 ++++++++++++++++++++++++++++++++----------
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index bb42551c1a42..a03cf2d889bf 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2902,6 +2902,7 @@ static int need_this_block(struct stripe_head *sh, struct stripe_head_state *s,
struct r5dev *dev = &sh->dev[disk_idx];
struct r5dev *fdev[2] = { &sh->dev[s->failed_num[0]],
&sh->dev[s->failed_num[1]] };
+ int i;
if (test_bit(R5_LOCKED, &dev->flags) ||
@@ -2949,16 +2950,37 @@ static int need_this_block(struct stripe_head *sh, struct stripe_head_state *s,
* and there is no need to delay that.
*/
return 0;
- if (
- (sh->raid_conf->level <= 5 && fdev[0]->towrite &&
- !test_bit(R5_OVERWRITE, &fdev[0]->flags)) ||
- ((sh->raid_conf->level == 6 ||
- sh->sector >= sh->raid_conf->mddev->recovery_cp)
- &&
- (s->to_write - s->non_overwrite <
- sh->raid_conf->raid_disks - sh->raid_conf->max_degraded)
- ))
- return 1;
+
+ for (i = 0; i < s->failed; i++) {
+ if (fdev[i]->towrite &&
+ !test_bit(R5_UPTODATE, &fdev[i]->flags) &&
+ !test_bit(R5_OVERWRITE, &fdev[i]->flags))
+ /* If we have a partial write to a failed
+ * device, then we will need to reconstruct
+ * the content of that device, so all other
+ * devices must be read.
+ */
+ return 1;
+ }
+
+ /* If we are forced to do a reconstruct-write, either because
+ * the current RAID6 implementation only supports that, or
+ * or because parity cannot be trusted and we are currently
+ * recovering it, there is extra need to be careful.
+ * If one of the devices that we would need to read, because
+ * it is not being overwritten (and maybe not written at all)
+ * is missing/faulty, then we need to read everything we can.
+ */
+ if (sh->raid_conf->level != 6 &&
+ sh->sector < sh->raid_conf->mddev->recovery_cp)
+ /* reconstruct-write isn't being forced */
+ return 0;
+ for (i = 0; i < s->failed; i++) {
+ if (!test_bit(R5_UPTODATE, &fdev[i]->flags) &&
+ !test_bit(R5_OVERWRITE, &fdev[i]->flags))
+ return 1;
+ }
+
return 0;
}
^ permalink raw reply related
* [md PATCH 06/26] md: rename mddev->write_lock to mddev->lock
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
This lock is used for (slightly) more than helping with writing
superblocks, and it will soon be extended further. So the
name is inappropriate.
Also, the _irq variant hasn't been needed since 2.6.37 as it is
never taking from interrupt or bh context.
So:
-rename write_lock to lock
-document what it protects
-remove _irq ... except in md_flush_request() as there
is no wait_event_lock() (with no _irq). This can be
cleaned up after appropriate changes to wait.h.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/md.c | 36 ++++++++++++++++++------------------
drivers/md/md.h | 8 +++++++-
2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 709755fb6d7b..17e7fd776034 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -397,12 +397,12 @@ static void md_submit_flush_data(struct work_struct *ws)
void md_flush_request(struct mddev *mddev, struct bio *bio)
{
- spin_lock_irq(&mddev->write_lock);
+ spin_lock_irq(&mddev->lock);
wait_event_lock_irq(mddev->sb_wait,
!mddev->flush_bio,
- mddev->write_lock);
+ mddev->lock);
mddev->flush_bio = bio;
- spin_unlock_irq(&mddev->write_lock);
+ spin_unlock_irq(&mddev->lock);
INIT_WORK(&mddev->flush_work, submit_flushes);
queue_work(md_wq, &mddev->flush_work);
@@ -465,7 +465,7 @@ void mddev_init(struct mddev *mddev)
atomic_set(&mddev->active, 1);
atomic_set(&mddev->openers, 0);
atomic_set(&mddev->active_io, 0);
- spin_lock_init(&mddev->write_lock);
+ spin_lock_init(&mddev->lock);
atomic_set(&mddev->flush_pending, 0);
init_waitqueue_head(&mddev->sb_wait);
init_waitqueue_head(&mddev->recovery_wait);
@@ -2230,7 +2230,7 @@ repeat:
return;
}
- spin_lock_irq(&mddev->write_lock);
+ spin_lock(&mddev->lock);
mddev->utime = get_seconds();
@@ -2287,7 +2287,7 @@ repeat:
}
sync_sbs(mddev, nospares);
- spin_unlock_irq(&mddev->write_lock);
+ spin_unlock(&mddev->lock);
pr_debug("md: updating %s RAID superblock on device (in sync %d)\n",
mdname(mddev), mddev->in_sync);
@@ -2326,15 +2326,15 @@ repeat:
md_super_wait(mddev);
/* if there was a failure, MD_CHANGE_DEVS was set, and we re-write super */
- spin_lock_irq(&mddev->write_lock);
+ spin_lock(&mddev->lock);
if (mddev->in_sync != sync_req ||
test_bit(MD_CHANGE_DEVS, &mddev->flags)) {
/* have to write it out again */
- spin_unlock_irq(&mddev->write_lock);
+ spin_unlock(&mddev->lock);
goto repeat;
}
clear_bit(MD_CHANGE_PENDING, &mddev->flags);
- spin_unlock_irq(&mddev->write_lock);
+ spin_unlock(&mddev->lock);
wake_up(&mddev->sb_wait);
if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
sysfs_notify(&mddev->kobj, NULL, "sync_completed");
@@ -3722,7 +3722,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
case clean:
if (mddev->pers) {
restart_array(mddev);
- spin_lock_irq(&mddev->write_lock);
+ spin_lock(&mddev->lock);
if (atomic_read(&mddev->writes_pending) == 0) {
if (mddev->in_sync == 0) {
mddev->in_sync = 1;
@@ -3733,7 +3733,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
err = 0;
} else
err = -EBUSY;
- spin_unlock_irq(&mddev->write_lock);
+ spin_unlock(&mddev->lock);
} else
err = -EINVAL;
break;
@@ -7102,7 +7102,7 @@ void md_write_start(struct mddev *mddev, struct bio *bi)
if (mddev->safemode == 1)
mddev->safemode = 0;
if (mddev->in_sync) {
- spin_lock_irq(&mddev->write_lock);
+ spin_lock(&mddev->lock);
if (mddev->in_sync) {
mddev->in_sync = 0;
set_bit(MD_CHANGE_CLEAN, &mddev->flags);
@@ -7110,7 +7110,7 @@ void md_write_start(struct mddev *mddev, struct bio *bi)
md_wakeup_thread(mddev->thread);
did_change = 1;
}
- spin_unlock_irq(&mddev->write_lock);
+ spin_unlock(&mddev->lock);
}
if (did_change)
sysfs_notify_dirent_safe(mddev->sysfs_state);
@@ -7148,7 +7148,7 @@ int md_allow_write(struct mddev *mddev)
if (!mddev->pers->sync_request)
return 0;
- spin_lock_irq(&mddev->write_lock);
+ spin_lock(&mddev->lock);
if (mddev->in_sync) {
mddev->in_sync = 0;
set_bit(MD_CHANGE_CLEAN, &mddev->flags);
@@ -7156,11 +7156,11 @@ int md_allow_write(struct mddev *mddev)
if (mddev->safemode_delay &&
mddev->safemode == 0)
mddev->safemode = 1;
- spin_unlock_irq(&mddev->write_lock);
+ spin_unlock(&mddev->lock);
md_update_sb(mddev, 0);
sysfs_notify_dirent_safe(mddev->sysfs_state);
} else
- spin_unlock_irq(&mddev->write_lock);
+ spin_unlock(&mddev->lock);
if (test_bit(MD_CHANGE_PENDING, &mddev->flags))
return -EAGAIN;
@@ -7688,7 +7688,7 @@ void md_check_recovery(struct mddev *mddev)
if (!mddev->external) {
int did_change = 0;
- spin_lock_irq(&mddev->write_lock);
+ spin_lock(&mddev->lock);
if (mddev->safemode &&
!atomic_read(&mddev->writes_pending) &&
!mddev->in_sync &&
@@ -7699,7 +7699,7 @@ void md_check_recovery(struct mddev *mddev)
}
if (mddev->safemode == 1)
mddev->safemode = 0;
- spin_unlock_irq(&mddev->write_lock);
+ spin_unlock(&mddev->lock);
if (did_change)
sysfs_notify_dirent_safe(mddev->sysfs_state);
}
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 03cec5bdcaae..f0d15bdd96d4 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -386,7 +386,13 @@ struct mddev {
struct work_struct del_work; /* used for delayed sysfs removal */
- spinlock_t write_lock;
+ /* "lock" protects:
+ * flush_bio transition from NULL to !NULL
+ * rdev superblocks, events
+ * clearing MD_CHANGE_*
+ * in_sync - and related safemode and MD_CHANGE changes
+ */
+ spinlock_t lock;
wait_queue_head_t sb_wait; /* for waiting on superblock updates */
atomic_t pending_writes; /* number of active superblock writes */
^ permalink raw reply related
* [md PATCH 07/26] md: make ->congested robust against personality changes.
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
There is currently no locking around calls to the 'congested'
bdi function. If called at an awkward time while an array is
being converted from one level (or personality) to another, there
is a tiny chance of running code in an unreferenced module etc.
So add a 'congested' function to the md_personality operations
structure, and call it with appropriate locking from a central
'mddev_congested'.
When the array personality is changing the array will be 'suspended'
so no IO is processed.
If mddev_congested detects this, it simply reports that the
array is congested, which is a safe guess.
As mddev_suspend calls synchronize_rcu(), mddev_congested can
avoid races by included the whole call inside an rcu_read_lock()
region.
This require that the congested functions for all subordinate devices
can be run under rcu_lock. Fortunately this is the case.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/dm-raid.c | 8 +-------
drivers/md/linear.c | 9 ++-------
drivers/md/md.c | 22 ++++++++++++++++++++--
drivers/md/md.h | 3 +++
drivers/md/multipath.c | 10 ++--------
drivers/md/raid0.c | 9 ++-------
drivers/md/raid1.c | 14 ++------------
drivers/md/raid1.h | 3 ---
drivers/md/raid10.c | 14 ++------------
drivers/md/raid10.h | 3 ---
drivers/md/raid5.c | 19 ++++---------------
drivers/md/raid5.h | 1 -
12 files changed, 38 insertions(+), 77 deletions(-)
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 07c0fa0fa284..777d9ba2acad 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -746,13 +746,7 @@ static int raid_is_congested(struct dm_target_callbacks *cb, int bits)
{
struct raid_set *rs = container_of(cb, struct raid_set, callbacks);
- if (rs->raid_type->level == 1)
- return md_raid1_congested(&rs->md, bits);
-
- if (rs->raid_type->level == 10)
- return md_raid10_congested(&rs->md, bits);
-
- return md_raid5_congested(&rs->md, bits);
+ return mddev_congested(&rs->md, bits);
}
/*
diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index 64713b77df1c..05108510d9cd 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -97,15 +97,11 @@ static int linear_mergeable_bvec(struct request_queue *q,
return maxsectors << 9;
}
-static int linear_congested(void *data, int bits)
+static int linear_congested(struct mddev *mddev, int bits)
{
- struct mddev *mddev = data;
struct linear_conf *conf;
int i, ret = 0;
- if (mddev_congested(mddev, bits))
- return 1;
-
rcu_read_lock();
conf = rcu_dereference(mddev->private);
@@ -218,8 +214,6 @@ static int linear_run (struct mddev *mddev)
md_set_array_sectors(mddev, linear_size(mddev, 0, 0));
blk_queue_merge_bvec(mddev->queue, linear_mergeable_bvec);
- mddev->queue->backing_dev_info.congested_fn = linear_congested;
- mddev->queue->backing_dev_info.congested_data = mddev;
ret = md_integrity_register(mddev);
if (ret) {
@@ -366,6 +360,7 @@ static struct md_personality linear_personality =
.status = linear_status,
.hot_add_disk = linear_add,
.size = linear_size,
+ .congested = linear_congested,
};
static int __init linear_init (void)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 17e7fd776034..d45f52edb314 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -321,9 +321,23 @@ EXPORT_SYMBOL_GPL(mddev_resume);
int mddev_congested(struct mddev *mddev, int bits)
{
- return mddev->suspended;
+ struct md_personality *pers = mddev->pers;
+ int ret = 0;
+
+ rcu_read_lock();
+ if (mddev->suspended)
+ ret = 1;
+ else if (pers && pers->congested)
+ ret = pers->congested(mddev, bits);
+ rcu_read_unlock();
+ return ret;
+}
+EXPORT_SYMBOL_GPL(mddev_congested);
+static int md_congested(void *data, int bits)
+{
+ struct mddev *mddev = data;
+ return mddev_congested(mddev, bits);
}
-EXPORT_SYMBOL(mddev_congested);
/*
* Generic flush handling for md
@@ -4908,6 +4922,10 @@ int md_run(struct mddev *mddev)
bitmap_destroy(mddev);
return err;
}
+ if (mddev->queue) {
+ mddev->queue->backing_dev_info.congested_data = mddev;
+ mddev->queue->backing_dev_info.congested_fn = md_congested;
+ }
if (mddev->pers->sync_request) {
if (mddev->kobj.sd &&
sysfs_create_group(&mddev->kobj, &md_redundancy_group))
diff --git a/drivers/md/md.h b/drivers/md/md.h
index f0d15bdd96d4..f2602280fac1 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -496,6 +496,9 @@ struct md_personality
* array.
*/
void *(*takeover) (struct mddev *mddev);
+ /* congested implements bdi.congested_fn().
+ * Will not be called while array is 'suspended' */
+ int (*congested)(struct mddev *mddev, int bits);
};
struct md_sysfs_entry {
diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index 399272f9c042..fedb1b31877d 100644
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -153,15 +153,11 @@ static void multipath_status (struct seq_file *seq, struct mddev *mddev)
seq_printf (seq, "]");
}
-static int multipath_congested(void *data, int bits)
+static int multipath_congested(struct mddev *mddev, int bits)
{
- struct mddev *mddev = data;
struct mpconf *conf = mddev->private;
int i, ret = 0;
- if (mddev_congested(mddev, bits))
- return 1;
-
rcu_read_lock();
for (i = 0; i < mddev->raid_disks ; i++) {
struct md_rdev *rdev = rcu_dereference(conf->multipaths[i].rdev);
@@ -489,9 +485,6 @@ static int multipath_run (struct mddev *mddev)
*/
md_set_array_sectors(mddev, multipath_size(mddev, 0, 0));
- mddev->queue->backing_dev_info.congested_fn = multipath_congested;
- mddev->queue->backing_dev_info.congested_data = mddev;
-
if (md_integrity_register(mddev))
goto out_free_conf;
@@ -533,6 +526,7 @@ static struct md_personality multipath_personality =
.hot_add_disk = multipath_add_disk,
.hot_remove_disk= multipath_remove_disk,
.size = multipath_size,
+ .congested = multipath_congested,
};
static int __init multipath_init (void)
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index ba6b85de96d2..4b521eac5b69 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -25,17 +25,13 @@
#include "raid0.h"
#include "raid5.h"
-static int raid0_congested(void *data, int bits)
+static int raid0_congested(struct mddev *mddev, int bits)
{
- struct mddev *mddev = data;
struct r0conf *conf = mddev->private;
struct md_rdev **devlist = conf->devlist;
int raid_disks = conf->strip_zone[0].nb_dev;
int i, ret = 0;
- if (mddev_congested(mddev, bits))
- return 1;
-
for (i = 0; i < raid_disks && !ret ; i++) {
struct request_queue *q = bdev_get_queue(devlist[i]->bdev);
@@ -263,8 +259,6 @@ static int create_strip_zones(struct mddev *mddev, struct r0conf **private_conf)
mdname(mddev),
(unsigned long long)smallest->sectors);
}
- mddev->queue->backing_dev_info.congested_fn = raid0_congested;
- mddev->queue->backing_dev_info.congested_data = mddev;
/*
* now since we have the hard sector sizes, we can make sure
@@ -729,6 +723,7 @@ static struct md_personality raid0_personality=
.size = raid0_size,
.takeover = raid0_takeover,
.quiesce = raid0_quiesce,
+ .congested = raid0_congested,
};
static int __init raid0_init (void)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 40b35be34f8d..9ad7ce7091be 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -734,7 +734,7 @@ static int raid1_mergeable_bvec(struct request_queue *q,
}
-int md_raid1_congested(struct mddev *mddev, int bits)
+static int raid1_congested(struct mddev *mddev, int bits)
{
struct r1conf *conf = mddev->private;
int i, ret = 0;
@@ -763,15 +763,6 @@ int md_raid1_congested(struct mddev *mddev, int bits)
rcu_read_unlock();
return ret;
}
-EXPORT_SYMBOL_GPL(md_raid1_congested);
-
-static int raid1_congested(void *data, int bits)
-{
- struct mddev *mddev = data;
-
- return mddev_congested(mddev, bits) ||
- md_raid1_congested(mddev, bits);
-}
static void flush_pending_writes(struct r1conf *conf)
{
@@ -2955,8 +2946,6 @@ static int run(struct mddev *mddev)
md_set_array_sectors(mddev, raid1_size(mddev, 0, 0));
if (mddev->queue) {
- mddev->queue->backing_dev_info.congested_fn = raid1_congested;
- mddev->queue->backing_dev_info.congested_data = mddev;
blk_queue_merge_bvec(mddev->queue, raid1_mergeable_bvec);
if (discard_supported)
@@ -3193,6 +3182,7 @@ static struct md_personality raid1_personality =
.check_reshape = raid1_reshape,
.quiesce = raid1_quiesce,
.takeover = raid1_takeover,
+ .congested = raid1_congested,
};
static int __init raid_init(void)
diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h
index 33bda55ef9f7..14ebb288c1ef 100644
--- a/drivers/md/raid1.h
+++ b/drivers/md/raid1.h
@@ -170,7 +170,4 @@ struct r1bio {
*/
#define R1BIO_MadeGood 7
#define R1BIO_WriteError 8
-
-extern int md_raid1_congested(struct mddev *mddev, int bits);
-
#endif
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 32e282f4c83c..fb6b88674e87 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -910,7 +910,7 @@ retry:
return rdev;
}
-int md_raid10_congested(struct mddev *mddev, int bits)
+static int raid10_congested(struct mddev *mddev, int bits)
{
struct r10conf *conf = mddev->private;
int i, ret = 0;
@@ -934,15 +934,6 @@ int md_raid10_congested(struct mddev *mddev, int bits)
rcu_read_unlock();
return ret;
}
-EXPORT_SYMBOL_GPL(md_raid10_congested);
-
-static int raid10_congested(void *data, int bits)
-{
- struct mddev *mddev = data;
-
- return mddev_congested(mddev, bits) ||
- md_raid10_congested(mddev, bits);
-}
static void flush_pending_writes(struct r10conf *conf)
{
@@ -3757,8 +3748,6 @@ static int run(struct mddev *mddev)
if (mddev->queue) {
int stripe = conf->geo.raid_disks *
((mddev->chunk_sectors << 9) / PAGE_SIZE);
- mddev->queue->backing_dev_info.congested_fn = raid10_congested;
- mddev->queue->backing_dev_info.congested_data = mddev;
/* Calculate max read-ahead size.
* We need to readahead at least twice a whole stripe....
@@ -4727,6 +4716,7 @@ static struct md_personality raid10_personality =
.check_reshape = raid10_check_reshape,
.start_reshape = raid10_start_reshape,
.finish_reshape = raid10_finish_reshape,
+ .congested = raid10_congested,
};
static int __init raid_init(void)
diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h
index 157d69e83ff4..5ee6473ddc2c 100644
--- a/drivers/md/raid10.h
+++ b/drivers/md/raid10.h
@@ -150,7 +150,4 @@ enum r10bio_state {
*/
R10BIO_Previous,
};
-
-extern int md_raid10_congested(struct mddev *mddev, int bits);
-
#endif
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index a03cf2d889bf..502a908149c6 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -4149,7 +4149,7 @@ static void activate_bit_delay(struct r5conf *conf,
}
}
-int md_raid5_congested(struct mddev *mddev, int bits)
+static int raid5_congested(struct mddev *mddev, int bits)
{
struct r5conf *conf = mddev->private;
@@ -4166,15 +4166,6 @@ int md_raid5_congested(struct mddev *mddev, int bits)
return 0;
}
-EXPORT_SYMBOL_GPL(md_raid5_congested);
-
-static int raid5_congested(void *data, int bits)
-{
- struct mddev *mddev = data;
-
- return mddev_congested(mddev, bits) ||
- md_raid5_congested(mddev, bits);
-}
/* We want read requests to align with chunks where possible,
* but write requests don't need to.
@@ -6248,9 +6239,6 @@ static int run(struct mddev *mddev)
blk_queue_merge_bvec(mddev->queue, raid5_mergeable_bvec);
- mddev->queue->backing_dev_info.congested_data = mddev;
- mddev->queue->backing_dev_info.congested_fn = raid5_congested;
-
chunk_size = mddev->chunk_sectors << 9;
blk_queue_io_min(mddev->queue, chunk_size);
blk_queue_io_opt(mddev->queue, chunk_size *
@@ -6333,8 +6321,6 @@ static int stop(struct mddev *mddev)
struct r5conf *conf = mddev->private;
md_unregister_thread(&mddev->thread);
- if (mddev->queue)
- mddev->queue->backing_dev_info.congested_fn = NULL;
free_conf(conf);
mddev->private = NULL;
mddev->to_remove = &raid5_attrs_group;
@@ -7126,6 +7112,7 @@ static struct md_personality raid6_personality =
.finish_reshape = raid5_finish_reshape,
.quiesce = raid5_quiesce,
.takeover = raid6_takeover,
+ .congested = raid5_congested,
};
static struct md_personality raid5_personality =
{
@@ -7148,6 +7135,7 @@ static struct md_personality raid5_personality =
.finish_reshape = raid5_finish_reshape,
.quiesce = raid5_quiesce,
.takeover = raid5_takeover,
+ .congested = raid5_congested,
};
static struct md_personality raid4_personality =
@@ -7171,6 +7159,7 @@ static struct md_personality raid4_personality =
.finish_reshape = raid5_finish_reshape,
.quiesce = raid5_quiesce,
.takeover = raid4_takeover,
+ .congested = raid5_congested,
};
static int __init raid5_init(void)
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index d59f5ca743cd..983e18a83db1 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -558,7 +558,6 @@ static inline int algorithm_is_DDF(int layout)
return layout >= 8 && layout <= 10;
}
-extern int md_raid5_congested(struct mddev *mddev, int bits);
extern void md_raid5_kick_device(struct r5conf *conf);
extern int raid5_set_cache_size(struct mddev *mddev, int size);
#endif
^ permalink raw reply related
* [md PATCH 08/26] md: make merge_bvec_fn more robust in face of personality changes.
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
There is no locking around calls to merge_bvec_fn(), so
it is possible that calls which coincide with a level (or personality)
change could go wrong.
So create a central dispatch point for these functions and use
rcu_read_lock().
If the array is suspended, reject any merge that can be rejected.
If not, we know it is safe to call the function.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/linear.c | 6 ++----
drivers/md/md.c | 24 ++++++++++++++++++++++++
drivers/md/md.h | 4 ++++
drivers/md/raid0.c | 7 +++----
drivers/md/raid1.c | 6 ++----
drivers/md/raid10.c | 7 +++----
drivers/md/raid5.c | 8 ++++----
7 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index 05108510d9cd..4c2a92ce2b0b 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -60,11 +60,10 @@ static inline struct dev_info *which_dev(struct mddev *mddev, sector_t sector)
*
* Return amount of bytes we can take at this offset
*/
-static int linear_mergeable_bvec(struct request_queue *q,
+static int linear_mergeable_bvec(struct mddev *mddev,
struct bvec_merge_data *bvm,
struct bio_vec *biovec)
{
- struct mddev *mddev = q->queuedata;
struct dev_info *dev0;
unsigned long maxsectors, bio_sectors = bvm->bi_size >> 9;
sector_t sector = bvm->bi_sector + get_start_sect(bvm->bi_bdev);
@@ -213,8 +212,6 @@ static int linear_run (struct mddev *mddev)
mddev->private = conf;
md_set_array_sectors(mddev, linear_size(mddev, 0, 0));
- blk_queue_merge_bvec(mddev->queue, linear_mergeable_bvec);
-
ret = md_integrity_register(mddev);
if (ret) {
kfree(conf);
@@ -361,6 +358,7 @@ static struct md_personality linear_personality =
.hot_add_disk = linear_add,
.size = linear_size,
.congested = linear_congested,
+ .mergeable_bvec = linear_mergeable_bvec,
};
static int __init linear_init (void)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d45f52edb314..9f0ff7187136 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -339,6 +339,29 @@ static int md_congested(void *data, int bits)
return mddev_congested(mddev, bits);
}
+static int md_mergeable_bvec(struct request_queue *q,
+ struct bvec_merge_data *bvm,
+ struct bio_vec *biovec)
+{
+ struct mddev *mddev = q->queuedata;
+ int ret;
+ rcu_read_lock();
+ if (mddev->suspended) {
+ /* Must always allow one vec */
+ if (bvm->bi_size == 0)
+ ret = biovec->bv_len;
+ else
+ ret = 0;
+ } else {
+ struct md_personality *pers = mddev->pers;
+ if (pers && pers->mergeable_bvec)
+ ret = pers->mergeable_bvec(mddev, bvm, biovec);
+ else
+ ret = biovec->bv_len;
+ }
+ rcu_read_unlock();
+ return ret;
+}
/*
* Generic flush handling for md
*/
@@ -4925,6 +4948,7 @@ int md_run(struct mddev *mddev)
if (mddev->queue) {
mddev->queue->backing_dev_info.congested_data = mddev;
mddev->queue->backing_dev_info.congested_fn = md_congested;
+ blk_queue_merge_bvec(mddev->queue, md_mergeable_bvec);
}
if (mddev->pers->sync_request) {
if (mddev->kobj.sd &&
diff --git a/drivers/md/md.h b/drivers/md/md.h
index f2602280fac1..bee5b852c33f 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -499,6 +499,10 @@ struct md_personality
/* congested implements bdi.congested_fn().
* Will not be called while array is 'suspended' */
int (*congested)(struct mddev *mddev, int bits);
+ /* mergeable_bvec is use to implement ->merge_bvec_fn */
+ int (*mergeable_bvec)(struct mddev *mddev,
+ struct bvec_merge_data *bvm,
+ struct bio_vec *biovec);
};
struct md_sysfs_entry {
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 4b521eac5b69..3770c9675b17 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -350,17 +350,16 @@ static struct md_rdev *map_sector(struct mddev *mddev, struct strip_zone *zone,
/**
* raid0_mergeable_bvec -- tell bio layer if two requests can be merged
- * @q: request queue
+ * @mddev: the md device
* @bvm: properties of new bio
* @biovec: the request that could be merged to it.
*
* Return amount of bytes we can accept at this offset
*/
-static int raid0_mergeable_bvec(struct request_queue *q,
+static int raid0_mergeable_bvec(struct mddev *mddev,
struct bvec_merge_data *bvm,
struct bio_vec *biovec)
{
- struct mddev *mddev = q->queuedata;
struct r0conf *conf = mddev->private;
sector_t sector = bvm->bi_sector + get_start_sect(bvm->bi_bdev);
sector_t sector_offset = sector;
@@ -465,7 +464,6 @@ static int raid0_run(struct mddev *mddev)
mddev->queue->backing_dev_info.ra_pages = 2* stripe;
}
- blk_queue_merge_bvec(mddev->queue, raid0_mergeable_bvec);
dump_zones(mddev);
ret = md_integrity_register(mddev);
@@ -724,6 +722,7 @@ static struct md_personality raid0_personality=
.takeover = raid0_takeover,
.quiesce = raid0_quiesce,
.congested = raid0_congested,
+ .mergeable_bvec = raid0_mergeable_bvec,
};
static int __init raid0_init (void)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 9ad7ce7091be..45c512a4b75d 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -701,11 +701,10 @@ static int read_balance(struct r1conf *conf, struct r1bio *r1_bio, int *max_sect
return best_disk;
}
-static int raid1_mergeable_bvec(struct request_queue *q,
+static int raid1_mergeable_bvec(struct mddev *mddev,
struct bvec_merge_data *bvm,
struct bio_vec *biovec)
{
- struct mddev *mddev = q->queuedata;
struct r1conf *conf = mddev->private;
sector_t sector = bvm->bi_sector + get_start_sect(bvm->bi_bdev);
int max = biovec->bv_len;
@@ -2946,8 +2945,6 @@ static int run(struct mddev *mddev)
md_set_array_sectors(mddev, raid1_size(mddev, 0, 0));
if (mddev->queue) {
- blk_queue_merge_bvec(mddev->queue, raid1_mergeable_bvec);
-
if (discard_supported)
queue_flag_set_unlocked(QUEUE_FLAG_DISCARD,
mddev->queue);
@@ -3183,6 +3180,7 @@ static struct md_personality raid1_personality =
.quiesce = raid1_quiesce,
.takeover = raid1_takeover,
.congested = raid1_congested,
+ .mergeable_bvec = raid1_mergeable_bvec,
};
static int __init raid_init(void)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index fb6b88674e87..407c81a820f4 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -674,7 +674,7 @@ static sector_t raid10_find_virt(struct r10conf *conf, sector_t sector, int dev)
/**
* raid10_mergeable_bvec -- tell bio layer if a two requests can be merged
- * @q: request queue
+ * @mddev: the md device
* @bvm: properties of new bio
* @biovec: the request that could be merged to it.
*
@@ -682,11 +682,10 @@ static sector_t raid10_find_virt(struct r10conf *conf, sector_t sector, int dev)
* This requires checking for end-of-chunk if near_copies != raid_disks,
* and for subordinate merge_bvec_fns if merge_check_needed.
*/
-static int raid10_mergeable_bvec(struct request_queue *q,
+static int raid10_mergeable_bvec(struct mddev *mddev,
struct bvec_merge_data *bvm,
struct bio_vec *biovec)
{
- struct mddev *mddev = q->queuedata;
struct r10conf *conf = mddev->private;
sector_t sector = bvm->bi_sector + get_start_sect(bvm->bi_bdev);
int max;
@@ -3756,7 +3755,6 @@ static int run(struct mddev *mddev)
stripe /= conf->geo.near_copies;
if (mddev->queue->backing_dev_info.ra_pages < 2 * stripe)
mddev->queue->backing_dev_info.ra_pages = 2 * stripe;
- blk_queue_merge_bvec(mddev->queue, raid10_mergeable_bvec);
}
if (md_integrity_register(mddev))
@@ -4717,6 +4715,7 @@ static struct md_personality raid10_personality =
.start_reshape = raid10_start_reshape,
.finish_reshape = raid10_finish_reshape,
.congested = raid10_congested,
+ .mergeable_bvec = raid10_mergeable_bvec,
};
static int __init raid_init(void)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 502a908149c6..2d4a2cc85eb2 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -4170,11 +4170,10 @@ static int raid5_congested(struct mddev *mddev, int bits)
/* We want read requests to align with chunks where possible,
* but write requests don't need to.
*/
-static int raid5_mergeable_bvec(struct request_queue *q,
+static int raid5_mergeable_bvec(struct mddev *mddev,
struct bvec_merge_data *bvm,
struct bio_vec *biovec)
{
- struct mddev *mddev = q->queuedata;
sector_t sector = bvm->bi_sector + get_start_sect(bvm->bi_bdev);
int max;
unsigned int chunk_sectors = mddev->chunk_sectors;
@@ -6237,8 +6236,6 @@ static int run(struct mddev *mddev)
if (mddev->queue->backing_dev_info.ra_pages < 2 * stripe)
mddev->queue->backing_dev_info.ra_pages = 2 * stripe;
- blk_queue_merge_bvec(mddev->queue, raid5_mergeable_bvec);
-
chunk_size = mddev->chunk_sectors << 9;
blk_queue_io_min(mddev->queue, chunk_size);
blk_queue_io_opt(mddev->queue, chunk_size *
@@ -7113,6 +7110,7 @@ static struct md_personality raid6_personality =
.quiesce = raid5_quiesce,
.takeover = raid6_takeover,
.congested = raid5_congested,
+ .mergeable_bvec = raid5_mergeable_bvec,
};
static struct md_personality raid5_personality =
{
@@ -7136,6 +7134,7 @@ static struct md_personality raid5_personality =
.quiesce = raid5_quiesce,
.takeover = raid5_takeover,
.congested = raid5_congested,
+ .mergeable_bvec = raid5_mergeable_bvec,
};
static struct md_personality raid4_personality =
@@ -7160,6 +7159,7 @@ static struct md_personality raid4_personality =
.quiesce = raid5_quiesce,
.takeover = raid4_takeover,
.congested = raid5_congested,
+ .mergeable_bvec = raid5_mergeable_bvec,
};
static int __init raid5_init(void)
^ permalink raw reply related
* [md PATCH 09/26] md/linear: remove rcu protections in favour of suspend/resume
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
The use of 'rcu' to protect accesses to ->private_data so that
the ->private_data could be updated predates the introduction
of mddev_suspend/mddev_resume.
These are a cleaner mechanism for providing stability while
swapping in a new ->private data - it is used by level_store()
to support changing of raid levels.
So get rid of the RCU stuff and just use mddev_suspend, mddev_resume.
As these function call ->quiesce(), we add an empty function for
linear just like for raid0.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/linear.c | 44 ++++++++++++++------------------------------
1 file changed, 14 insertions(+), 30 deletions(-)
diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index 4c2a92ce2b0b..b3e717adbc9b 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -34,7 +34,7 @@ static inline struct dev_info *which_dev(struct mddev *mddev, sector_t sector)
lo = 0;
hi = mddev->raid_disks - 1;
- conf = rcu_dereference(mddev->private);
+ conf = mddev->private;
/*
* Binary Search
@@ -70,7 +70,6 @@ static int linear_mergeable_bvec(struct mddev *mddev,
int maxbytes = biovec->bv_len;
struct request_queue *subq;
- rcu_read_lock();
dev0 = which_dev(mddev, sector);
maxsectors = dev0->end_sector - sector;
subq = bdev_get_queue(dev0->rdev->bdev);
@@ -80,7 +79,6 @@ static int linear_mergeable_bvec(struct mddev *mddev,
maxbytes = min(maxbytes, subq->merge_bvec_fn(subq, bvm,
biovec));
}
- rcu_read_unlock();
if (maxsectors < bio_sectors)
maxsectors = 0;
@@ -101,15 +99,13 @@ static int linear_congested(struct mddev *mddev, int bits)
struct linear_conf *conf;
int i, ret = 0;
- rcu_read_lock();
- conf = rcu_dereference(mddev->private);
+ conf = mddev->private;
for (i = 0; i < mddev->raid_disks && !ret ; i++) {
struct request_queue *q = bdev_get_queue(conf->disks[i].rdev->bdev);
ret |= bdi_congested(&q->backing_dev_info, bits);
}
- rcu_read_unlock();
return ret;
}
@@ -118,12 +114,10 @@ static sector_t linear_size(struct mddev *mddev, sector_t sectors, int raid_disk
struct linear_conf *conf;
sector_t array_sectors;
- rcu_read_lock();
- conf = rcu_dereference(mddev->private);
+ conf = mddev->private;
WARN_ONCE(sectors || raid_disks,
"%s does not support generic reshape\n", __func__);
array_sectors = conf->array_sectors;
- rcu_read_unlock();
return array_sectors;
}
@@ -243,33 +237,22 @@ static int linear_add(struct mddev *mddev, struct md_rdev *rdev)
if (!newconf)
return -ENOMEM;
- oldconf = rcu_dereference_protected(mddev->private,
- lockdep_is_held(
- &mddev->reconfig_mutex));
+ mddev_suspend(mddev);
+ oldconf = mddev->private;
mddev->raid_disks++;
- rcu_assign_pointer(mddev->private, newconf);
+ mddev->private = newconf;
md_set_array_sectors(mddev, linear_size(mddev, 0, 0));
set_capacity(mddev->gendisk, mddev->array_sectors);
+ mddev_resume(mddev);
revalidate_disk(mddev->gendisk);
- kfree_rcu(oldconf, rcu);
+ kfree(oldconf);
return 0;
}
static int linear_stop (struct mddev *mddev)
{
- struct linear_conf *conf =
- rcu_dereference_protected(mddev->private,
- lockdep_is_held(
- &mddev->reconfig_mutex));
+ struct linear_conf *conf = mddev->private;
- /*
- * We do not require rcu protection here since
- * we hold reconfig_mutex for both linear_add and
- * linear_stop, so they cannot race.
- * We should make sure any old 'conf's are properly
- * freed though.
- */
- rcu_barrier();
blk_sync_queue(mddev->queue); /* the unplug fn references 'conf'*/
kfree(conf);
mddev->private = NULL;
@@ -290,16 +273,12 @@ static void linear_make_request(struct mddev *mddev, struct bio *bio)
}
do {
- rcu_read_lock();
-
tmp_dev = which_dev(mddev, bio->bi_iter.bi_sector);
start_sector = tmp_dev->end_sector - tmp_dev->rdev->sectors;
end_sector = tmp_dev->end_sector;
data_offset = tmp_dev->rdev->data_offset;
bio->bi_bdev = tmp_dev->rdev->bdev;
- rcu_read_unlock();
-
if (unlikely(bio->bi_iter.bi_sector >= end_sector ||
bio->bi_iter.bi_sector < start_sector))
goto out_of_bounds;
@@ -346,6 +325,10 @@ static void linear_status (struct seq_file *seq, struct mddev *mddev)
seq_printf(seq, " %dk rounding", mddev->chunk_sectors / 2);
}
+static void linear_quiesce(struct mddev *mddev, int state)
+{
+}
+
static struct md_personality linear_personality =
{
.name = "linear",
@@ -357,6 +340,7 @@ static struct md_personality linear_personality =
.status = linear_status,
.hot_add_disk = linear_add,
.size = linear_size,
+ .quiesce = linear_quiesce,
.congested = linear_congested,
.mergeable_bvec = linear_mergeable_bvec,
};
^ permalink raw reply related
* [md PATCH 10/26] md: split detach operation out from ->stop.
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
Each md personality has a 'stop' operation which does two
things:
1/ it finalizes some aspects of the array to ensure nothing
is accessing the ->private data
2/ it frees the ->private data.
All the steps in '1' can apply to all arrays and so can be
performed in common code.
This is useful as in the case where we change the personality which
manages an array (in level_store()), it would be helpful to do
step 1 early, and step 2 later.
So split the 'step 1' functionality out into a new mddev_detach().
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/linear.c | 1 -
drivers/md/md.c | 30 ++++++++++++++++++++++++++----
drivers/md/multipath.c | 2 --
drivers/md/raid0.c | 1 -
drivers/md/raid1.c | 18 +++---------------
drivers/md/raid10.c | 8 --------
drivers/md/raid5.c | 1 -
7 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index b3e717adbc9b..c201555b9c6c 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -253,7 +253,6 @@ static int linear_stop (struct mddev *mddev)
{
struct linear_conf *conf = mddev->private;
- blk_sync_queue(mddev->queue); /* the unplug fn references 'conf'*/
kfree(conf);
mddev->private = NULL;
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 9f0ff7187136..58f140bef999 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -72,6 +72,7 @@ static struct workqueue_struct *md_misc_wq;
static int remove_and_add_spares(struct mddev *mddev,
struct md_rdev *this);
+static void mddev_detach(struct mddev *mddev);
/*
* Default number of read corrections we'll attempt on an rdev
@@ -3372,6 +3373,7 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
/* Looks like we have a winner */
mddev_suspend(mddev);
+ mddev_detach(mddev);
mddev->pers->stop(mddev);
if (mddev->pers->sync_request == NULL &&
@@ -4928,18 +4930,17 @@ int md_run(struct mddev *mddev)
(unsigned long long)mddev->array_sectors / 2,
(unsigned long long)mddev->pers->size(mddev, 0, 0) / 2);
err = -EINVAL;
- mddev->pers->stop(mddev);
}
if (err == 0 && mddev->pers->sync_request &&
(mddev->bitmap_info.file || mddev->bitmap_info.offset)) {
err = bitmap_create(mddev);
- if (err) {
+ if (err)
printk(KERN_ERR "%s: failed to create bitmap (%d)\n",
mdname(mddev), err);
- mddev->pers->stop(mddev);
- }
}
if (err) {
+ mddev_detach(mddev);
+ mddev->pers->stop(mddev);
module_put(mddev->pers->owner);
mddev->pers = NULL;
bitmap_destroy(mddev);
@@ -5112,9 +5113,30 @@ void md_stop_writes(struct mddev *mddev)
}
EXPORT_SYMBOL_GPL(md_stop_writes);
+static void mddev_detach(struct mddev *mddev)
+{
+ struct bitmap *bitmap = mddev->bitmap;
+ /* wait for behind writes to complete */
+ if (bitmap && atomic_read(&bitmap->behind_writes) > 0) {
+ printk(KERN_INFO "md:%s: behind writes in progress - waiting to stop.\n",
+ mdname(mddev));
+ /* need to kick something here to make sure I/O goes? */
+ wait_event(bitmap->behind_wait,
+ atomic_read(&bitmap->behind_writes) == 0);
+ }
+ if (mddev->pers->quiesce) {
+ mddev->pers->quiesce(mddev, 1);
+ mddev->pers->quiesce(mddev, 0);
+ }
+ md_unregister_thread(&mddev->thread);
+ if (mddev->queue)
+ blk_sync_queue(mddev->queue); /* the unplug fn references 'conf'*/
+}
+
static void __md_stop(struct mddev *mddev)
{
mddev->ready = 0;
+ mddev_detach(mddev);
mddev->pers->stop(mddev);
if (mddev->pers->sync_request && mddev->to_remove == NULL)
mddev->to_remove = &md_redundancy_group;
diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index fedb1b31877d..9fe34453835b 100644
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -504,8 +504,6 @@ static int multipath_stop (struct mddev *mddev)
{
struct mpconf *conf = mddev->private;
- md_unregister_thread(&mddev->thread);
- blk_sync_queue(mddev->queue); /* the unplug fn references 'conf'*/
mempool_destroy(conf->pool);
kfree(conf->multipaths);
kfree(conf);
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 3770c9675b17..01dfca94b663 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -477,7 +477,6 @@ static int raid0_stop(struct mddev *mddev)
{
struct r0conf *conf = mddev->private;
- blk_sync_queue(mddev->queue); /* the unplug fn references 'conf'*/
kfree(conf->strip_zone);
kfree(conf->devlist);
kfree(conf);
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 45c512a4b75d..fccea0b39808 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2954,29 +2954,17 @@ static int run(struct mddev *mddev)
}
ret = md_integrity_register(mddev);
- if (ret)
+ if (ret) {
+ md_unregister_thread(&mddev->thread);
stop(mddev);
+ }
return ret;
}
static int stop(struct mddev *mddev)
{
struct r1conf *conf = mddev->private;
- struct bitmap *bitmap = mddev->bitmap;
-
- /* wait for behind writes to complete */
- if (bitmap && atomic_read(&bitmap->behind_writes) > 0) {
- printk(KERN_INFO "md/raid1:%s: behind writes in progress - waiting to stop.\n",
- mdname(mddev));
- /* need to kick something here to make sure I/O goes? */
- wait_event(bitmap->behind_wait,
- atomic_read(&bitmap->behind_writes) == 0);
- }
-
- freeze_array(conf, 0);
- unfreeze_array(conf);
- md_unregister_thread(&mddev->thread);
if (conf->r1bio_pool)
mempool_destroy(conf->r1bio_pool);
kfree(conf->mirrors);
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 407c81a820f4..654fdae906aa 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3802,14 +3802,6 @@ static int stop(struct mddev *mddev)
{
struct r10conf *conf = mddev->private;
- raise_barrier(conf, 0);
- lower_barrier(conf);
-
- md_unregister_thread(&mddev->thread);
- if (mddev->queue)
- /* the unplug fn references 'conf'*/
- blk_sync_queue(mddev->queue);
-
if (conf->r10bio_pool)
mempool_destroy(conf->r10bio_pool);
safe_put_page(conf->tmppage);
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 2d4a2cc85eb2..482526077647 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6317,7 +6317,6 @@ static int stop(struct mddev *mddev)
{
struct r5conf *conf = mddev->private;
- md_unregister_thread(&mddev->thread);
free_conf(conf);
mddev->private = NULL;
mddev->to_remove = &raid5_attrs_group;
^ permalink raw reply related
* [md PATCH 11/26] md: rename ->stop to ->free
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
Now that the ->stop function only frees the private data,
rename is accordingly.
Also pass in the private pointer as an arg rather than using
mddev->private. This flexibility will be useful in level_store().
Finally, don't clear ->private. It doesn't make sense to clear
it seeing that isn't what we free, and it is no longer necessary
to clear ->private (it was some time ago before ->to_remove was
introduced).
Setting ->to_remove in ->free() is a bit of a wart, but not a
big problem at the moment.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/faulty.c | 8 +++-----
drivers/md/linear.c | 9 +++------
drivers/md/md.c | 10 +++++-----
drivers/md/md.h | 2 +-
drivers/md/multipath.c | 10 ++++------
drivers/md/raid0.c | 12 +++++-------
drivers/md/raid1.c | 14 ++++++--------
drivers/md/raid10.c | 8 +++-----
drivers/md/raid5.c | 12 +++++-------
9 files changed, 35 insertions(+), 50 deletions(-)
diff --git a/drivers/md/faulty.c b/drivers/md/faulty.c
index e8b4574956c7..1277eb26b58a 100644
--- a/drivers/md/faulty.c
+++ b/drivers/md/faulty.c
@@ -332,13 +332,11 @@ static int run(struct mddev *mddev)
return 0;
}
-static int stop(struct mddev *mddev)
+static void faulty_free(struct mddev *mddev, void *priv)
{
- struct faulty_conf *conf = mddev->private;
+ struct faulty_conf *conf = priv;
kfree(conf);
- mddev->private = NULL;
- return 0;
}
static struct md_personality faulty_personality =
@@ -348,7 +346,7 @@ static struct md_personality faulty_personality =
.owner = THIS_MODULE,
.make_request = make_request,
.run = run,
- .stop = stop,
+ .free = faulty_free,
.status = status,
.check_reshape = reshape,
.size = faulty_size,
diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index c201555b9c6c..fa7d577f3d12 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -249,14 +249,11 @@ static int linear_add(struct mddev *mddev, struct md_rdev *rdev)
return 0;
}
-static int linear_stop (struct mddev *mddev)
+static void linear_free(struct mddev *mddev, void *priv)
{
- struct linear_conf *conf = mddev->private;
+ struct linear_conf *conf = priv;
kfree(conf);
- mddev->private = NULL;
-
- return 0;
}
static void linear_make_request(struct mddev *mddev, struct bio *bio)
@@ -335,7 +332,7 @@ static struct md_personality linear_personality =
.owner = THIS_MODULE,
.make_request = linear_make_request,
.run = linear_run,
- .stop = linear_stop,
+ .free = linear_free,
.status = linear_status,
.hot_add_disk = linear_add,
.size = linear_size,
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 58f140bef999..2920fd004865 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -293,8 +293,8 @@ static void md_make_request(struct request_queue *q, struct bio *bio)
/* mddev_suspend makes sure no new requests are submitted
* to the device, and that any requests that have been submitted
* are completely handled.
- * Once ->stop is called and completes, the module will be completely
- * unused.
+ * Once mddev_detach() is called and completes, the module will be
+ * completely unused.
*/
void mddev_suspend(struct mddev *mddev)
{
@@ -3374,7 +3374,7 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
/* Looks like we have a winner */
mddev_suspend(mddev);
mddev_detach(mddev);
- mddev->pers->stop(mddev);
+ mddev->pers->free(mddev, mddev->private);
if (mddev->pers->sync_request == NULL &&
pers->sync_request != NULL) {
@@ -4940,7 +4940,7 @@ int md_run(struct mddev *mddev)
}
if (err) {
mddev_detach(mddev);
- mddev->pers->stop(mddev);
+ mddev->pers->free(mddev, mddev->private);
module_put(mddev->pers->owner);
mddev->pers = NULL;
bitmap_destroy(mddev);
@@ -5137,7 +5137,7 @@ static void __md_stop(struct mddev *mddev)
{
mddev->ready = 0;
mddev_detach(mddev);
- mddev->pers->stop(mddev);
+ mddev->pers->free(mddev, mddev->private);
if (mddev->pers->sync_request && mddev->to_remove == NULL)
mddev->to_remove = &md_redundancy_group;
module_put(mddev->pers->owner);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index bee5b852c33f..37e7c17e56a6 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -465,7 +465,7 @@ struct md_personality
struct module *owner;
void (*make_request)(struct mddev *mddev, struct bio *bio);
int (*run)(struct mddev *mddev);
- int (*stop)(struct mddev *mddev);
+ void (*free)(struct mddev *mddev, void *priv);
void (*status)(struct seq_file *seq, struct mddev *mddev);
/* error_handler must set ->faulty and clear ->in_sync
* if appropriate, and should abort recovery if needed
diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index 9fe34453835b..ac3ede2bd00e 100644
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -399,7 +399,7 @@ static int multipath_run (struct mddev *mddev)
/*
* copy the already verified devices into our private MULTIPATH
* bookkeeping area. [whatever we allocate in multipath_run(),
- * should be freed in multipath_stop()]
+ * should be freed in multipath_free()]
*/
conf = kzalloc(sizeof(struct mpconf), GFP_KERNEL);
@@ -500,15 +500,13 @@ out:
return -EIO;
}
-static int multipath_stop (struct mddev *mddev)
+static void multipath_free(struct mddev *mddev, void *priv)
{
- struct mpconf *conf = mddev->private;
+ struct mpconf *conf = priv;
mempool_destroy(conf->pool);
kfree(conf->multipaths);
kfree(conf);
- mddev->private = NULL;
- return 0;
}
static struct md_personality multipath_personality =
@@ -518,7 +516,7 @@ static struct md_personality multipath_personality =
.owner = THIS_MODULE,
.make_request = multipath_make_request,
.run = multipath_run,
- .stop = multipath_stop,
+ .free = multipath_free,
.status = multipath_status,
.error_handler = multipath_error,
.hot_add_disk = multipath_add_disk,
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 01dfca94b663..a13f738a7b39 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -415,7 +415,7 @@ static sector_t raid0_size(struct mddev *mddev, sector_t sectors, int raid_disks
return array_sectors;
}
-static int raid0_stop(struct mddev *mddev);
+static void raid0_free(struct mddev *mddev, void *priv);
static int raid0_run(struct mddev *mddev)
{
@@ -468,20 +468,18 @@ static int raid0_run(struct mddev *mddev)
ret = md_integrity_register(mddev);
if (ret)
- raid0_stop(mddev);
+ raid0_free(mddev, conf);
return ret;
}
-static int raid0_stop(struct mddev *mddev)
+static void raid0_free(struct mddev *mddev, void *priv)
{
- struct r0conf *conf = mddev->private;
+ struct r0conf *conf = priv;
kfree(conf->strip_zone);
kfree(conf->devlist);
kfree(conf);
- mddev->private = NULL;
- return 0;
}
/*
@@ -715,7 +713,7 @@ static struct md_personality raid0_personality=
.owner = THIS_MODULE,
.make_request = raid0_make_request,
.run = raid0_run,
- .stop = raid0_stop,
+ .free = raid0_free,
.status = raid0_status,
.size = raid0_size,
.takeover = raid0_takeover,
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index fccea0b39808..5dd0c2e59ab9 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2872,7 +2872,7 @@ static struct r1conf *setup_conf(struct mddev *mddev)
return ERR_PTR(err);
}
-static int stop(struct mddev *mddev);
+static void raid1_free(struct mddev *mddev, void *priv);
static int run(struct mddev *mddev)
{
struct r1conf *conf;
@@ -2894,7 +2894,7 @@ static int run(struct mddev *mddev)
/*
* copy the already verified devices into our private RAID1
* bookkeeping area. [whatever we allocate in run(),
- * should be freed in stop()]
+ * should be freed in raid1_free()]
*/
if (mddev->private == NULL)
conf = setup_conf(mddev);
@@ -2956,14 +2956,14 @@ static int run(struct mddev *mddev)
ret = md_integrity_register(mddev);
if (ret) {
md_unregister_thread(&mddev->thread);
- stop(mddev);
+ raid1_free(mddev, conf);
}
return ret;
}
-static int stop(struct mddev *mddev)
+static void raid1_free(struct mddev *mddev, void *priv)
{
- struct r1conf *conf = mddev->private;
+ struct r1conf *conf = priv;
if (conf->r1bio_pool)
mempool_destroy(conf->r1bio_pool);
@@ -2971,8 +2971,6 @@ static int stop(struct mddev *mddev)
safe_put_page(conf->tmppage);
kfree(conf->poolinfo);
kfree(conf);
- mddev->private = NULL;
- return 0;
}
static int raid1_resize(struct mddev *mddev, sector_t sectors)
@@ -3155,7 +3153,7 @@ static struct md_personality raid1_personality =
.owner = THIS_MODULE,
.make_request = make_request,
.run = run,
- .stop = stop,
+ .free = raid1_free,
.status = status,
.error_handler = error,
.hot_add_disk = raid1_add_disk,
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 654fdae906aa..d1203cddb024 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3798,9 +3798,9 @@ out:
return -EIO;
}
-static int stop(struct mddev *mddev)
+static void raid10_free(struct mddev *mddev, void *priv)
{
- struct r10conf *conf = mddev->private;
+ struct r10conf *conf = priv;
if (conf->r10bio_pool)
mempool_destroy(conf->r10bio_pool);
@@ -3809,8 +3809,6 @@ static int stop(struct mddev *mddev)
kfree(conf->mirrors_old);
kfree(conf->mirrors_new);
kfree(conf);
- mddev->private = NULL;
- return 0;
}
static void raid10_quiesce(struct mddev *mddev, int state)
@@ -4692,7 +4690,7 @@ static struct md_personality raid10_personality =
.owner = THIS_MODULE,
.make_request = make_request,
.run = run,
- .stop = stop,
+ .free = raid10_free,
.status = status,
.error_handler = error,
.hot_add_disk = raid10_add_disk,
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 482526077647..dab908b2aa9a 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6313,14 +6313,12 @@ abort:
return -EIO;
}
-static int stop(struct mddev *mddev)
+static void raid5_free(struct mddev *mddev, void *priv)
{
- struct r5conf *conf = mddev->private;
+ struct r5conf *conf = priv;
free_conf(conf);
- mddev->private = NULL;
mddev->to_remove = &raid5_attrs_group;
- return 0;
}
static void status(struct seq_file *seq, struct mddev *mddev)
@@ -7094,7 +7092,7 @@ static struct md_personality raid6_personality =
.owner = THIS_MODULE,
.make_request = make_request,
.run = run,
- .stop = stop,
+ .free = raid5_free,
.status = status,
.error_handler = error,
.hot_add_disk = raid5_add_disk,
@@ -7118,7 +7116,7 @@ static struct md_personality raid5_personality =
.owner = THIS_MODULE,
.make_request = make_request,
.run = run,
- .stop = stop,
+ .free = raid5_free,
.status = status,
.error_handler = error,
.hot_add_disk = raid5_add_disk,
@@ -7143,7 +7141,7 @@ static struct md_personality raid4_personality =
.owner = THIS_MODULE,
.make_request = make_request,
.run = run,
- .stop = stop,
+ .free = raid5_free,
.status = status,
.error_handler = error,
.hot_add_disk = raid5_add_disk,
^ permalink raw reply related
* [md PATCH 12/26] md: level_store: group all important changes into one place.
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
Gather all the changes that can happen atomically and might
be relevant to other code into one place. This will
make it easier to refine the locking.
Note that this puts quite a few things between mddev_detach()
and ->free(). Enabling this was the point of some recent patches.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/md.c | 62 ++++++++++++++++++++++++++++---------------------------
1 file changed, 32 insertions(+), 30 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 2920fd004865..58f531f8dcc2 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -3280,9 +3280,9 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
{
char clevel[16];
ssize_t rv = len;
- struct md_personality *pers;
+ struct md_personality *pers, *oldpers;
long level;
- void *priv;
+ void *priv, *oldpriv;
struct md_rdev *rdev;
if (mddev->pers == NULL) {
@@ -3374,9 +3374,35 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
/* Looks like we have a winner */
mddev_suspend(mddev);
mddev_detach(mddev);
- mddev->pers->free(mddev, mddev->private);
+ oldpers = mddev->pers;
+ oldpriv = mddev->private;
+ mddev->pers = pers;
+ mddev->private = priv;
+ strlcpy(mddev->clevel, pers->name, sizeof(mddev->clevel));
+ mddev->level = mddev->new_level;
+ mddev->layout = mddev->new_layout;
+ mddev->chunk_sectors = mddev->new_chunk_sectors;
+ mddev->delta_disks = 0;
+ mddev->reshape_backwards = 0;
+ mddev->degraded = 0;
+
+ if (oldpers->sync_request == NULL &&
+ mddev->external) {
+ /* We are converting from a no-redundancy array
+ * to a redundancy array and metadata is managed
+ * externally so we need to be sure that writes
+ * won't block due to a need to transition
+ * clean->dirty
+ * until external management is started.
+ */
+ mddev->in_sync = 0;
+ mddev->safemode_delay = 0;
+ mddev->safemode = 0;
+ }
- if (mddev->pers->sync_request == NULL &&
+ oldpers->free(mddev, oldpriv);
+
+ if (oldpers->sync_request == NULL &&
pers->sync_request != NULL) {
/* need to add the md_redundancy_group */
if (sysfs_create_group(&mddev->kobj, &md_redundancy_group))
@@ -3385,27 +3411,13 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
mdname(mddev));
mddev->sysfs_action = sysfs_get_dirent(mddev->kobj.sd, "sync_action");
}
- if (mddev->pers->sync_request != NULL &&
+ if (oldpers->sync_request != NULL &&
pers->sync_request == NULL) {
/* need to remove the md_redundancy_group */
if (mddev->to_remove == NULL)
mddev->to_remove = &md_redundancy_group;
}
- if (mddev->pers->sync_request == NULL &&
- mddev->external) {
- /* We are converting from a no-redundancy array
- * to a redundancy array and metadata is managed
- * externally so we need to be sure that writes
- * won't block due to a need to transition
- * clean->dirty
- * until external management is started.
- */
- mddev->in_sync = 0;
- mddev->safemode_delay = 0;
- mddev->safemode = 0;
- }
-
rdev_for_each(rdev, mddev) {
if (rdev->raid_disk < 0)
continue;
@@ -3431,17 +3443,7 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
}
}
- module_put(mddev->pers->owner);
- mddev->pers = pers;
- mddev->private = priv;
- strlcpy(mddev->clevel, pers->name, sizeof(mddev->clevel));
- mddev->level = mddev->new_level;
- mddev->layout = mddev->new_layout;
- mddev->chunk_sectors = mddev->new_chunk_sectors;
- mddev->delta_disks = 0;
- mddev->reshape_backwards = 0;
- mddev->degraded = 0;
- if (mddev->pers->sync_request == NULL) {
+ if (pers->sync_request == NULL) {
/* this is now an array without redundancy, so
* it must always be in_sync
*/
^ permalink raw reply related
* [md PATCH 13/26] md: protect ->pers changes with mddev->lock
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
->pers is already protected by ->reconfig_mutex, and
cannot possibly change when there are threads running or
outstanding IO.
However there are some places where we access ->pers
not in a thread or IO context, and where ->reconfig_mutex
is unnecessarily heavy-weight: level_show and md_seq_show().
So protect all changes, and those accesses, with ->lock.
This is a step toward taking those accesses out from under
reconfig_mutex.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/md.c | 51 ++++++++++++++++++++++++++++++++-------------------
drivers/md/md.h | 1 +
2 files changed, 33 insertions(+), 19 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 58f531f8dcc2..421d47e2d457 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -3264,15 +3264,20 @@ __ATTR(safe_mode_delay, S_IRUGO|S_IWUSR,safe_delay_show, safe_delay_store);
static ssize_t
level_show(struct mddev *mddev, char *page)
{
- struct md_personality *p = mddev->pers;
+ struct md_personality *p;
+ int ret;
+ spin_lock(&mddev->lock);
+ p = mddev->pers;
if (p)
- return sprintf(page, "%s\n", p->name);
+ ret = sprintf(page, "%s\n", p->name);
else if (mddev->clevel[0])
- return sprintf(page, "%s\n", mddev->clevel);
+ ret = sprintf(page, "%s\n", mddev->clevel);
else if (mddev->level != LEVEL_NONE)
- return sprintf(page, "%d\n", mddev->level);
+ ret = sprintf(page, "%d\n", mddev->level);
else
- return 0;
+ ret = 0;
+ spin_unlock(&mddev->lock);
+ return ret;
}
static ssize_t
@@ -3374,6 +3379,8 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
/* Looks like we have a winner */
mddev_suspend(mddev);
mddev_detach(mddev);
+
+ spin_lock(&mddev->lock);
oldpers = mddev->pers;
oldpriv = mddev->private;
mddev->pers = pers;
@@ -3385,6 +3392,7 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
mddev->delta_disks = 0;
mddev->reshape_backwards = 0;
mddev->degraded = 0;
+ spin_unlock(&mddev->lock);
if (oldpers->sync_request == NULL &&
mddev->external) {
@@ -4866,7 +4874,6 @@ int md_run(struct mddev *mddev)
mddev->clevel);
return -EINVAL;
}
- mddev->pers = pers;
spin_unlock(&pers_lock);
if (mddev->level != pers->level) {
mddev->level = pers->level;
@@ -4877,7 +4884,6 @@ int md_run(struct mddev *mddev)
if (mddev->reshape_position != MaxSector &&
pers->start_reshape == NULL) {
/* This personality cannot handle reshaping... */
- mddev->pers = NULL;
module_put(pers->owner);
return -EINVAL;
}
@@ -4921,10 +4927,10 @@ int md_run(struct mddev *mddev)
if (start_readonly && mddev->ro == 0)
mddev->ro = 2; /* read-only, but switch on first write */
- err = mddev->pers->run(mddev);
+ err = pers->run(mddev);
if (err)
printk(KERN_ERR "md: pers->run() failed ...\n");
- else if (mddev->pers->size(mddev, 0, 0) < mddev->array_sectors) {
+ else if (pers->size(mddev, 0, 0) < mddev->array_sectors) {
WARN_ONCE(!mddev->external_size, "%s: default size too small,"
" but 'external_size' not in effect?\n", __func__);
printk(KERN_ERR
@@ -4933,7 +4939,7 @@ int md_run(struct mddev *mddev)
(unsigned long long)mddev->pers->size(mddev, 0, 0) / 2);
err = -EINVAL;
}
- if (err == 0 && mddev->pers->sync_request &&
+ if (err == 0 && pers->sync_request &&
(mddev->bitmap_info.file || mddev->bitmap_info.offset)) {
err = bitmap_create(mddev);
if (err)
@@ -4942,9 +4948,8 @@ int md_run(struct mddev *mddev)
}
if (err) {
mddev_detach(mddev);
- mddev->pers->free(mddev, mddev->private);
- module_put(mddev->pers->owner);
- mddev->pers = NULL;
+ pers->free(mddev, mddev->private);
+ module_put(pers->owner);
bitmap_destroy(mddev);
return err;
}
@@ -4953,7 +4958,7 @@ int md_run(struct mddev *mddev)
mddev->queue->backing_dev_info.congested_fn = md_congested;
blk_queue_merge_bvec(mddev->queue, md_mergeable_bvec);
}
- if (mddev->pers->sync_request) {
+ if (pers->sync_request) {
if (mddev->kobj.sd &&
sysfs_create_group(&mddev->kobj, &md_redundancy_group))
printk(KERN_WARNING
@@ -4972,7 +4977,10 @@ int md_run(struct mddev *mddev)
mddev->safemode_delay = (200 * HZ)/1000 +1; /* 200 msec delay */
mddev->in_sync = 1;
smp_wmb();
+ spin_lock(&mddev->lock);
+ mddev->pers = pers;
mddev->ready = 1;
+ spin_unlock(&mddev->lock);
rdev_for_each(rdev, mddev)
if (rdev->raid_disk >= 0)
if (sysfs_link_rdev(mddev, rdev))
@@ -5137,13 +5145,16 @@ static void mddev_detach(struct mddev *mddev)
static void __md_stop(struct mddev *mddev)
{
- mddev->ready = 0;
+ struct md_personality *pers = mddev->pers;
mddev_detach(mddev);
- mddev->pers->free(mddev, mddev->private);
- if (mddev->pers->sync_request && mddev->to_remove == NULL)
- mddev->to_remove = &md_redundancy_group;
- module_put(mddev->pers->owner);
+ spin_lock(&mddev->lock);
+ mddev->ready = 0;
mddev->pers = NULL;
+ spin_unlock(&mddev->lock);
+ pers->free(mddev, mddev->private);
+ if (pers->sync_request && mddev->to_remove == NULL)
+ mddev->to_remove = &md_redundancy_group;
+ module_put(pers->owner);
clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
}
@@ -6942,6 +6953,7 @@ static int md_seq_show(struct seq_file *seq, void *v)
if (mddev_lock(mddev) < 0)
return -EINTR;
+ spin_lock(&mddev->lock);
if (mddev->pers || mddev->raid_disks || !list_empty(&mddev->disks)) {
seq_printf(seq, "%s : %sactive", mdname(mddev),
mddev->pers ? "" : "in");
@@ -7012,6 +7024,7 @@ static int md_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, "\n");
}
+ spin_unlock(&mddev->lock);
mddev_unlock(mddev);
return 0;
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 37e7c17e56a6..e41559dccdc9 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -391,6 +391,7 @@ struct mddev {
* rdev superblocks, events
* clearing MD_CHANGE_*
* in_sync - and related safemode and MD_CHANGE changes
+ * pers (also protected by reconfig_mutex and pending IO).
*/
spinlock_t lock;
wait_queue_head_t sb_wait; /* for waiting on superblock updates */
^ permalink raw reply related
* [md PATCH 14/26] md/bitmap: protect clearing of ->bitmap by mddev->lock
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
This makes it safe to inspect the struct while holding only
the spinlock.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/bitmap.c | 2 ++
drivers/md/md.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index 1695ee5f3ffc..3424b1915fc4 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -1619,7 +1619,9 @@ void bitmap_destroy(struct mddev *mddev)
return;
mutex_lock(&mddev->bitmap_info.mutex);
+ spin_lock(&mddev->lock);
mddev->bitmap = NULL; /* disconnect from the md device */
+ spin_unlock(&mddev->lock);
mutex_unlock(&mddev->bitmap_info.mutex);
if (mddev->thread)
mddev->thread->timeout = MAX_SCHEDULE_TIMEOUT;
diff --git a/drivers/md/md.h b/drivers/md/md.h
index e41559dccdc9..8770308a8052 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -392,6 +392,7 @@ struct mddev {
* clearing MD_CHANGE_*
* in_sync - and related safemode and MD_CHANGE changes
* pers (also protected by reconfig_mutex and pending IO).
+ * clearing ->bitmap
*/
spinlock_t lock;
wait_queue_head_t sb_wait; /* for waiting on superblock updates */
^ permalink raw reply related
* [md PATCH 16/26] md/raid5: use ->lock to protect accessing raid5 sysfs attributes.
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
It is important that mddev->private isn't freed while
a sysfs attribute function is accessing it.
So use mddev->lock to protect the setting of ->private to NULL, and
take that lock when checking ->private for NULL and de-referencing it
in the sysfs access functions.
This only applies to the read ('show') side of access. Write
access will be handled separately.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/raid5.c | 44 ++++++++++++++++++++++++++++----------------
1 file changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index dab908b2aa9a..d5b80174b3b3 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5354,11 +5354,14 @@ static void raid5d(struct md_thread *thread)
static ssize_t
raid5_show_stripe_cache_size(struct mddev *mddev, char *page)
{
- struct r5conf *conf = mddev->private;
+ struct r5conf *conf;
+ int ret = 0;
+ spin_lock(&mddev->lock);
+ conf = mddev->private;
if (conf)
- return sprintf(page, "%d\n", conf->max_nr_stripes);
- else
- return 0;
+ ret = sprintf(page, "%d\n", conf->max_nr_stripes);
+ spin_unlock(&mddev->lock);
+ return ret;
}
int
@@ -5422,11 +5425,14 @@ raid5_stripecache_size = __ATTR(stripe_cache_size, S_IRUGO | S_IWUSR,
static ssize_t
raid5_show_preread_threshold(struct mddev *mddev, char *page)
{
- struct r5conf *conf = mddev->private;
+ struct r5conf *conf;
+ int ret = 0;
+ spin_lock(&mddev->lock);
+ conf = mddev->private;
if (conf)
- return sprintf(page, "%d\n", conf->bypass_threshold);
- else
- return 0;
+ ret = sprintf(page, "%d\n", conf->bypass_threshold);
+ spin_unlock(&mddev->lock);
+ return ret;
}
static ssize_t
@@ -5456,11 +5462,14 @@ raid5_preread_bypass_threshold = __ATTR(preread_bypass_threshold,
static ssize_t
raid5_show_skip_copy(struct mddev *mddev, char *page)
{
- struct r5conf *conf = mddev->private;
+ struct r5conf *conf;
+ int ret = 0;
+ spin_lock(&mddev->lock);
+ conf = mddev->private;
if (conf)
- return sprintf(page, "%d\n", conf->skip_copy);
- else
- return 0;
+ ret = sprintf(page, "%d\n", conf->skip_copy);
+ spin_unlock(&mddev->lock);
+ return ret;
}
static ssize_t
@@ -5512,11 +5521,14 @@ raid5_stripecache_active = __ATTR_RO(stripe_cache_active);
static ssize_t
raid5_show_group_thread_cnt(struct mddev *mddev, char *page)
{
- struct r5conf *conf = mddev->private;
+ struct r5conf *conf;
+ int ret = 0;
+ spin_lock(&mddev->lock);
+ conf = mddev->private;
if (conf)
- return sprintf(page, "%d\n", conf->worker_cnt_per_group);
- else
- return 0;
+ ret = sprintf(page, "%d\n", conf->worker_cnt_per_group);
+ spin_unlock(&mddev->lock);
+ return ret;
}
static int alloc_thread_groups(struct r5conf *conf, int cnt,
^ permalink raw reply related
* [md PATCH 15/26] md: remove need for mddev_lock() in md_seq_show()
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
The only access in md_seq_show that could suffer from races
not protected by ->lock is walking the rdev list.
This can receive sufficient protection from 'rcu'.
So use rdev_for_each_rcu() and get rid of mddev_lock().
Now reading /proc/mdstat will never block in md_seq_show.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/md.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 421d47e2d457..4caa414fe6a3 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6950,9 +6950,6 @@ static int md_seq_show(struct seq_file *seq, void *v)
return 0;
}
- if (mddev_lock(mddev) < 0)
- return -EINTR;
-
spin_lock(&mddev->lock);
if (mddev->pers || mddev->raid_disks || !list_empty(&mddev->disks)) {
seq_printf(seq, "%s : %sactive", mdname(mddev),
@@ -6966,7 +6963,8 @@ static int md_seq_show(struct seq_file *seq, void *v)
}
sectors = 0;
- rdev_for_each(rdev, mddev) {
+ rcu_read_lock();
+ rdev_for_each_rcu(rdev, mddev) {
char b[BDEVNAME_SIZE];
seq_printf(seq, " %s[%d]",
bdevname(rdev->bdev,b), rdev->desc_nr);
@@ -6982,6 +6980,7 @@ static int md_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, "(R)");
sectors += rdev->sectors;
}
+ rcu_read_unlock();
if (!list_empty(&mddev->disks)) {
if (mddev->pers)
@@ -7025,7 +7024,6 @@ static int md_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, "\n");
}
spin_unlock(&mddev->lock);
- mddev_unlock(mddev);
return 0;
}
^ permalink raw reply related
* [md PATCH 17/26] md: remove mddev_lock() from md_attr_show()
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
Most attributes can be read safely without any locking.
A race might lead to a slightly out-dated value, but nothing wrong.
We already have locking in some places where needed.
All that remains is can_clear_show(), behind_writes_used_show()
and action_show() which are easily fixed.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/bitmap.c | 13 ++++++++++---
drivers/md/md.c | 23 ++++++++++-------------
2 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index 3424b1915fc4..3a5767968ba0 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -2211,11 +2211,13 @@ __ATTR(metadata, S_IRUGO|S_IWUSR, metadata_show, metadata_store);
static ssize_t can_clear_show(struct mddev *mddev, char *page)
{
int len;
+ spin_lock(&mddev->lock);
if (mddev->bitmap)
len = sprintf(page, "%s\n", (mddev->bitmap->need_sync ?
"false" : "true"));
else
len = sprintf(page, "\n");
+ spin_unlock(&mddev->lock);
return len;
}
@@ -2240,10 +2242,15 @@ __ATTR(can_clear, S_IRUGO|S_IWUSR, can_clear_show, can_clear_store);
static ssize_t
behind_writes_used_show(struct mddev *mddev, char *page)
{
+ ssize_t ret;
+ spin_lock(&mddev->lock);
if (mddev->bitmap == NULL)
- return sprintf(page, "0\n");
- return sprintf(page, "%lu\n",
- mddev->bitmap->behind_writes_used);
+ ret = sprintf(page, "0\n");
+ else
+ ret = sprintf(page, "%lu\n",
+ mddev->bitmap->behind_writes_used);
+ spin_unlock(&mddev->lock);
+ return ret;
}
static ssize_t
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 4caa414fe6a3..b7f48238a92d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4042,20 +4042,21 @@ static ssize_t
action_show(struct mddev *mddev, char *page)
{
char *type = "idle";
- if (test_bit(MD_RECOVERY_FROZEN, &mddev->recovery))
+ unsigned long recovery = mddev->recovery;
+ if (test_bit(MD_RECOVERY_FROZEN, &recovery))
type = "frozen";
- else if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
- (!mddev->ro && test_bit(MD_RECOVERY_NEEDED, &mddev->recovery))) {
- if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery))
+ else if (test_bit(MD_RECOVERY_RUNNING, &recovery) ||
+ (!mddev->ro && test_bit(MD_RECOVERY_NEEDED, &recovery))) {
+ if (test_bit(MD_RECOVERY_RESHAPE, &recovery))
type = "reshape";
- else if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery)) {
- if (!test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery))
+ else if (test_bit(MD_RECOVERY_SYNC, &recovery)) {
+ if (!test_bit(MD_RECOVERY_REQUESTED, &recovery))
type = "resync";
- else if (test_bit(MD_RECOVERY_CHECK, &mddev->recovery))
+ else if (test_bit(MD_RECOVERY_CHECK, &recovery))
type = "check";
else
type = "repair";
- } else if (test_bit(MD_RECOVERY_RECOVER, &mddev->recovery))
+ } else if (test_bit(MD_RECOVERY_RECOVER, &recovery))
type = "recover";
}
return sprintf(page, "%s\n", type);
@@ -4572,11 +4573,7 @@ md_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
mddev_get(mddev);
spin_unlock(&all_mddevs_lock);
- rv = mddev_lock(mddev);
- if (!rv) {
- rv = entry->show(mddev, page);
- mddev_unlock(mddev);
- }
+ rv = entry->show(mddev, page);
mddev_put(mddev);
return rv;
}
^ permalink raw reply related
* [md PATCH 18/26] md: remove mddev_lock from rdev_attr_show()
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
No rdev attributes need locking for 'show', though
state_show() might benefit from ensuring it sees a
consistent set of flags.
None even use rdev->mddev, so testing for it isn't really
needed and it certainly doesn't need to be held constant.
So improve state_show() and remove the locking.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/md.c | 36 ++++++++++++++----------------------
1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index b7f48238a92d..cbbac7b7238a 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2419,40 +2419,41 @@ state_show(struct md_rdev *rdev, char *page)
{
char *sep = "";
size_t len = 0;
+ unsigned long flags = ACCESS_ONCE(rdev->flags);
- if (test_bit(Faulty, &rdev->flags) ||
+ if (test_bit(Faulty, &flags) ||
rdev->badblocks.unacked_exist) {
len+= sprintf(page+len, "%sfaulty",sep);
sep = ",";
}
- if (test_bit(In_sync, &rdev->flags)) {
+ if (test_bit(In_sync, &flags)) {
len += sprintf(page+len, "%sin_sync",sep);
sep = ",";
}
- if (test_bit(WriteMostly, &rdev->flags)) {
+ if (test_bit(WriteMostly, &flags)) {
len += sprintf(page+len, "%swrite_mostly",sep);
sep = ",";
}
- if (test_bit(Blocked, &rdev->flags) ||
+ if (test_bit(Blocked, &flags) ||
(rdev->badblocks.unacked_exist
- && !test_bit(Faulty, &rdev->flags))) {
+ && !test_bit(Faulty, &flags))) {
len += sprintf(page+len, "%sblocked", sep);
sep = ",";
}
- if (!test_bit(Faulty, &rdev->flags) &&
- !test_bit(In_sync, &rdev->flags)) {
+ if (!test_bit(Faulty, &flags) &&
+ !test_bit(In_sync, &flags)) {
len += sprintf(page+len, "%sspare", sep);
sep = ",";
}
- if (test_bit(WriteErrorSeen, &rdev->flags)) {
+ if (test_bit(WriteErrorSeen, &flags)) {
len += sprintf(page+len, "%swrite_error", sep);
sep = ",";
}
- if (test_bit(WantReplacement, &rdev->flags)) {
+ if (test_bit(WantReplacement, &flags)) {
len += sprintf(page+len, "%swant_replacement", sep);
sep = ",";
}
- if (test_bit(Replacement, &rdev->flags)) {
+ if (test_bit(Replacement, &flags)) {
len += sprintf(page+len, "%sreplacement", sep);
sep = ",";
}
@@ -2965,21 +2966,12 @@ rdev_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
{
struct rdev_sysfs_entry *entry = container_of(attr, struct rdev_sysfs_entry, attr);
struct md_rdev *rdev = container_of(kobj, struct md_rdev, kobj);
- struct mddev *mddev = rdev->mddev;
- ssize_t rv;
if (!entry->show)
return -EIO;
-
- rv = mddev ? mddev_lock(mddev) : -EBUSY;
- if (!rv) {
- if (rdev->mddev == NULL)
- rv = -EBUSY;
- else
- rv = entry->show(rdev, page);
- mddev_unlock(mddev);
- }
- return rv;
+ if (!rdev->mddev)
+ return -EBUSY;
+ return entry->show(rdev, page);
}
static ssize_t
^ permalink raw reply related
* [md PATCH 19/26] md: remove unnecessary 'buf' from get_bitmap_file.
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
'buf' is only used because d_path fills from the end of the
buffer instead of from the start.
We don't need a separate buf to handle that, we just need to use
memmove() to move the string to the start.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/md.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index cbbac7b7238a..01793f24f8de 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5502,7 +5502,7 @@ static int get_array_info(struct mddev *mddev, void __user *arg)
static int get_bitmap_file(struct mddev *mddev, void __user * arg)
{
mdu_bitmap_file_t *file = NULL; /* too big for stack allocation */
- char *ptr, *buf = NULL;
+ char *ptr;
int err = -ENOMEM;
file = kmalloc(sizeof(*file), GFP_NOIO);
@@ -5516,23 +5516,19 @@ static int get_bitmap_file(struct mddev *mddev, void __user * arg)
goto copy_out;
}
- buf = kmalloc(sizeof(file->pathname), GFP_KERNEL);
- if (!buf)
- goto out;
-
ptr = d_path(&mddev->bitmap->storage.file->f_path,
- buf, sizeof(file->pathname));
+ file->pathname, sizeof(file->pathname));
if (IS_ERR(ptr))
goto out;
- strcpy(file->pathname, ptr);
+ memmove(file->pathname, ptr,
+ sizeof(file->pathname)-(ptr-file->pathname));
copy_out:
err = 0;
if (copy_to_user(arg, file, sizeof(*file)))
err = -EFAULT;
out:
- kfree(buf);
kfree(file);
return err;
}
^ permalink raw reply related
* [md PATCH 20/26] md: tidy up set_bitmap_file
From: NeilBrown @ 2015-02-03 21:42 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20150203213948.3448.80258.stgit@notabene.brown>
1/ delay setting mddev->bitmap_info.file until 'f' looks
usable, so we don't have to unset it.
2/ Don't allow bitmap file to be set if bitmap_info.file
is already set.
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/md/md.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 01793f24f8de..1e32f2e93d1d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5851,22 +5851,24 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
if (fd >= 0) {
struct inode *inode;
- if (mddev->bitmap)
+ struct file *f;
+
+ if (mddev->bitmap || mddev->bitmap_info.file)
return -EEXIST; /* cannot add when bitmap is present */
- mddev->bitmap_info.file = fget(fd);
+ f = fget(fd);
- if (mddev->bitmap_info.file == NULL) {
+ if (f == NULL) {
printk(KERN_ERR "%s: error: failed to get bitmap file\n",
mdname(mddev));
return -EBADF;
}
- inode = mddev->bitmap_info.file->f_mapping->host;
+ inode = f->f_mapping->host;
if (!S_ISREG(inode->i_mode)) {
printk(KERN_ERR "%s: error: bitmap file must be a regular file\n",
mdname(mddev));
err = -EBADF;
- } else if (!(mddev->bitmap_info.file->f_mode & FMODE_WRITE)) {
+ } else if (!(f->f_mode & FMODE_WRITE)) {
printk(KERN_ERR "%s: error: bitmap file must open for write\n",
mdname(mddev));
err = -EBADF;
@@ -5876,10 +5878,10 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
err = -EBUSY;
}
if (err) {
- fput(mddev->bitmap_info.file);
- mddev->bitmap_info.file = NULL;
+ fput(f);
return err;
}
+ mddev->bitmap_info.file = f;
mddev->bitmap_info.offset = 0; /* file overrides offset */
} else if (mddev->bitmap == NULL)
return -ENOENT; /* cannot remove what isn't there */
^ 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