Re: [PATCH v3 2/3] percpu: Do not trust hint starts when they are not set

[Joonwon Kang <joonwonkang@google.com>]

Hi, I am sorry for the late response. I was occupied by other issues lately.

> On Fri, Apr 10, 2026 at 05:44:16PM +0000, Joonwon Kang wrote:
> > contig_hint_start can be trusted outside the hint update function since
> > it will be updated everytime contig_hint is broken. On the other hand,
> > scan_hint_start might still be invalid anywhere in the code due to the
> > broken scan_hint not being updated promptly. If those starts are trusted
> > when they are not set, it could lead to false invalidation or update of
> > the hints.
> > 
> > Signed-off-by: Joonwon Kang <joonwonkang@google.com>
> > ---
> > v3: Initial version.
> > 
> >  mm/percpu.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/mm/percpu.c b/mm/percpu.c
> > index 3ecd86096641..f16533ed4a49 100644
> > --- a/mm/percpu.c
> > +++ b/mm/percpu.c
> > @@ -638,6 +638,13 @@ static void pcpu_block_update(struct pcpu_block_md *block, int start, int end)
> >  	if (end == block->nr_bits)
> >  		block->right_free = contig;
> >  
> > +	if (block->contig_hint == 0) {
> > +		block->contig_hint = contig;
> > +		block->contig_hint_start = start;
> > +		block->scan_hint = 0;
> > +		return;
> > +	}
> > +
> 
> This change isn't described in the commit log. I'd prefer to clean up
> the logic below than adding this additional if empty case.
> 

Oh, by the commit message I meant indirectly that contig_scan_hint should
not be trusted inside this hint update function while it can be outside.
Let me clean up the logic below instead of changing the commit message.
Since the control flow will always reach the first if-block below when
contig_hint is zero, it should be simple enough. Thanks!

> >  	if (contig > block->contig_hint) {
> >  		/* promote the old contig_hint to be the new scan_hint */
> >  		if (start > block->contig_hint_start) {
> > @@ -845,7 +852,8 @@ static void pcpu_block_update_hint_alloc(struct pcpu_chunk *chunk, int bit_off,
> >  					PCPU_BITMAP_BLOCK_BITS,
> >  					s_off + bits);
> >  
> > -	if (pcpu_region_overlap(s_block->scan_hint_start,
> > +	if (s_block->scan_hint &&
> > +	    pcpu_region_overlap(s_block->scan_hint_start,
> >  				s_block->scan_hint_start + s_block->scan_hint,
> >  				s_off,
> >  				s_off + bits))
> > @@ -889,7 +897,7 @@ static void pcpu_block_update_hint_alloc(struct pcpu_chunk *chunk, int bit_off,
> >  			/* reset the block */
> >  			e_block++;
> >  		} else {
> > -			if (e_off > e_block->scan_hint_start)
> > +			if (e_block->scan_hint && e_off > e_block->scan_hint_start)
> >  				e_block->scan_hint = 0;
> >  
> >  			e_block->left_free = 0;
> > @@ -922,7 +930,8 @@ static void pcpu_block_update_hint_alloc(struct pcpu_chunk *chunk, int bit_off,
> >  	if (nr_empty_pages)
> >  		pcpu_update_empty_pages(chunk, -nr_empty_pages);
> >  
> > -	if (pcpu_region_overlap(chunk_md->scan_hint_start,
> > +	if (chunk_md->scan_hint &&
> > +	    pcpu_region_overlap(chunk_md->scan_hint_start,
> >  				chunk_md->scan_hint_start +
> >  				chunk_md->scan_hint,
> >  				bit_off,
> > -- 
> > 2.53.0.1213.gd9a14994de-goog
> >