2023-06-28 09:52:26

by 李扬韬

[permalink] [raw]
Subject: [PATCH 1/7] block: add queue_logical_block_mask() and bdev_logical_block_mask()

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 <[email protected]>
---
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



2023-06-28 09:53:42

by 李扬韬

[permalink] [raw]
Subject: [PATCH 2/7] block: Convert to bdev_logical_block_mask()

Use bdev_logical_block_mask() to simplify code.

Signed-off-by: Yangtao Li <[email protected]>
---
block/bio.c | 2 +-
block/fops.c | 4 ++--
block/ioctl.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 8672179213b9..42ccf5a21696 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1281,7 +1281,7 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)

nr_pages = DIV_ROUND_UP(offset + size, PAGE_SIZE);

- trim = size & (bdev_logical_block_size(bio->bi_bdev) - 1);
+ trim = size & bdev_logical_block_mask(bio->bi_bdev);
iov_iter_revert(iter, trim);

size -= trim;
diff --git a/block/fops.c b/block/fops.c
index a286bf3325c5..754f7014172a 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -45,7 +45,7 @@ static blk_opf_t dio_bio_write_op(struct kiocb *iocb)
static bool blkdev_dio_unaligned(struct block_device *bdev, loff_t pos,
struct iov_iter *iter)
{
- return pos & (bdev_logical_block_size(bdev) - 1) ||
+ return pos & bdev_logical_block_mask(bdev) ||
!bdev_iter_is_aligned(bdev, iter);
}

@@ -653,7 +653,7 @@ static long blkdev_fallocate(struct file *file, int mode, loff_t start,
/*
* Don't allow IO that isn't aligned to logical block size.
*/
- if ((start | len) & (bdev_logical_block_size(bdev) - 1))
+ if ((start | len) & bdev_logical_block_mask(bdev))
return -EINVAL;

filemap_invalidate_lock(inode->i_mapping);
diff --git a/block/ioctl.c b/block/ioctl.c
index 3be11941fb2d..8544702d6263 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -39,7 +39,7 @@ static int blkpg_do_ioctl(struct block_device *bdev,
switch (op) {
case BLKPG_ADD_PARTITION:
/* check if partition is aligned to blocksize */
- if (p.start & (bdev_logical_block_size(bdev) - 1))
+ if (p.start & bdev_logical_block_mask(bdev))
return -EINVAL;
return bdev_add_partition(disk, p.pno, start, length);
case BLKPG_RESIZE_PARTITION:
--
2.39.0


2023-06-28 09:54:09

by 李扬韬

[permalink] [raw]
Subject: [PATCH 7/7] erofs: Convert to bdev_logical_block_mask()

Use bdev_logical_block_mask() to simplify code.

Signed-off-by: Yangtao Li <[email protected]>
---
fs/erofs/data.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index db5e4b7636ec..13bd185ef3b3 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -387,7 +387,7 @@ static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
unsigned int blksize_mask;

if (bdev)
- blksize_mask = bdev_logical_block_size(bdev) - 1;
+ blksize_mask = bdev_logical_block_mask(bdev);
else
blksize_mask = i_blocksize(inode) - 1;

--
2.39.0


2023-06-28 10:06:35

by 李扬韬

[permalink] [raw]
Subject: [PATCH 4/7] buffer: Convert to bdev_logical_block_mask()

Use bdev_logical_block_mask() to simplify code.

Signed-off-by: Yangtao Li <[email protected]>
---
fs/buffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index bd091329026c..fd2705465a5e 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1128,7 +1128,7 @@ __getblk_slow(struct block_device *bdev, sector_t block,
unsigned size, gfp_t gfp)
{
/* Size must be multiple of hard sectorsize */
- if (unlikely(size & (bdev_logical_block_size(bdev)-1) ||
+ if (unlikely(size & bdev_logical_block_mask(bdev) ||
(size < 512 || size > PAGE_SIZE))) {
printk(KERN_ERR "getblk(): invalid block size %d requested\n",
size);
--
2.39.0


2023-06-28 10:07:57

by 李扬韬

[permalink] [raw]
Subject: [PATCH 3/7] md: Convert to queue_logical_block_mask()

Use queue_logical_block_mask() to simplify code.

Signed-off-by: Yangtao Li <[email protected]>
---
drivers/md/md.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 2e38ef421d69..fd646e5ed082 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1649,7 +1649,7 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
atomic_set(&rdev->corrected_errors, le32_to_cpu(sb->cnt_corrected_read));

rdev->sb_size = le32_to_cpu(sb->max_dev) * 2 + 256;
- bmask = queue_logical_block_size(rdev->bdev->bd_disk->queue)-1;
+ bmask = queue_logical_block_mask(rdev->bdev->bd_disk->queue);
if (rdev->sb_size & bmask)
rdev->sb_size = (rdev->sb_size | bmask) + 1;

@@ -2062,7 +2062,7 @@ static void super_1_sync(struct mddev *mddev, struct md_rdev *rdev)
int bmask;
sb->max_dev = cpu_to_le32(max_dev);
rdev->sb_size = max_dev * 2 + 256;
- bmask = queue_logical_block_size(rdev->bdev->bd_disk->queue)-1;
+ bmask = queue_logical_block_mask(rdev->bdev->bd_disk->queue);
if (rdev->sb_size & bmask)
rdev->sb_size = (rdev->sb_size | bmask) + 1;
} else
--
2.39.0


2023-06-28 17:06:54

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH 1/7] block: add queue_logical_block_mask() and bdev_logical_block_mask()

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.

2023-06-28 17:26:17

by Bill O'Donnell

[permalink] [raw]
Subject: Re: [PATCH 1/7] block: add queue_logical_block_mask() and bdev_logical_block_mask()

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 <[email protected]>

Looks fine.
Reviewed-by: Bill O'Donnell <[email protected]>

> ---
> 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
>


2023-06-29 04:00:33

by 李扬韬

[permalink] [raw]
Subject: Re: [PATCH 1/7] block: add queue_logical_block_mask() and bdev_logical_block_mask()

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.

2023-06-29 04:59:42

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH 1/7] block: add queue_logical_block_mask() and bdev_logical_block_mask()

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.