[PATCH v2 0/2] docs/ksm: fix advisor documentation and comment

[PATCH v2 0/2] docs/ksm: fix advisor documentation and comment

[Jinjiang Tu]

Fix typos and misleading description of advisor_min_pages_to_scan.

Changes since v1:
 * remove fix tags for both patches
 * update commit message and comment for patch2
 * collect Acked-by for patch1

Jinjiang Tu (2):
  docs: ksm: fix typos in sysfs knob names
  mm/ksm: fix advisor_min_pages_to_scan description

 Documentation/admin-guide/mm/ksm.rst | 8 ++++----
 mm/ksm.c                             | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] docs: ksm: fix typos in sysfs knob names

[Jinjiang Tu]

The sysfs knob names in mm/ksm.c are "advisor_max_cpu" and
"advisor_max_pages_to_scan", but the ksm.rst documentation spelled
both as "adivsor_*", fix the two typos.

Acked-by: David Hildenbrand (Arm) <david@kernel.org>
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
---
 Documentation/admin-guide/mm/ksm.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/mm/ksm.rst b/Documentation/admin-guide/mm/ksm.rst
index ad8e7a41f3b5..c9f533b10f6f 100644
--- a/Documentation/admin-guide/mm/ksm.rst
+++ b/Documentation/admin-guide/mm/ksm.rst
@@ -174,7 +174,7 @@ advisor_mode
         The section about ``advisor`` explains in detail how the scan time
         advisor works.
 
-adivsor_max_cpu
+advisor_max_cpu
         specifies the upper limit of the cpu percent usage of the ksmd
         background thread. The default is 70.
 
@@ -186,7 +186,7 @@ advisor_min_pages_to_scan
         specifies the lower limit of the ``pages_to_scan`` parameter of the
         scan time advisor. The default is 500.
 
-adivsor_max_pages_to_scan
+advisor_max_pages_to_scan
         specifies the upper limit of the ``pages_to_scan`` parameter of the
         scan time advisor. The default is 30000.
 
-- 
2.43.0

Re: [PATCH v2 1/2] docs: ksm: fix typos in sysfs knob names

[Randy Dunlap]

On 8/12/26 8:17 PM, Jinjiang Tu wrote:
> The sysfs knob names in mm/ksm.c are "advisor_max_cpu" and
> "advisor_max_pages_to_scan", but the ksm.rst documentation spelled
> both as "adivsor_*", fix the two typos.
> 
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>

Acked-by: Randy Dunlap <rdunlap@infradead.org>
Thanks.

> ---
>  Documentation/admin-guide/mm/ksm.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/mm/ksm.rst b/Documentation/admin-guide/mm/ksm.rst
> index ad8e7a41f3b5..c9f533b10f6f 100644
> --- a/Documentation/admin-guide/mm/ksm.rst
> +++ b/Documentation/admin-guide/mm/ksm.rst
> @@ -174,7 +174,7 @@ advisor_mode
>          The section about ``advisor`` explains in detail how the scan time
>          advisor works.
>  
> -adivsor_max_cpu
> +advisor_max_cpu
>          specifies the upper limit of the cpu percent usage of the ksmd
>          background thread. The default is 70.
>  
> @@ -186,7 +186,7 @@ advisor_min_pages_to_scan
>          specifies the lower limit of the ``pages_to_scan`` parameter of the
>          scan time advisor. The default is 500.
>  
> -adivsor_max_pages_to_scan
> +advisor_max_pages_to_scan
>          specifies the upper limit of the ``pages_to_scan`` parameter of the
>          scan time advisor. The default is 30000.
>  

-- 
~Randy

[PATCH v2 2/2] mm/ksm: fix advisor_min_pages_to_scan description

[Jinjiang Tu]

