* [PATCH v2 01/10] block: move PAGE_SECTORS definition into <linux/blkdev.h>
2020-05-07 7:50 [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Zhen Lei
@ 2020-05-07 7:50 ` Zhen Lei
2020-05-07 7:50 ` [PATCH v2 02/10] zram: abolish macro SECTORS_PER_PAGE Zhen Lei
` (9 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Zhen Lei @ 2020-05-07 7:50 UTC (permalink / raw)
To: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Matthew Wilcox,
Jens Axboe, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, linux-block, Andrew Morton, linux-mm, dm-devel,
Song Liu, linux-raid, linux-kernel
Cc: Zhen Lei
Too many duplicated PAGE_SECTORS definitions, eliminate it.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/block/brd.c | 1 -
drivers/block/null_blk_main.c | 1 -
drivers/md/bcache/util.h | 2 --
include/linux/blkdev.h | 5 +++--
include/linux/device-mapper.h | 1 -
5 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 2fb25c348d53..30df6daa9dbc 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -26,7 +26,6 @@
#include <linux/uaccess.h>
#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
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 8efd8778e209..25048ff15858 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -12,7 +12,6 @@
#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)
#define FREE_BATCH 16
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
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 32868fbedc9e..934f31fc15cd 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -904,11 +904,12 @@ static inline struct request_queue *bdev_get_queue(struct block_device *bdev)
* multiple of 512 bytes. Hence these two constants.
*/
#ifndef SECTOR_SHIFT
-#define SECTOR_SHIFT 9
+#define SECTOR_SHIFT 9
#endif
#ifndef SECTOR_SIZE
-#define SECTOR_SIZE (1 << SECTOR_SHIFT)
+#define SECTOR_SIZE (1 << SECTOR_SHIFT)
#endif
+#define PAGE_SECTORS (PAGE_SIZE / SECTOR_SIZE)
/*
* blk_rq_pos() : the current sector
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index af48d9da3916..83e018ed8c21 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -143,7 +143,6 @@ typedef size_t (*dm_dax_copy_iter_fn)(struct dm_target *ti, pgoff_t pgoff,
void *addr, size_t bytes, struct iov_iter *i);
typedef int (*dm_dax_zero_page_range_fn)(struct dm_target *ti, pgoff_t pgoff,
size_t nr_pages);
-#define PAGE_SECTORS (PAGE_SIZE / 512)
void dm_error(const char *message);
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 02/10] zram: abolish macro SECTORS_PER_PAGE
2020-05-07 7:50 [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Zhen Lei
2020-05-07 7:50 ` [PATCH v2 01/10] block: move PAGE_SECTORS definition into <linux/blkdev.h> Zhen Lei
@ 2020-05-07 7:50 ` Zhen Lei
2020-05-07 7:50 ` [PATCH v2 03/10] block: add sectors_to_npage()/npage_to_sectors() helpers Zhen Lei
` (8 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Zhen Lei @ 2020-05-07 7:50 UTC (permalink / raw)
To: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Matthew Wilcox,
Jens Axboe, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, linux-block, Andrew Morton, linux-mm, dm-devel,
Song Liu, linux-raid, linux-kernel
Cc: Zhen Lei
SECTORS_PER_PAGE is equivalent to PAGE_SECTORS.
Although I prefer SECTORS_PER_PAGE better than PAGE_SECTORS, the former
is more clearer, I think. But the latter was defined in
<linux/device-mapper.h> before, rename it may impact users.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/block/zram/zram_drv.c | 4 ++--
drivers/block/zram/zram_drv.h | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index ebb234f36909..e2fbf7a847e7 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1551,7 +1551,7 @@ static void __zram_make_request(struct zram *zram, struct bio *bio)
index = bio->bi_iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
offset = (bio->bi_iter.bi_sector &
- (SECTORS_PER_PAGE - 1)) << SECTOR_SHIFT;
+ (PAGE_SECTORS - 1)) << SECTOR_SHIFT;
switch (bio_op(bio)) {
case REQ_OP_DISCARD:
@@ -1645,7 +1645,7 @@ static int zram_rw_page(struct block_device *bdev, sector_t sector,
}
index = sector >> SECTORS_PER_PAGE_SHIFT;
- offset = (sector & (SECTORS_PER_PAGE - 1)) << SECTOR_SHIFT;
+ offset = (sector & (PAGE_SECTORS - 1)) << SECTOR_SHIFT;
bv.bv_page = page;
bv.bv_len = PAGE_SIZE;
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index f2fd46daa760..10fdf413dd6e 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -22,7 +22,6 @@
#include "zcomp.h"
#define SECTORS_PER_PAGE_SHIFT (PAGE_SHIFT - SECTOR_SHIFT)
-#define SECTORS_PER_PAGE (1 << SECTORS_PER_PAGE_SHIFT)
#define ZRAM_LOGICAL_BLOCK_SHIFT 12
#define ZRAM_LOGICAL_BLOCK_SIZE (1 << ZRAM_LOGICAL_BLOCK_SHIFT)
#define ZRAM_SECTOR_PER_LOGICAL_BLOCK \
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 03/10] block: add sectors_to_npage()/npage_to_sectors() helpers
2020-05-07 7:50 [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Zhen Lei
2020-05-07 7:50 ` [PATCH v2 01/10] block: move PAGE_SECTORS definition into <linux/blkdev.h> Zhen Lei
2020-05-07 7:50 ` [PATCH v2 02/10] zram: abolish macro SECTORS_PER_PAGE Zhen Lei
@ 2020-05-07 7:50 ` Zhen Lei
2020-05-07 7:50 ` [PATCH v2 04/10] zram: abolish macro SECTORS_PER_PAGE_SHIFT Zhen Lei
` (7 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Zhen Lei @ 2020-05-07 7:50 UTC (permalink / raw)
To: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Matthew Wilcox,
Jens Axboe, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, linux-block, Andrew Morton, linux-mm, dm-devel,
Song Liu, linux-raid, linux-kernel
Cc: Zhen Lei
Provide the conversion of "number of sectors"/"sector number" and
"number of pages"/"page number".
Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
include/linux/blkdev.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 934f31fc15cd..5d8daaffc38b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -910,6 +910,8 @@ static inline struct request_queue *bdev_get_queue(struct block_device *bdev)
#define SECTOR_SIZE (1 << SECTOR_SHIFT)
#endif
#define PAGE_SECTORS (PAGE_SIZE / SECTOR_SIZE)
+#define sectors_to_npage(nr) ((nr) / PAGE_SECTORS)
+#define npage_to_sectors(nr) ((nr) * PAGE_SECTORS)
/*
* blk_rq_pos() : the current sector
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 04/10] zram: abolish macro SECTORS_PER_PAGE_SHIFT
2020-05-07 7:50 [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Zhen Lei
` (2 preceding siblings ...)
2020-05-07 7:50 ` [PATCH v2 03/10] block: add sectors_to_npage()/npage_to_sectors() helpers Zhen Lei
@ 2020-05-07 7:50 ` Zhen Lei
2020-05-07 7:50 ` [PATCH v2 05/10] block: abolish macro PAGE_SECTORS_SHIFT Zhen Lei
` (6 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Zhen Lei @ 2020-05-07 7:50 UTC (permalink / raw)
To: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Matthew Wilcox,
Jens Axboe, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, linux-block, Andrew Morton, linux-mm, dm-devel,
Song Liu, linux-raid, linux-kernel
Cc: Zhen Lei
The name of SECTORS_PER_PAGE_SHIFT is quite hard to read. So use
sectors_to_npage() to replace ">> SECTORS_PER_PAGE_SHIFT"
Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/block/zram/zram_drv.c | 4 ++--
drivers/block/zram/zram_drv.h | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index e2fbf7a847e7..918b77f9bce4 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1549,7 +1549,7 @@ static void __zram_make_request(struct zram *zram, struct bio *bio)
struct bio_vec bvec;
struct bvec_iter iter;
- index = bio->bi_iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
+ index = sectors_to_npage(bio->bi_iter.bi_sector);
offset = (bio->bi_iter.bi_sector &
(PAGE_SECTORS - 1)) << SECTOR_SHIFT;
@@ -1644,7 +1644,7 @@ static int zram_rw_page(struct block_device *bdev, sector_t sector,
goto out;
}
- index = sector >> SECTORS_PER_PAGE_SHIFT;
+ index = sectors_to_npage(sector);
offset = (sector & (PAGE_SECTORS - 1)) << SECTOR_SHIFT;
bv.bv_page = page;
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 10fdf413dd6e..12309175d55e 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -21,7 +21,6 @@
#include "zcomp.h"
-#define SECTORS_PER_PAGE_SHIFT (PAGE_SHIFT - SECTOR_SHIFT)
#define ZRAM_LOGICAL_BLOCK_SHIFT 12
#define ZRAM_LOGICAL_BLOCK_SIZE (1 << ZRAM_LOGICAL_BLOCK_SHIFT)
#define ZRAM_SECTOR_PER_LOGICAL_BLOCK \
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 05/10] block: abolish macro PAGE_SECTORS_SHIFT
2020-05-07 7:50 [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Zhen Lei
` (3 preceding siblings ...)
2020-05-07 7:50 ` [PATCH v2 04/10] zram: abolish macro SECTORS_PER_PAGE_SHIFT Zhen Lei
@ 2020-05-07 7:50 ` Zhen Lei
2020-05-07 7:50 ` [PATCH v2 06/10] mm/swap: use npage_to_sectors() and PAGE_SECTORS to clean up code Zhen Lei
` (5 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Zhen Lei @ 2020-05-07 7:50 UTC (permalink / raw)
To: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Matthew Wilcox,
Jens Axboe, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, linux-block, Andrew Morton, linux-mm, dm-devel,
Song Liu, linux-raid, linux-kernel
Cc: Zhen Lei
The name of PAGE_SECTORS_SHIFT is quite hard to read.
1. use sectors_to_npage() to replace ">> PAGE_SECTORS_SHIFT"
2. use npage_to_sectors() to replace "<< PAGE_SECTORS_SHIFT"
Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/block/brd.c | 6 ++----
drivers/block/null_blk_main.c | 9 ++++-----
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 30df6daa9dbc..051c5a50497f 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -25,8 +25,6 @@
#include <linux/uaccess.h>
-#define PAGE_SECTORS_SHIFT (PAGE_SHIFT - SECTOR_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
@@ -69,7 +67,7 @@ static struct page *brd_lookup_page(struct brd_device *brd, sector_t sector)
* here, only deletes).
*/
rcu_read_lock();
- idx = sector >> PAGE_SECTORS_SHIFT; /* sector to page index */
+ idx = sectors_to_npage(sector);
page = radix_tree_lookup(&brd->brd_pages, idx);
rcu_read_unlock();
@@ -108,7 +106,7 @@ static struct page *brd_insert_page(struct brd_device *brd, sector_t sector)
}
spin_lock(&brd->brd_lock);
- idx = sector >> PAGE_SECTORS_SHIFT;
+ idx = sectors_to_npage(sector);
page->index = idx;
if (radix_tree_insert(&brd->brd_pages, idx, page)) {
__free_page(page);
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 25048ff15858..81485f47dcf0 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -11,7 +11,6 @@
#include <linux/init.h>
#include "null_blk.h"
-#define PAGE_SECTORS_SHIFT (PAGE_SHIFT - SECTOR_SHIFT)
#define SECTOR_MASK (PAGE_SECTORS - 1)
#define FREE_BATCH 16
@@ -737,7 +736,7 @@ static void null_free_sector(struct nullb *nullb, sector_t sector,
struct radix_tree_root *root;
root = is_cache ? &nullb->dev->cache : &nullb->dev->data;
- idx = sector >> PAGE_SECTORS_SHIFT;
+ idx = sectors_to_npage(sector);
sector_bit = (sector & SECTOR_MASK);
t_page = radix_tree_lookup(root, idx);
@@ -808,7 +807,7 @@ static struct nullb_page *__null_lookup_page(struct nullb *nullb,
struct nullb_page *t_page;
struct radix_tree_root *root;
- idx = sector >> PAGE_SECTORS_SHIFT;
+ idx = sectors_to_npage(sector);
sector_bit = (sector & SECTOR_MASK);
root = is_cache ? &nullb->dev->cache : &nullb->dev->data;
@@ -855,7 +854,7 @@ static struct nullb_page *null_insert_page(struct nullb *nullb,
goto out_freepage;
spin_lock_irq(&nullb->lock);
- idx = sector >> PAGE_SECTORS_SHIFT;
+ idx = sectors_to_npage(sector);
t_page->page->index = idx;
t_page = null_radix_tree_insert(nullb, idx, t_page, !ignore_cache);
radix_tree_preload_end();
@@ -878,7 +877,7 @@ static int null_flush_cache_page(struct nullb *nullb, struct nullb_page *c_page)
idx = c_page->page->index;
- t_page = null_insert_page(nullb, idx << PAGE_SECTORS_SHIFT, true);
+ t_page = null_insert_page(nullb, npage_to_sectors(idx), true);
__clear_bit(NULLB_PAGE_LOCK, c_page->bitmap);
if (test_bit(NULLB_PAGE_FREE, c_page->bitmap)) {
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 06/10] mm/swap: use npage_to_sectors() and PAGE_SECTORS to clean up code
2020-05-07 7:50 [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Zhen Lei
` (4 preceding siblings ...)
2020-05-07 7:50 ` [PATCH v2 05/10] block: abolish macro PAGE_SECTORS_SHIFT Zhen Lei
@ 2020-05-07 7:50 ` Zhen Lei
2020-05-15 4:06 ` Matthew Wilcox
2020-05-15 4:14 ` Matthew Wilcox
2020-05-07 7:50 ` [PATCH v2 07/10] block: use sectors_to_npage() " Zhen Lei
` (4 subsequent siblings)
10 siblings, 2 replies; 18+ messages in thread
From: Zhen Lei @ 2020-05-07 7:50 UTC (permalink / raw)
To: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Matthew Wilcox,
Jens Axboe, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, linux-block, Andrew Morton, linux-mm, dm-devel,
Song Liu, linux-raid, linux-kernel
Cc: Zhen Lei
1. Replace "<<= (PAGE_SHIFT - 9)" with "*= PAGE_SECTORS"
2. Replace "<< (PAGE_SHIFT - 9)" with npage_to_sectors()
Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
mm/page_io.c | 4 ++--
mm/swapfile.c | 12 ++++++------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/mm/page_io.c b/mm/page_io.c
index 76965be1d40e..23291a49ab91 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -38,7 +38,7 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
bio->bi_iter.bi_sector = map_swap_page(page, &bdev);
bio_set_dev(bio, bdev);
- bio->bi_iter.bi_sector <<= PAGE_SHIFT - 9;
+ bio->bi_iter.bi_sector *= PAGE_SECTORS;
bio->bi_end_io = end_io;
bio_add_page(bio, page, PAGE_SIZE * hpage_nr_pages(page), 0);
@@ -266,7 +266,7 @@ int swap_writepage(struct page *page, struct writeback_control *wbc)
static sector_t swap_page_sector(struct page *page)
{
- return (sector_t)__page_file_index(page) << (PAGE_SHIFT - 9);
+ return npage_to_sectors((sector_t)__page_file_index(page));
}
static inline void count_swpout_vm_event(struct page *page)
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 5871a2aa86a5..c8be92f972a4 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -177,8 +177,8 @@ static int discard_swap(struct swap_info_struct *si)
/* Do not discard the swap header page! */
se = first_se(si);
- start_block = (se->start_block + 1) << (PAGE_SHIFT - 9);
- nr_blocks = ((sector_t)se->nr_pages - 1) << (PAGE_SHIFT - 9);
+ start_block = npage_to_sectors(se->start_block + 1);
+ nr_blocks = npage_to_sectors((sector_t)se->nr_pages - 1);
if (nr_blocks) {
err = blkdev_issue_discard(si->bdev, start_block,
nr_blocks, GFP_KERNEL, 0);
@@ -188,8 +188,8 @@ static int discard_swap(struct swap_info_struct *si)
}
for (se = next_se(se); se; se = next_se(se)) {
- start_block = se->start_block << (PAGE_SHIFT - 9);
- nr_blocks = (sector_t)se->nr_pages << (PAGE_SHIFT - 9);
+ start_block = npage_to_sectors(se->start_block);
+ nr_blocks = npage_to_sectors((sector_t)se->nr_pages);
err = blkdev_issue_discard(si->bdev, start_block,
nr_blocks, GFP_KERNEL, 0);
@@ -240,8 +240,8 @@ static void discard_swap_cluster(struct swap_info_struct *si,
start_page += nr_blocks;
nr_pages -= nr_blocks;
- start_block <<= PAGE_SHIFT - 9;
- nr_blocks <<= PAGE_SHIFT - 9;
+ start_block *= PAGE_SECTORS;
+ nr_blocks *= PAGE_SECTORS;
if (blkdev_issue_discard(si->bdev, start_block,
nr_blocks, GFP_NOIO, 0))
break;
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 06/10] mm/swap: use npage_to_sectors() and PAGE_SECTORS to clean up code
2020-05-07 7:50 ` [PATCH v2 06/10] mm/swap: use npage_to_sectors() and PAGE_SECTORS to clean up code Zhen Lei
@ 2020-05-15 4:06 ` Matthew Wilcox
2020-05-15 6:28 ` Leizhen (ThunderTown)
2020-05-15 4:14 ` Matthew Wilcox
1 sibling, 1 reply; 18+ messages in thread
From: Matthew Wilcox @ 2020-05-15 4:06 UTC (permalink / raw)
To: Zhen Lei
Cc: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Jens Axboe,
Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer,
linux-block, Andrew Morton, linux-mm, dm-devel, Song Liu,
linux-raid, linux-kernel
On Thu, May 07, 2020 at 03:50:56PM +0800, Zhen Lei wrote:
> @@ -266,7 +266,7 @@ int swap_writepage(struct page *page, struct writeback_control *wbc)
>
> static sector_t swap_page_sector(struct page *page)
> {
> - return (sector_t)__page_file_index(page) << (PAGE_SHIFT - 9);
> + return npage_to_sectors((sector_t)__page_file_index(page));
If you make npage_to_sectors() a proper function instead of a macro,
you can do the casting inside the function instead of in the callers
(which is prone to bugs).
Also, this is a great example of why page_to_sector() was a better name
than npage_to_sectors(). This function doesn't return a count of sectors,
it returns a sector number within the swap device.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 06/10] mm/swap: use npage_to_sectors() and PAGE_SECTORS to clean up code
2020-05-15 4:06 ` Matthew Wilcox
@ 2020-05-15 6:28 ` Leizhen (ThunderTown)
0 siblings, 0 replies; 18+ messages in thread
From: Leizhen (ThunderTown) @ 2020-05-15 6:28 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Jens Axboe,
Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer,
linux-block, Andrew Morton, linux-mm, dm-devel, Song Liu,
linux-raid, linux-kernel
On 2020/5/15 12:06, Matthew Wilcox wrote:
> On Thu, May 07, 2020 at 03:50:56PM +0800, Zhen Lei wrote:
>> @@ -266,7 +266,7 @@ int swap_writepage(struct page *page, struct writeback_control *wbc)
>>
>> static sector_t swap_page_sector(struct page *page)
>> {
>> - return (sector_t)__page_file_index(page) << (PAGE_SHIFT - 9);
>> + return npage_to_sectors((sector_t)__page_file_index(page));
>
> If you make npage_to_sectors() a proper function instead of a macro,
> you can do the casting inside the function instead of in the callers
> (which is prone to bugs).
Oh, yes. __page_file_index(page) maybe called many times in marco, althouth currently
it is not. So that, not all are suitable for page_to_sector(). And for this example,
still need to use "<< PAGE_SECTORS_SHIFT".
>
> Also, this is a great example of why page_to_sector() was a better name
> than npage_to_sectors(). This function doesn't return a count of sectors,
> it returns a sector number within the swap device.
OK, so I will change to page_to_sector()/sector_to_page().
>
>
> .
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 06/10] mm/swap: use npage_to_sectors() and PAGE_SECTORS to clean up code
2020-05-07 7:50 ` [PATCH v2 06/10] mm/swap: use npage_to_sectors() and PAGE_SECTORS to clean up code Zhen Lei
2020-05-15 4:06 ` Matthew Wilcox
@ 2020-05-15 4:14 ` Matthew Wilcox
2020-05-15 6:42 ` Leizhen (ThunderTown)
1 sibling, 1 reply; 18+ messages in thread
From: Matthew Wilcox @ 2020-05-15 4:14 UTC (permalink / raw)
To: Zhen Lei
Cc: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Jens Axboe,
Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer,
linux-block, Andrew Morton, linux-mm, dm-devel, Song Liu,
linux-raid, linux-kernel
On Thu, May 07, 2020 at 03:50:56PM +0800, Zhen Lei wrote:
> +++ b/mm/page_io.c
> @@ -38,7 +38,7 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
>
> bio->bi_iter.bi_sector = map_swap_page(page, &bdev);
> bio_set_dev(bio, bdev);
> - bio->bi_iter.bi_sector <<= PAGE_SHIFT - 9;
> + bio->bi_iter.bi_sector *= PAGE_SECTORS;
> bio->bi_end_io = end_io;
This just doesn't look right. Why is map_swap_page() returning a sector_t
which isn't actually a sector_t?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 06/10] mm/swap: use npage_to_sectors() and PAGE_SECTORS to clean up code
2020-05-15 4:14 ` Matthew Wilcox
@ 2020-05-15 6:42 ` Leizhen (ThunderTown)
0 siblings, 0 replies; 18+ messages in thread
From: Leizhen (ThunderTown) @ 2020-05-15 6:42 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Jens Axboe,
Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer,
linux-block, Andrew Morton, linux-mm, dm-devel, Song Liu,
linux-raid, linux-kernel
On 2020/5/15 12:14, Matthew Wilcox wrote:
> On Thu, May 07, 2020 at 03:50:56PM +0800, Zhen Lei wrote:
>> +++ b/mm/page_io.c
>> @@ -38,7 +38,7 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
>>
>> bio->bi_iter.bi_sector = map_swap_page(page, &bdev);
>> bio_set_dev(bio, bdev);
>> - bio->bi_iter.bi_sector <<= PAGE_SHIFT - 9;
>> + bio->bi_iter.bi_sector *= PAGE_SECTORS;
>> bio->bi_end_io = end_io;
>
> This just doesn't look right. Why is map_swap_page() returning a sector_t
> which isn't actually a sector_t?
I try to understand map_swap_page(). Here maybe a bug. Otherwise, it would be
better to add a temporary variable to cache the return value of map_swap_page(page, &bdev).
>
>
> .
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 07/10] block: use sectors_to_npage() and PAGE_SECTORS to clean up code
2020-05-07 7:50 [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Zhen Lei
` (5 preceding siblings ...)
2020-05-07 7:50 ` [PATCH v2 06/10] mm/swap: use npage_to_sectors() and PAGE_SECTORS to clean up code Zhen Lei
@ 2020-05-07 7:50 ` Zhen Lei
2020-05-15 4:19 ` Matthew Wilcox
2020-05-07 7:50 ` [PATCH v2 08/10] md: use sectors_to_npage() and npage_to_sectors() " Zhen Lei
` (3 subsequent siblings)
10 siblings, 1 reply; 18+ messages in thread
From: Zhen Lei @ 2020-05-07 7:50 UTC (permalink / raw)
To: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Matthew Wilcox,
Jens Axboe, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, linux-block, Andrew Morton, linux-mm, dm-devel,
Song Liu, linux-raid, linux-kernel
Cc: Zhen Lei
1. Replace "1 << (PAGE_SHIFT - 9)" with PAGE_SECTORS
2. Replace ">> (PAGE_SHIFT - 9)" with sectors_to_npage()
Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
block/blk-settings.c | 6 +++---
block/partitions/core.c | 5 ++---
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 14397b4c4b53..171665ed8318 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -150,7 +150,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 = PAGE_SECTORS;
printk(KERN_INFO "%s: set to minimum %d\n",
__func__, max_hw_sectors);
}
@@ -159,7 +159,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 = sectors_to_npage(max_sectors);
}
EXPORT_SYMBOL(blk_queue_max_hw_sectors);
@@ -630,7 +630,7 @@ void disk_stack_limits(struct gendisk *disk, struct block_device *bdev,
}
t->backing_dev_info->io_pages =
- t->limits.max_sectors >> (PAGE_SHIFT - 9);
+ sectors_to_npage(t->limits.max_sectors);
}
EXPORT_SYMBOL(disk_stack_limits);
diff --git a/block/partitions/core.c b/block/partitions/core.c
index 9ef48a8cff86..4859739a2414 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -640,8 +640,7 @@ void *read_part_sector(struct parsed_partitions *state, sector_t n, Sector *p)
return NULL;
}
- page = read_mapping_page(mapping,
- (pgoff_t)(n >> (PAGE_SHIFT - 9)), NULL);
+ page = read_mapping_page(mapping, (pgoff_t)sectors_to_npage(n), NULL);
if (IS_ERR(page))
goto out;
if (PageError(page))
@@ -649,7 +648,7 @@ void *read_part_sector(struct parsed_partitions *state, sector_t n, Sector *p)
p->v = page;
return (unsigned char *)page_address(page) +
- ((n & ((1 << (PAGE_SHIFT - 9)) - 1)) << SECTOR_SHIFT);
+ ((n & (PAGE_SECTORS - 1)) << SECTOR_SHIFT);
out_put_page:
put_page(page);
out:
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 07/10] block: use sectors_to_npage() and PAGE_SECTORS to clean up code
2020-05-07 7:50 ` [PATCH v2 07/10] block: use sectors_to_npage() " Zhen Lei
@ 2020-05-15 4:19 ` Matthew Wilcox
2020-05-15 6:52 ` Leizhen (ThunderTown)
0 siblings, 1 reply; 18+ messages in thread
From: Matthew Wilcox @ 2020-05-15 4:19 UTC (permalink / raw)
To: Zhen Lei
Cc: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Jens Axboe,
Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer,
linux-block, Andrew Morton, linux-mm, dm-devel, Song Liu,
linux-raid, linux-kernel
On Thu, May 07, 2020 at 03:50:57PM +0800, Zhen Lei wrote:
> +++ b/block/blk-settings.c
> @@ -150,7 +150,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 = PAGE_SECTORS;
Surely this should be:
if (max_hw_sectors < PAGE_SECTORS) {
max_hw_sectors = PAGE_SECTORS;
... no?
> - page = read_mapping_page(mapping,
> - (pgoff_t)(n >> (PAGE_SHIFT - 9)), NULL);
> + page = read_mapping_page(mapping, (pgoff_t)sectors_to_npage(n), NULL);
... again, get the type right, and you won't need the cast.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 07/10] block: use sectors_to_npage() and PAGE_SECTORS to clean up code
2020-05-15 4:19 ` Matthew Wilcox
@ 2020-05-15 6:52 ` Leizhen (ThunderTown)
0 siblings, 0 replies; 18+ messages in thread
From: Leizhen (ThunderTown) @ 2020-05-15 6:52 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Jens Axboe,
Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer,
linux-block, Andrew Morton, linux-mm, dm-devel, Song Liu,
linux-raid, linux-kernel
On 2020/5/15 12:19, Matthew Wilcox wrote:
> On Thu, May 07, 2020 at 03:50:57PM +0800, Zhen Lei wrote:
>> +++ b/block/blk-settings.c
>> @@ -150,7 +150,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 = PAGE_SECTORS;
>
> Surely this should be:
>
> if (max_hw_sectors < PAGE_SECTORS) {
> max_hw_sectors = PAGE_SECTORS;
>
> ... no?
I've noticed this place before. "(max_hw_sectors << 9) < PAGE_SIZE" can also make sure
that max_hw_sectors is not too large, that means (max_hw_sectors << 9) may overflow.
>
>> - page = read_mapping_page(mapping,
>> - (pgoff_t)(n >> (PAGE_SHIFT - 9)), NULL);
>> + page = read_mapping_page(mapping, (pgoff_t)sectors_to_npage(n), NULL);
>
> ... again, get the type right, and you won't need the cast.
OK, I'll consider it.
>
>
> .
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 08/10] md: use sectors_to_npage() and npage_to_sectors() to clean up code
2020-05-07 7:50 [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Zhen Lei
` (6 preceding siblings ...)
2020-05-07 7:50 ` [PATCH v2 07/10] block: use sectors_to_npage() " Zhen Lei
@ 2020-05-07 7:50 ` Zhen Lei
2020-05-07 7:50 ` [PATCH v2 09/10] md: use existing definition RESYNC_SECTORS Zhen Lei
` (2 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Zhen Lei @ 2020-05-07 7:50 UTC (permalink / raw)
To: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Matthew Wilcox,
Jens Axboe, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, linux-block, Andrew Morton, linux-mm, dm-devel,
Song Liu, linux-raid, linux-kernel
Cc: Zhen Lei
1. Replace ">> (PAGE_SHIFT - 9)" with sectors_to_npage()
2. Replace "<< (PAGE_SHIFT - 9)" with npage_to_sectors()
Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/md/dm-table.c | 2 +-
drivers/md/raid1.c | 2 +-
drivers/md/raid10.c | 2 +-
drivers/md/raid5-cache.c | 11 +++++------
4 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 0a2cc197f62b..e1f176bda528 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1964,7 +1964,7 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
#endif
/* Allow reads to exceed readahead limits */
- q->backing_dev_info->io_pages = limits->max_sectors >> (PAGE_SHIFT - 9);
+ q->backing_dev_info->io_pages = sectors_to_npage(limits->max_sectors);
}
unsigned int dm_table_get_num_targets(struct dm_table *t)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index cd810e195086..44ffe1b6d77a 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2129,7 +2129,7 @@ static void process_checks(struct r1bio *r1_bio)
int vcnt;
/* Fix variable parts of all bios */
- vcnt = (r1_bio->sectors + PAGE_SIZE / 512 - 1) >> (PAGE_SHIFT - 9);
+ vcnt = sectors_to_npage(r1_bio->sectors + PAGE_SIZE / 512 - 1);
for (i = 0; i < conf->raid_disks * 2; i++) {
blk_status_t status;
struct bio *b = r1_bio->bios[i];
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index ec136e44aef7..948afe720fca 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2029,7 +2029,7 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
fbio->bi_iter.bi_idx = 0;
fpages = get_resync_pages(fbio)->pages;
- vcnt = (r10_bio->sectors + (PAGE_SIZE >> 9) - 1) >> (PAGE_SHIFT - 9);
+ vcnt = sectors_to_npage(r10_bio->sectors + (PAGE_SIZE >> 9) - 1);
/* now find blocks with errors */
for (i=0 ; i < conf->copies ; i++) {
int j, d;
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 9b6da759dca2..0b9cd810466a 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -832,8 +832,7 @@ static void r5l_append_payload_meta(struct r5l_log *log, u16 type,
payload = page_address(io->meta_page) + io->meta_offset;
payload->header.type = cpu_to_le16(type);
payload->header.flags = cpu_to_le16(0);
- payload->size = cpu_to_le32((1 + !!checksum2_valid) <<
- (PAGE_SHIFT - 9));
+ payload->size = cpu_to_le32(npage_to_sectors(1 + !!checksum2_valid));
payload->location = cpu_to_le64(location);
payload->checksum[0] = cpu_to_le32(checksum1);
if (checksum2_valid)
@@ -1042,7 +1041,7 @@ int r5l_write_stripe(struct r5l_log *log, struct stripe_head *sh)
mutex_lock(&log->io_mutex);
/* meta + data */
- reserve = (1 + write_disks) << (PAGE_SHIFT - 9);
+ reserve = npage_to_sectors(1 + write_disks);
if (log->r5c_journal_mode == R5C_JOURNAL_MODE_WRITE_THROUGH) {
if (!r5l_has_free_space(log, reserve)) {
@@ -2053,7 +2052,7 @@ r5l_recovery_verify_data_checksum_for_mb(struct r5l_log *log,
le32_to_cpu(payload->size));
mb_offset += sizeof(struct r5l_payload_data_parity) +
sizeof(__le32) *
- (le32_to_cpu(payload->size) >> (PAGE_SHIFT - 9));
+ sectors_to_npage(le32_to_cpu(payload->size));
}
}
@@ -2199,7 +2198,7 @@ r5c_recovery_analyze_meta_block(struct r5l_log *log,
mb_offset += sizeof(struct r5l_payload_data_parity) +
sizeof(__le32) *
- (le32_to_cpu(payload->size) >> (PAGE_SHIFT - 9));
+ sectors_to_npage(le32_to_cpu(payload->size));
}
return 0;
@@ -2916,7 +2915,7 @@ int r5c_cache_data(struct r5l_log *log, struct stripe_head *sh)
mutex_lock(&log->io_mutex);
/* meta + data */
- reserve = (1 + pages) << (PAGE_SHIFT - 9);
+ reserve = npage_to_sectors(1 + pages);
if (test_bit(R5C_LOG_CRITICAL, &conf->cache_state) &&
sh->log_start == MaxSector)
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 09/10] md: use existing definition RESYNC_SECTORS
2020-05-07 7:50 [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Zhen Lei
` (7 preceding siblings ...)
2020-05-07 7:50 ` [PATCH v2 08/10] md: use sectors_to_npage() and npage_to_sectors() " Zhen Lei
@ 2020-05-07 7:50 ` Zhen Lei
2020-05-07 7:51 ` [PATCH v2 10/10] md: use PAGE_SECTORS to clean up code Zhen Lei
2020-05-15 2:05 ` [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Leizhen (ThunderTown)
10 siblings, 0 replies; 18+ messages in thread
From: Zhen Lei @ 2020-05-07 7:50 UTC (permalink / raw)
To: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Matthew Wilcox,
Jens Axboe, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, linux-block, Andrew Morton, linux-mm, dm-devel,
Song Liu, linux-raid, linux-kernel
Cc: Zhen Lei
"RESYNC_BLOCK_SIZE/512" is equal to "RESYNC_BLOCK_SIZE >> 9", replace it
with RESYNC_SECTORS.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/md/raid10.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 948afe720fca..ac4273f804e8 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4483,8 +4483,8 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
last = conf->reshape_progress - 1;
sector_nr = last & ~(sector_t)(conf->geo.chunk_mask
& conf->prev.chunk_mask);
- if (sector_nr + RESYNC_BLOCK_SIZE/512 < last)
- sector_nr = last + 1 - RESYNC_BLOCK_SIZE/512;
+ if (sector_nr + RESYNC_SECTORS < last)
+ sector_nr = last + 1 - RESYNC_SECTORS;
} else {
/* 'next' is after the last device address that we
* might write to for this chunk in the new layout
@@ -4506,8 +4506,8 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
last = sector_nr | (conf->geo.chunk_mask
& conf->prev.chunk_mask);
- if (sector_nr + RESYNC_BLOCK_SIZE/512 <= last)
- last = sector_nr + RESYNC_BLOCK_SIZE/512 - 1;
+ if (sector_nr + RESYNC_SECTORS <= last)
+ last = sector_nr + RESYNC_SECTORS - 1;
}
if (need_flush ||
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 10/10] md: use PAGE_SECTORS to clean up code
2020-05-07 7:50 [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Zhen Lei
` (8 preceding siblings ...)
2020-05-07 7:50 ` [PATCH v2 09/10] md: use existing definition RESYNC_SECTORS Zhen Lei
@ 2020-05-07 7:51 ` Zhen Lei
2020-05-15 2:05 ` [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Leizhen (ThunderTown)
10 siblings, 0 replies; 18+ messages in thread
From: Zhen Lei @ 2020-05-07 7:51 UTC (permalink / raw)
To: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Matthew Wilcox,
Jens Axboe, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, linux-block, Andrew Morton, linux-mm, dm-devel,
Song Liu, linux-raid, linux-kernel
Cc: Zhen Lei
Execute the following shell script:
C_FILES=`find drivers/md/ -name "*.c"`
for file in $C_FILES
do
#with brace
sed -i 's/(PAGE_SIZE \/ 512)/PAGE_SECTORS/g' $file
sed -i 's/(PAGE_SIZE\/512)/PAGE_SECTORS/g' $file
sed -i 's/(PAGE_SIZE >> 9)/PAGE_SECTORS/g' $file
sed -i 's/(PAGE_SIZE>>9)/PAGE_SECTORS/g' $file
#without brace
sed -i 's/PAGE_SIZE \/ 512/PAGE_SECTORS/g' $file
sed -i 's/PAGE_SIZE\/512/PAGE_SECTORS/g' $file
sed -i 's/PAGE_SIZE >> 9/PAGE_SECTORS/g' $file
sed -i 's/PAGE_SIZE>>9/PAGE_SECTORS/g' $file
done
In addition, eliminate below scripts/checkpatch.pl warning:
#44: FILE: drivers/md/dm-kcopyd.c:587:
+ unsigned nr_pages = dm_div_up(job->dests[0].count, PAGE_SECTORS);
Change to "unsigned int nr_pages".
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/md/dm-kcopyd.c | 2 +-
drivers/md/md-bitmap.c | 16 ++++++++--------
drivers/md/md.c | 6 +++---
drivers/md/raid1.c | 10 +++++-----
drivers/md/raid10.c | 20 ++++++++++----------
drivers/md/raid5.c | 4 ++--
6 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/md/dm-kcopyd.c b/drivers/md/dm-kcopyd.c
index 1bbe4a34ef4c..ad861a3d648e 100644
--- a/drivers/md/dm-kcopyd.c
+++ b/drivers/md/dm-kcopyd.c
@@ -584,7 +584,7 @@ static int run_io_job(struct kcopyd_job *job)
static int run_pages_job(struct kcopyd_job *job)
{
int r;
- unsigned nr_pages = dm_div_up(job->dests[0].count, PAGE_SIZE >> 9);
+ unsigned int nr_pages = dm_div_up(job->dests[0].count, PAGE_SECTORS);
r = kcopyd_get_pages(job->kc, nr_pages, &job->pages);
if (!r) {
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index b952bd45bd6a..12ccf1c81661 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -161,7 +161,7 @@ static int read_sb_page(struct mddev *mddev, loff_t offset,
|| test_bit(Bitmap_sync, &rdev->flags))
continue;
- target = offset + index * (PAGE_SIZE/512);
+ target = offset + index * PAGE_SECTORS;
if (sync_page_io(rdev, target,
roundup(size, bdev_logical_block_size(rdev->bdev)),
@@ -237,17 +237,17 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
if (mddev->external) {
/* Bitmap could be anywhere. */
if (rdev->sb_start + offset + (page->index
- * (PAGE_SIZE/512))
+ * PAGE_SECTORS)
> rdev->data_offset
&&
rdev->sb_start + offset
< (rdev->data_offset + mddev->dev_sectors
- + (PAGE_SIZE/512)))
+ + PAGE_SECTORS))
goto bad_alignment;
} else if (offset < 0) {
/* DATA BITMAP METADATA */
if (offset
- + (long)(page->index * (PAGE_SIZE/512))
+ + (long)(page->index * PAGE_SECTORS)
+ size/512 > 0)
/* bitmap runs in to metadata */
goto bad_alignment;
@@ -259,7 +259,7 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
/* METADATA BITMAP DATA */
if (rdev->sb_start
+ offset
- + page->index*(PAGE_SIZE/512) + size/512
+ + page->index*PAGE_SECTORS + size/512
> rdev->data_offset)
/* bitmap runs in to data */
goto bad_alignment;
@@ -268,7 +268,7 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
}
md_super_write(mddev, rdev,
rdev->sb_start + offset
- + page->index * (PAGE_SIZE/512),
+ + page->index * PAGE_SECTORS,
size,
page);
}
@@ -1548,14 +1548,14 @@ int md_bitmap_start_sync(struct bitmap *bitmap, sector_t offset, sector_t *block
* pages, otherwise resync (which is very PAGE_SIZE based) will
* get confused.
* So call __bitmap_start_sync repeatedly (if needed) until
- * At least PAGE_SIZE>>9 blocks are covered.
+ * At least PAGE_SECTORS blocks are covered.
* Return the 'or' of the result.
*/
int rv = 0;
sector_t blocks1;
*blocks = 0;
- while (*blocks < (PAGE_SIZE>>9)) {
+ while (*blocks < PAGE_SECTORS) {
rv |= __bitmap_start_sync(bitmap, offset,
&blocks1, degraded);
offset += blocks1;
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 271e8a587354..a7572b17cf2e 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1736,7 +1736,7 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
__le64 *bbp;
int i;
int sectors = le16_to_cpu(sb->bblog_size);
- if (sectors > (PAGE_SIZE / 512))
+ if (sectors > PAGE_SECTORS)
return -EINVAL;
offset = le32_to_cpu(sb->bblog_offset);
if (offset == 0)
@@ -2223,7 +2223,7 @@ super_1_allow_new_offset(struct md_rdev *rdev,
bitmap = rdev->mddev->bitmap;
if (bitmap && !rdev->mddev->bitmap_info.file &&
rdev->sb_start + rdev->mddev->bitmap_info.offset +
- bitmap->storage.file_pages * (PAGE_SIZE>>9) > new_offset)
+ bitmap->storage.file_pages * PAGE_SECTORS > new_offset)
return 0;
if (rdev->badblocks.sector + rdev->badblocks.size > new_offset)
return 0;
@@ -8734,7 +8734,7 @@ void md_do_sync(struct md_thread *thread)
/*
* Tune reconstruction:
*/
- window = 32 * (PAGE_SIZE / 512);
+ window = 32 * PAGE_SECTORS;
pr_debug("md: using %dk window, over a total of %lluk.\n",
window/2, (unsigned long long)max_sectors/2);
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 44ffe1b6d77a..717c6e397cad 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2021,8 +2021,8 @@ static int fix_sync_read_error(struct r1bio *r1_bio)
int success = 0;
int start;
- if (s > (PAGE_SIZE>>9))
- s = PAGE_SIZE >> 9;
+ if (s > PAGE_SECTORS)
+ s = PAGE_SECTORS;
do {
if (r1_bio->bios[d]->bi_end_io == end_sync_read) {
/* No rcu protection needed here devices
@@ -2129,7 +2129,7 @@ static void process_checks(struct r1bio *r1_bio)
int vcnt;
/* Fix variable parts of all bios */
- vcnt = sectors_to_npage(r1_bio->sectors + PAGE_SIZE / 512 - 1);
+ vcnt = sectors_to_npage(r1_bio->sectors + PAGE_SECTORS - 1);
for (i = 0; i < conf->raid_disks * 2; i++) {
blk_status_t status;
struct bio *b = r1_bio->bios[i];
@@ -2264,8 +2264,8 @@ static void fix_read_error(struct r1conf *conf, int read_disk,
int start;
struct md_rdev *rdev;
- if (s > (PAGE_SIZE>>9))
- s = PAGE_SIZE >> 9;
+ if (s > PAGE_SECTORS)
+ s = PAGE_SECTORS;
do {
sector_t first_bad;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index ac4273f804e8..21bc6e33c093 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2029,7 +2029,7 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
fbio->bi_iter.bi_idx = 0;
fpages = get_resync_pages(fbio)->pages;
- vcnt = sectors_to_npage(r10_bio->sectors + (PAGE_SIZE >> 9) - 1);
+ vcnt = sectors_to_npage(r10_bio->sectors + PAGE_SECTORS - 1);
/* now find blocks with errors */
for (i=0 ; i < conf->copies ; i++) {
int j, d;
@@ -2163,8 +2163,8 @@ static void fix_recovery_read_error(struct r10bio *r10_bio)
sector_t addr;
int ok;
- if (s > (PAGE_SIZE>>9))
- s = PAGE_SIZE >> 9;
+ if (s > PAGE_SECTORS)
+ s = PAGE_SECTORS;
rdev = conf->mirrors[dr].rdev;
addr = r10_bio->devs[0].addr + sect,
@@ -2367,8 +2367,8 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
int success = 0;
int start;
- if (s > (PAGE_SIZE>>9))
- s = PAGE_SIZE >> 9;
+ if (s > PAGE_SECTORS)
+ s = PAGE_SECTORS;
rcu_read_lock();
do {
@@ -3597,7 +3597,7 @@ static int setup_geo(struct geom *geo, struct mddev *mddev, enum geo_type new)
}
if (layout >> 19)
return -1;
- if (chunk < (PAGE_SIZE >> 9) ||
+ if (chunk < PAGE_SECTORS ||
!is_power_of_2(chunk))
return -2;
nc = layout & 255;
@@ -4632,8 +4632,8 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
nr_sectors = 0;
pages = get_resync_pages(r10_bio->devs[0].bio)->pages;
- for (s = 0 ; s < max_sectors; s += PAGE_SIZE >> 9) {
- struct page *page = pages[s / (PAGE_SIZE >> 9)];
+ for (s = 0 ; s < max_sectors; s += PAGE_SECTORS) {
+ struct page *page = pages[s / PAGE_SECTORS];
int len = (max_sectors - s) << 9;
if (len > PAGE_SIZE)
len = PAGE_SIZE;
@@ -4789,8 +4789,8 @@ static int handle_reshape_read_error(struct mddev *mddev,
int success = 0;
int first_slot = slot;
- if (s > (PAGE_SIZE >> 9))
- s = PAGE_SIZE >> 9;
+ if (s > PAGE_SECTORS)
+ s = PAGE_SECTORS;
rcu_read_lock();
while (!success) {
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ba00e9877f02..5b316538b9ea 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -8197,7 +8197,7 @@ static int raid5_check_reshape(struct mddev *mddev)
if (new_chunk > 0) {
if (!is_power_of_2(new_chunk))
return -EINVAL;
- if (new_chunk < (PAGE_SIZE>>9))
+ if (new_chunk < PAGE_SECTORS)
return -EINVAL;
if (mddev->array_sectors & (new_chunk-1))
/* not factor of array size */
@@ -8231,7 +8231,7 @@ static int raid6_check_reshape(struct mddev *mddev)
if (new_chunk > 0) {
if (!is_power_of_2(new_chunk))
return -EINVAL;
- if (new_chunk < (PAGE_SIZE >> 9))
+ if (new_chunk < PAGE_SECTORS)
return -EINVAL;
if (mddev->array_sectors & (new_chunk-1))
/* not factor of array size */
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions
2020-05-07 7:50 [PATCH v2 00/10] clean up SECTOR related macros and sectors/pages conversions Zhen Lei
` (9 preceding siblings ...)
2020-05-07 7:51 ` [PATCH v2 10/10] md: use PAGE_SECTORS to clean up code Zhen Lei
@ 2020-05-15 2:05 ` Leizhen (ThunderTown)
10 siblings, 0 replies; 18+ messages in thread
From: Leizhen (ThunderTown) @ 2020-05-15 2:05 UTC (permalink / raw)
To: Minchan Kim, Nitin Gupta, Sergey Senozhatsky, Matthew Wilcox,
Jens Axboe, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, linux-block, Andrew Morton, linux-mm, dm-devel,
Song Liu, linux-raid, linux-kernel
Hi, all:
It seems no one take care about these patches. But I think patch 1 is need. And
the main discussion points of others is whether we should add
sectors_to_npage()/npage_to_sectors() or keep PAGE_SECTORS_SHIFT. And which marco
name do we prefer: PAGE_SECTORS vs SECTORS_PER_PAGE, PAGE_SECTORS_SHIFT vs
SECTORS_PER_PAGE_SHIFT.
Hi, Jens Axboe, Coly Li, Kent Overstreet,Alasdair Kergon. Mike Snitzer:
Can you take a look at patch 1?
On 2020/5/7 15:50, Zhen Lei wrote:
> v1 --> v2:
> As Matthew Wilcox's suggestion, add sectors_to_npage()/npage_to_sectors()
> helpers to eliminate SECTORS_PER_PAGE_SHIFT, because it's quite hard to read.
> In further, I also eliminated PAGE_SECTORS_SHIFT.
>
> I tried to eliminate all magic number "9" and "512", but it's too many, maybe
> no one want to review it, so I gave up. In the process of searching, I found
> the existing macro PAGE_SECTORS, it's equivalent to SECTORS_PER_PAGE. Because
> PAGE_SECTORS was defined in include/linux/device-mapper.h, and SECTORS_PER_PAGE
> was defined in drivers/block/zram/zram_drv.h, so I discarded SECTORS_PER_PAGE,
> althrough I prefer it so much.
>
> v1:
> When I studied the code of mm/swap, I found "1 << (PAGE_SHIFT - 9)" appears
> many times. So I try to clean up it.
>
> 1. Replace "1 << (PAGE_SHIFT - 9)" or similar with SECTORS_PER_PAGE
> 2. Replace "PAGE_SHIFT - 9" with SECTORS_PER_PAGE_SHIFT
> 3. Replace "9" with SECTOR_SHIFT
> 4. Replace "512" with SECTOR_SIZE
>
> Zhen Lei (10):
> block: move PAGE_SECTORS definition into <linux/blkdev.h>
> zram: abolish macro SECTORS_PER_PAGE
> block: add sectors_to_npage()/npage_to_sectors() helpers
> zram: abolish macro SECTORS_PER_PAGE_SHIFT
> block: abolish macro PAGE_SECTORS_SHIFT
> mm/swap: use npage_to_sectors() and PAGE_SECTORS to clean up code
> block: use sectors_to_npage() and PAGE_SECTORS to clean up code
> md: use sectors_to_npage() and npage_to_sectors() to clean up code
> md: use existing definition RESYNC_SECTORS
> md: use PAGE_SECTORS to clean up code
>
> block/blk-settings.c | 6 +++---
> block/partitions/core.c | 5 ++---
> drivers/block/brd.c | 7 ++-----
> drivers/block/null_blk_main.c | 10 ++++------
> drivers/block/zram/zram_drv.c | 8 ++++----
> drivers/block/zram/zram_drv.h | 2 --
> drivers/md/bcache/util.h | 2 --
> drivers/md/dm-kcopyd.c | 2 +-
> drivers/md/dm-table.c | 2 +-
> drivers/md/md-bitmap.c | 16 ++++++++--------
> drivers/md/md.c | 6 +++---
> drivers/md/raid1.c | 10 +++++-----
> drivers/md/raid10.c | 28 ++++++++++++++--------------
> drivers/md/raid5-cache.c | 11 +++++------
> drivers/md/raid5.c | 4 ++--
> include/linux/blkdev.h | 7 +++++--
> include/linux/device-mapper.h | 1 -
> mm/page_io.c | 4 ++--
> mm/swapfile.c | 12 ++++++------
> 19 files changed, 67 insertions(+), 76 deletions(-)
>
^ permalink raw reply [flat|nested] 18+ messages in thread