Re: [PATCH 2/4] mm/swap: use SECTORS_PER_PAGE_SHIFT to clean up code
On 2020/5/6 11:47, Leizhen (ThunderTown) wrote: > > > On 2020/5/6 9:33, Leizhen (ThunderTown) wrote: >> >> >> On 2020/5/6 1:25, Matthew Wilcox wrote: >>> On Tue, May 05, 2020 at 07:55:41PM +0800, Zhen Lei wrote: +++ 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 = (se->start_block + 1) << SECTORS_PER_PAGE_SHIFT; + nr_blocks = ((sector_t)se->nr_pages - 1) << SECTORS_PER_PAGE_SHIFT; >>> >>> Thinking about this some more, wouldn't this look better? >>> >>> start_block = page_sectors(se->start_block + 1); >>> nr_block = page_sectors(se->nr_pages - 1); >>> >> >> OK,That's fine, it's clearer. And in this way, there won't be more than 80 >> columns. > > Should we rename "page_sectors" to "page_to_sectors"? Because we may need to > define > "sectors_to_page" also. Change the "sectors_to_page" to "sectors_to_npage", npage means "number of pages" or "page number". To distinguish the use case of "pfn_to_page()" etc. The latter returns the pointer of "struct page". > >> >>> >>> . >>>
Re: [PATCH 2/4] mm/swap: use SECTORS_PER_PAGE_SHIFT to clean up code
On 2020/5/6 9:33, Leizhen (ThunderTown) wrote: > > > On 2020/5/6 1:25, Matthew Wilcox wrote: >> On Tue, May 05, 2020 at 07:55:41PM +0800, Zhen Lei wrote: >>> +++ 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 = (se->start_block + 1) << SECTORS_PER_PAGE_SHIFT; >>> + nr_blocks = ((sector_t)se->nr_pages - 1) << SECTORS_PER_PAGE_SHIFT; >> >> Thinking about this some more, wouldn't this look better? >> >> start_block = page_sectors(se->start_block + 1); >> nr_block = page_sectors(se->nr_pages - 1); >> > > OK,That's fine, it's clearer. And in this way, there won't be more than 80 > columns. Should we rename "page_sectors" to "page_to_sectors"? Because we may need to define "sectors_to_page" also. > >> >> . >>
Re: [PATCH 2/4] mm/swap: use SECTORS_PER_PAGE_SHIFT to clean up code
On 2020/5/6 1:25, Matthew Wilcox wrote: > On Tue, May 05, 2020 at 07:55:41PM +0800, Zhen Lei wrote: >> +++ 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 = (se->start_block + 1) << SECTORS_PER_PAGE_SHIFT; >> +nr_blocks = ((sector_t)se->nr_pages - 1) << SECTORS_PER_PAGE_SHIFT; > > Thinking about this some more, wouldn't this look better? > > start_block = page_sectors(se->start_block + 1); > nr_block = page_sectors(se->nr_pages - 1); > OK,That's fine, it's clearer. And in this way, there won't be more than 80 columns. > > . >
Re: [PATCH 2/4] mm/swap: use SECTORS_PER_PAGE_SHIFT to clean up code
On Tue, May 05, 2020 at 07:55:41PM +0800, Zhen Lei wrote: > +++ 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 = (se->start_block + 1) << SECTORS_PER_PAGE_SHIFT; > + nr_blocks = ((sector_t)se->nr_pages - 1) << SECTORS_PER_PAGE_SHIFT; Thinking about this some more, wouldn't this look better? start_block = page_sectors(se->start_block + 1); nr_block = page_sectors(se->nr_pages - 1);
[PATCH 2/4] mm/swap: use SECTORS_PER_PAGE_SHIFT to clean up code
1. Replace "PAGE_SHIFT - 9" with SECTORS_PER_PAGE_SHIFT Signed-off-by: Zhen Lei --- 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 b1d4f4558e6b..51f25238fd9a 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 <<= SECTORS_PER_PAGE_SHIFT; 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 (sector_t)__page_file_index(page) << SECTORS_PER_PAGE_SHIFT; } static inline void count_swpout_vm_event(struct page *page) diff --git a/mm/swapfile.c b/mm/swapfile.c index 5871a2aa86a5..0871023c0166 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 = (se->start_block + 1) << SECTORS_PER_PAGE_SHIFT; + nr_blocks = ((sector_t)se->nr_pages - 1) << SECTORS_PER_PAGE_SHIFT; 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 = se->start_block << SECTORS_PER_PAGE_SHIFT; + nr_blocks = (sector_t)se->nr_pages << SECTORS_PER_PAGE_SHIFT; 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 <<= SECTORS_PER_PAGE_SHIFT; + nr_blocks <<= SECTORS_PER_PAGE_SHIFT; if (blkdev_issue_discard(si->bdev, start_block, nr_blocks, GFP_NOIO, 0)) break; -- 2.26.0.106.g9fadedd