1. Patchew
  2. linux
  3. [PATCH v2 0/5] add static PMD zero page support
  4. View patch

[PATCH v2 2/5] huge_memory: add huge_zero_page_shrinker_(init|exit) function

Pankaj Raghav (Samsung) posted 5 patches 3 months ago
[PATCH v2 2/5] huge_memory: add huge_zero_page_shrinker_(init|exit) function
Posted by Pankaj Raghav (Samsung) 3 months ago 
From: Pankaj Raghav <p.raghav@samsung.com>

Add huge_zero_page_shrinker_init() and huge_zero_page_shrinker_exit().
As shrinker will not be needed when static PMD zero page is enabled,
these two functions can be a no-op.

This is a preparation patch for static PMD zero page. No functional
changes.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 mm/huge_memory.c | 38 +++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index d3e66136e41a..101b67ab2eb6 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -289,6 +289,24 @@ static unsigned long shrink_huge_zero_page_scan(struct shrinker *shrink,
 }
 
 static struct shrinker *huge_zero_page_shrinker;
+static int huge_zero_page_shrinker_init(void)
+{
+	huge_zero_page_shrinker = shrinker_alloc(0, "thp-zero");
+	if (!huge_zero_page_shrinker)
+		return -ENOMEM;
+
+	huge_zero_page_shrinker->count_objects = shrink_huge_zero_page_count;
+	huge_zero_page_shrinker->scan_objects = shrink_huge_zero_page_scan;
+	shrinker_register(huge_zero_page_shrinker);
+	return 0;
+}
+
+static void huge_zero_page_shrinker_exit(void)
+{
+	shrinker_free(huge_zero_page_shrinker);
+	return;
+}
+
 
 #ifdef CONFIG_SYSFS
 static ssize_t enabled_show(struct kobject *kobj,
@@ -850,33 +868,31 @@ static inline void hugepage_exit_sysfs(struct kobject *hugepage_kobj)
 
 static int __init thp_shrinker_init(void)
 {
-	huge_zero_page_shrinker = shrinker_alloc(0, "thp-zero");
-	if (!huge_zero_page_shrinker)
-		return -ENOMEM;
+	int ret = 0;
 
 	deferred_split_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE |
 						 SHRINKER_MEMCG_AWARE |
 						 SHRINKER_NONSLAB,
 						 "thp-deferred_split");
-	if (!deferred_split_shrinker) {
-		shrinker_free(huge_zero_page_shrinker);
+	if (!deferred_split_shrinker)
 		return -ENOMEM;
-	}
-
-	huge_zero_page_shrinker->count_objects = shrink_huge_zero_page_count;
-	huge_zero_page_shrinker->scan_objects = shrink_huge_zero_page_scan;
-	shrinker_register(huge_zero_page_shrinker);
 
 	deferred_split_shrinker->count_objects = deferred_split_count;
 	deferred_split_shrinker->scan_objects = deferred_split_scan;
 	shrinker_register(deferred_split_shrinker);
 
+	ret = huge_zero_page_shrinker_init();
+	if (ret) {
+		shrinker_free(deferred_split_shrinker);
+		return ret;
+	}
+
 	return 0;
 }
 
 static void __init thp_shrinker_exit(void)
 {
-	shrinker_free(huge_zero_page_shrinker);
+	huge_zero_page_shrinker_exit();
 	shrinker_free(deferred_split_shrinker);
 }
 
-- 
2.49.0
Re: [PATCH v2 2/5] huge_memory: add huge_zero_page_shrinker_(init|exit) function
Posted by Lorenzo Stoakes 2 months, 3 weeks ago 
Nit on subject, function -> functions.

On Mon, Jul 07, 2025 at 04:23:16PM +0200, Pankaj Raghav (Samsung) wrote:
> From: Pankaj Raghav <p.raghav@samsung.com>
>
> Add huge_zero_page_shrinker_init() and huge_zero_page_shrinker_exit().
> As shrinker will not be needed when static PMD zero page is enabled,
> these two functions can be a no-op.
>
> This is a preparation patch for static PMD zero page. No functional
> changes.

This is nitty stuff, but I think this is a little unclear, maybe something
like:

	We will soon be determining whether to use a shrinker depending on
	whether a static PMD zero page is available, therefore abstract out
	shrink initialisation and teardown such that we can more easily
	handle both the shrinker and static PMD zero page cases.

>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>

