The value (HPAGE_PMD_NR - 1) is used often in the khugepaged code to
signify the limit of the max_ptes_* values. Add a define for this to
increase code readability and reuse.
Signed-off-by: Nico Pache <npache@redhat.com>
---
mm/khugepaged.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index c362b3b2e08a..3dcce6018f20 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -85,6 +85,7 @@ static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);
*
* Note that these are only respected if collapse was initiated by khugepaged.
*/
+#define COLLAPSE_MAX_PTES_LIMIT (HPAGE_PMD_NR - 1)
unsigned int khugepaged_max_ptes_none __read_mostly;
static unsigned int khugepaged_max_ptes_swap __read_mostly;
static unsigned int khugepaged_max_ptes_shared __read_mostly;
@@ -252,7 +253,7 @@ static ssize_t max_ptes_none_store(struct kobject *kobj,
unsigned long max_ptes_none;
err = kstrtoul(buf, 10, &max_ptes_none);
- if (err || max_ptes_none > HPAGE_PMD_NR - 1)
+ if (err || max_ptes_none > COLLAPSE_MAX_PTES_LIMIT)
return -EINVAL;
khugepaged_max_ptes_none = max_ptes_none;
@@ -277,7 +278,7 @@ static ssize_t max_ptes_swap_store(struct kobject *kobj,
unsigned long max_ptes_swap;
err = kstrtoul(buf, 10, &max_ptes_swap);
- if (err || max_ptes_swap > HPAGE_PMD_NR - 1)
+ if (err || max_ptes_swap > COLLAPSE_MAX_PTES_LIMIT)
return -EINVAL;
khugepaged_max_ptes_swap = max_ptes_swap;
@@ -303,7 +304,7 @@ static ssize_t max_ptes_shared_store(struct kobject *kobj,
unsigned long max_ptes_shared;
err = kstrtoul(buf, 10, &max_ptes_shared);
- if (err || max_ptes_shared > HPAGE_PMD_NR - 1)
+ if (err || max_ptes_shared > COLLAPSE_MAX_PTES_LIMIT)
return -EINVAL;
khugepaged_max_ptes_shared = max_ptes_shared;
@@ -384,7 +385,7 @@ int __init khugepaged_init(void)
return -ENOMEM;
khugepaged_pages_to_scan = HPAGE_PMD_NR * 8;
- khugepaged_max_ptes_none = HPAGE_PMD_NR - 1;
+ khugepaged_max_ptes_none = COLLAPSE_MAX_PTES_LIMIT;
khugepaged_max_ptes_swap = HPAGE_PMD_NR / 8;
khugepaged_max_ptes_shared = HPAGE_PMD_NR / 2;
@@ -1869,7 +1870,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
bool is_shmem = shmem_file(file);
VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
- VM_BUG_ON(start & (HPAGE_PMD_NR - 1));
+ VM_BUG_ON(start & (COLLAPSE_MAX_PTES_LIMIT));
result = alloc_charge_folio(&new_folio, mm, cc);
if (result != SCAN_SUCCEED)
@@ -2209,7 +2210,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
* unwritten page.
*/
folio_mark_uptodate(new_folio);
- folio_ref_add(new_folio, HPAGE_PMD_NR - 1);
+ folio_ref_add(new_folio, COLLAPSE_MAX_PTES_LIMIT);
if (is_shmem)
folio_mark_dirty(new_folio);
@@ -2303,7 +2304,7 @@ static enum scan_result hpage_collapse_scan_file(struct mm_struct *mm, unsigned
memset(cc->node_load, 0, sizeof(cc->node_load));
nodes_clear(cc->alloc_nmask);
rcu_read_lock();
- xas_for_each(&xas, folio, start + HPAGE_PMD_NR - 1) {
+ xas_for_each(&xas, folio, start + COLLAPSE_MAX_PTES_LIMIT) {
if (xas_retry(&xas, folio))
continue;
--
2.53.0On 2/12/26 03:18, Nico Pache wrote:
> The value (HPAGE_PMD_NR - 1) is used often in the khugepaged code to
> signify the limit of the max_ptes_* values. Add a define for this to
> increase code readability and reuse.
>
> Signed-off-by: Nico Pache <npache@redhat.com>
> ---
> mm/khugepaged.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index c362b3b2e08a..3dcce6018f20 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -85,6 +85,7 @@ static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);
> *
> * Note that these are only respected if collapse was initiated by khugepaged.
> */
> +#define COLLAPSE_MAX_PTES_LIMIT (HPAGE_PMD_NR - 1)
> unsigned int khugepaged_max_ptes_none __read_mostly;
> static unsigned int khugepaged_max_ptes_swap __read_mostly;
> static unsigned int khugepaged_max_ptes_shared __read_mostly;
> @@ -252,7 +253,7 @@ static ssize_t max_ptes_none_store(struct kobject *kobj,
> unsigned long max_ptes_none;
>
> err = kstrtoul(buf, 10, &max_ptes_none);
> - if (err || max_ptes_none > HPAGE_PMD_NR - 1)
> + if (err || max_ptes_none > COLLAPSE_MAX_PTES_LIMIT)
> return -EINVAL;
>
> khugepaged_max_ptes_none = max_ptes_none;
> @@ -277,7 +278,7 @@ static ssize_t max_ptes_swap_store(struct kobject *kobj,
> unsigned long max_ptes_swap;
>
> err = kstrtoul(buf, 10, &max_ptes_swap);
> - if (err || max_ptes_swap > HPAGE_PMD_NR - 1)
> + if (err || max_ptes_swap > COLLAPSE_MAX_PTES_LIMIT)
> return -EINVAL;
>
> khugepaged_max_ptes_swap = max_ptes_swap;
> @@ -303,7 +304,7 @@ static ssize_t max_ptes_shared_store(struct kobject *kobj,
> unsigned long max_ptes_shared;
>
> err = kstrtoul(buf, 10, &max_ptes_shared);
> - if (err || max_ptes_shared > HPAGE_PMD_NR - 1)
> + if (err || max_ptes_shared > COLLAPSE_MAX_PTES_LIMIT)
> return -EINVAL;
>
> khugepaged_max_ptes_shared = max_ptes_shared;
> @@ -384,7 +385,7 @@ int __init khugepaged_init(void)
> return -ENOMEM;
>
> khugepaged_pages_to_scan = HPAGE_PMD_NR * 8;
> - khugepaged_max_ptes_none = HPAGE_PMD_NR - 1;
> + khugepaged_max_ptes_none = COLLAPSE_MAX_PTES_LIMIT;
> khugepaged_max_ptes_swap = HPAGE_PMD_NR / 8;
> khugepaged_max_ptes_shared = HPAGE_PMD_NR / 2;
Changing these is ok. I don't like the name either.
It should be clear that they all belong to the "khugepaged_max_ptes" *
values. (see below)
>
> @@ -1869,7 +1870,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> bool is_shmem = shmem_file(file);
>
> VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
> - VM_BUG_ON(start & (HPAGE_PMD_NR - 1));
> + VM_BUG_ON(start & (COLLAPSE_MAX_PTES_LIMIT));
>
> result = alloc_charge_folio(&new_folio, mm, cc);
> if (result != SCAN_SUCCEED)
> @@ -2209,7 +2210,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> * unwritten page.
> */
> folio_mark_uptodate(new_folio);
> - folio_ref_add(new_folio, HPAGE_PMD_NR - 1);
> + folio_ref_add(new_folio, COLLAPSE_MAX_PTES_LIMIT);
>
> if (is_shmem)
> folio_mark_dirty(new_folio);
> @@ -2303,7 +2304,7 @@ static enum scan_result hpage_collapse_scan_file(struct mm_struct *mm, unsigned
> memset(cc->node_load, 0, sizeof(cc->node_load));
> nodes_clear(cc->alloc_nmask);
> rcu_read_lock();
> - xas_for_each(&xas, folio, start + HPAGE_PMD_NR - 1) {
> + xas_for_each(&xas, folio, start + COLLAPSE_MAX_PTES_LIMIT) {
> if (xas_retry(&xas, folio))
> continue;
>
Changing these is not. They are semantically something different.
E.g., folio_ref_add() adds "HPAGE_PMD_NR - 1" because we already
obtained a reference, and we need one per page -- HPAGE_PMD_NR.
So it's something different than when messing with the magical
khugepaged_max_ptes_none values.
--
Cheers,
David
On Thu, Feb 12, 2026 at 12:51 PM David Hildenbrand (Arm)
<david@kernel.org> wrote:
>
> On 2/12/26 03:18, Nico Pache wrote:
> > The value (HPAGE_PMD_NR - 1) is used often in the khugepaged code to
> > signify the limit of the max_ptes_* values. Add a define for this to
> > increase code readability and reuse.
> >
> > Signed-off-by: Nico Pache <npache@redhat.com>
> > ---
> > mm/khugepaged.c | 15 ++++++++-------
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index c362b3b2e08a..3dcce6018f20 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -85,6 +85,7 @@ static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);
> > *
> > * Note that these are only respected if collapse was initiated by khugepaged.
> > */
> > +#define COLLAPSE_MAX_PTES_LIMIT (HPAGE_PMD_NR - 1)
> > unsigned int khugepaged_max_ptes_none __read_mostly;
> > static unsigned int khugepaged_max_ptes_swap __read_mostly;
> > static unsigned int khugepaged_max_ptes_shared __read_mostly;
> > @@ -252,7 +253,7 @@ static ssize_t max_ptes_none_store(struct kobject *kobj,
> > unsigned long max_ptes_none;
> >
> > err = kstrtoul(buf, 10, &max_ptes_none);
> > - if (err || max_ptes_none > HPAGE_PMD_NR - 1)
> > + if (err || max_ptes_none > COLLAPSE_MAX_PTES_LIMIT)
> > return -EINVAL;
> >
> > khugepaged_max_ptes_none = max_ptes_none;
> > @@ -277,7 +278,7 @@ static ssize_t max_ptes_swap_store(struct kobject *kobj,
> > unsigned long max_ptes_swap;
> >
> > err = kstrtoul(buf, 10, &max_ptes_swap);
> > - if (err || max_ptes_swap > HPAGE_PMD_NR - 1)
> > + if (err || max_ptes_swap > COLLAPSE_MAX_PTES_LIMIT)
> > return -EINVAL;
> >
> > khugepaged_max_ptes_swap = max_ptes_swap;
> > @@ -303,7 +304,7 @@ static ssize_t max_ptes_shared_store(struct kobject *kobj,
> > unsigned long max_ptes_shared;
> >
> > err = kstrtoul(buf, 10, &max_ptes_shared);
> > - if (err || max_ptes_shared > HPAGE_PMD_NR - 1)
> > + if (err || max_ptes_shared > COLLAPSE_MAX_PTES_LIMIT)
> > return -EINVAL;
> >
> > khugepaged_max_ptes_shared = max_ptes_shared;
> > @@ -384,7 +385,7 @@ int __init khugepaged_init(void)
> > return -ENOMEM;
> >
> > khugepaged_pages_to_scan = HPAGE_PMD_NR * 8;
> > - khugepaged_max_ptes_none = HPAGE_PMD_NR - 1;
> > + khugepaged_max_ptes_none = COLLAPSE_MAX_PTES_LIMIT;
> > khugepaged_max_ptes_swap = HPAGE_PMD_NR / 8;
> > khugepaged_max_ptes_shared = HPAGE_PMD_NR / 2;
>
>
> Changing these is ok. I don't like the name either.
Yeah I prefer COLLAPSE_MAX_PTES as others suggested, its shorter and
more general.
>
> It should be clear that they all belong to the "khugepaged_max_ptes" *
> values. (see below)
>
> >
> > @@ -1869,7 +1870,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> > bool is_shmem = shmem_file(file);
> >
> > VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
> > - VM_BUG_ON(start & (HPAGE_PMD_NR - 1));
> > + VM_BUG_ON(start & (COLLAPSE_MAX_PTES_LIMIT));
> >
> > result = alloc_charge_folio(&new_folio, mm, cc);
> > if (result != SCAN_SUCCEED)
> > @@ -2209,7 +2210,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> > * unwritten page.
> > */
> > folio_mark_uptodate(new_folio);
> > - folio_ref_add(new_folio, HPAGE_PMD_NR - 1);
> > + folio_ref_add(new_folio, COLLAPSE_MAX_PTES_LIMIT);
> >
> > if (is_shmem)
> > folio_mark_dirty(new_folio);
> > @@ -2303,7 +2304,7 @@ static enum scan_result hpage_collapse_scan_file(struct mm_struct *mm, unsigned
> > memset(cc->node_load, 0, sizeof(cc->node_load));
> > nodes_clear(cc->alloc_nmask);
> > rcu_read_lock();
> > - xas_for_each(&xas, folio, start + HPAGE_PMD_NR - 1) {
> > + xas_for_each(&xas, folio, start + COLLAPSE_MAX_PTES_LIMIT) {
> > if (xas_retry(&xas, folio))
> > continue;
> >
>
> Changing these is not. They are semantically something different.
With the new name these are ok right? given we dropped the LIMIT
aspect. The only thing that comes to mind is that COLLAPSE_MAX_PTES
would seemingly refer to 512 not 511, but I'm ok with that if no one
objects.
-- Nico
>
> E.g., folio_ref_add() adds "HPAGE_PMD_NR - 1" because we already
> obtained a reference, and we need one per page -- HPAGE_PMD_NR.
>
> So it's something different than when messing with the magical
> khugepaged_max_ptes_none values.
>
> --
> Cheers,
>
> David
>
On 2/25/26 01:35, Nico Pache wrote:
> On Thu, Feb 12, 2026 at 12:51 PM David Hildenbrand (Arm)
> <david@kernel.org> wrote:
>>
>> On 2/12/26 03:18, Nico Pache wrote:
>>> The value (HPAGE_PMD_NR - 1) is used often in the khugepaged code to
>>> signify the limit of the max_ptes_* values. Add a define for this to
>>> increase code readability and reuse.
>>>
>>> Signed-off-by: Nico Pache <npache@redhat.com>
>>> ---
>>> mm/khugepaged.c | 15 ++++++++-------
>>> 1 file changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index c362b3b2e08a..3dcce6018f20 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -85,6 +85,7 @@ static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);
>>> *
>>> * Note that these are only respected if collapse was initiated by khugepaged.
>>> */
>>> +#define COLLAPSE_MAX_PTES_LIMIT (HPAGE_PMD_NR - 1)
>>> unsigned int khugepaged_max_ptes_none __read_mostly;
>>> static unsigned int khugepaged_max_ptes_swap __read_mostly;
>>> static unsigned int khugepaged_max_ptes_shared __read_mostly;
>>> @@ -252,7 +253,7 @@ static ssize_t max_ptes_none_store(struct kobject *kobj,
>>> unsigned long max_ptes_none;
>>>
>>> err = kstrtoul(buf, 10, &max_ptes_none);
>>> - if (err || max_ptes_none > HPAGE_PMD_NR - 1)
>>> + if (err || max_ptes_none > COLLAPSE_MAX_PTES_LIMIT)
>>> return -EINVAL;
>>>
>>> khugepaged_max_ptes_none = max_ptes_none;
>>> @@ -277,7 +278,7 @@ static ssize_t max_ptes_swap_store(struct kobject *kobj,
>>> unsigned long max_ptes_swap;
>>>
>>> err = kstrtoul(buf, 10, &max_ptes_swap);
>>> - if (err || max_ptes_swap > HPAGE_PMD_NR - 1)
>>> + if (err || max_ptes_swap > COLLAPSE_MAX_PTES_LIMIT)
>>> return -EINVAL;
>>>
>>> khugepaged_max_ptes_swap = max_ptes_swap;
>>> @@ -303,7 +304,7 @@ static ssize_t max_ptes_shared_store(struct kobject *kobj,
>>> unsigned long max_ptes_shared;
>>>
>>> err = kstrtoul(buf, 10, &max_ptes_shared);
>>> - if (err || max_ptes_shared > HPAGE_PMD_NR - 1)
>>> + if (err || max_ptes_shared > COLLAPSE_MAX_PTES_LIMIT)
>>> return -EINVAL;
>>>
>>> khugepaged_max_ptes_shared = max_ptes_shared;
>>> @@ -384,7 +385,7 @@ int __init khugepaged_init(void)
>>> return -ENOMEM;
>>>
>>> khugepaged_pages_to_scan = HPAGE_PMD_NR * 8;
>>> - khugepaged_max_ptes_none = HPAGE_PMD_NR - 1;
>>> + khugepaged_max_ptes_none = COLLAPSE_MAX_PTES_LIMIT;
>>> khugepaged_max_ptes_swap = HPAGE_PMD_NR / 8;
>>> khugepaged_max_ptes_shared = HPAGE_PMD_NR / 2;
>>
>>
>> Changing these is ok. I don't like the name either.
>
> Yeah I prefer COLLAPSE_MAX_PTES as others suggested, its shorter and
> more general.
>
COLLAPSE_MAX_PTES is misleading. We will happily collapse for 512 PTEs.
It's all about khugepaged_max_ptes_* semantics.
>>
>> It should be clear that they all belong to the "khugepaged_max_ptes" *
>> values. (see below)
>>
>>>
>>> @@ -1869,7 +1870,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>>> bool is_shmem = shmem_file(file);
>>>
>>> VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
>>> - VM_BUG_ON(start & (HPAGE_PMD_NR - 1));
>>> + VM_BUG_ON(start & (COLLAPSE_MAX_PTES_LIMIT));
>>>
>>> result = alloc_charge_folio(&new_folio, mm, cc);
>>> if (result != SCAN_SUCCEED)
>>> @@ -2209,7 +2210,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>>> * unwritten page.
>>> */
>>> folio_mark_uptodate(new_folio);
>>> - folio_ref_add(new_folio, HPAGE_PMD_NR - 1);
>>> + folio_ref_add(new_folio, COLLAPSE_MAX_PTES_LIMIT);
>>>
>>> if (is_shmem)
>>> folio_mark_dirty(new_folio);
>>> @@ -2303,7 +2304,7 @@ static enum scan_result hpage_collapse_scan_file(struct mm_struct *mm, unsigned
>>> memset(cc->node_load, 0, sizeof(cc->node_load));
>>> nodes_clear(cc->alloc_nmask);
>>> rcu_read_lock();
>>> - xas_for_each(&xas, folio, start + HPAGE_PMD_NR - 1) {
>>> + xas_for_each(&xas, folio, start + COLLAPSE_MAX_PTES_LIMIT) {
>>> if (xas_retry(&xas, folio))
>>> continue;
>>>
>>
>> Changing these is not. They are semantically something different.
>
> With the new name these are ok right? given we dropped the LIMIT
> aspect. The only thing that comes to mind is that COLLAPSE_MAX_PTES
> would seemingly refer to 512 not 511, but I'm ok with that if no one
> objects.
Just leave it at HPAGE_PMD_NR - 1, any magical constant will make this
code only more confusing.
--
Cheers,
David
On 11 Feb 2026, at 21:18, Nico Pache wrote: > The value (HPAGE_PMD_NR - 1) is used often in the khugepaged code to > signify the limit of the max_ptes_* values. Add a define for this to > increase code readability and reuse. > > Signed-off-by: Nico Pache <npache@redhat.com> > --- > mm/khugepaged.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com> Best Regards, Yan, Zi
On Wed, Feb 11, 2026 at 07:18:33PM -0700, Nico Pache wrote: > The value (HPAGE_PMD_NR - 1) is used often in the khugepaged code to > signify the limit of the max_ptes_* values. Add a define for this to > increase code readability and reuse. > > Signed-off-by: Nico Pache <npache@redhat.com> Acked-by: Pedro Falcato <pfalcato@suse.de> > --- > mm/khugepaged.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index c362b3b2e08a..3dcce6018f20 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -85,6 +85,7 @@ static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait); > * > * Note that these are only respected if collapse was initiated by khugepaged. > */ > +#define COLLAPSE_MAX_PTES_LIMIT (HPAGE_PMD_NR - 1) > unsigned int khugepaged_max_ptes_none __read_mostly; > static unsigned int khugepaged_max_ptes_swap __read_mostly; > static unsigned int khugepaged_max_ptes_shared __read_mostly; > @@ -252,7 +253,7 @@ static ssize_t max_ptes_none_store(struct kobject *kobj, > unsigned long max_ptes_none; > > err = kstrtoul(buf, 10, &max_ptes_none); > - if (err || max_ptes_none > HPAGE_PMD_NR - 1) > + if (err || max_ptes_none > COLLAPSE_MAX_PTES_LIMIT) > return -EINVAL; > > khugepaged_max_ptes_none = max_ptes_none; > @@ -277,7 +278,7 @@ static ssize_t max_ptes_swap_store(struct kobject *kobj, > unsigned long max_ptes_swap; > > err = kstrtoul(buf, 10, &max_ptes_swap); > - if (err || max_ptes_swap > HPAGE_PMD_NR - 1) > + if (err || max_ptes_swap > COLLAPSE_MAX_PTES_LIMIT) > return -EINVAL; > > khugepaged_max_ptes_swap = max_ptes_swap; > @@ -303,7 +304,7 @@ static ssize_t max_ptes_shared_store(struct kobject *kobj, > unsigned long max_ptes_shared; > > err = kstrtoul(buf, 10, &max_ptes_shared); > - if (err || max_ptes_shared > HPAGE_PMD_NR - 1) > + if (err || max_ptes_shared > COLLAPSE_MAX_PTES_LIMIT) > return -EINVAL; > > khugepaged_max_ptes_shared = max_ptes_shared; > @@ -384,7 +385,7 @@ int __init khugepaged_init(void) > return -ENOMEM; > > khugepaged_pages_to_scan = HPAGE_PMD_NR * 8; > - khugepaged_max_ptes_none = HPAGE_PMD_NR - 1; > + khugepaged_max_ptes_none = COLLAPSE_MAX_PTES_LIMIT; > khugepaged_max_ptes_swap = HPAGE_PMD_NR / 8; > khugepaged_max_ptes_shared = HPAGE_PMD_NR / 2; > > @@ -1869,7 +1870,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr, > bool is_shmem = shmem_file(file); > > VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem); > - VM_BUG_ON(start & (HPAGE_PMD_NR - 1)); > + VM_BUG_ON(start & (COLLAPSE_MAX_PTES_LIMIT)); nit: no need for the () around COLLAPSE_MAX_PTES_LIMIT here -- Pedro
On Wed, Feb 11, 2026 at 07:18:33PM -0700, Nico Pache wrote:
> The value (HPAGE_PMD_NR - 1) is used often in the khugepaged code to
> signify the limit of the max_ptes_* values. Add a define for this to
> increase code readability and reuse.
>
> Signed-off-by: Nico Pache <npache@redhat.com>
> ---
> mm/khugepaged.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index c362b3b2e08a..3dcce6018f20 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -85,6 +85,7 @@ static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);
> *
> * Note that these are only respected if collapse was initiated by khugepaged.
> */
> +#define COLLAPSE_MAX_PTES_LIMIT (HPAGE_PMD_NR - 1)
> unsigned int khugepaged_max_ptes_none __read_mostly;
> static unsigned int khugepaged_max_ptes_swap __read_mostly;
> static unsigned int khugepaged_max_ptes_shared __read_mostly;
> @@ -252,7 +253,7 @@ static ssize_t max_ptes_none_store(struct kobject *kobj,
> unsigned long max_ptes_none;
>
> err = kstrtoul(buf, 10, &max_ptes_none);
> - if (err || max_ptes_none > HPAGE_PMD_NR - 1)
> + if (err || max_ptes_none > COLLAPSE_MAX_PTES_LIMIT)
> return -EINVAL;
>
> khugepaged_max_ptes_none = max_ptes_none;
> @@ -277,7 +278,7 @@ static ssize_t max_ptes_swap_store(struct kobject *kobj,
> unsigned long max_ptes_swap;
>
> err = kstrtoul(buf, 10, &max_ptes_swap);
> - if (err || max_ptes_swap > HPAGE_PMD_NR - 1)
> + if (err || max_ptes_swap > COLLAPSE_MAX_PTES_LIMIT)
> return -EINVAL;
>
> khugepaged_max_ptes_swap = max_ptes_swap;
> @@ -303,7 +304,7 @@ static ssize_t max_ptes_shared_store(struct kobject *kobj,
> unsigned long max_ptes_shared;
>
> err = kstrtoul(buf, 10, &max_ptes_shared);
> - if (err || max_ptes_shared > HPAGE_PMD_NR - 1)
> + if (err || max_ptes_shared > COLLAPSE_MAX_PTES_LIMIT)
> return -EINVAL;
>
> khugepaged_max_ptes_shared = max_ptes_shared;
> @@ -384,7 +385,7 @@ int __init khugepaged_init(void)
> return -ENOMEM;
>
> khugepaged_pages_to_scan = HPAGE_PMD_NR * 8;
> - khugepaged_max_ptes_none = HPAGE_PMD_NR - 1;
> + khugepaged_max_ptes_none = COLLAPSE_MAX_PTES_LIMIT;
> khugepaged_max_ptes_swap = HPAGE_PMD_NR / 8;
> khugepaged_max_ptes_shared = HPAGE_PMD_NR / 2;
>
> @@ -1869,7 +1870,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> bool is_shmem = shmem_file(file);
>
> VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
> - VM_BUG_ON(start & (HPAGE_PMD_NR - 1));
> + VM_BUG_ON(start & (COLLAPSE_MAX_PTES_LIMIT));
>
> result = alloc_charge_folio(&new_folio, mm, cc);
> if (result != SCAN_SUCCEED)
> @@ -2209,7 +2210,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> * unwritten page.
> */
> folio_mark_uptodate(new_folio);
> - folio_ref_add(new_folio, HPAGE_PMD_NR - 1);
> + folio_ref_add(new_folio, COLLAPSE_MAX_PTES_LIMIT);
Hi Nico,
Thank you for cleanup.
The readability is better in other places, but here it confuses me.
Why is it LIMIT? I had to look at the implementation code.
Would COLLAPSE_MAX_PTES be better?
> if (is_shmem)
> folio_mark_dirty(new_folio);
> @@ -2303,7 +2304,7 @@ static enum scan_result hpage_collapse_scan_file(struct mm_struct *mm, unsigned
> memset(cc->node_load, 0, sizeof(cc->node_load));
> nodes_clear(cc->alloc_nmask);
> rcu_read_lock();
> - xas_for_each(&xas, folio, start + HPAGE_PMD_NR - 1) {
> + xas_for_each(&xas, folio, start + COLLAPSE_MAX_PTES_LIMIT) {
> if (xas_retry(&xas, folio))
> continue;
>
> --
> 2.53.0
>
>
--
Cheers,
Vernon
© 2016 - 2026 Red Hat, Inc.