[PATCH] block: shift with PAGE_SHIFT instead of dividing with PAGE_SIZE

[PATCH] block: shift with PAGE_SHIFT instead of dividing with PAGE_SIZE

[Pankaj Raghav]

No functional change. Division will be costly, especially in the hot
path (page_is_mergeable() and bio_copy_user_iov())

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 block/bio.c     | 3 ++-
 block/blk-map.c | 2 +-
 block/ioctl.c   | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 2e421c0dad13..2dc248e03ec2 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -922,7 +922,8 @@ static inline bool page_is_mergeable(const struct bio_vec *bv,
 		return true;
 	else if (IS_ENABLED(CONFIG_KMSAN))
 		return false;
-	return (bv->bv_page + bv_end / PAGE_SIZE) == (page + off / PAGE_SIZE);
+	return (bv->bv_page + (bv_end >> PAGE_SHIFT)) ==
+	       (page + (off >> PAGE_SHIFT));
 }
 
 /**
diff --git a/block/blk-map.c b/block/blk-map.c
index 9137d16cecdc..22a0b65cafce 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -160,7 +160,7 @@ static int bio_copy_user_iov(struct request *rq, struct rq_map_data *map_data,
 
 	if (map_data) {
 		nr_pages = 1U << map_data->page_order;
-		i = map_data->offset / PAGE_SIZE;
+		i = map_data->offset >> PAGE_SHIFT;
 	}
 	while (len) {
 		unsigned int bytes = PAGE_SIZE;
diff --git a/block/ioctl.c b/block/ioctl.c
index 9c5f637ff153..afb435adce71 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -521,7 +521,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
 	case BLKFRASET:
 		if(!capable(CAP_SYS_ADMIN))
 			return -EACCES;
-		bdev->bd_disk->bdi->ra_pages = (arg * 512) / PAGE_SIZE;
+		bdev->bd_disk->bdi->ra_pages = arg >> PAGE_SECTORS_SHIFT;
 		return 0;
 	case BLKRRPART:
 		if (!capable(CAP_SYS_ADMIN))
-- 
2.39.1

Re: [PATCH] block: shift with PAGE_SHIFT instead of dividing with PAGE_SIZE

[Bart Van Assche]

On 2/22/23 06:34, Pankaj Raghav wrote:
> No functional change. Division will be costly, especially in the hot
> path (page_is_mergeable() and bio_copy_user_iov())

Although the change looks fine to me, is there any compiler for which 
this patch makes a difference? I would expect that a compiler performs 
this optimization even without this patch.

Thanks,

Bart.

Re: [PATCH] block: shift with PAGE_SHIFT instead of dividing with PAGE_SIZE

[Pankaj Raghav]

On 2023-02-22 20:57, Bart Van Assche wrote:
> On 2/22/23 06:34, Pankaj Raghav wrote:
>> No functional change. Division will be costly, especially in the hot
>> path (page_is_mergeable() and bio_copy_user_iov())
> 
> Although the change looks fine to me, is there any compiler for which this
> patch makes a difference? I would expect that a compiler performs this
> optimization even without this patch.
>

I didn't notice any for x86_64. But I was thinking this also as a way to
maintain consistency across block code where we do a shift with PAGE_SHIFT
instead of dividing with PAGE_SIZE.

> Thanks,
> 
> Bart.
> 

Re: [PATCH] block: shift with PAGE_SHIFT instead of dividing with PAGE_SIZE

[Jens Axboe]

On 2/22/23 9:25 AM, Pankaj Raghav wrote:
> 
> On 2023-02-22 20:57, Bart Van Assche wrote:
>> On 2/22/23 06:34, Pankaj Raghav wrote:
>>> No functional change. Division will be costly, especially in the hot
>>> path (page_is_mergeable() and bio_copy_user_iov())
>>
>> Although the change looks fine to me, is there any compiler for which this
>> patch makes a difference? I would expect that a compiler performs this
>> optimization even without this patch.
>>
> 
> I didn't notice any for x86_64. But I was thinking this also as a way to
> maintain consistency across block code where we do a shift with PAGE_SHIFT
> instead of dividing with PAGE_SIZE.

It won't make a difference on any architecture, it'd be a pretty
awful compiler that didn't turn a division by a constant power-of-2
into a shift.

-- 
Jens Axboe

Re: [PATCH] block: shift with PAGE_SHIFT instead of dividing with PAGE_SIZE

[Chaitanya Kulkarni]

On 2/22/2023 6:34 AM, Pankaj Raghav wrote:
> No functional change. Division will be costly, especially in the hot
> path (page_is_mergeable() and bio_copy_user_iov())
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>


without any quantitative data provided in the commit log on different
architectures it is hard to ratify this patch.

-ck