Re: [PATCH] raid0: Set discard_granularity to correct value after reshape.
From: NeilBrown <hidden>
Date: 2013-11-07 05:03:55
Also in:
lkml
Subsystem:
software raid (multiple disks) support, the rest · Maintainers:
Song Liu, Yu Kuai, Linus Torvalds
On Tue, 5 Nov 2013 14:25:19 +0000 "Baldysiak, Pawel" [off-list ref] wrote:
On Thursday, October 31, 2013 1:16 AM NeilBrown [off-list ref] wrote:quoted
On Wed, 30 Oct 2013 13:20:22 +0100 Pawel Baldysiak [off-list ref] wrote:quoted
In case of reshape of raid0 through raid4 a value of discard_granularity will be set to stripe size. MD driver should re-set this value to correct one when migration will be finished. Otherwise array will be left with wrong value and discard operations will notwork properly.quoted
Signed-off-by: Pawel Baldysiak <redacted> Cc: Shaohua Li <shli@kernel.org> --- drivers/md/raid0.c | 2 ++ 1 file changed, 2 insertions(+)diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c indexc4d420b..807ca3a 100644--- a/drivers/md/raid0.c +++ b/drivers/md/raid0.c@@ -266,6 +266,8 @@ static int create_strip_zones(struct mddev *mddev,struct r0conf **private_conf)quoted
} mddev->queue->backing_dev_info.congested_fn = raid0_congested; mddev->queue->backing_dev_info.congested_data = mddev; + mddev->queue->limits.discard_granularity = + queue_logical_block_size(mddev->queue); /* * now since we have the hard sector sizes, we can make sureThanks, but this doesn't seem like the right sort of fix. It is to specific to the symptom rather than trying to address the underlying problem. Maybe something like this? Can you review and test? Thanks, NeilBrowndiff --git a/drivers/md/md.c b/drivers/md/md.c index628cd529343f..740b6340f980 100644--- a/drivers/md/md.c +++ b/drivers/md/md.c@@ -3620,6 +3620,7 @@ level_store(struct mddev *mddev, const char*buf, size_t len) mddev->in_sync = 1; del_timer_sync(&mddev->safemode_timer); } + blk_set_stacking_limit(&mddev->queue->limits); pers->run(mddev); set_bit(MD_CHANGE_DEVS, &mddev->flags); mddev_resume(mddev);Hi Neil, I have tested Yours patch, and everything works well. TRIM operation are made correctly. Could You apply it to upstream? BTW. Correct name of this function is blk_set_stacking_limits(); Thanks, Paweł Baldysiak
Thanks for testing. I've queue the following up for submission soonish. NeilBrown From 3d2b31bb23ccb8170fafdf358098bb3cb0836080 Mon Sep 17 00:00:00 2001 From: NeilBrown <redacted> Date: Thu, 7 Nov 2013 13:36:36 +1100 Subject: [PATCH] md: fix calculation of stacking limits on level change. The various ->run routines of md personalities assume that the 'queue' has been initialised by the blk_set_stacking_limits() call in md_alloc(). However when the level is changed (by level_store()) the ->run routine for the new level is called for an array which has already had the stacking limits modified. This can result in incorrect final settings. So call blk_set_stacking_limits() before ->run in level_store(). A specific consequence of this bug is that it causes discard_granularity to be set incorrectly when reshaping a RAID4 to a RAID0. This is suitable for any -stable kernel since 3.3 in which blk_set_stacking_limits() was introduced. Cc: stable@vger.kernel.org (3.3+) Reported-by: "Baldysiak, Pawel" <redacted> Signed-off-by: NeilBrown <redacted>
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 628cd529343f..b3a75afee560 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c@@ -3620,6 +3620,7 @@ level_store(struct mddev *mddev, const char *buf, size_t len) mddev->in_sync = 1; del_timer_sync(&mddev->safemode_timer); } + blk_set_stacking_limits(&mddev->queue->limits); pers->run(mddev); set_bit(MD_CHANGE_DEVS, &mddev->flags); mddev_resume(mddev);
Attachments
- signature.asc [application/pgp-signature] 828 bytes