All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/khugepaged: use page_cache_sync_ra() directly
@ 2026-07-16  8:32 Aditya Prakash Srivastava
  2026-07-16  9:02 ` Dev Jain
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Aditya Prakash Srivastava @ 2026-07-16  8:32 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand, Lorenzo Stoakes
  Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, linux-mm, linux-kernel,
	Aditya Prakash Srivastava

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.

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.

No functional change is introduced, as the logic and behavior
remain identical.

Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm/khugepaged: use page_cache_sync_ra() directly
  2026-07-16  8:32 [PATCH] mm/khugepaged: use page_cache_sync_ra() directly Aditya Prakash Srivastava
@ 2026-07-16  9:02 ` Dev Jain
  2026-07-16  9:08 ` David Hildenbrand (Arm)
  2026-07-16  9:11 ` Lorenzo Stoakes (ARM)
  2 siblings, 0 replies; 5+ messages in thread
From: Dev Jain @ 2026-07-16  9:02 UTC (permalink / raw)
  To: Aditya Prakash Srivastava, Andrew Morton, David Hildenbrand,
	Lorenzo Stoakes
  Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
	Barry Song, Lance Yang, linux-mm, linux-kernel



On 16/07/26 2:02 pm, 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

How exactly is this a legacy interface?

> a local readahead_control structure using DEFINE_READAHEAD.
> 
> 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.

I can see multiple callers of page_cache_sync_readahead.
> 
> No functional change is introduced, as the logic and behavior
> remain identical.
> 
> Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> ---
>  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);


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm/khugepaged: use page_cache_sync_ra() directly
  2026-07-16  8:32 [PATCH] mm/khugepaged: use page_cache_sync_ra() directly Aditya Prakash Srivastava
  2026-07-16  9:02 ` Dev Jain
@ 2026-07-16  9:08 ` David Hildenbrand (Arm)
  2026-07-16  9:11 ` Lorenzo Stoakes (ARM)
  2 siblings, 0 replies; 5+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-16  9:08 UTC (permalink / raw)
  To: Aditya Prakash Srivastava, Andrew Morton, Lorenzo Stoakes
  Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, linux-mm, linux-kernel

On 7/16/26 10:32, 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.
> 
> 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.
> 
> No functional change is introduced, as the logic and behavior
> remain identical.
> 
> Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> ---
>  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);

The page_cache_sync_readahead() call nicely wraps these two things.

You are essentially duplicating code, no?

So I also miss the point.

-- 
Cheers,

David

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm/khugepaged: use page_cache_sync_ra() directly
  2026-07-16  8:32 [PATCH] mm/khugepaged: use page_cache_sync_ra() directly Aditya Prakash Srivastava
  2026-07-16  9:02 ` Dev Jain
  2026-07-16  9:08 ` David Hildenbrand (Arm)
@ 2026-07-16  9:11 ` Lorenzo Stoakes (ARM)
  2026-07-16  9:16   ` Aditya Prakash Srivastava
  2 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-07-16  9:11 UTC (permalink / raw)
  To: Aditya Prakash Srivastava
  Cc: Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
	Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
	Lance Yang, linux-mm, linux-kernel

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@gmail.com>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm/khugepaged: use page_cache_sync_ra() directly
  2026-07-16  9:11 ` Lorenzo Stoakes (ARM)
@ 2026-07-16  9:16   ` Aditya Prakash Srivastava
  0 siblings, 0 replies; 5+ messages in thread
From: Aditya Prakash Srivastava @ 2026-07-16  9:16 UTC (permalink / raw)
  To: Lorenzo Stoakes (ARM)
  Cc: Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
	Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
	Lance Yang, linux-mm, linux-kernel

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@kernel.org> 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@gmail.com>
>
> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-16  9:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16  8:32 [PATCH] mm/khugepaged: use page_cache_sync_ra() directly Aditya Prakash Srivastava
2026-07-16  9:02 ` Dev Jain
2026-07-16  9:08 ` David Hildenbrand (Arm)
2026-07-16  9:11 ` Lorenzo Stoakes (ARM)
2026-07-16  9:16   ` Aditya Prakash Srivastava

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.