* [PATCH] block: make blk_queue_zone_sectors() return sector_t type
@ 2019-04-10 6:10 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 6:10 UTC (permalink / raw)
To: Jens Axboe, Damien Le Moal
Cc: Hannes Reinecke, Omar Sandoval, Martin K. Petersen,
Bart Van Assche, Ming Lei, Greg Edwards, linux-block,
kernel-janitors
My static checker complains about this line from dmz_get_zoned_device()
aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
The problem is that "aligned_capacity" and "dev->capacity" are sector_t
type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
higher 32 bits in aligned_capacity are always cleared to zero.
Declaring blk_queue_zone_sectors() as a sector_t addresses this warning
and it feels intuitive based on the function name. I updated
bdev_zone_sectors() as well just to be consistent.
Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Please review this one extra carefully. I'm not positive it's correct.
include/linux/blkdev.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4b85dc066264..1785a7f506be 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -670,7 +670,7 @@ static inline bool blk_queue_is_zoned(struct request_queue *q)
}
}
-static inline unsigned int blk_queue_zone_sectors(struct request_queue *q)
+static inline sector_t blk_queue_zone_sectors(struct request_queue *q)
{
return blk_queue_is_zoned(q) ? q->limits.chunk_sectors : 0;
}
@@ -1419,7 +1419,7 @@ static inline bool bdev_is_zoned(struct block_device *bdev)
return false;
}
-static inline unsigned int bdev_zone_sectors(struct block_device *bdev)
+static inline sector_t bdev_zone_sectors(struct block_device *bdev)
{
struct request_queue *q = bdev_get_queue(bdev);
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] block: make blk_queue_zone_sectors() return sector_t type
@ 2019-04-10 6:10 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 6:10 UTC (permalink / raw)
To: Jens Axboe, Damien Le Moal
Cc: Hannes Reinecke, Omar Sandoval, Martin K. Petersen,
Bart Van Assche, Ming Lei, Greg Edwards, linux-block,
kernel-janitors
My static checker complains about this line from dmz_get_zoned_device()
aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
The problem is that "aligned_capacity" and "dev->capacity" are sector_t
type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
higher 32 bits in aligned_capacity are always cleared to zero.
Declaring blk_queue_zone_sectors() as a sector_t addresses this warning
and it feels intuitive based on the function name. I updated
bdev_zone_sectors() as well just to be consistent.
Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Please review this one extra carefully. I'm not positive it's correct.
include/linux/blkdev.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4b85dc066264..1785a7f506be 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -670,7 +670,7 @@ static inline bool blk_queue_is_zoned(struct request_queue *q)
}
}
-static inline unsigned int blk_queue_zone_sectors(struct request_queue *q)
+static inline sector_t blk_queue_zone_sectors(struct request_queue *q)
{
return blk_queue_is_zoned(q) ? q->limits.chunk_sectors : 0;
}
@@ -1419,7 +1419,7 @@ static inline bool bdev_is_zoned(struct block_device *bdev)
return false;
}
-static inline unsigned int bdev_zone_sectors(struct block_device *bdev)
+static inline sector_t bdev_zone_sectors(struct block_device *bdev)
{
struct request_queue *q = bdev_get_queue(bdev);
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] block: make blk_queue_zone_sectors() return sector_t type
2019-04-10 6:10 ` Dan Carpenter
(?)
@ 2019-04-10 7:20 ` Damien Le Moal
2019-04-10 7:31 ` Dan Carpenter
2019-04-10 7:47 ` Dan Carpenter
-1 siblings, 2 replies; 20+ messages in thread
From: Damien Le Moal @ 2019-04-10 7:20 UTC (permalink / raw)
To: Dan Carpenter, Jens Axboe
Cc: Hannes Reinecke, Omar Sandoval, Martin K. Petersen,
Bart Van Assche, Ming Lei, Greg Edwards, linux-block,
kernel-janitors
On 2019/04/10 15:10, Dan Carpenter wrote:
> My static checker complains about this line from dmz_get_zoned_device()
>
> aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
>
> The problem is that "aligned_capacity" and "dev->capacity" are sector_t
> type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
> higher 32 bits in aligned_capacity are always cleared to zero.
>
> Declaring blk_queue_zone_sectors() as a sector_t addresses this warning
> and it feels intuitive based on the function name. I updated
> bdev_zone_sectors() as well just to be consistent.
>
> Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Please review this one extra carefully. I'm not positive it's correct.
>
> include/linux/blkdev.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 4b85dc066264..1785a7f506be 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -670,7 +670,7 @@ static inline bool blk_queue_is_zoned(struct request_queue *q)
> }
> }
>
> -static inline unsigned int blk_queue_zone_sectors(struct request_queue *q)
> +static inline sector_t blk_queue_zone_sectors(struct request_queue *q)
> {
> return blk_queue_is_zoned(q) ? q->limits.chunk_sectors : 0;
> }
> @@ -1419,7 +1419,7 @@ static inline bool bdev_is_zoned(struct block_device *bdev)
> return false;
> }
>
> -static inline unsigned int bdev_zone_sectors(struct block_device *bdev)
> +static inline sector_t bdev_zone_sectors(struct block_device *bdev)
> {
> struct request_queue *q = bdev_get_queue(bdev);
Indeed, using sector_t instead of unsigned int is more intuitive and changing to
sector_t matches the 64bits zone length in struct blk_zone too, so no problem on
this front.
However, q->limits.chunk_sectors is an unsigned int, which is why I used that
type, to reflect the fact that even though the API allows values larger than 4G
sectors, the handling through the queue limits truncates that to at most 4G-1
sectors. Furthermore, the unsigned int type is used wherever
blk_queue_zone_sectors() and bdev_zone_sectors() are called (block/ioctl.c,
block/blk-zoned.c, f2fs, etc). That needs in depth checking.
I would rather keep the unsigned int since the queue limits will give only 32
bit values anyway. What about a cast for the aligned capacity line ? Something like:
aligned_capacity = dev->capacity & ~((u64)blk_queue_zone_sectors(q) - 1);
Would that make your checker happy ?
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] block: make blk_queue_zone_sectors() return sector_t type
2019-04-10 7:20 ` Damien Le Moal
@ 2019-04-10 7:31 ` Dan Carpenter
2019-04-10 7:47 ` Dan Carpenter
1 sibling, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 7:31 UTC (permalink / raw)
To: Damien Le Moal
Cc: Jens Axboe, Hannes Reinecke, Omar Sandoval, Martin K. Petersen,
Bart Van Assche, Ming Lei, Greg Edwards, linux-block,
kernel-janitors
On Wed, Apr 10, 2019 at 07:20:27AM +0000, Damien Le Moal wrote:
> On 2019/04/10 15:10, Dan Carpenter wrote:
> > My static checker complains about this line from dmz_get_zoned_device()
> >
> > aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> >
> > The problem is that "aligned_capacity" and "dev->capacity" are sector_t
> > type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
> > higher 32 bits in aligned_capacity are always cleared to zero.
> >
> > Declaring blk_queue_zone_sectors() as a sector_t addresses this warning
> > and it feels intuitive based on the function name. I updated
> > bdev_zone_sectors() as well just to be consistent.
> >
> > Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > Please review this one extra carefully. I'm not positive it's correct.
> >
> > include/linux/blkdev.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > index 4b85dc066264..1785a7f506be 100644
> > --- a/include/linux/blkdev.h
> > +++ b/include/linux/blkdev.h
> > @@ -670,7 +670,7 @@ static inline bool blk_queue_is_zoned(struct request_queue *q)
> > }
> > }
> >
> > -static inline unsigned int blk_queue_zone_sectors(struct request_queue *q)
> > +static inline sector_t blk_queue_zone_sectors(struct request_queue *q)
> > {
> > return blk_queue_is_zoned(q) ? q->limits.chunk_sectors : 0;
> > }
> > @@ -1419,7 +1419,7 @@ static inline bool bdev_is_zoned(struct block_device *bdev)
> > return false;
> > }
> >
> > -static inline unsigned int bdev_zone_sectors(struct block_device *bdev)
> > +static inline sector_t bdev_zone_sectors(struct block_device *bdev)
> > {
> > struct request_queue *q = bdev_get_queue(bdev);
>
> Indeed, using sector_t instead of unsigned int is more intuitive and changing to
> sector_t matches the 64bits zone length in struct blk_zone too, so no problem on
> this front.
>
> However, q->limits.chunk_sectors is an unsigned int, which is why I used that
> type, to reflect the fact that even though the API allows values larger than 4G
> sectors, the handling through the queue limits truncates that to at most 4G-1
> sectors. Furthermore, the unsigned int type is used wherever
> blk_queue_zone_sectors() and bdev_zone_sectors() are called (block/ioctl.c,
> block/blk-zoned.c, f2fs, etc). That needs in depth checking.
>
> I would rather keep the unsigned int since the queue limits will give only 32
> bit values anyway. What about a cast for the aligned capacity line ? Something like:
>
> aligned_capacity = dev->capacity & ~((u64)blk_queue_zone_sectors(q) - 1);
>
> Would that make your checker happy ?
>
Yeah. Or a sector_t cast.
aligned_capacity = dev->capacity & ~((sector_t)blk_queue_zone_sectors(q) - 1);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] block: make blk_queue_zone_sectors() return sector_t type
@ 2019-04-10 7:31 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 7:31 UTC (permalink / raw)
To: Damien Le Moal
Cc: Jens Axboe, Hannes Reinecke, Omar Sandoval, Martin K. Petersen,
Bart Van Assche, Ming Lei, Greg Edwards, linux-block,
kernel-janitors
On Wed, Apr 10, 2019 at 07:20:27AM +0000, Damien Le Moal wrote:
> On 2019/04/10 15:10, Dan Carpenter wrote:
> > My static checker complains about this line from dmz_get_zoned_device()
> >
> > aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> >
> > The problem is that "aligned_capacity" and "dev->capacity" are sector_t
> > type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
> > higher 32 bits in aligned_capacity are always cleared to zero.
> >
> > Declaring blk_queue_zone_sectors() as a sector_t addresses this warning
> > and it feels intuitive based on the function name. I updated
> > bdev_zone_sectors() as well just to be consistent.
> >
> > Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > Please review this one extra carefully. I'm not positive it's correct.
> >
> > include/linux/blkdev.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > index 4b85dc066264..1785a7f506be 100644
> > --- a/include/linux/blkdev.h
> > +++ b/include/linux/blkdev.h
> > @@ -670,7 +670,7 @@ static inline bool blk_queue_is_zoned(struct request_queue *q)
> > }
> > }
> >
> > -static inline unsigned int blk_queue_zone_sectors(struct request_queue *q)
> > +static inline sector_t blk_queue_zone_sectors(struct request_queue *q)
> > {
> > return blk_queue_is_zoned(q) ? q->limits.chunk_sectors : 0;
> > }
> > @@ -1419,7 +1419,7 @@ static inline bool bdev_is_zoned(struct block_device *bdev)
> > return false;
> > }
> >
> > -static inline unsigned int bdev_zone_sectors(struct block_device *bdev)
> > +static inline sector_t bdev_zone_sectors(struct block_device *bdev)
> > {
> > struct request_queue *q = bdev_get_queue(bdev);
>
> Indeed, using sector_t instead of unsigned int is more intuitive and changing to
> sector_t matches the 64bits zone length in struct blk_zone too, so no problem on
> this front.
>
> However, q->limits.chunk_sectors is an unsigned int, which is why I used that
> type, to reflect the fact that even though the API allows values larger than 4G
> sectors, the handling through the queue limits truncates that to at most 4G-1
> sectors. Furthermore, the unsigned int type is used wherever
> blk_queue_zone_sectors() and bdev_zone_sectors() are called (block/ioctl.c,
> block/blk-zoned.c, f2fs, etc). That needs in depth checking.
>
> I would rather keep the unsigned int since the queue limits will give only 32
> bit values anyway. What about a cast for the aligned capacity line ? Something like:
>
> aligned_capacity = dev->capacity & ~((u64)blk_queue_zone_sectors(q) - 1);
>
> Would that make your checker happy ?
>
Yeah. Or a sector_t cast.
aligned_capacity = dev->capacity & ~((sector_t)blk_queue_zone_sectors(q) - 1);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] dm zoned: Silence a static checker warning
2019-04-10 7:20 ` Damien Le Moal
2019-04-10 7:31 ` Dan Carpenter
@ 2019-04-10 7:47 ` Dan Carpenter
1 sibling, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 7:47 UTC (permalink / raw)
To: Alasdair Kergon, Damien Le Moal
Cc: Mike Snitzer, dm-devel, kernel-janitors, Hannes Reinecke,
Omar Sandoval, Martin K. Petersen, Bart Van Assche, Ming Lei,
Greg Edwards, linux-block, Jens Axboe
My static checker complains about this line from dmz_get_zoned_device()
aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
The problem is that "aligned_capacity" and "dev->capacity" are sector_t
type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
higher 32 bits in "aligned_capacity" are always cleared to zero. This
patch adds a cast to u64 to address this issue.
Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: In v1 I changed blk_queue_zone_sectors() to return a sector_t type,
but in v2 I just add a cast. The v2 fix would end up going through
different maintainers and reviewers so the CC list has grown...
Original discussion: https://marc.info/?l=kernel-janitors&m=155487663405737&w=2
drivers/md/dm-zoned-target.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index 8865c1709e16..b6cb44fa946d 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
q = bdev_get_queue(dev->bdev);
dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
- aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
+ aligned_capacity = dev->capacity &
+ ~((u64)blk_queue_zone_sectors(q) - 1);
if (ti->begin ||
((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
ti->error = "Partial mapping not supported";
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2] dm zoned: Silence a static checker warning
@ 2019-04-10 7:47 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 7:47 UTC (permalink / raw)
To: Alasdair Kergon, Damien Le Moal
Cc: Jens Axboe, Hannes Reinecke, Mike Snitzer, Martin K. Petersen,
Greg Edwards, kernel-janitors, Ming Lei, linux-block, dm-devel,
Omar Sandoval, Bart Van Assche
My static checker complains about this line from dmz_get_zoned_device()
aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
The problem is that "aligned_capacity" and "dev->capacity" are sector_t
type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
higher 32 bits in "aligned_capacity" are always cleared to zero. This
patch adds a cast to u64 to address this issue.
Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: In v1 I changed blk_queue_zone_sectors() to return a sector_t type,
but in v2 I just add a cast. The v2 fix would end up going through
different maintainers and reviewers so the CC list has grown...
Original discussion: https://marc.info/?l=kernel-janitors&m\x155487663405737&w=2
drivers/md/dm-zoned-target.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index 8865c1709e16..b6cb44fa946d 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
q = bdev_get_queue(dev->bdev);
dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
- aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
+ aligned_capacity = dev->capacity &
+ ~((u64)blk_queue_zone_sectors(q) - 1);
if (ti->begin ||
((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
ti->error = "Partial mapping not supported";
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2] dm zoned: Silence a static checker warning
@ 2019-04-10 7:47 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 7:47 UTC (permalink / raw)
To: Alasdair Kergon, Damien Le Moal
Cc: Jens Axboe, Hannes Reinecke, Mike Snitzer, Martin K. Petersen,
Greg Edwards, kernel-janitors, Ming Lei, linux-block, dm-devel,
Omar Sandoval, Bart Van Assche
My static checker complains about this line from dmz_get_zoned_device()
aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
The problem is that "aligned_capacity" and "dev->capacity" are sector_t
type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
higher 32 bits in "aligned_capacity" are always cleared to zero. This
patch adds a cast to u64 to address this issue.
Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: In v1 I changed blk_queue_zone_sectors() to return a sector_t type,
but in v2 I just add a cast. The v2 fix would end up going through
different maintainers and reviewers so the CC list has grown...
Original discussion: https://marc.info/?l=kernel-janitors&m=155487663405737&w=2
drivers/md/dm-zoned-target.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index 8865c1709e16..b6cb44fa946d 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
q = bdev_get_queue(dev->bdev);
dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
- aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
+ aligned_capacity = dev->capacity &
+ ~((u64)blk_queue_zone_sectors(q) - 1);
if (ti->begin ||
((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
ti->error = "Partial mapping not supported";
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2] dm zoned: Silence a static checker warning
2019-04-10 7:47 ` Dan Carpenter
@ 2019-04-10 7:56 ` Damien Le Moal
-1 siblings, 0 replies; 20+ messages in thread
From: Damien Le Moal @ 2019-04-10 7:56 UTC (permalink / raw)
To: Dan Carpenter, Alasdair Kergon
Cc: Mike Snitzer, dm-devel, kernel-janitors, Hannes Reinecke,
Omar Sandoval, Martin K. Petersen, Bart Van Assche, Ming Lei,
Greg Edwards, linux-block, Jens Axboe
On 2019/04/10 16:48, Dan Carpenter wrote:
> My static checker complains about this line from dmz_get_zoned_device()
>
> aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
>
> The problem is that "aligned_capacity" and "dev->capacity" are sector_t
> type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
> higher 32 bits in "aligned_capacity" are always cleared to zero. This
> patch adds a cast to u64 to address this issue.
>
> Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: In v1 I changed blk_queue_zone_sectors() to return a sector_t type,
> but in v2 I just add a cast. The v2 fix would end up going through
> different maintainers and reviewers so the CC list has grown...
>
> Original discussion: https://marc.info/?l=kernel-janitors&m=155487663405737&w=2
>
> drivers/md/dm-zoned-target.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
> index 8865c1709e16..b6cb44fa946d 100644
> --- a/drivers/md/dm-zoned-target.c
> +++ b/drivers/md/dm-zoned-target.c
> @@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
>
> q = bdev_get_queue(dev->bdev);
> dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
> - aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> + aligned_capacity = dev->capacity &
> + ~((u64)blk_queue_zone_sectors(q) - 1);
sector_t is an u64 only if CONFIG_LBDAF is defined (I think this option is going
away though). Otherwise it is an unsigned long which would be u32 on 32 bits
arch. Not a problem in terms of arithmetic, but why not cast to sector_t directly ?
> if (ti->begin ||
> ((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
> ti->error = "Partial mapping not supported";
>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] dm zoned: Silence a static checker warning
@ 2019-04-10 7:56 ` Damien Le Moal
0 siblings, 0 replies; 20+ messages in thread
From: Damien Le Moal @ 2019-04-10 7:56 UTC (permalink / raw)
To: Dan Carpenter, Alasdair Kergon
Cc: Jens Axboe, Hannes Reinecke, Martin K. Petersen, Mike Snitzer,
Edwards, kernel-janitors, Ming Lei, linux-block, dm-devel, Greg,
Omar, Sandoval, Bart Van Assche
On 2019/04/10 16:48, Dan Carpenter wrote:
> My static checker complains about this line from dmz_get_zoned_device()
>
> aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
>
> The problem is that "aligned_capacity" and "dev->capacity" are sector_t
> type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
> higher 32 bits in "aligned_capacity" are always cleared to zero. This
> patch adds a cast to u64 to address this issue.
>
> Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: In v1 I changed blk_queue_zone_sectors() to return a sector_t type,
> but in v2 I just add a cast. The v2 fix would end up going through
> different maintainers and reviewers so the CC list has grown...
>
> Original discussion: https://marc.info/?l=kernel-janitors&m=155487663405737&w=2
>
> drivers/md/dm-zoned-target.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
> index 8865c1709e16..b6cb44fa946d 100644
> --- a/drivers/md/dm-zoned-target.c
> +++ b/drivers/md/dm-zoned-target.c
> @@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
>
> q = bdev_get_queue(dev->bdev);
> dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
> - aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> + aligned_capacity = dev->capacity &
> + ~((u64)blk_queue_zone_sectors(q) - 1);
sector_t is an u64 only if CONFIG_LBDAF is defined (I think this option is going
away though). Otherwise it is an unsigned long which would be u32 on 32 bits
arch. Not a problem in terms of arithmetic, but why not cast to sector_t directly ?
> if (ti->begin ||
> ((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
> ti->error = "Partial mapping not supported";
>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] dm zoned: Silence a static checker warning
2019-04-10 7:56 ` Damien Le Moal
(?)
@ 2019-04-10 8:03 ` Dan Carpenter
-1 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 8:03 UTC (permalink / raw)
To: Damien Le Moal
Cc: Alasdair Kergon, Mike Snitzer, dm-devel, kernel-janitors,
Hannes Reinecke, Omar Sandoval, Martin K. Petersen,
Bart Van Assche, Ming Lei, Greg Edwards, linux-block, Jens Axboe
On Wed, Apr 10, 2019 at 07:56:14AM +0000, Damien Le Moal wrote:
> On 2019/04/10 16:48, Dan Carpenter wrote:
> > My static checker complains about this line from dmz_get_zoned_device()
> >
> > aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> >
> > The problem is that "aligned_capacity" and "dev->capacity" are sector_t
> > type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
> > higher 32 bits in "aligned_capacity" are always cleared to zero. This
> > patch adds a cast to u64 to address this issue.
> >
> > Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: In v1 I changed blk_queue_zone_sectors() to return a sector_t type,
> > but in v2 I just add a cast. The v2 fix would end up going through
> > different maintainers and reviewers so the CC list has grown...
> >
> > Original discussion: https://marc.info/?l=kernel-janitors&m=155487663405737&w=2
> >
> > drivers/md/dm-zoned-target.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
> > index 8865c1709e16..b6cb44fa946d 100644
> > --- a/drivers/md/dm-zoned-target.c
> > +++ b/drivers/md/dm-zoned-target.c
> > @@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
> >
> > q = bdev_get_queue(dev->bdev);
> > dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
> > - aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> > + aligned_capacity = dev->capacity &
> > + ~((u64)blk_queue_zone_sectors(q) - 1);
>
> sector_t is an u64 only if CONFIG_LBDAF is defined (I think this option is going
> away though). Otherwise it is an unsigned long which would be u32 on 32 bits
> arch. Not a problem in terms of arithmetic, but why not cast to sector_t directly ?
>
I would have prefered to do that but I didn't have strong feelings
either way. I am always slight annoyed when people don't just copy and
paste my code when I tell them how to fix a patch so I decided to go
with your version... :P
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] dm zoned: Silence a static checker warning
@ 2019-04-10 8:03 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 8:03 UTC (permalink / raw)
To: Damien Le Moal
Cc: Jens Axboe, Hannes Reinecke, Martin K. Petersen, Mike Snitzer,
Greg Edwards, kernel-janitors, Ming Lei, linux-block, dm-devel,
Omar Sandoval, Alasdair Kergon, Bart Van Assche
On Wed, Apr 10, 2019 at 07:56:14AM +0000, Damien Le Moal wrote:
> On 2019/04/10 16:48, Dan Carpenter wrote:
> > My static checker complains about this line from dmz_get_zoned_device()
> >
> > aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> >
> > The problem is that "aligned_capacity" and "dev->capacity" are sector_t
> > type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
> > higher 32 bits in "aligned_capacity" are always cleared to zero. This
> > patch adds a cast to u64 to address this issue.
> >
> > Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: In v1 I changed blk_queue_zone_sectors() to return a sector_t type,
> > but in v2 I just add a cast. The v2 fix would end up going through
> > different maintainers and reviewers so the CC list has grown...
> >
> > Original discussion: https://marc.info/?l=kernel-janitors&m\x155487663405737&w=2
> >
> > drivers/md/dm-zoned-target.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
> > index 8865c1709e16..b6cb44fa946d 100644
> > --- a/drivers/md/dm-zoned-target.c
> > +++ b/drivers/md/dm-zoned-target.c
> > @@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
> >
> > q = bdev_get_queue(dev->bdev);
> > dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
> > - aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> > + aligned_capacity = dev->capacity &
> > + ~((u64)blk_queue_zone_sectors(q) - 1);
>
> sector_t is an u64 only if CONFIG_LBDAF is defined (I think this option is going
> away though). Otherwise it is an unsigned long which would be u32 on 32 bits
> arch. Not a problem in terms of arithmetic, but why not cast to sector_t directly ?
>
I would have prefered to do that but I didn't have strong feelings
either way. I am always slight annoyed when people don't just copy and
paste my code when I tell them how to fix a patch so I decided to go
with your version... :P
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] dm zoned: Silence a static checker warning
@ 2019-04-10 8:03 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 8:03 UTC (permalink / raw)
To: Damien Le Moal
Cc: Jens Axboe, Hannes Reinecke, Martin K. Petersen, Mike Snitzer,
Greg Edwards, kernel-janitors, Ming Lei, linux-block, dm-devel,
Omar Sandoval, Alasdair Kergon, Bart Van Assche
On Wed, Apr 10, 2019 at 07:56:14AM +0000, Damien Le Moal wrote:
> On 2019/04/10 16:48, Dan Carpenter wrote:
> > My static checker complains about this line from dmz_get_zoned_device()
> >
> > aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> >
> > The problem is that "aligned_capacity" and "dev->capacity" are sector_t
> > type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
> > higher 32 bits in "aligned_capacity" are always cleared to zero. This
> > patch adds a cast to u64 to address this issue.
> >
> > Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: In v1 I changed blk_queue_zone_sectors() to return a sector_t type,
> > but in v2 I just add a cast. The v2 fix would end up going through
> > different maintainers and reviewers so the CC list has grown...
> >
> > Original discussion: https://marc.info/?l=kernel-janitors&m=155487663405737&w=2
> >
> > drivers/md/dm-zoned-target.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
> > index 8865c1709e16..b6cb44fa946d 100644
> > --- a/drivers/md/dm-zoned-target.c
> > +++ b/drivers/md/dm-zoned-target.c
> > @@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
> >
> > q = bdev_get_queue(dev->bdev);
> > dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
> > - aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> > + aligned_capacity = dev->capacity &
> > + ~((u64)blk_queue_zone_sectors(q) - 1);
>
> sector_t is an u64 only if CONFIG_LBDAF is defined (I think this option is going
> away though). Otherwise it is an unsigned long which would be u32 on 32 bits
> arch. Not a problem in terms of arithmetic, but why not cast to sector_t directly ?
>
I would have prefered to do that but I didn't have strong feelings
either way. I am always slight annoyed when people don't just copy and
paste my code when I tell them how to fix a patch so I decided to go
with your version... :P
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dm-devel] [PATCH v2] dm zoned: Silence a static checker warning
2019-04-10 8:03 ` Dan Carpenter
@ 2019-04-10 8:06 ` Damien Le Moal
-1 siblings, 0 replies; 20+ messages in thread
From: Damien Le Moal @ 2019-04-10 8:06 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jens Axboe, Hannes Reinecke, Martin K. Petersen, Mike Snitzer,
Greg Edwards, kernel-janitors, Ming Lei, linux-block, dm-devel,
Omar Sandoval, Alasdair Kergon, Bart Van Assche
On 2019/04/10 17:03, Dan Carpenter wrote:
> On Wed, Apr 10, 2019 at 07:56:14AM +0000, Damien Le Moal wrote:
>> On 2019/04/10 16:48, Dan Carpenter wrote:
>>> My static checker complains about this line from dmz_get_zoned_device()
>>>
>>> aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
>>>
>>> The problem is that "aligned_capacity" and "dev->capacity" are sector_t
>>> type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
>>> higher 32 bits in "aligned_capacity" are always cleared to zero. This
>>> patch adds a cast to u64 to address this issue.
>>>
>>> Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>> ---
>>> v2: In v1 I changed blk_queue_zone_sectors() to return a sector_t type,
>>> but in v2 I just add a cast. The v2 fix would end up going through
>>> different maintainers and reviewers so the CC list has grown...
>>>
>>> Original discussion: https://marc.info/?l=kernel-janitors&m=155487663405737&w=2
>>>
>>> drivers/md/dm-zoned-target.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
>>> index 8865c1709e16..b6cb44fa946d 100644
>>> --- a/drivers/md/dm-zoned-target.c
>>> +++ b/drivers/md/dm-zoned-target.c
>>> @@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
>>>
>>> q = bdev_get_queue(dev->bdev);
>>> dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
>>> - aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
>>> + aligned_capacity = dev->capacity &
>>> + ~((u64)blk_queue_zone_sectors(q) - 1);
>>
>> sector_t is an u64 only if CONFIG_LBDAF is defined (I think this option is going
>> away though). Otherwise it is an unsigned long which would be u32 on 32 bits
>> arch. Not a problem in terms of arithmetic, but why not cast to sector_t directly ?
>>
>
> I would have prefered to do that but I didn't have strong feelings
> either way. I am always slight annoyed when people don't just copy and
> paste my code when I tell them how to fix a patch so I decided to go
> with your version... :P
I proposed u64 cast without checking sector_t definition.
After doing that, I liked your sector_t cast more than my own misinformed
proposal :)
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] dm zoned: Silence a static checker warning
@ 2019-04-10 8:06 ` Damien Le Moal
0 siblings, 0 replies; 20+ messages in thread
From: Damien Le Moal @ 2019-04-10 8:06 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jens Axboe, Hannes Reinecke, Martin K. Petersen, Mike Snitzer,
Greg Edwards, kernel-janitors, Ming Lei, linux-block, dm-devel,
Omar Sandoval, Alasdair Kergon, Bart Van Assche
On 2019/04/10 17:03, Dan Carpenter wrote:
> On Wed, Apr 10, 2019 at 07:56:14AM +0000, Damien Le Moal wrote:
>> On 2019/04/10 16:48, Dan Carpenter wrote:
>>> My static checker complains about this line from dmz_get_zoned_device()
>>>
>>> aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
>>>
>>> The problem is that "aligned_capacity" and "dev->capacity" are sector_t
>>> type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
>>> higher 32 bits in "aligned_capacity" are always cleared to zero. This
>>> patch adds a cast to u64 to address this issue.
>>>
>>> Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>> ---
>>> v2: In v1 I changed blk_queue_zone_sectors() to return a sector_t type,
>>> but in v2 I just add a cast. The v2 fix would end up going through
>>> different maintainers and reviewers so the CC list has grown...
>>>
>>> Original discussion: https://marc.info/?l=kernel-janitors&m=155487663405737&w=2
>>>
>>> drivers/md/dm-zoned-target.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
>>> index 8865c1709e16..b6cb44fa946d 100644
>>> --- a/drivers/md/dm-zoned-target.c
>>> +++ b/drivers/md/dm-zoned-target.c
>>> @@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
>>>
>>> q = bdev_get_queue(dev->bdev);
>>> dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
>>> - aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
>>> + aligned_capacity = dev->capacity &
>>> + ~((u64)blk_queue_zone_sectors(q) - 1);
>>
>> sector_t is an u64 only if CONFIG_LBDAF is defined (I think this option is going
>> away though). Otherwise it is an unsigned long which would be u32 on 32 bits
>> arch. Not a problem in terms of arithmetic, but why not cast to sector_t directly ?
>>
>
> I would have prefered to do that but I didn't have strong feelings
> either way. I am always slight annoyed when people don't just copy and
> paste my code when I tell them how to fix a patch so I decided to go
> with your version... :P
I proposed u64 cast without checking sector_t definition.
After doing that, I liked your sector_t cast more than my own misinformed
proposal :)
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3] dm zoned: Silence a static checker warning
2019-04-10 7:56 ` Damien Le Moal
(?)
@ 2019-04-10 8:12 ` Dan Carpenter
-1 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 8:12 UTC (permalink / raw)
To: Alasdair Kergon, Damien Le Moal
Cc: Mike Snitzer, dm-devel, kernel-janitors, Hannes Reinecke,
Omar Sandoval, Martin K. Petersen, Bart Van Assche, Ming Lei,
Greg Edwards, linux-block, Jens Axboe
My static checker complains about this line from dmz_get_zoned_device()
aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
The problem is that "aligned_capacity" and "dev->capacity" are sector_t
type (which is a u64 under most configs) but blk_queue_zone_sectors(q)
returns a u32 so the higher 32 bits in aligned_capacity are cleared to
zero. This patch adds a cast to address the issue.
Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: The v1 patch declared blk_queue_zone_sectors() as sector_t but the
v2 cast added a cast to u64.
v3: Cast it to sector_t instead
drivers/md/dm-zoned-target.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index 8865c1709e16..51d029bbb740 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
q = bdev_get_queue(dev->bdev);
dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
- aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
+ aligned_capacity = dev->capacity &
+ ~((sector_t)blk_queue_zone_sectors(q) - 1);
if (ti->begin ||
((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
ti->error = "Partial mapping not supported";
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3] dm zoned: Silence a static checker warning
@ 2019-04-10 8:12 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 8:12 UTC (permalink / raw)
To: Alasdair Kergon, Damien Le Moal
Cc: Jens Axboe, Hannes Reinecke, Mike Snitzer, Martin K. Petersen,
Greg Edwards, kernel-janitors, Ming Lei, linux-block, dm-devel,
Omar Sandoval, Bart Van Assche
My static checker complains about this line from dmz_get_zoned_device()
aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
The problem is that "aligned_capacity" and "dev->capacity" are sector_t
type (which is a u64 under most configs) but blk_queue_zone_sectors(q)
returns a u32 so the higher 32 bits in aligned_capacity are cleared to
zero. This patch adds a cast to address the issue.
Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: The v1 patch declared blk_queue_zone_sectors() as sector_t but the
v2 cast added a cast to u64.
v3: Cast it to sector_t instead
drivers/md/dm-zoned-target.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index 8865c1709e16..51d029bbb740 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
q = bdev_get_queue(dev->bdev);
dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
- aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
+ aligned_capacity = dev->capacity &
+ ~((sector_t)blk_queue_zone_sectors(q) - 1);
if (ti->begin ||
((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
ti->error = "Partial mapping not supported";
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3] dm zoned: Silence a static checker warning
@ 2019-04-10 8:12 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2019-04-10 8:12 UTC (permalink / raw)
To: Alasdair Kergon, Damien Le Moal
Cc: Jens Axboe, Hannes Reinecke, Mike Snitzer, Martin K. Petersen,
Greg Edwards, kernel-janitors, Ming Lei, linux-block, dm-devel,
Omar Sandoval, Bart Van Assche
My static checker complains about this line from dmz_get_zoned_device()
aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
The problem is that "aligned_capacity" and "dev->capacity" are sector_t
type (which is a u64 under most configs) but blk_queue_zone_sectors(q)
returns a u32 so the higher 32 bits in aligned_capacity are cleared to
zero. This patch adds a cast to address the issue.
Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: The v1 patch declared blk_queue_zone_sectors() as sector_t but the
v2 cast added a cast to u64.
v3: Cast it to sector_t instead
drivers/md/dm-zoned-target.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index 8865c1709e16..51d029bbb740 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
q = bdev_get_queue(dev->bdev);
dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
- aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
+ aligned_capacity = dev->capacity &
+ ~((sector_t)blk_queue_zone_sectors(q) - 1);
if (ti->begin ||
((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
ti->error = "Partial mapping not supported";
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3] dm zoned: Silence a static checker warning
2019-04-10 8:12 ` Dan Carpenter
@ 2019-04-10 8:14 ` Damien Le Moal
-1 siblings, 0 replies; 20+ messages in thread
From: Damien Le Moal @ 2019-04-10 8:14 UTC (permalink / raw)
To: Dan Carpenter, Alasdair Kergon
Cc: Mike Snitzer, dm-devel, kernel-janitors, Hannes Reinecke,
Omar Sandoval, Martin K. Petersen, Bart Van Assche, Ming Lei,
Greg Edwards, linux-block, Jens Axboe
On 2019/04/10 17:13, Dan Carpenter wrote:
> My static checker complains about this line from dmz_get_zoned_device()
>
> aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
>
> The problem is that "aligned_capacity" and "dev->capacity" are sector_t
> type (which is a u64 under most configs) but blk_queue_zone_sectors(q)
> returns a u32 so the higher 32 bits in aligned_capacity are cleared to
> zero. This patch adds a cast to address the issue.
>
> Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: The v1 patch declared blk_queue_zone_sectors() as sector_t but the
> v2 cast added a cast to u64.
> v3: Cast it to sector_t instead
>
> drivers/md/dm-zoned-target.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
> index 8865c1709e16..51d029bbb740 100644
> --- a/drivers/md/dm-zoned-target.c
> +++ b/drivers/md/dm-zoned-target.c
> @@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
>
> q = bdev_get_queue(dev->bdev);
> dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
> - aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> + aligned_capacity = dev->capacity &
> + ~((sector_t)blk_queue_zone_sectors(q) - 1);
> if (ti->begin ||
> ((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
> ti->error = "Partial mapping not supported";
>
Thanks for fixing this !
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] dm zoned: Silence a static checker warning
@ 2019-04-10 8:14 ` Damien Le Moal
0 siblings, 0 replies; 20+ messages in thread
From: Damien Le Moal @ 2019-04-10 8:14 UTC (permalink / raw)
To: Dan Carpenter, Alasdair Kergon
Cc: Jens Axboe, Hannes Reinecke, Martin K. Petersen, Mike Snitzer,
Edwards, kernel-janitors, Ming Lei, linux-block, dm-devel, Greg,
Omar, Sandoval, Bart Van Assche
On 2019/04/10 17:13, Dan Carpenter wrote:
> My static checker complains about this line from dmz_get_zoned_device()
>
> aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
>
> The problem is that "aligned_capacity" and "dev->capacity" are sector_t
> type (which is a u64 under most configs) but blk_queue_zone_sectors(q)
> returns a u32 so the higher 32 bits in aligned_capacity are cleared to
> zero. This patch adds a cast to address the issue.
>
> Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: The v1 patch declared blk_queue_zone_sectors() as sector_t but the
> v2 cast added a cast to u64.
> v3: Cast it to sector_t instead
>
> drivers/md/dm-zoned-target.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
> index 8865c1709e16..51d029bbb740 100644
> --- a/drivers/md/dm-zoned-target.c
> +++ b/drivers/md/dm-zoned-target.c
> @@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
>
> q = bdev_get_queue(dev->bdev);
> dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
> - aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> + aligned_capacity = dev->capacity &
> + ~((sector_t)blk_queue_zone_sectors(q) - 1);
> if (ti->begin ||
> ((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
> ti->error = "Partial mapping not supported";
>
Thanks for fixing this !
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-04-10 8:14 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-10 6:10 [PATCH] block: make blk_queue_zone_sectors() return sector_t type Dan Carpenter
2019-04-10 6:10 ` Dan Carpenter
2019-04-10 7:20 ` Damien Le Moal
2019-04-10 7:31 ` Dan Carpenter
2019-04-10 7:31 ` Dan Carpenter
2019-04-10 7:47 ` [PATCH v2] dm zoned: Silence a static checker warning Dan Carpenter
2019-04-10 7:47 ` Dan Carpenter
2019-04-10 7:47 ` Dan Carpenter
2019-04-10 7:56 ` Damien Le Moal
2019-04-10 7:56 ` Damien Le Moal
2019-04-10 8:03 ` Dan Carpenter
2019-04-10 8:03 ` Dan Carpenter
2019-04-10 8:03 ` Dan Carpenter
2019-04-10 8:06 ` [dm-devel] " Damien Le Moal
2019-04-10 8:06 ` Damien Le Moal
2019-04-10 8:12 ` [PATCH v3] " Dan Carpenter
2019-04-10 8:12 ` Dan Carpenter
2019-04-10 8:12 ` Dan Carpenter
2019-04-10 8:14 ` Damien Le Moal
2019-04-10 8:14 ` Damien Le Moal
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.