Other than nits, this LGTM, so with those addressed:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  mm/huge_memory.c | 38 +++++++++++++++++++++++++++-----------
>  1 file changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index d3e66136e41a..101b67ab2eb6 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -289,6 +289,24 @@ static unsigned long shrink_huge_zero_page_scan(struct shrinker *shrink,
>  }
>
>  static struct shrinker *huge_zero_page_shrinker;
> +static int huge_zero_page_shrinker_init(void)
> +{
> +	huge_zero_page_shrinker = shrinker_alloc(0, "thp-zero");
> +	if (!huge_zero_page_shrinker)
> +		return -ENOMEM;
> +
> +	huge_zero_page_shrinker->count_objects = shrink_huge_zero_page_count;
> +	huge_zero_page_shrinker->scan_objects = shrink_huge_zero_page_scan;
> +	shrinker_register(huge_zero_page_shrinker);
> +	return 0;
> +}
> +
> +static void huge_zero_page_shrinker_exit(void)
> +{
> +	shrinker_free(huge_zero_page_shrinker);
> +	return;
> +}
> +
>
>  #ifdef CONFIG_SYSFS
>  static ssize_t enabled_show(struct kobject *kobj,
> @@ -850,33 +868,31 @@ static inline void hugepage_exit_sysfs(struct kobject *hugepage_kobj)
>
>  static int __init thp_shrinker_init(void)
>  {
> -	huge_zero_page_shrinker = shrinker_alloc(0, "thp-zero");
> -	if (!huge_zero_page_shrinker)
> -		return -ENOMEM;
> +	int ret = 0;

Kinda no point in initialising to zero, unless...

>
>  	deferred_split_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE |
>  						 SHRINKER_MEMCG_AWARE |
>  						 SHRINKER_NONSLAB,
>  						 "thp-deferred_split");
> -	if (!deferred_split_shrinker) {
> -		shrinker_free(huge_zero_page_shrinker);
> +	if (!deferred_split_shrinker)
>  		return -ENOMEM;
> -	}
> -
> -	huge_zero_page_shrinker->count_objects = shrink_huge_zero_page_count;
> -	huge_zero_page_shrinker->scan_objects = shrink_huge_zero_page_scan;
> -	shrinker_register(huge_zero_page_shrinker);
>
>  	deferred_split_shrinker->count_objects = deferred_split_count;
>  	deferred_split_shrinker->scan_objects = deferred_split_scan;
>  	shrinker_register(deferred_split_shrinker);
>
> +	ret = huge_zero_page_shrinker_init();
> +	if (ret) {
> +		shrinker_free(deferred_split_shrinker);
> +		return ret;
> +	}

... you change this to:

	if (ret)
		shrinker_free(deferred_split_shrinker);

	return ret;

But it's not a big deal. Maybe I'd rename ret -> err if you keep things as
they are (but don't init to 0).

> +
>  	return 0;
>  }
>
>  static void __init thp_shrinker_exit(void)
>  {
> -	shrinker_free(huge_zero_page_shrinker);
> +	huge_zero_page_shrinker_exit();
>  	shrinker_free(deferred_split_shrinker);
>  }
>
> --
> 2.49.0
>
Re: [PATCH v2 2/5] huge_memory: add huge_zero_page_shrinker_(init|exit) function
Posted by Pankaj Raghav (Samsung) 2 months, 3 weeks ago 
On Tue, Jul 15, 2025 at 03:29:08PM +0100, Lorenzo Stoakes wrote:
> Nit on subject, function -> functions.
> 
> On Mon, Jul 07, 2025 at 04:23:16PM +0200, Pankaj Raghav (Samsung) wrote:
> > From: Pankaj Raghav <p.raghav@samsung.com>
> >
> > Add huge_zero_page_shrinker_init() and huge_zero_page_shrinker_exit().
> > As shrinker will not be needed when static PMD zero page is enabled,
> > these two functions can be a no-op.
> >
> > This is a preparation patch for static PMD zero page. No functional
> > changes.
> 
> This is nitty stuff, but I think this is a little unclear, maybe something
> like:
> 
> 	We will soon be determining whether to use a shrinker depending on
> 	whether a static PMD zero page is available, therefore abstract out
> 	shrink initialisation and teardown such that we can more easily
> 	handle both the shrinker and static PMD zero page cases.
> 
This looks good. I will use add this to the commit message.

> >
> > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> 
> Other than nits, this LGTM, so with those addressed:
> 
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Thanks.

> >  #ifdef CONFIG_SYSFS
> >  static ssize_t enabled_show(struct kobject *kobj,
> > @@ -850,33 +868,31 @@ static inline void hugepage_exit_sysfs(struct kobject *hugepage_kobj)
> >
> >  static int __init thp_shrinker_init(void)
> >  {
> > -	huge_zero_page_shrinker = shrinker_alloc(0, "thp-zero");
> > -	if (!huge_zero_page_shrinker)
> > -		return -ENOMEM;
> > +	int ret = 0;
> 
> Kinda no point in initialising to zero, unless...
> 
> >
> >  	deferred_split_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE |
> >  						 SHRINKER_MEMCG_AWARE |
> >  						 SHRINKER_NONSLAB,
> >  						 "thp-deferred_split");
> > -	if (!deferred_split_shrinker) {
> > -		shrinker_free(huge_zero_page_shrinker);
> > +	if (!deferred_split_shrinker)
> >  		return -ENOMEM;
> > -	}
> > -
> > -	huge_zero_page_shrinker->count_objects = shrink_huge_zero_page_count;
> > -	huge_zero_page_shrinker->scan_objects = shrink_huge_zero_page_scan;
> > -	shrinker_register(huge_zero_page_shrinker);
> >
> >  	deferred_split_shrinker->count_objects = deferred_split_count;
> >  	deferred_split_shrinker->scan_objects = deferred_split_scan;
> >  	shrinker_register(deferred_split_shrinker);
> >
> > +	ret = huge_zero_page_shrinker_init();
> > +	if (ret) {
> > +		shrinker_free(deferred_split_shrinker);
> > +		return ret;
> > +	}
> 
> ... you change this to:
> 
> 	if (ret)
> 		shrinker_free(deferred_split_shrinker);
> 
> 	return ret;
> 
> But it's not a big deal. Maybe I'd rename ret -> err if you keep things as
> they are (but don't init to 0).

Sounds good.

--
Pankaj
Re: [PATCH v2 2/5] huge_memory: add huge_zero_page_shrinker_(init|exit) function
Posted by David Hildenbrand 2 months, 3 weeks ago 
On 07.07.25 16:23, Pankaj Raghav (Samsung) wrote:
> From: Pankaj Raghav <p.raghav@samsung.com>
> 
> Add huge_zero_page_shrinker_init() and huge_zero_page_shrinker_exit().
> As shrinker will not be needed when static PMD zero page is enabled,
> these two functions can be a no-op.
> 
> This is a preparation patch for static PMD zero page. No functional
> changes.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>   mm/huge_memory.c | 38 +++++++++++++++++++++++++++-----------
>   1 file changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index d3e66136e41a..101b67ab2eb6 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -289,6 +289,24 @@ static unsigned long shrink_huge_zero_page_scan(struct shrinker *shrink,
>   }
>   
>   static struct shrinker *huge_zero_page_shrinker;
> +static int huge_zero_page_shrinker_init(void)
> +{
> +	huge_zero_page_shrinker = shrinker_alloc(0, "thp-zero");
> +	if (!huge_zero_page_shrinker)
> +		return -ENOMEM;
> +
> +	huge_zero_page_shrinker->count_objects = shrink_huge_zero_page_count;
> +	huge_zero_page_shrinker->scan_objects = shrink_huge_zero_page_scan;
> +	shrinker_register(huge_zero_page_shrinker);
> +	return 0;
> +}
> +
> +static void huge_zero_page_shrinker_exit(void)
> +{
> +	shrinker_free(huge_zero_page_shrinker);
> +	return;
> +}

While at it, we should rename most of that to "huge_zero_folio" I assume.

-- 
Cheers,

David / dhildenb
Re: [PATCH v2 2/5] huge_memory: add huge_zero_page_shrinker_(init|exit) function
Posted by Pankaj Raghav (Samsung) 2 months, 3 weeks ago 
On Tue, Jul 15, 2025 at 04:18:26PM +0200, David Hildenbrand wrote:
> On 07.07.25 16:23, Pankaj Raghav (Samsung) wrote:
> > From: Pankaj Raghav <p.raghav@samsung.com>
> > 
> > Add huge_zero_page_shrinker_init() and huge_zero_page_shrinker_exit().
> > As shrinker will not be needed when static PMD zero page is enabled,
> > these two functions can be a no-op.
> > 
> > This is a preparation patch for static PMD zero page. No functional
> > changes.
> > 
> > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> > ---
> >   mm/huge_memory.c | 38 +++++++++++++++++++++++++++-----------
> >   1 file changed, 27 insertions(+), 11 deletions(-)
> > 
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index d3e66136e41a..101b67ab2eb6 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -289,6 +289,24 @@ static unsigned long shrink_huge_zero_page_scan(struct shrinker *shrink,
> >   }
> >   static struct shrinker *huge_zero_page_shrinker;
> > +static int huge_zero_page_shrinker_init(void)
> > +{
> > +	huge_zero_page_shrinker = shrinker_alloc(0, "thp-zero");
> > +	if (!huge_zero_page_shrinker)
> > +		return -ENOMEM;
> > +
> > +	huge_zero_page_shrinker->count_objects = shrink_huge_zero_page_count;
> > +	huge_zero_page_shrinker->scan_objects = shrink_huge_zero_page_scan;
> > +	shrinker_register(huge_zero_page_shrinker);
> > +	return 0;
> > +}
> > +
> > +static void huge_zero_page_shrinker_exit(void)
> > +{
> > +	shrinker_free(huge_zero_page_shrinker);
> > +	return;
> > +}
> 
> While at it, we should rename most of that to "huge_zero_folio" I assume.
Sounds good.
> 
> -- 
> Cheers,
> 
> David / dhildenb
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.