| Message ID | 20230628093500.68779-1-frank.li@vivo.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | [1/7] block: add queue_logical_block_mask() and bdev_logical_block_mask() | expand |
On Wed, Jun 28, 2023 at 05:34:54PM +0800, Yangtao Li wrote: > Introduce queue_logical_block_mask() and bdev_logical_block_mask() > to simplify code, which replace (queue_logical_block_size(q) - 1) > and (bdev_logical_block_size(bdev) - 1). > > Signed-off-by: Yangtao Li <frank.li@vivo.com> Looks fine. Reviewed-by: Bill O'Donnell <bodonnel@redhat.com> > --- > include/linux/blkdev.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index ed44a997f629..0cc0d1694ef6 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -1150,11 +1150,21 @@ static inline unsigned queue_logical_block_size(const struct request_queue *q) > return retval; > } > > +static inline unsigned int queue_logical_block_mask(const struct request_queue *q) > +{ > + return queue_logical_block_size(q) - 1; > +} > + > static inline unsigned int bdev_logical_block_size(struct block_device *bdev) > { > return queue_logical_block_size(bdev_get_queue(bdev)); > } > > +static inline unsigned int bdev_logical_block_mask(struct block_device *bdev) > +{ > + return bdev_logical_block_size(bdev) - 1; > +} > + > static inline unsigned int queue_physical_block_size(const struct request_queue *q) > { > return q->limits.physical_block_size; > -- > 2.39.0 >
On Wed, Jun 28, 2023 at 05:34:54PM +0800, Yangtao Li wrote: > Introduce queue_logical_block_mask() and bdev_logical_block_mask() > to simplify code, which replace (queue_logical_block_size(q) - 1) > and (bdev_logical_block_size(bdev) - 1). The thing is that I know what queue_logical_block_size - 1 does. That's the low bits. _Which_ bits are queue_logical_block_mask? The high bits or the low bits? And before you say "It's obviously", we have both ways round in the kernel today. I am not in favour of this change. I might be in favour of bool queue_logical_block_aligned(q, x), but even then it doesn't seem worth the bits.
On 2023/6/29 0:46, Matthew Wilcox wrote: > On Wed, Jun 28, 2023 at 05:34:54PM +0800, Yangtao Li wrote: >> Introduce queue_logical_block_mask() and bdev_logical_block_mask() >> to simplify code, which replace (queue_logical_block_size(q) - 1) >> and (bdev_logical_block_size(bdev) - 1). > The thing is that I know what queue_logical_block_size - 1 does. > That's the low bits. _Which_ bits are queue_logical_block_mask? > The high bits or the low bits? And before you say "It's obviously", > we have both ways round in the kernel today. I guess for this you mentioned, can we name it bdev_logical_block_lmask and queue_logical_block_lmask? Thx, > > I am not in favour of this change. I might be in favour of bool > queue_logical_block_aligned(q, x), but even then it doesn't seem worth > the bits.
On Thu, Jun 29, 2023 at 11:44:35AM +0800, Yangtao Li wrote: > On 2023/6/29 0:46, Matthew Wilcox wrote: > > > On Wed, Jun 28, 2023 at 05:34:54PM +0800, Yangtao Li wrote: > > > Introduce queue_logical_block_mask() and bdev_logical_block_mask() > > > to simplify code, which replace (queue_logical_block_size(q) - 1) > > > and (bdev_logical_block_size(bdev) - 1). > > The thing is that I know what queue_logical_block_size - 1 does. > > That's the low bits. _Which_ bits are queue_logical_block_mask? > > The high bits or the low bits? And before you say "It's obviously", > > we have both ways round in the kernel today. > > > I guess for this you mentioned, can we name it bdev_logical_block_lmask and > queue_logical_block_lmask? {bdev,queue}_offset_in_lba() ? --D > > Thx, > > > > > I am not in favour of this change. I might be in favour of bool > > queue_logical_block_aligned(q, x), but even then it doesn't seem worth > > the bits.
What is the value add of this series?
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index ed44a997f629..0cc0d1694ef6 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1150,11 +1150,21 @@ static inline unsigned queue_logical_block_size(const struct request_queue *q) return retval; } +static inline unsigned int queue_logical_block_mask(const struct request_queue *q) +{ + return queue_logical_block_size(q) - 1; +} + static inline unsigned int bdev_logical_block_size(struct block_device *bdev) { return queue_logical_block_size(bdev_get_queue(bdev)); } +static inline unsigned int bdev_logical_block_mask(struct block_device *bdev) +{ + return bdev_logical_block_size(bdev) - 1; +} + static inline unsigned int queue_physical_block_size(const struct request_queue *q) { return q->limits.physical_block_size;
Introduce queue_logical_block_mask() and bdev_logical_block_mask() to simplify code, which replace (queue_logical_block_size(q) - 1) and (bdev_logical_block_size(bdev) - 1). Signed-off-by: Yangtao Li <frank.li@vivo.com> --- include/linux/blkdev.h | 10 ++++++++++ 1 file changed, 10 insertions(+)