Re: [Xen-devel] [PATCH v3] block: refactor duplicated macros

2020-03-11 Thread Jens Axboe
On 3/10/20 6:22 PM, Matteo Croce wrote:
> The macros PAGE_SECTORS, PAGE_SECTORS_SHIFT and SECTOR_MASK are defined
> several times in different flavours across the whole tree.
> Define them just once in a common header.
> 
> While at it, replace replace "PAGE_SHIFT - 9" with "PAGE_SECTORS_SHIFT" too
> and rename SECTOR_MASK to PAGE_SECTORS_MASK.

Applied, thanks.

-- 
Jens Axboe


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3] block: refactor duplicated macros

2020-03-11 Thread Coly Li
On 2020/3/11 8:22 上午, Matteo Croce wrote:
> The macros PAGE_SECTORS, PAGE_SECTORS_SHIFT and SECTOR_MASK are defined
> several times in different flavours across the whole tree.
> Define them just once in a common header.
> 
> While at it, replace replace "PAGE_SHIFT - 9" with "PAGE_SECTORS_SHIFT" too
> and rename SECTOR_MASK to PAGE_SECTORS_MASK.
> 
> Signed-off-by: Matteo Croce 

Hi Matteo,

For the bcache part, it looks good to me.

Acked-by: Coly Li 

> ---
> v3:
> As Guoqing Jiang suggested, replace "PAGE_SHIFT - 9" with "PAGE_SECTORS_SHIFT"
> 
> v2:
> As Dan Williams suggested:
> 
>  #define PAGE_SECTORS_MASK(~(PAGE_SECTORS - 1))
> 
>  block/blk-lib.c  |  2 +-
>  block/blk-settings.c |  4 ++--
>  block/partition-generic.c|  2 +-
>  drivers/block/brd.c  |  3 ---
>  drivers/block/null_blk_main.c| 14 +-
>  drivers/block/zram/zram_drv.c|  8 
>  drivers/block/zram/zram_drv.h|  2 --
>  drivers/dax/super.c  |  2 +-
>  drivers/md/bcache/util.h |  2 --
>  drivers/md/dm-bufio.c|  6 +++---
>  drivers/md/dm-integrity.c| 10 +-
>  drivers/md/dm-table.c|  2 +-
>  drivers/md/md.c  |  4 ++--
>  drivers/md/raid1.c   |  2 +-
>  drivers/md/raid10.c  |  2 +-
>  drivers/md/raid5-cache.c | 10 +-
>  drivers/md/raid5.h   |  2 +-
>  drivers/mmc/core/host.c  |  3 ++-
>  drivers/nvme/host/fc.c   |  2 +-
>  drivers/nvme/target/loop.c   |  2 +-
>  drivers/scsi/xen-scsifront.c |  4 ++--
>  fs/erofs/internal.h  |  2 +-
>  fs/ext2/dir.c|  2 +-
>  fs/iomap/buffered-io.c   |  2 +-
>  fs/libfs.c   |  2 +-
>  fs/nfs/blocklayout/blocklayout.h |  2 --
>  fs/nilfs2/dir.c  |  2 +-
>  include/linux/blkdev.h   |  4 
>  include/linux/device-mapper.h|  1 -
>  mm/page_io.c |  4 ++--
>  mm/swapfile.c| 12 ++--
>  31 files changed, 56 insertions(+), 65 deletions(-)
> 

[snipped]

> diff --git a/drivers/md/bcache/util.h b/drivers/md/bcache/util.h
> index c029f7443190..55196e0f37c3 100644
> --- a/drivers/md/bcache/util.h
> +++ b/drivers/md/bcache/util.h
> @@ -15,8 +15,6 @@
>  
>  #include "closure.h"
>  
> -#define PAGE_SECTORS (PAGE_SIZE / 512)
> -
>  struct closure;
>  
>  #ifdef CONFIG_BCACHE_DEBUG

[snipped]


-- 

Coly Li

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3] block: refactor duplicated macros

2020-03-10 Thread Martin K. Petersen

Matteo,

> The macros PAGE_SECTORS, PAGE_SECTORS_SHIFT and SECTOR_MASK are
> defined several times in different flavours across the whole tree.
> Define them just once in a common header.
>
> While at it, replace replace "PAGE_SHIFT - 9" with
> "PAGE_SECTORS_SHIFT" too and rename SECTOR_MASK to PAGE_SECTORS_MASK.

Looks OK to me.

Reviewed-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v3] block: refactor duplicated macros

2020-03-10 Thread Matteo Croce
The macros PAGE_SECTORS, PAGE_SECTORS_SHIFT and SECTOR_MASK are defined
several times in different flavours across the whole tree.
Define them just once in a common header.

While at it, replace replace "PAGE_SHIFT - 9" with "PAGE_SECTORS_SHIFT" too
and rename SECTOR_MASK to PAGE_SECTORS_MASK.

Signed-off-by: Matteo Croce 
---
v3:
As Guoqing Jiang suggested, replace "PAGE_SHIFT - 9" with "PAGE_SECTORS_SHIFT"