Both Documentation/admin-guide/mm/ksm.rst and the comment next to the
variable definition in mm/ksm.c describe advisor_min_pages_to_scan as a
lower limit of the pages_to_scan parameter, but that is not how the
scan-time advisor actually uses it. commit 4e5fa4f5eff6 ("mm/ksm: add ksm
advisor") only uses it to initialize ksm_thread_pages_to_scan when the
scan-time advisor is enabled. This will mislead the users.

The semantics of advisor_min_pages_to_scan was updated in the v2 patchset
[1], but the documentation wasn't updated.

Update the documentation and comment to match the semantics of
advisor_min_pages_to_scan.

Link: https://lore.kernel.org/linux-mm/20231028000945.2428830-2-shr@devkernel.io/ [1]
Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
---
 Documentation/admin-guide/mm/ksm.rst | 4 ++--
 mm/ksm.c                             | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/mm/ksm.rst b/Documentation/admin-guide/mm/ksm.rst
index c9f533b10f6f..c329ca747b8c 100644
--- a/Documentation/admin-guide/mm/ksm.rst
+++ b/Documentation/admin-guide/mm/ksm.rst
@@ -183,8 +183,8 @@ advisor_target_scan_time
         pages. The default value is 200 seconds.
 
 advisor_min_pages_to_scan
-        specifies the lower limit of the ``pages_to_scan`` parameter of the
-        scan time advisor. The default is 500.
+        specifies the initial value of the ``pages_to_scan`` parameter of
+        the scan time advisor. The default is 500.
 
 advisor_max_pages_to_scan
         specifies the upper limit of the ``pages_to_scan`` parameter of the
diff --git a/mm/ksm.c b/mm/ksm.c
index 7d5b76478f0b..4a6cf8cf5d60 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -342,7 +342,7 @@ static enum ksm_advisor_type ksm_advisor;
  * Only called through the sysfs control interface:
  */
 
-/* At least scan this many pages per batch. */
+/* Initial number of pages to scan per batch. */
 static unsigned long ksm_advisor_min_pages_to_scan = 500;
 
 static void set_advisor_defaults(void)
-- 
2.43.0

Re: [PATCH v2 2/2] mm/ksm: fix advisor_min_pages_to_scan description

[Lorenzo Stoakes (ARM)]

On Thu, Aug 13, 2026 at 11:17:22AM +0800, Jinjiang Tu wrote:
> Both Documentation/admin-guide/mm/ksm.rst and the comment next to the
> variable definition in mm/ksm.c describe advisor_min_pages_to_scan as a
> lower limit of the pages_to_scan parameter, but that is not how the
> scan-time advisor actually uses it. commit 4e5fa4f5eff6 ("mm/ksm: add ksm
> advisor") only uses it to initialize ksm_thread_pages_to_scan when the
> scan-time advisor is enabled. This will mislead the users.
>
> The semantics of advisor_min_pages_to_scan was updated in the v2 patchset
> [1], but the documentation wasn't updated.
>
> Update the documentation and comment to match the semantics of
> advisor_min_pages_to_scan.

Umm, firstly if this were wrong you'd need to change the name of the variable
too instad of documenting it as something completely distinct.

But secondly AFAICT the logic in scan_time_advisor() suggests that it truly does
act as a minimum bound as well as being the initial value for
ksm_thread_pages_to_scan.

So this patch just looks wrong to me.

But in any case, you are making a claim here yet have provided no evidence for
it. 'It is just the initial assignment' means nothing - if the logic can then
only increase it up to the maximum value then it still acts as a minimum.

A brief look at the logic suggests so.

But in any case - the onus is on _you_ to prove that that's not happening.

And even if you could do that, since it is _intended to be a lower bound_, the
fix wouldn't be alterting comments or documentation, it would be to re-establish
the minimum as a minimum.

>
> Link: https://lore.kernel.org/linux-mm/20231028000945.2428830-2-shr@devkernel.io/ [1]

This patch repeatedly states that it is the minimum time. You have failed to
provide any analysis to suggest otherwise.

> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>

Assisted-by: ?

> ---
>  Documentation/admin-guide/mm/ksm.rst | 4 ++--
>  mm/ksm.c                             | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/admin-guide/mm/ksm.rst b/Documentation/admin-guide/mm/ksm.rst
> index c9f533b10f6f..c329ca747b8c 100644
> --- a/Documentation/admin-guide/mm/ksm.rst
> +++ b/Documentation/admin-guide/mm/ksm.rst
> @@ -183,8 +183,8 @@ advisor_target_scan_time
>          pages. The default value is 200 seconds.
>
>  advisor_min_pages_to_scan
> -        specifies the lower limit of the ``pages_to_scan`` parameter of the
> -        scan time advisor. The default is 500.
> +        specifies the initial value of the ``pages_to_scan`` parameter of
> +        the scan time advisor. The default is 500.
>
>  advisor_max_pages_to_scan
>          specifies the upper limit of the ``pages_to_scan`` parameter of the
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 7d5b76478f0b..4a6cf8cf5d60 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -342,7 +342,7 @@ static enum ksm_advisor_type ksm_advisor;
>   * Only called through the sysfs control interface:
>   */
>
> -/* At least scan this many pages per batch. */
> +/* Initial number of pages to scan per batch. */
>  static unsigned long ksm_advisor_min_pages_to_scan = 500;
>
>  static void set_advisor_defaults(void)
> --
> 2.43.0
>

--
Cheers, Lorenzo

Re: [PATCH v2 2/2] mm/ksm: fix advisor_min_pages_to_scan description

[Jinjiang Tu]

在 2026/8/13 18:56, Lorenzo Stoakes (ARM) 写道:
> On Thu, Aug 13, 2026 at 11:17:22AM +0800, Jinjiang Tu wrote:
>> Both Documentation/admin-guide/mm/ksm.rst and the comment next to the
>> variable definition in mm/ksm.c describe advisor_min_pages_to_scan as a
>> lower limit of the pages_to_scan parameter, but that is not how the
>> scan-time advisor actually uses it. commit 4e5fa4f5eff6 ("mm/ksm: add ksm
>> advisor") only uses it to initialize ksm_thread_pages_to_scan when the
>> scan-time advisor is enabled. This will mislead the users.
>>
>> The semantics of advisor_min_pages_to_scan was updated in the v2 patchset
>> [1], but the documentation wasn't updated.
>>
>> Update the documentation and comment to match the semantics of
>> advisor_min_pages_to_scan.
> Umm, firstly if this were wrong you'd need to change the name of the variable
> too instad of documenting it as something completely distinct.
>
> But secondly AFAICT the logic in scan_time_advisor() suggests that it truly does
> act as a minimum bound as well as being the initial value for
> ksm_thread_pages_to_scan.

As the cover letter[1] said, 'The initial value and the max value for the pages_to_scan parameter can
be limited with:', ksm_advisor_min_pages is used as the initial value, not the min value of pages_to_scan.

The v1 patchset[2] uses ksm_advisor_min_pages as the min value of pages_to_scan, but as David suggested, the
author changed the semantics of advisor_min_pages_to_scan in v2, but forgot to update the comments and variable names.

[2] https://lore.kernel.org/all/20231004190249.829015-1-shr@devkernel.io/

>
> So this patch just looks wrong to me.
>
> But in any case, you are making a claim here yet have provided no evidence for
> it. 'It is just the initial assignment' means nothing - if the logic can then
> only increase it up to the maximum value then it still acts as a minimum.
>
> A brief look at the logic suggests so.
>
> But in any case - the onus is on _you_ to prove that that's not happening.
>
> And even if you could do that, since it is _intended to be a lower bound_, the
> fix wouldn't be alterting comments or documentation, it would be to re-establish
> the minimum as a minimum.
>
>> Link: https://lore.kernel.org/linux-mm/20231028000945.2428830-2-shr@devkernel.io/ [1]
> This patch repeatedly states that it is the minimum time. You have failed to
> provide any analysis to suggest otherwise.
>
>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> Assisted-by: ?
>
>> ---
>>   Documentation/admin-guide/mm/ksm.rst | 4 ++--
>>   mm/ksm.c                             | 2 +-
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/mm/ksm.rst b/Documentation/admin-guide/mm/ksm.rst
>> index c9f533b10f6f..c329ca747b8c 100644
>> --- a/Documentation/admin-guide/mm/ksm.rst
>> +++ b/Documentation/admin-guide/mm/ksm.rst
>> @@ -183,8 +183,8 @@ advisor_target_scan_time
>>           pages. The default value is 200 seconds.
>>
>>   advisor_min_pages_to_scan
>> -        specifies the lower limit of the ``pages_to_scan`` parameter of the
>> -        scan time advisor. The default is 500.
>> +        specifies the initial value of the ``pages_to_scan`` parameter of
>> +        the scan time advisor. The default is 500.
>>
>>   advisor_max_pages_to_scan
>>           specifies the upper limit of the ``pages_to_scan`` parameter of the
>> diff --git a/mm/ksm.c b/mm/ksm.c
>> index 7d5b76478f0b..4a6cf8cf5d60 100644
>> --- a/mm/ksm.c
>> +++ b/mm/ksm.c
>> @@ -342,7 +342,7 @@ static enum ksm_advisor_type ksm_advisor;
>>    * Only called through the sysfs control interface:
>>    */
>>
>> -/* At least scan this many pages per batch. */
>> +/* Initial number of pages to scan per batch. */
>>   static unsigned long ksm_advisor_min_pages_to_scan = 500;
>>
>>   static void set_advisor_defaults(void)
>> --
>> 2.43.0
>>
> --
> Cheers, Lorenzo

Re: [PATCH v2 2/2] mm/ksm: fix advisor_min_pages_to_scan description

[Lorenzo Stoakes (ARM)]

Please show me the common courtesy of responding to _all_ the review given,
not only the one point you want to respond to.

I'm not happy with this patch, sorry, for the reasons stated that you chose
to ignore.

On Thu, Aug 13, 2026 at 07:17:05PM +0800, Jinjiang Tu wrote:
>
> 在 2026/8/13 18:56, Lorenzo Stoakes (ARM) 写道:
> > On Thu, Aug 13, 2026 at 11:17:22AM +0800, Jinjiang Tu wrote:
> > > Both Documentation/admin-guide/mm/ksm.rst and the comment next to the
> > > variable definition in mm/ksm.c describe advisor_min_pages_to_scan as a
> > > lower limit of the pages_to_scan parameter, but that is not how the
> > > scan-time advisor actually uses it. commit 4e5fa4f5eff6 ("mm/ksm: add ksm
> > > advisor") only uses it to initialize ksm_thread_pages_to_scan when the
> > > scan-time advisor is enabled. This will mislead the users.
> > >
> > > The semantics of advisor_min_pages_to_scan was updated in the v2 patchset
> > > [1], but the documentation wasn't updated.
> > >
> > > Update the documentation and comment to match the semantics of
> > > advisor_min_pages_to_scan.
> > Umm, firstly if this were wrong you'd need to change the name of the variable
> > too instad of documenting it as something completely distinct.
> >
> > But secondly AFAICT the logic in scan_time_advisor() suggests that it truly does
> > act as a minimum bound as well as being the initial value for
> > ksm_thread_pages_to_scan.
>
> As the cover letter[1] said, 'The initial value and the max value for the pages_to_scan parameter can
> be limited with:', ksm_advisor_min_pages is used as the initial value, not the min value of pages_to_scan.

(Wrap your lines properly please, it's a common courtesy to learn how to
send mail according to community conventions.)

One of the part of my review you ignored:

	But in any case, you are making a claim here yet have provided no
	evidence for it. 'It is just the initial assignment' means nothing
	- if the logic can then only increase it up to the maximum value
	then it still acts as a minimum.

>
> The v1 patchset[2] uses ksm_advisor_min_pages as the min value of pages_to_scan, but as David suggested, the
> author changed the semantics of advisor_min_pages_to_scan in v2, but forgot to update the comments and variable names.
>
> [2] https://lore.kernel.org/all/20231004190249.829015-1-shr@devkernel.io/
>
> >
> > So this patch just looks wrong to me.
> >
> > But in any case, you are making a claim here yet have provided no evidence for
> > it. 'It is just the initial assignment' means nothing - if the logic can then
> > only increase it up to the maximum value then it still acts as a minimum.
> >
> > A brief look at the logic suggests so.
> >
> > But in any case - the onus is on _you_ to prove that that's not happening.
> >
> > And even if you could do that, since it is _intended to be a lower bound_, the
> > fix wouldn't be alterting comments or documentation, it would be to re-establish
> > the minimum as a minimum.
> >
> > > Link: https://lore.kernel.org/linux-mm/20231028000945.2428830-2-shr@devkernel.io/ [1]
> > This patch repeatedly states that it is the minimum time. You have failed to
> > provide any analysis to suggest otherwise.
> >
> > > Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> > Assisted-by: ?
> >
> > > ---
> > >   Documentation/admin-guide/mm/ksm.rst | 4 ++--
> > >   mm/ksm.c                             | 2 +-
> > >   2 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/admin-guide/mm/ksm.rst b/Documentation/admin-guide/mm/ksm.rst
> > > index c9f533b10f6f..c329ca747b8c 100644
> > > --- a/Documentation/admin-guide/mm/ksm.rst
> > > +++ b/Documentation/admin-guide/mm/ksm.rst
> > > @@ -183,8 +183,8 @@ advisor_target_scan_time
> > >           pages. The default value is 200 seconds.
> > >
> > >   advisor_min_pages_to_scan
> > > -        specifies the lower limit of the ``pages_to_scan`` parameter of the
> > > -        scan time advisor. The default is 500.
> > > +        specifies the initial value of the ``pages_to_scan`` parameter of
> > > +        the scan time advisor. The default is 500.
> > >
> > >   advisor_max_pages_to_scan
> > >           specifies the upper limit of the ``pages_to_scan`` parameter of the
> > > diff --git a/mm/ksm.c b/mm/ksm.c
> > > index 7d5b76478f0b..4a6cf8cf5d60 100644
> > > --- a/mm/ksm.c
> > > +++ b/mm/ksm.c
> > > @@ -342,7 +342,7 @@ static enum ksm_advisor_type ksm_advisor;
> > >    * Only called through the sysfs control interface:
> > >    */
> > >
> > > -/* At least scan this many pages per batch. */
> > > +/* Initial number of pages to scan per batch. */
> > >   static unsigned long ksm_advisor_min_pages_to_scan = 500;
> > >
> > >   static void set_advisor_defaults(void)
> > > --
> > > 2.43.0
> > >
> > --
> > Cheers, Lorenzo

--
Cheers, Lorenzo

Re: [PATCH v2 0/2] docs/ksm: fix advisor documentation and comment

[Lorenzo Stoakes (ARM)]

On Thu, Aug 13, 2026 at 11:17:20AM +0800, Jinjiang Tu wrote:
> Fix typos and misleading description of advisor_min_pages_to_scan.

This is a very bare cover letter...

>
> Changes since v1:
>  * remove fix tags for both patches
>  * update commit message and comment for patch2
>  * collect Acked-by for patch1
>
> Jinjiang Tu (2):
>   docs: ksm: fix typos in sysfs knob names
>   mm/ksm: fix advisor_min_pages_to_scan description
>
>  Documentation/admin-guide/mm/ksm.rst | 8 ++++----
>  mm/ksm.c                             | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> --
> 2.43.0
>

--
Cheers, Lorenzo