Thread (5 messages) 5 messages, 3 authors, 2020-12-12

Re: [PATCH] md: change mddev 'chunk_sectors' from int to unsigned

From: Song Liu <hidden>
Date: 2020-12-12 17:05:30
Also in: dm-devel

On Dec 12, 2020, at 8:55 AM, Mike Snitzer [off-list ref] 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 <redacted>
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 <redacted>
Reviewed-by: Song Liu <song@kernel.org>
quoted hunk ↗ jump to hunk
---
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
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help