* [PATCH] md/raid10: remove unnecessary checks with 'first'
@ 2017-04-01 3:38 Guoqing Jiang
2017-04-03 2:21 ` NeilBrown
2017-04-06 1:12 ` [PATCH V2] md/raid10: reset the 'first' at the end of loop Guoqing Jiang
0 siblings, 2 replies; 5+ messages in thread
From: Guoqing Jiang @ 2017-04-01 3:38 UTC (permalink / raw)
To: linux-raid; +Cc: shli, neilb
Since the value of first is always '1', we can
set min_offset_diff directly.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
drivers/md/raid10.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0f13d57..edaf8f4 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3702,7 +3702,6 @@ static int raid10_run(struct mddev *mddev)
struct md_rdev *rdev;
sector_t size;
sector_t min_offset_diff = 0;
- int first = 1;
bool discard_supported = false;
if (mddev->private == NULL) {
@@ -3758,8 +3757,7 @@ static int raid10_run(struct mddev *mddev)
diff = -diff;
if (diff < 0)
diff = 0;
- if (first || diff < min_offset_diff)
- min_offset_diff = diff;
+ min_offset_diff = diff;
if (mddev->gendisk)
disk_stack_limits(mddev->gendisk, rdev->bdev,
@@ -4140,7 +4138,6 @@ static int raid10_start_reshape(struct mddev *mddev)
unsigned long before_length, after_length;
sector_t min_offset_diff = 0;
- int first = 1;
struct geom new;
struct r10conf *conf = mddev->private;
struct md_rdev *rdev;
@@ -4169,8 +4166,7 @@ static int raid10_start_reshape(struct mddev *mddev)
diff = -diff;
if (diff < 0)
diff = 0;
- if (first || diff < min_offset_diff)
- min_offset_diff = diff;
+ min_offset_diff = diff;
}
}
--
2.6.6
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] md/raid10: remove unnecessary checks with 'first'
2017-04-01 3:38 [PATCH] md/raid10: remove unnecessary checks with 'first' Guoqing Jiang
@ 2017-04-03 2:21 ` NeilBrown
2017-04-06 1:12 ` [PATCH V2] md/raid10: reset the 'first' at the end of loop Guoqing Jiang
1 sibling, 0 replies; 5+ messages in thread
From: NeilBrown @ 2017-04-03 2:21 UTC (permalink / raw)
To: Guoqing Jiang, linux-raid; +Cc: shli
[-- Attachment #1: Type: text/plain, Size: 1890 bytes --]
On Sat, Apr 01 2017, Guoqing Jiang wrote:
> Since the value of first is always '1', we can
> set min_offset_diff directly.
Alternately, maybe the fact that first is always '1', is a sign of a bug
that should be fixed.
The calculation of "min_offset_diff" doesn't make much sense now. It is
just the last_offset_diff.
I think the correct fix is to set "first = 0;" at the end of the
rdev_for_each() loop.
Thanks for spotting this.
NeilBrown
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
> drivers/md/raid10.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0f13d57..edaf8f4 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -3702,7 +3702,6 @@ static int raid10_run(struct mddev *mddev)
> struct md_rdev *rdev;
> sector_t size;
> sector_t min_offset_diff = 0;
> - int first = 1;
> bool discard_supported = false;
>
> if (mddev->private == NULL) {
> @@ -3758,8 +3757,7 @@ static int raid10_run(struct mddev *mddev)
> diff = -diff;
> if (diff < 0)
> diff = 0;
> - if (first || diff < min_offset_diff)
> - min_offset_diff = diff;
> + min_offset_diff = diff;
>
> if (mddev->gendisk)
> disk_stack_limits(mddev->gendisk, rdev->bdev,
> @@ -4140,7 +4138,6 @@ static int raid10_start_reshape(struct mddev *mddev)
>
> unsigned long before_length, after_length;
> sector_t min_offset_diff = 0;
> - int first = 1;
> struct geom new;
> struct r10conf *conf = mddev->private;
> struct md_rdev *rdev;
> @@ -4169,8 +4166,7 @@ static int raid10_start_reshape(struct mddev *mddev)
> diff = -diff;
> if (diff < 0)
> diff = 0;
> - if (first || diff < min_offset_diff)
> - min_offset_diff = diff;
> + min_offset_diff = diff;
> }
> }
>
> --
> 2.6.6
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH V2] md/raid10: reset the 'first' at the end of loop
2017-04-01 3:38 [PATCH] md/raid10: remove unnecessary checks with 'first' Guoqing Jiang
2017-04-03 2:21 ` NeilBrown
@ 2017-04-06 1:12 ` Guoqing Jiang
2017-04-06 2:27 ` NeilBrown
2017-04-10 17:41 ` Shaohua Li
1 sibling, 2 replies; 5+ messages in thread
From: Guoqing Jiang @ 2017-04-06 1:12 UTC (permalink / raw)
To: linux-raid; +Cc: shli, neilb
We need to set "first = 0' at the end of rdev_for_each
loop, so we can get the array's min_offset_diff correctly
otherwise min_offset_diff just means the last rdev's
offset diff.
Suggested-by: NeilBrown <neilb@suse.com>
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
drivers/md/raid10.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0f13d57..e055ec9 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3769,6 +3769,7 @@ static int raid10_run(struct mddev *mddev)
if (blk_queue_discard(bdev_get_queue(rdev->bdev)))
discard_supported = true;
+ first = 0;
}
if (mddev->queue) {
@@ -4172,6 +4173,7 @@ static int raid10_start_reshape(struct mddev *mddev)
if (first || diff < min_offset_diff)
min_offset_diff = diff;
}
+ first = 0;
}
if (max(before_length, after_length) > min_offset_diff)
--
2.6.6
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH V2] md/raid10: reset the 'first' at the end of loop
2017-04-06 1:12 ` [PATCH V2] md/raid10: reset the 'first' at the end of loop Guoqing Jiang
@ 2017-04-06 2:27 ` NeilBrown
2017-04-10 17:41 ` Shaohua Li
1 sibling, 0 replies; 5+ messages in thread
From: NeilBrown @ 2017-04-06 2:27 UTC (permalink / raw)
To: Guoqing Jiang, linux-raid; +Cc: shli
[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]
On Thu, Apr 06 2017, Guoqing Jiang wrote:
> We need to set "first = 0' at the end of rdev_for_each
> loop, so we can get the array's min_offset_diff correctly
> otherwise min_offset_diff just means the last rdev's
> offset diff.
>
> Suggested-by: NeilBrown <neilb@suse.com>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
Reviewed-by: NeilBrown <neilb@suse.com>
Thanks,
NeilBrown
> ---
> drivers/md/raid10.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0f13d57..e055ec9 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -3769,6 +3769,7 @@ static int raid10_run(struct mddev *mddev)
>
> if (blk_queue_discard(bdev_get_queue(rdev->bdev)))
> discard_supported = true;
> + first = 0;
> }
>
> if (mddev->queue) {
> @@ -4172,6 +4173,7 @@ static int raid10_start_reshape(struct mddev *mddev)
> if (first || diff < min_offset_diff)
> min_offset_diff = diff;
> }
> + first = 0;
> }
>
> if (max(before_length, after_length) > min_offset_diff)
> --
> 2.6.6
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH V2] md/raid10: reset the 'first' at the end of loop
2017-04-06 1:12 ` [PATCH V2] md/raid10: reset the 'first' at the end of loop Guoqing Jiang
2017-04-06 2:27 ` NeilBrown
@ 2017-04-10 17:41 ` Shaohua Li
1 sibling, 0 replies; 5+ messages in thread
From: Shaohua Li @ 2017-04-10 17:41 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid, shli, neilb
On Thu, Apr 06, 2017 at 09:12:18AM +0800, Guoqing Jiang wrote:
> We need to set "first = 0' at the end of rdev_for_each
> loop, so we can get the array's min_offset_diff correctly
> otherwise min_offset_diff just means the last rdev's
> offset diff.
>
> Suggested-by: NeilBrown <neilb@suse.com>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
applied, thanks!
> ---
> drivers/md/raid10.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0f13d57..e055ec9 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -3769,6 +3769,7 @@ static int raid10_run(struct mddev *mddev)
>
> if (blk_queue_discard(bdev_get_queue(rdev->bdev)))
> discard_supported = true;
> + first = 0;
> }
>
> if (mddev->queue) {
> @@ -4172,6 +4173,7 @@ static int raid10_start_reshape(struct mddev *mddev)
> if (first || diff < min_offset_diff)
> min_offset_diff = diff;
> }
> + first = 0;
> }
>
> if (max(before_length, after_length) > min_offset_diff)
> --
> 2.6.6
>
> --
> 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 [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-10 17:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-01 3:38 [PATCH] md/raid10: remove unnecessary checks with 'first' Guoqing Jiang
2017-04-03 2:21 ` NeilBrown
2017-04-06 1:12 ` [PATCH V2] md/raid10: reset the 'first' at the end of loop Guoqing Jiang
2017-04-06 2:27 ` NeilBrown
2017-04-10 17:41 ` Shaohua Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).