* [GIT PULL v3] md-fixes 20201212
@ 2020-12-12 9:12 Song Liu
2020-12-12 14:42 ` [dm-devel] " Mike Snitzer
0 siblings, 1 reply; 9+ messages in thread
From: Song Liu @ 2020-12-12 9:12 UTC (permalink / raw)
To: Jens Axboe, linux-raid; +Cc: Mike Snitzer, Matthew Ruffell
Hi Jens,
Please consider pulling the following changes on top of tag
block-5.10-2020-12-05. This is to fix raid10 data corruption [1] in 5.10-rc7.
Tests run on this change:
1. md raid10: tested discard on raid10 device works properly (zero mismatch_cnt).
2. dm raid10: tested discard_granularity and discard_max_bytes was set properly.
Thanks,
Song
[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1907262/
The following changes since commit 7e7986f9d3ba69a7375a41080a1f8c8012cb0923:
block: use gcd() to fix chunk_sectors limit stacking (2020-12-01 11:02:55 -0700)
are available in the Git repository at:
ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/song/md.git md-fixes
for you to fetch changes up to 0d5c7b890229f8a9bb4b588b34ffe70c62691143:
Revert "md: add md_submit_discard_bio() for submitting discard bio" (2020-12-12 00:51:41 -0800)
----------------------------------------------------------------
Song Liu (7):
Revert "dm raid: remove unnecessary discard limits for raid10"
Revert "dm raid: fix discard limits for raid1 and raid10"
Revert "md/raid10: improve discard request for far layout"
Revert "md/raid10: improve raid10 discard request"
Revert "md/raid10: pull codes that wait for blocked dev into one function"
Revert "md/raid10: extend r10bio devs to raid disks"
Revert "md: add md_submit_discard_bio() for submitting discard bio"
drivers/md/dm-raid.c | 9 +++++
drivers/md/md.c | 20 ----------
drivers/md/md.h | 2 -
drivers/md/raid0.c | 14 ++++++-
drivers/md/raid10.c | 423 +++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
drivers/md/raid10.h | 1 -
6 files changed, 78 insertions(+), 391 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [GIT PULL v3] md-fixes 20201212 2020-12-12 9:12 [GIT PULL v3] md-fixes 20201212 Song Liu @ 2020-12-12 14:42 ` Mike Snitzer 0 siblings, 0 replies; 9+ messages in thread From: Mike Snitzer @ 2020-12-12 14:42 UTC (permalink / raw) To: Song Liu Cc: Jens Axboe, linux-raid, Matthew Ruffell, Xiao Ni, Heinz Mauelshagen, dm-devel On Sat, Dec 12 2020 at 4:12am -0500, Song Liu <songliubraving@fb.com> wrote: > Hi Jens, > > Please consider pulling the following changes on top of tag > block-5.10-2020-12-05. This is to fix raid10 data corruption [1] in 5.10-rc7. > > Tests run on this change: > > 1. md raid10: tested discard on raid10 device works properly (zero mismatch_cnt). > 2. dm raid10: tested discard_granularity and discard_max_bytes was set properly. > > Thanks, > Song > > > [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1907262/ > > > The following changes since commit 7e7986f9d3ba69a7375a41080a1f8c8012cb0923: > > block: use gcd() to fix chunk_sectors limit stacking (2020-12-01 11:02:55 -0700) > > are available in the Git repository at: > > ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/song/md.git md-fixes > > for you to fetch changes up to 0d5c7b890229f8a9bb4b588b34ffe70c62691143: > > Revert "md: add md_submit_discard_bio() for submitting discard bio" (2020-12-12 00:51:41 -0800) > > ---------------------------------------------------------------- > Song Liu (7): > Revert "dm raid: remove unnecessary discard limits for raid10" > Revert "dm raid: fix discard limits for raid1 and raid10" > Revert "md/raid10: improve discard request for far layout" > Revert "md/raid10: improve raid10 discard request" > Revert "md/raid10: pull codes that wait for blocked dev into one function" > Revert "md/raid10: extend r10bio devs to raid disks" > Revert "md: add md_submit_discard_bio() for submitting discard bio" > > drivers/md/dm-raid.c | 9 +++++ > drivers/md/md.c | 20 ---------- > drivers/md/md.h | 2 - > drivers/md/raid0.c | 14 ++++++- > drivers/md/raid10.c | 423 +++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ > drivers/md/raid10.h | 1 - > 6 files changed, 78 insertions(+), 391 deletions(-) > Jens already pulled 95% of these changes no? why are you sending a pull that ignores that fact? And as I stated here: https://www.redhat.com/archives/dm-devel/2020-December/msg00253.html I don't agree with reverting commit e0910c8e4f87bb9 ("dm raid: fix discard limits for raid1 and raid10"). Espeiclaly not like you've done, given it was marked for stable any follow-on fix/revert needs to be as well. Simply changing 'struct mddev' chunk_sectors members from int to 'unsigned int' is the better way forward I think. Mike ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dm-devel] [GIT PULL v3] md-fixes 20201212 @ 2020-12-12 14:42 ` Mike Snitzer 0 siblings, 0 replies; 9+ messages in thread From: Mike Snitzer @ 2020-12-12 14:42 UTC (permalink / raw) To: Song Liu Cc: Jens Axboe, Heinz Mauelshagen, dm-devel, Matthew Ruffell, linux-raid, Xiao Ni On Sat, Dec 12 2020 at 4:12am -0500, Song Liu <songliubraving@fb.com> wrote: > Hi Jens, > > Please consider pulling the following changes on top of tag > block-5.10-2020-12-05. This is to fix raid10 data corruption [1] in 5.10-rc7. > > Tests run on this change: > > 1. md raid10: tested discard on raid10 device works properly (zero mismatch_cnt). > 2. dm raid10: tested discard_granularity and discard_max_bytes was set properly. > > Thanks, > Song > > > [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1907262/ > > > The following changes since commit 7e7986f9d3ba69a7375a41080a1f8c8012cb0923: > > block: use gcd() to fix chunk_sectors limit stacking (2020-12-01 11:02:55 -0700) > > are available in the Git repository at: > > ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/song/md.git md-fixes > > for you to fetch changes up to 0d5c7b890229f8a9bb4b588b34ffe70c62691143: > > Revert "md: add md_submit_discard_bio() for submitting discard bio" (2020-12-12 00:51:41 -0800) > > ---------------------------------------------------------------- > Song Liu (7): > Revert "dm raid: remove unnecessary discard limits for raid10" > Revert "dm raid: fix discard limits for raid1 and raid10" > Revert "md/raid10: improve discard request for far layout" > Revert "md/raid10: improve raid10 discard request" > Revert "md/raid10: pull codes that wait for blocked dev into one function" > Revert "md/raid10: extend r10bio devs to raid disks" > Revert "md: add md_submit_discard_bio() for submitting discard bio" > > drivers/md/dm-raid.c | 9 +++++ > drivers/md/md.c | 20 ---------- > drivers/md/md.h | 2 - > drivers/md/raid0.c | 14 ++++++- > drivers/md/raid10.c | 423 +++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ > drivers/md/raid10.h | 1 - > 6 files changed, 78 insertions(+), 391 deletions(-) > Jens already pulled 95% of these changes no? why are you sending a pull that ignores that fact? And as I stated here: https://www.redhat.com/archives/dm-devel/2020-December/msg00253.html I don't agree with reverting commit e0910c8e4f87bb9 ("dm raid: fix discard limits for raid1 and raid10"). Espeiclaly not like you've done, given it was marked for stable any follow-on fix/revert needs to be as well. Simply changing 'struct mddev' chunk_sectors members from int to 'unsigned int' is the better way forward I think. Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL v3] md-fixes 20201212 2020-12-12 14:42 ` [dm-devel] " Mike Snitzer @ 2020-12-12 16:19 ` Jens Axboe -1 siblings, 0 replies; 9+ messages in thread From: Jens Axboe @ 2020-12-12 16:19 UTC (permalink / raw) To: Mike Snitzer, Song Liu Cc: linux-raid, Matthew Ruffell, Xiao Ni, Heinz Mauelshagen, dm-devel On 12/12/20 7:42 AM, Mike Snitzer wrote: > On Sat, Dec 12 2020 at 4:12am -0500, > Song Liu <songliubraving@fb.com> wrote: > >> Hi Jens, >> >> Please consider pulling the following changes on top of tag >> block-5.10-2020-12-05. This is to fix raid10 data corruption [1] in 5.10-rc7. >> >> Tests run on this change: >> >> 1. md raid10: tested discard on raid10 device works properly (zero mismatch_cnt). >> 2. dm raid10: tested discard_granularity and discard_max_bytes was set properly. >> >> Thanks, >> Song >> >> >> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1907262/ >> >> >> The following changes since commit 7e7986f9d3ba69a7375a41080a1f8c8012cb0923: >> >> block: use gcd() to fix chunk_sectors limit stacking (2020-12-01 11:02:55 -0700) >> >> are available in the Git repository at: >> >> ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/song/md.git md-fixes >> >> for you to fetch changes up to 0d5c7b890229f8a9bb4b588b34ffe70c62691143: >> >> Revert "md: add md_submit_discard_bio() for submitting discard bio" (2020-12-12 00:51:41 -0800) >> >> ---------------------------------------------------------------- >> Song Liu (7): >> Revert "dm raid: remove unnecessary discard limits for raid10" >> Revert "dm raid: fix discard limits for raid1 and raid10" >> Revert "md/raid10: improve discard request for far layout" >> Revert "md/raid10: improve raid10 discard request" >> Revert "md/raid10: pull codes that wait for blocked dev into one function" >> Revert "md/raid10: extend r10bio devs to raid disks" >> Revert "md: add md_submit_discard_bio() for submitting discard bio" >> >> drivers/md/dm-raid.c | 9 +++++ >> drivers/md/md.c | 20 ---------- >> drivers/md/md.h | 2 - >> drivers/md/raid0.c | 14 ++++++- >> drivers/md/raid10.c | 423 +++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ >> drivers/md/raid10.h | 1 - >> 6 files changed, 78 insertions(+), 391 deletions(-) >> > > Jens already pulled 95% of these changes no? why are you sending a pull > that ignores that fact? > > And as I stated here: > https://www.redhat.com/archives/dm-devel/2020-December/msg00253.html > > I don't agree with reverting commit e0910c8e4f87bb9 ("dm raid: fix > discard limits for raid1 and raid10"). Espeiclaly not like you've done, > given it was marked for stable any follow-on fix/revert needs to be as > well. > > Simply changing 'struct mddev' chunk_sectors members from int to > 'unsigned int' is the better way forward I think. I agree, that'd be simpler. Mike, care to send a real patch for that against block-5.10? -- Jens Axboe ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dm-devel] [GIT PULL v3] md-fixes 20201212 @ 2020-12-12 16:19 ` Jens Axboe 0 siblings, 0 replies; 9+ messages in thread From: Jens Axboe @ 2020-12-12 16:19 UTC (permalink / raw) To: Mike Snitzer, Song Liu Cc: linux-raid, Heinz Mauelshagen, Xiao Ni, dm-devel, Matthew Ruffell On 12/12/20 7:42 AM, Mike Snitzer wrote: > On Sat, Dec 12 2020 at 4:12am -0500, > Song Liu <songliubraving@fb.com> wrote: > >> Hi Jens, >> >> Please consider pulling the following changes on top of tag >> block-5.10-2020-12-05. This is to fix raid10 data corruption [1] in 5.10-rc7. >> >> Tests run on this change: >> >> 1. md raid10: tested discard on raid10 device works properly (zero mismatch_cnt). >> 2. dm raid10: tested discard_granularity and discard_max_bytes was set properly. >> >> Thanks, >> Song >> >> >> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1907262/ >> >> >> The following changes since commit 7e7986f9d3ba69a7375a41080a1f8c8012cb0923: >> >> block: use gcd() to fix chunk_sectors limit stacking (2020-12-01 11:02:55 -0700) >> >> are available in the Git repository at: >> >> ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/song/md.git md-fixes >> >> for you to fetch changes up to 0d5c7b890229f8a9bb4b588b34ffe70c62691143: >> >> Revert "md: add md_submit_discard_bio() for submitting discard bio" (2020-12-12 00:51:41 -0800) >> >> ---------------------------------------------------------------- >> Song Liu (7): >> Revert "dm raid: remove unnecessary discard limits for raid10" >> Revert "dm raid: fix discard limits for raid1 and raid10" >> Revert "md/raid10: improve discard request for far layout" >> Revert "md/raid10: improve raid10 discard request" >> Revert "md/raid10: pull codes that wait for blocked dev into one function" >> Revert "md/raid10: extend r10bio devs to raid disks" >> Revert "md: add md_submit_discard_bio() for submitting discard bio" >> >> drivers/md/dm-raid.c | 9 +++++ >> drivers/md/md.c | 20 ---------- >> drivers/md/md.h | 2 - >> drivers/md/raid0.c | 14 ++++++- >> drivers/md/raid10.c | 423 +++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ >> drivers/md/raid10.h | 1 - >> 6 files changed, 78 insertions(+), 391 deletions(-) >> > > Jens already pulled 95% of these changes no? why are you sending a pull > that ignores that fact? > > And as I stated here: > https://www.redhat.com/archives/dm-devel/2020-December/msg00253.html > > I don't agree with reverting commit e0910c8e4f87bb9 ("dm raid: fix > discard limits for raid1 and raid10"). Espeiclaly not like you've done, > given it was marked for stable any follow-on fix/revert needs to be as > well. > > Simply changing 'struct mddev' chunk_sectors members from int to > 'unsigned int' is the better way forward I think. I agree, that'd be simpler. Mike, care to send a real patch for that against block-5.10? -- Jens Axboe -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] md: change mddev 'chunk_sectors' from int to unsigned 2020-12-12 16:19 ` [dm-devel] " Jens Axboe @ 2020-12-12 16:55 ` Mike Snitzer -1 siblings, 0 replies; 9+ messages in thread From: Mike Snitzer @ 2020-12-12 16:55 UTC (permalink / raw) To: Jens Axboe Cc: Song Liu, linux-raid, Matthew Ruffell, Xiao Ni, Heinz Mauelshagen, dm-devel Commit e2782f560c29 ("Revert "dm raid: remove unnecessary discard limits for raid10"") exposed compiler warnings introduced by commit e0910c8e4f87 ("dm raid: fix discard limits for raid1 and raid10"): In file included from ./include/linux/kernel.h:14, from ./include/asm-generic/bug.h:20, from ./arch/x86/include/asm/bug.h:93, from ./include/linux/bug.h:5, from ./include/linux/mmdebug.h:5, from ./include/linux/gfp.h:5, from ./include/linux/slab.h:15, from drivers/md/dm-raid.c:8: drivers/md/dm-raid.c: In function ‘raid_io_hints’: ./include/linux/minmax.h:18:28: warning: comparison of distinct pointer types lacks a cast (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) ^~ ./include/linux/minmax.h:32:4: note: in expansion of macro ‘__typecheck’ (__typecheck(x, y) && __no_side_effects(x, y)) ^~~~~~~~~~~ ./include/linux/minmax.h:42:24: note: in expansion of macro ‘__safe_cmp’ __builtin_choose_expr(__safe_cmp(x, y), \ ^~~~~~~~~~ ./include/linux/minmax.h:51:19: note: in expansion of macro ‘__careful_cmp’ #define min(x, y) __careful_cmp(x, y, <) ^~~~~~~~~~~~~ ./include/linux/minmax.h:84:39: note: in expansion of macro ‘min’ __x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); }) ^~~ drivers/md/dm-raid.c:3739:33: note: in expansion of macro ‘min_not_zero’ limits->max_discard_sectors = min_not_zero(rs->md.chunk_sectors, ^~~~~~~~~~~~ Fix this by changing the chunk_sectors member of 'struct mddev' from int to 'unsigned int' to match the type used for the 'chunk_sectors' member of 'struct queue_limits'. Various MD code still uses 'int' but none of it appears to ever make use of signed int; and storing positive signed int in unsigned is perfectly safe. Reported-by: Song Liu <songliubraving@fb.com> Fixes: e2782f560c29 ("Revert "dm raid: remove unnecessary discard limits for raid10"") Fixes: e0910c8e4f87 ("dm raid: fix discard limits for raid1 and raid10") Cc: stable@vger,kernel.org # e0910c8e4f87 was marked for stable@ Signed-off-by: Mike Snitzer <snitzer@redhat.com> --- drivers/md/md.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/md/md.h b/drivers/md/md.h index 2175a5ac4f7c..bb645bc3ba6d 100644 --- a/drivers/md/md.h +++ b/drivers/md/md.h @@ -311,7 +311,7 @@ struct mddev { int external; /* metadata is * managed externally */ char metadata_type[17]; /* externally set*/ - int chunk_sectors; + unsigned int chunk_sectors; time64_t ctime, utime; int level, layout; char clevel[16]; @@ -339,7 +339,7 @@ struct mddev { */ sector_t reshape_position; int delta_disks, new_level, new_layout; - int new_chunk_sectors; + unsigned int new_chunk_sectors; int reshape_backwards; struct md_thread *thread; /* management thread */ -- 2.15.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [dm-devel] [PATCH] md: change mddev 'chunk_sectors' from int to unsigned @ 2020-12-12 16:55 ` Mike Snitzer 0 siblings, 0 replies; 9+ messages in thread From: Mike Snitzer @ 2020-12-12 16:55 UTC (permalink / raw) To: Jens Axboe Cc: Song Liu, Heinz Mauelshagen, Xiao Ni, Matthew Ruffell, linux-raid, dm-devel Commit e2782f560c29 ("Revert "dm raid: remove unnecessary discard limits for raid10"") exposed compiler warnings introduced by commit e0910c8e4f87 ("dm raid: fix discard limits for raid1 and raid10"): In file included from ./include/linux/kernel.h:14, from ./include/asm-generic/bug.h:20, from ./arch/x86/include/asm/bug.h:93, from ./include/linux/bug.h:5, from ./include/linux/mmdebug.h:5, from ./include/linux/gfp.h:5, from ./include/linux/slab.h:15, from drivers/md/dm-raid.c:8: drivers/md/dm-raid.c: In function ‘raid_io_hints’: ./include/linux/minmax.h:18:28: warning: comparison of distinct pointer types lacks a cast (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) ^~ ./include/linux/minmax.h:32:4: note: in expansion of macro ‘__typecheck’ (__typecheck(x, y) && __no_side_effects(x, y)) ^~~~~~~~~~~ ./include/linux/minmax.h:42:24: note: in expansion of macro ‘__safe_cmp’ __builtin_choose_expr(__safe_cmp(x, y), \ ^~~~~~~~~~ ./include/linux/minmax.h:51:19: note: in expansion of macro ‘__careful_cmp’ #define min(x, y) __careful_cmp(x, y, <) ^~~~~~~~~~~~~ ./include/linux/minmax.h:84:39: note: in expansion of macro ‘min’ __x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); }) ^~~ drivers/md/dm-raid.c:3739:33: note: in expansion of macro ‘min_not_zero’ limits->max_discard_sectors = min_not_zero(rs->md.chunk_sectors, ^~~~~~~~~~~~ Fix this by changing the chunk_sectors member of 'struct mddev' from int to 'unsigned int' to match the type used for the 'chunk_sectors' member of 'struct queue_limits'. Various MD code still uses 'int' but none of it appears to ever make use of signed int; and storing positive signed int in unsigned is perfectly safe. Reported-by: Song Liu <songliubraving@fb.com> Fixes: e2782f560c29 ("Revert "dm raid: remove unnecessary discard limits for raid10"") Fixes: e0910c8e4f87 ("dm raid: fix discard limits for raid1 and raid10") Cc: stable@vger,kernel.org # e0910c8e4f87 was marked for stable@ Signed-off-by: Mike Snitzer <snitzer@redhat.com> --- drivers/md/md.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/md/md.h b/drivers/md/md.h index 2175a5ac4f7c..bb645bc3ba6d 100644 --- a/drivers/md/md.h +++ b/drivers/md/md.h @@ -311,7 +311,7 @@ struct mddev { int external; /* metadata is * managed externally */ char metadata_type[17]; /* externally set*/ - int chunk_sectors; + unsigned int chunk_sectors; time64_t ctime, utime; int level, layout; char clevel[16]; @@ -339,7 +339,7 @@ struct mddev { */ sector_t reshape_position; int delta_disks, new_level, new_layout; - int new_chunk_sectors; + unsigned int new_chunk_sectors; int reshape_backwards; struct md_thread *thread; /* management thread */ -- 2.15.0 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] md: change mddev 'chunk_sectors' from int to unsigned 2020-12-12 16:55 ` [dm-devel] " Mike Snitzer @ 2020-12-12 17:04 ` Song Liu -1 siblings, 0 replies; 9+ messages in thread From: Song Liu @ 2020-12-12 17:04 UTC (permalink / raw) To: Mike Snitzer Cc: Jens Axboe, linux-raid, Matthew Ruffell, Xiao Ni, Heinz Mauelshagen, dm-devel > On Dec 12, 2020, at 8:55 AM, Mike Snitzer <snitzer@redhat.com> wrote: > > Commit e2782f560c29 ("Revert "dm raid: remove unnecessary discard > limits for raid10"") exposed compiler warnings introduced by commit > e0910c8e4f87 ("dm raid: fix discard limits for raid1 and raid10"): > > In file included from ./include/linux/kernel.h:14, > from ./include/asm-generic/bug.h:20, > from ./arch/x86/include/asm/bug.h:93, > from ./include/linux/bug.h:5, > from ./include/linux/mmdebug.h:5, > from ./include/linux/gfp.h:5, > from ./include/linux/slab.h:15, > from drivers/md/dm-raid.c:8: > drivers/md/dm-raid.c: In function ‘raid_io_hints’: > ./include/linux/minmax.h:18:28: warning: comparison of distinct pointer types lacks a cast > (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) > ^~ > ./include/linux/minmax.h:32:4: note: in expansion of macro ‘__typecheck’ > (__typecheck(x, y) && __no_side_effects(x, y)) > ^~~~~~~~~~~ > ./include/linux/minmax.h:42:24: note: in expansion of macro ‘__safe_cmp’ > __builtin_choose_expr(__safe_cmp(x, y), \ > ^~~~~~~~~~ > ./include/linux/minmax.h:51:19: note: in expansion of macro ‘__careful_cmp’ > #define min(x, y) __careful_cmp(x, y, <) > ^~~~~~~~~~~~~ > ./include/linux/minmax.h:84:39: note: in expansion of macro ‘min’ > __x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); }) > ^~~ > drivers/md/dm-raid.c:3739:33: note: in expansion of macro ‘min_not_zero’ > limits->max_discard_sectors = min_not_zero(rs->md.chunk_sectors, > ^~~~~~~~~~~~ > > Fix this by changing the chunk_sectors member of 'struct mddev' from > int to 'unsigned int' to match the type used for the 'chunk_sectors' > member of 'struct queue_limits'. Various MD code still uses 'int' but > none of it appears to ever make use of signed int; and storing > positive signed int in unsigned is perfectly safe. Thanks for the quick fix and thorough analysis. I also checked MD code and didn't see any use of negative chunk_sectors, so this change is safe. I will convert the rest use of signed chunk_sectors in 5.11. > > Reported-by: Song Liu <songliubraving@fb.com> > Fixes: e2782f560c29 ("Revert "dm raid: remove unnecessary discard limits for raid10"") > Fixes: e0910c8e4f87 ("dm raid: fix discard limits for raid1 and raid10") > Cc: stable@vger,kernel.org # e0910c8e4f87 was marked for stable@ > Signed-off-by: Mike Snitzer <snitzer@redhat.com> Reviewed-by: Song Liu <song@kernel.org> > --- > drivers/md/md.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/md.h b/drivers/md/md.h > index 2175a5ac4f7c..bb645bc3ba6d 100644 > --- a/drivers/md/md.h > +++ b/drivers/md/md.h > @@ -311,7 +311,7 @@ struct mddev { > int external; /* metadata is > * managed externally */ > char metadata_type[17]; /* externally set*/ > - int chunk_sectors; > + unsigned int chunk_sectors; > time64_t ctime, utime; > int level, layout; > char clevel[16]; > @@ -339,7 +339,7 @@ struct mddev { > */ > sector_t reshape_position; > int delta_disks, new_level, new_layout; > - int new_chunk_sectors; > + unsigned int new_chunk_sectors; > int reshape_backwards; > > struct md_thread *thread; /* management thread */ > -- > 2.15.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dm-devel] [PATCH] md: change mddev 'chunk_sectors' from int to unsigned @ 2020-12-12 17:04 ` Song Liu 0 siblings, 0 replies; 9+ messages in thread From: Song Liu @ 2020-12-12 17:04 UTC (permalink / raw) To: Mike Snitzer Cc: Jens Axboe, Heinz Mauelshagen, dm-devel, Matthew Ruffell, linux-raid, Xiao Ni > On Dec 12, 2020, at 8:55 AM, Mike Snitzer <snitzer@redhat.com> wrote: > > Commit e2782f560c29 ("Revert "dm raid: remove unnecessary discard > limits for raid10"") exposed compiler warnings introduced by commit > e0910c8e4f87 ("dm raid: fix discard limits for raid1 and raid10"): > > In file included from ./include/linux/kernel.h:14, > from ./include/asm-generic/bug.h:20, > from ./arch/x86/include/asm/bug.h:93, > from ./include/linux/bug.h:5, > from ./include/linux/mmdebug.h:5, > from ./include/linux/gfp.h:5, > from ./include/linux/slab.h:15, > from drivers/md/dm-raid.c:8: > drivers/md/dm-raid.c: In function ‘raid_io_hints’: > ./include/linux/minmax.h:18:28: warning: comparison of distinct pointer types lacks a cast > (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) > ^~ > ./include/linux/minmax.h:32:4: note: in expansion of macro ‘__typecheck’ > (__typecheck(x, y) && __no_side_effects(x, y)) > ^~~~~~~~~~~ > ./include/linux/minmax.h:42:24: note: in expansion of macro ‘__safe_cmp’ > __builtin_choose_expr(__safe_cmp(x, y), \ > ^~~~~~~~~~ > ./include/linux/minmax.h:51:19: note: in expansion of macro ‘__careful_cmp’ > #define min(x, y) __careful_cmp(x, y, <) > ^~~~~~~~~~~~~ > ./include/linux/minmax.h:84:39: note: in expansion of macro ‘min’ > __x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); }) > ^~~ > drivers/md/dm-raid.c:3739:33: note: in expansion of macro ‘min_not_zero’ > limits->max_discard_sectors = min_not_zero(rs->md.chunk_sectors, > ^~~~~~~~~~~~ > > Fix this by changing the chunk_sectors member of 'struct mddev' from > int to 'unsigned int' to match the type used for the 'chunk_sectors' > member of 'struct queue_limits'. Various MD code still uses 'int' but > none of it appears to ever make use of signed int; and storing > positive signed int in unsigned is perfectly safe. Thanks for the quick fix and thorough analysis. I also checked MD code and didn't see any use of negative chunk_sectors, so this change is safe. I will convert the rest use of signed chunk_sectors in 5.11. > > Reported-by: Song Liu <songliubraving@fb.com> > Fixes: e2782f560c29 ("Revert "dm raid: remove unnecessary discard limits for raid10"") > Fixes: e0910c8e4f87 ("dm raid: fix discard limits for raid1 and raid10") > Cc: stable@vger,kernel.org # e0910c8e4f87 was marked for stable@ > Signed-off-by: Mike Snitzer <snitzer@redhat.com> Reviewed-by: Song Liu <song@kernel.org> > --- > drivers/md/md.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/md.h b/drivers/md/md.h > index 2175a5ac4f7c..bb645bc3ba6d 100644 > --- a/drivers/md/md.h > +++ b/drivers/md/md.h > @@ -311,7 +311,7 @@ struct mddev { > int external; /* metadata is > * managed externally */ > char metadata_type[17]; /* externally set*/ > - int chunk_sectors; > + unsigned int chunk_sectors; > time64_t ctime, utime; > int level, layout; > char clevel[16]; > @@ -339,7 +339,7 @@ struct mddev { > */ > sector_t reshape_position; > int delta_disks, new_level, new_layout; > - int new_chunk_sectors; > + unsigned int new_chunk_sectors; > int reshape_backwards; > > struct md_thread *thread; /* management thread */ > -- > 2.15.0 > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-12-12 17:05 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-12 9:12 [GIT PULL v3] md-fixes 20201212 Song Liu 2020-12-12 14:42 ` Mike Snitzer 2020-12-12 14:42 ` [dm-devel] " Mike Snitzer 2020-12-12 16:19 ` Jens Axboe 2020-12-12 16:19 ` [dm-devel] " Jens Axboe 2020-12-12 16:55 ` [PATCH] md: change mddev 'chunk_sectors' from int to unsigned Mike Snitzer 2020-12-12 16:55 ` [dm-devel] " Mike Snitzer 2020-12-12 17:04 ` Song Liu 2020-12-12 17:04 ` [dm-devel] " Song Liu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.