[PATCHv2] mm: bail out when the PMD has been set in bloom filter

[PATCHv2] mm: bail out when the PMD has been set in bloom filter

[zhaoyang.huang]

From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>

Part of bloom filter utilization in MGLRU are listed below, in which we
can see that the step '3' will prevent the hot PMD to be carried to new
gen since the new arrived rmap_walk clears the page's young flag.
This commit would like to suggest to query the PMD in bloom filter
before starting the rmap walk to improve this. In terms of the cost,
test_bloom_filter only consume 20~30 instructions in modern processors(25
instructions in ARM64).

1. rmap_walk set suitable PMD in filters[max_seq] while all page's turn
   to be non-young status
   * rmap_walk->lru_gen_look_around->update_bloom_filter(max_seq)
2. young pages gathering on the PMD if it is a hot VM area
3. newly arrived rmap_walk in the same PMD clears page's young which
   are set in step 2
4. walk_mm test the PMD again during aging, which will bring the
   suitable PMD to filters[max_seq+1]
   * walk_mm->walk_pmd_range->test_bloom_fitler->update_bloom_filter(
   walk->seq + 1)

Tested-by: syzbot@syzkaller.appspotmail.com
Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
---
v2: fix null-ptr-ref of mm_state and update commit message
---
---
 mm/vmscan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 10f1e7d716ca..5558a24d1564 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4234,6 +4234,10 @@ bool lru_gen_look_around(struct page_vma_mapped_walk *pvmw)
 	/* avoid taking the LRU lock under the PTL when possible */
 	walk = current->reclaim_state ? current->reclaim_state->mm_walk : NULL;
 
+	/* may the pmd has been set in bloom filter */
+	if (mm_state && test_bloom_filter(mm_state, max_seq, pvmw->pmd))
+		return true;
+
 	start = max(addr & PMD_MASK, vma->vm_start);
 	end = min(addr | ~PMD_MASK, vma->vm_end - 1) + 1;
 
-- 
2.25.1

Re: [PATCHv2] mm: bail out when the PMD has been set in bloom filter

[David Hildenbrand (Arm)]

On 3/4/26 04:15, zhaoyang.huang wrote:
> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> 
> Part of bloom filter utilization in MGLRU are listed below, in which we
> can see that the step '3' will prevent the hot PMD to be carried to new
> gen since the new arrived rmap_walk clears the page's young flag.
> This commit would like to suggest to query the PMD in bloom filter
> before starting the rmap walk to improve this. In terms of the cost,
> test_bloom_filter only consume 20~30 instructions in modern processors(25
> instructions in ARM64).

This is hard to follow, for example because

* You forward-reference things
* You talk about "this commit". A commit does not suggest anything,
  people to.
* It's unclear what the real benefit of the patch is or which problem it
  tries to solve.


You could start with a template like the following, where you clearly
express what is currently problematic, and why, and how you are
intending to optimize:

"Currently, lru_gen_look_around() does not properly consider the bloom
filter when processing a PTE. This is suboptimal, because TODO ...

To improve performance, let's consult the bloom filter for the PMD we
are processing, to just skip processing all PTEs in the PMD table.
"

In general, for performance optimizations that are not completely
obvious, we prefer actual proof that it is worth the additional code.

If it's a correctness issue, we should spell that out. I'm still not
sure what it is :)

> 
> 1. rmap_walk set suitable PMD in filters[max_seq] while all page's turn
>    to be non-young status
>    * rmap_walk->lru_gen_look_around->update_bloom_filter(max_seq)
> 2. young pages gathering on the PMD if it is a hot VM area
> 3. newly arrived rmap_walk in the same PMD clears page's young which
>    are set in step 2
> 4. walk_mm test the PMD again during aging, which will bring the
>    suitable PMD to filters[max_seq+1]
>    * walk_mm->walk_pmd_range->test_bloom_fitler->update_bloom_filter(
>    walk->seq + 1)
> 
> Tested-by: syzbot@syzkaller.appspotmail.com

That doesn't really count as "Tested". It just verified that the problem
in v1 no longer results in a splat, but it didn't really test if the
patch does the right thing.


> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> ---
> v2: fix null-ptr-ref of mm_state and update commit message
> ---
> ---
>  mm/vmscan.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 10f1e7d716ca..5558a24d1564 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4234,6 +4234,10 @@ bool lru_gen_look_around(struct page_vma_mapped_walk *pvmw)
>  	/* avoid taking the LRU lock under the PTL when possible */
>  	walk = current->reclaim_state ? current->reclaim_state->mm_walk : NULL;
>  
> +	/* may the pmd has been set in bloom filter */

That comment is not really helpful given that the code does exactly that.

> +	if (mm_state && test_bloom_filter(mm_state, max_seq, pvmw->pmd))
> +		return true;


-- 
Cheers,

David