v2:
As Dan Williams suggested:

 #define PAGE_SECTORS_MASK(~(PAGE_SECTORS - 1))

 block/blk-lib.c  |  2 +-
 block/blk-settings.c |  4 ++--
 block/partition-generic.c|  2 +-
 drivers/block/brd.c  |  3 ---
 drivers/block/null_blk_main.c| 14 +-
 drivers/block/zram/zram_drv.c|  8 
 drivers/block/zram/zram_drv.h|  2 --
 drivers/dax/super.c  |  2 +-
 drivers/md/bcache/util.h |  2 --
 drivers/md/dm-bufio.c|  6 +++---
 drivers/md/dm-integrity.c| 10 +-
 drivers/md/dm-table.c|  2 +-
 drivers/md/md.c  |  4 ++--
 drivers/md/raid1.c   |  2 +-
 drivers/md/raid10.c  |  2 +-
 drivers/md/raid5-cache.c | 10 +-
 drivers/md/raid5.h   |  2 +-
 drivers/mmc/core/host.c  |  3 ++-
 drivers/nvme/host/fc.c   |  2 +-
 drivers/nvme/target/loop.c   |  2 +-
 drivers/scsi/xen-scsifront.c |  4 ++--
 fs/erofs/internal.h  |  2 +-
 fs/ext2/dir.c|  2 +-
 fs/iomap/buffered-io.c   |  2 +-
 fs/libfs.c   |  2 +-
 fs/nfs/blocklayout/blocklayout.h |  2 --
 fs/nilfs2/dir.c  |  2 +-
 include/linux/blkdev.h   |  4 
 include/linux/device-mapper.h|  1 -
 mm/page_io.c |  4 ++--
 mm/swapfile.c| 12 ++--
 31 files changed, 56 insertions(+), 65 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 5f2c429d4378..f5e705d307e0 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -260,7 +260,7 @@ static int __blkdev_issue_write_zeroes(struct block_device 
*bdev,
  */
 static unsigned int __blkdev_sectors_to_bio_pages(sector_t nr_sects)
 {
-   sector_t pages = DIV_ROUND_UP_SECTOR_T(nr_sects, PAGE_SIZE / 512);
+   sector_t pages = DIV_ROUND_UP_SECTOR_T(nr_sects, PAGE_SECTORS);
 
return min(pages, (sector_t)BIO_MAX_PAGES);
 }
diff --git a/block/blk-settings.c b/block/blk-settings.c
index c8eda2e7b91e..e29933581ac0 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -186,7 +186,7 @@ void blk_queue_max_hw_sectors(struct request_queue *q, 
unsigned int max_hw_secto
unsigned int max_sectors;
 
if ((max_hw_sectors << 9) < PAGE_SIZE) {
-   max_hw_sectors = 1 << (PAGE_SHIFT - 9);
+   max_hw_sectors = 1 << PAGE_SECTORS_SHIFT;
printk(KERN_INFO "%s: set to minimum %d\n",
   __func__, max_hw_sectors);
}
@@ -195,7 +195,7 @@ void blk_queue_max_hw_sectors(struct request_queue *q, 
unsigned int max_hw_secto
max_sectors = min_not_zero(max_hw_sectors, limits->max_dev_sectors);
max_sectors = min_t(unsigned int, max_sectors, BLK_DEF_MAX_SECTORS);
limits->max_sectors = max_sectors;
-   q->backing_dev_info->io_pages = max_sectors >> (PAGE_SHIFT - 9);
+   q->backing_dev_info->io_pages = max_sectors >> PAGE_SECTORS_SHIFT;
 }
 EXPORT_SYMBOL(blk_queue_max_hw_sectors);
 
diff --git a/block/partition-generic.c b/block/partition-generic.c
index 564fae77711d..5f06b7a6c119 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -612,7 +612,7 @@ unsigned char *read_dev_sector(struct block_device *bdev, 
sector_t n, Sector *p)
if (PageError(page))
goto fail;
p->v = page;
-   return (unsigned char *)page_address(page) +  ((n & ((1 << 
(PAGE_SHIFT - 9)) - 1)) << 9);
+   return (unsigned char *)page_address(page) +  ((n & ((1 << 
PAGE_SECTORS_SHIFT) - 1)) << 9);
 fail:
put_page(page);
}
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 220c5e18aba0..33e2cbe11400 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -25,9 +25,6 @@
 
 #include 
 
-#define PAGE_SECTORS_SHIFT (PAGE_SHIFT - SECTOR_SHIFT)
-#define PAGE_SECTORS   (1 << PAGE_SECTORS_SHIFT)
-
 /*
  * Each block ramdisk device has a radix_tree brd_pages of pages that stores
  * the pages containing the block device's contents. A brd page's ->index is
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 16510795e377..b5ebde97bf0b 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -11,10 +11,6 @@
 #include 
 #include "null_blk.h"
 
-#define PAGE_SECTORS_SHIFT (PAGE_SHIFT - SECTOR_SHIFT)
-#define PAGE_SECTORS   (1 << PAGE_SECTORS_SHIFT)
-#define SECTOR_MASK(PAGE_SECTORS - 1)
-