Re: [PATCH] mm/khugepaged: use page_cache_sync_ra() directly

[Aditya Prakash Srivastava]

Thanks for the feedback. I agree that replacing the helper with
its open-coded implementation at a single call site doesn't
provide a meaningful improvement. I'll drop this patch.

Thanks,
Aditya

On Thu, Jul 16, 2026 at 2:41 PM Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx> wrote:
>
> On Thu, Jul 16, 2026 at 08:32:11AM +0000, Aditya Prakash Srivastava wrote:
> > The static inline page_cache_sync_readahead() wrapper in pagemap.h
> > is legacy and only wraps page_cache_sync_ra() after initializing
> > a local readahead_control structure using DEFINE_READAHEAD.
>
> As Dev notes there's nothing legacy about it, it's used in multiple places.
>
> >
> > Modernize the khugepaged readahead call by using page_cache_sync_ra()
> > directly. This avoids the legacy wrapper and aligns the khugepaged
> > readahead implementation with other subsystems in the kernel.
>
> Nope, it's replacing a helper function that wraps something with open coding it
> instead for no good reason.
>
> >
> > No functional change is introduced, as the logic and behavior
> > remain identical.
> >
> > Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@xxxxxxxxx>
>
> Sorry this patch is replacing a wrapper with open code for not really much
> benefit.
>
> > ---
> >  mm/khugepaged.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 617bca76db49..2682b39401b2 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -2331,10 +2331,10 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> >                       }
> >               } else {        /* !is_shmem */
> >                       if (!folio || xa_is_value(folio)) {
> > +                             DEFINE_READAHEAD(ractl, file, &file->f_ra, mapping, index);
> > +
> >                               xas_unlock_irq(&xas);
> > -                             page_cache_sync_readahead(mapping, &file->f_ra,
> > -                                                       file, index,
> > -                                                       end - index);
> > +                             page_cache_sync_ra(&ractl, end - index);
> >                               /* drain lru cache to help folio_isolate_lru() */
> >                               lru_add_drain();
> >                               folio = filemap_lock_folio(mapping, index);
> > --
> > 2.47.3
> >
>
> Cheers, Lorenzo