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

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

[Yangtao Li]

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(+)

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

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

[Yangtao Li]

Use bdev_logical_block_mask() to simplify code.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 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

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

[Yangtao Li]

Use queue_logical_block_mask() to simplify code.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 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

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

[Yangtao Li]

Use bdev_logical_block_mask() to simplify code.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 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

[PATCH 5/7] iomap: Convert to bdev_logical_block_mask()

[Yangtao Li]

Use bdev_logical_block_mask() to simplify code.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/iomap/direct-io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index ea3b868c8355..8d4a06bc14b3 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -238,7 +238,7 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
 	size_t copied = 0;
 	size_t orig_count;
 
-	if ((pos | length) & (bdev_logical_block_size(iomap->bdev) - 1) ||
+	if ((pos | length) & bdev_logical_block_mask(iomap->bdev) ||
 	    !bdev_iter_is_aligned(iomap->bdev, dio->submit.iter))
 		return -EINVAL;
 
-- 
2.39.0

[PATCH 6/7] xfs: Convert to bdev_logical_block_mask()

[Yangtao Li]

Use bdev_logical_block_mask() to simplify code.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/xfs/xfs_buf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 15d1e5a7c2d3..f784daa21219 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1968,7 +1968,7 @@ xfs_setsize_buftarg(
 
 	/* Set up device logical sector size mask */
 	btp->bt_logical_sectorsize = bdev_logical_block_size(btp->bt_bdev);
-	btp->bt_logical_sectormask = bdev_logical_block_size(btp->bt_bdev) - 1;
+	btp->bt_logical_sectormask = bdev_logical_block_mask(btp->bt_bdev);
 
 	return 0;
 }
-- 
2.39.0

Re: [PATCH 6/7] xfs: Convert to bdev_logical_block_mask()

[Bill O'Donnell]

On Wed, Jun 28, 2023 at 05:34:59PM +0800, Yangtao Li wrote:
> Use bdev_logical_block_mask() to simplify code.
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>

Reviewed-by: Bill O'Donnell <bodonnel@redhat.com>

> ---
>  fs/xfs/xfs_buf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 15d1e5a7c2d3..f784daa21219 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1968,7 +1968,7 @@ xfs_setsize_buftarg(
>  
>  	/* Set up device logical sector size mask */
>  	btp->bt_logical_sectorsize = bdev_logical_block_size(btp->bt_bdev);
> -	btp->bt_logical_sectormask = bdev_logical_block_size(btp->bt_bdev) - 1;
> +	btp->bt_logical_sectormask = bdev_logical_block_mask(btp->bt_bdev);
>  
>  	return 0;
>  }
> -- 
> 2.39.0
> 

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

[Yangtao Li]

Use bdev_logical_block_mask() to simplify code.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 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

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

[Bill O'Donnell]

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
> 

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

[Matthew Wilcox]

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.

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

[Yangtao Li]

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.

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

[Darrick J. Wong]

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.

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

[Christoph Hellwig]

What is the value add of this series?