* [PATCH] mm,swap: cleanup VMA based swap readahead window calculation
@ 2024-04-24 6:30 Huang Ying
2024-04-24 9:23 ` David Hildenbrand
0 siblings, 1 reply; 3+ messages in thread
From: Huang Ying @ 2024-04-24 6:30 UTC (permalink / raw
To: Andrew Morton
Cc: linux-mm, linux-kernel, Huang Ying, Hugh Dickins, Alistair Popple,
Anshuman Khandual, David Hildenbrand, Mel Gorman, Miaohe Lin,
Minchan Kim, Ryan Roberts, Yang Shi, Yu Zhao, Kairui Song,
Barry Song, Chris Li, Yosry Ahmed
When VMA based swap readahead is introduced in commit
ec560175c0b6 ("mm, swap: VMA based swap readahead"), "struct
vma_swap_readahead" is defined to describe the readahead window.
Because we wanted to save the PTE entries in the struct at that time.
But after commit 4f8fcf4ced0b ("mm/swap: swap_vma_readahead() do the
pte_offset_map()"), we no longer save PTE entries in the struct. The
size of the struct becomes so small, that it's better to use the
fields of the struct directly. This can simplify the code to improve
the code readability. The line number of source code reduces too.
No functionality change is expected in this patch.
Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Alistair Popple <apopple@nvidia.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Miaohe Lin <linmiaohe@huawei.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Yang Shi <shy828301@gmail.com>
Cc: Yu Zhao <yuzhao@google.com>
Cc: Kairui Song <kasong@tencent.com>
Cc: Barry Song <v-songbaohua@oppo.com>
Cc: Chris Li <chrisl@kernel.org>
Cc: Yosry Ahmed <yosryahmed@google.com>
---
mm/swap_state.c | 86 ++++++++++++++++++-------------------------------
1 file changed, 32 insertions(+), 54 deletions(-)
diff --git a/mm/swap_state.c b/mm/swap_state.c
index bfc7e8c58a6d..19874e7bab70 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -42,6 +42,8 @@ struct address_space *swapper_spaces[MAX_SWAPFILES] __read_mostly;
static unsigned int nr_swapper_spaces[MAX_SWAPFILES] __read_mostly;
static bool enable_vma_readahead __read_mostly = true;
+#define SWAP_RA_ORDER_CEILING 5
+
#define SWAP_RA_WIN_SHIFT (PAGE_SHIFT / 2)
#define SWAP_RA_HITS_MASK ((1UL << SWAP_RA_WIN_SHIFT) - 1)
#define SWAP_RA_HITS_MAX SWAP_RA_HITS_MASK
@@ -738,62 +740,42 @@ void exit_swap_address_space(unsigned int type)
swapper_spaces[type] = NULL;
}
-#define SWAP_RA_ORDER_CEILING 5
-
-struct vma_swap_readahead {
- unsigned short win;
- unsigned short offset;
- unsigned short nr_pte;
-};
-
-static void swap_ra_info(struct vm_fault *vmf,
- struct vma_swap_readahead *ra_info)
+static int swap_vma_ra_win(struct vm_fault *vmf, unsigned long *start,
+ unsigned long *end)
{
struct vm_area_struct *vma = vmf->vma;
unsigned long ra_val;
- unsigned long faddr, pfn, fpfn, lpfn, rpfn;
- unsigned long start, end;
+ unsigned long faddr, prev_faddr, left, right;
unsigned int max_win, hits, prev_win, win;
- max_win = 1 << min_t(unsigned int, READ_ONCE(page_cluster),
- SWAP_RA_ORDER_CEILING);
- if (max_win == 1) {
- ra_info->win = 1;
- return;
- }
+ max_win = 1 << min(READ_ONCE(page_cluster), SWAP_RA_ORDER_CEILING);
+ if (max_win == 1)
+ return 1;
faddr = vmf->address;
- fpfn = PFN_DOWN(faddr);
ra_val = GET_SWAP_RA_VAL(vma);
- pfn = PFN_DOWN(SWAP_RA_ADDR(ra_val));
+ prev_faddr = SWAP_RA_ADDR(ra_val);
prev_win = SWAP_RA_WIN(ra_val);
hits = SWAP_RA_HITS(ra_val);
- ra_info->win = win = __swapin_nr_pages(pfn, fpfn, hits,
- max_win, prev_win);
- atomic_long_set(&vma->swap_readahead_info,
- SWAP_RA_VAL(faddr, win, 0));
+ win = __swapin_nr_pages(PFN_DOWN(prev_faddr), PFN_DOWN(faddr), hits,
+ max_win, prev_win);
+ atomic_long_set(&vma->swap_readahead_info, SWAP_RA_VAL(faddr, win, 0));
if (win == 1)
- return;
+ return 1;
- if (fpfn == pfn + 1) {
- lpfn = fpfn;
- rpfn = fpfn + win;
- } else if (pfn == fpfn + 1) {
- lpfn = fpfn - win + 1;
- rpfn = fpfn + 1;
- } else {
- unsigned int left = (win - 1) / 2;
+ if (faddr == prev_faddr + PAGE_SIZE)
+ left = faddr;
+ else if (prev_faddr == faddr + PAGE_SIZE)
+ left = faddr - (win << PAGE_SHIFT) + PAGE_SIZE;
+ else
+ left = faddr - (((win - 1) / 2) << PAGE_SHIFT);
+ right = left + (win << PAGE_SHIFT);
+ if ((long)left < 0)
+ left = 0;
+ *start = max3(left, vma->vm_start, faddr & PMD_MASK);
+ *end = min3(right, vma->vm_end, (faddr & PMD_MASK) + PMD_SIZE);
- lpfn = fpfn - left;
- rpfn = fpfn + win - left;
- }
- start = max3(lpfn, PFN_DOWN(vma->vm_start),
- PFN_DOWN(faddr & PMD_MASK));
- end = min3(rpfn, PFN_DOWN(vma->vm_end),
- PFN_DOWN((faddr & PMD_MASK) + PMD_SIZE));
-
- ra_info->nr_pte = end - start;
- ra_info->offset = fpfn - start;
+ return win;
}
/**
@@ -819,24 +801,20 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
struct swap_iocb *splug = NULL;
struct folio *folio;
pte_t *pte = NULL, pentry;
- unsigned long addr;
+ int win;
+ unsigned long start, end, addr;
swp_entry_t entry;
pgoff_t ilx;
- unsigned int i;
bool page_allocated;
- struct vma_swap_readahead ra_info = {
- .win = 1,
- };
- swap_ra_info(vmf, &ra_info);
- if (ra_info.win == 1)
+ win = swap_vma_ra_win(vmf, &start, &end);
+ if (win == 1)
goto skip;
- addr = vmf->address - (ra_info.offset * PAGE_SIZE);
- ilx = targ_ilx - ra_info.offset;
+ ilx = targ_ilx - PFN_DOWN(vmf->address - start);
blk_start_plug(&plug);
- for (i = 0; i < ra_info.nr_pte; i++, ilx++, addr += PAGE_SIZE) {
+ for (addr = start; addr < end; ilx++, addr += PAGE_SIZE) {
if (!pte++) {
pte = pte_offset_map(vmf->pmd, addr);
if (!pte)
@@ -856,7 +834,7 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
continue;
if (page_allocated) {
swap_read_folio(folio, false, &splug);
- if (i != ra_info.offset) {
+ if (addr != vmf->address) {
folio_set_readahead(folio);
count_vm_event(SWAP_RA);
}
--
2.39.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mm,swap: cleanup VMA based swap readahead window calculation
2024-04-24 6:30 [PATCH] mm,swap: cleanup VMA based swap readahead window calculation Huang Ying
@ 2024-04-24 9:23 ` David Hildenbrand
2024-04-25 1:45 ` Huang, Ying
0 siblings, 1 reply; 3+ messages in thread
From: David Hildenbrand @ 2024-04-24 9:23 UTC (permalink / raw
To: Huang Ying, Andrew Morton
Cc: linux-mm, linux-kernel, Hugh Dickins, Alistair Popple,
Anshuman Khandual, Mel Gorman, Miaohe Lin, Minchan Kim,
Ryan Roberts, Yang Shi, Yu Zhao, Kairui Song, Barry Song,
Chris Li, Yosry Ahmed
On 24.04.24 08:30, Huang Ying wrote:
> When VMA based swap readahead is introduced in commit
> ec560175c0b6 ("mm, swap: VMA based swap readahead"), "struct
> vma_swap_readahead" is defined to describe the readahead window.
> Because we wanted to save the PTE entries in the struct at that time.
> But after commit 4f8fcf4ced0b ("mm/swap: swap_vma_readahead() do the
> pte_offset_map()"), we no longer save PTE entries in the struct. The
> size of the struct becomes so small, that it's better to use the
> fields of the struct directly. This can simplify the code to improve
> the code readability. The line number of source code reduces too.
>
> No functionality change is expected in this patch.
From a quick scan, you perform quite some unrelated changes that make
the code harder to review than it should be. Consider separating any
cleanups from the core change of removing the struct.
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mm,swap: cleanup VMA based swap readahead window calculation
2024-04-24 9:23 ` David Hildenbrand
@ 2024-04-25 1:45 ` Huang, Ying
0 siblings, 0 replies; 3+ messages in thread
From: Huang, Ying @ 2024-04-25 1:45 UTC (permalink / raw
To: David Hildenbrand
Cc: Andrew Morton, linux-mm, linux-kernel, Hugh Dickins,
Alistair Popple, Anshuman Khandual, Mel Gorman, Miaohe Lin,
Minchan Kim, Ryan Roberts, Yang Shi, Yu Zhao, Kairui Song,
Barry Song, Chris Li, Yosry Ahmed
David Hildenbrand <david@redhat.com> writes:
> On 24.04.24 08:30, Huang Ying wrote:
>> When VMA based swap readahead is introduced in commit
>> ec560175c0b6 ("mm, swap: VMA based swap readahead"), "struct
>> vma_swap_readahead" is defined to describe the readahead window.
>> Because we wanted to save the PTE entries in the struct at that time.
>> But after commit 4f8fcf4ced0b ("mm/swap: swap_vma_readahead() do the
>> pte_offset_map()"), we no longer save PTE entries in the struct. The
>> size of the struct becomes so small, that it's better to use the
>> fields of the struct directly. This can simplify the code to improve
>> the code readability. The line number of source code reduces too.
>> No functionality change is expected in this patch.
>
> From a quick scan, you perform quite some unrelated changes that make
> the code harder to review than it should be. Consider separating any
> cleanups from the core change of removing the struct.
Sure. Will separate it.
--
Best Regards,
Huang, Ying
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-04-25 1:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-24 6:30 [PATCH] mm,swap: cleanup VMA based swap readahead window calculation Huang Ying
2024-04-24 9:23 ` David Hildenbrand
2024-04-25 1:45 ` Huang, Ying
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).