[Patch mm-stable-fixup 0/2] mm_slot: fix the usage of mm_slot_entry()

[Patch mm-stable-fixup 0/2] mm_slot: fix the usage of mm_slot_entry()

[Wei Yang]

This is the fixup based on mm-stable.

The base commit:

   mm: swap: check for stable address space before operating on the VMA

The reviewed version is at [1].

[1]: https://lkml.kernel.org/r/20250927004539.19308-1-richard.weiyang@gmail.com

Wei Yang (2):
  mm/ksm: don't call mm_slot_entry() when the slot is NULL
  mm/khugepaged: remove definition of struct khugepaged_mm_slot

 mm/khugepaged.c |  5 +----
 mm/ksm.c        | 27 ++++++++++++++-------------
 2 files changed, 15 insertions(+), 17 deletions(-)

-- 
2.34.1

[Patch mm-stable-fixup 1/2] mm/ksm: don't call mm_slot_entry() when the slot is NULL

[Wei Yang]

When using mm_slot in ksm, there is code like:

     slot = mm_slot_lookup(mm_slots_hash, mm);
     mm_slot = mm_slot_entry(slot, struct ksm_mm_slot, slot);
     if (mm_slot && ..) {
     }

The mm_slot_entry() won't return a valid value if slot is NULL generally.
But currently it works since slot is the first element of struct
ksm_mm_slot.

To reduce the ambiguity and make it robust, only call mm_slot_entry()
when we have a valid slot.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Lance Yang <lance.yang@linux.dev>
Cc: Kiryl Shutsemau <kirill@shutemov.name>
Cc: xu xin <xu.xin16@zte.com.cn>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Chengming Zhou <chengming.zhou@linux.dev>
Reviewed-by: Dev Jain <dev.jain@arm.com>
Acked-by: David Hildenbrand <david@redhat.com>
Acked-by: Kiryl Shutsemau <kas@kernel.org>
Acked-by: Zi Yan <ziy@nvidia.com>
---
 mm/ksm.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index 04019a15b25d..7bc726b50b2f 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -2921,7 +2921,7 @@ int __ksm_enter(struct mm_struct *mm)
 
 void __ksm_exit(struct mm_struct *mm)
 {
-	struct ksm_mm_slot *mm_slot;
+	struct ksm_mm_slot *mm_slot = NULL;
 	struct mm_slot *slot;
 	int easy_to_free = 0;
 
@@ -2936,19 +2936,20 @@ void __ksm_exit(struct mm_struct *mm)
 
 	spin_lock(&ksm_mmlist_lock);
 	slot = mm_slot_lookup(mm_slots_hash, mm);
-	if (slot) {
-		mm_slot = mm_slot_entry(slot, struct ksm_mm_slot, slot);
-		if (ksm_scan.mm_slot != mm_slot) {
-			if (!mm_slot->rmap_list) {
-				hash_del(&slot->hash);
-				list_del(&slot->mm_node);
-				easy_to_free = 1;
-			} else {
-				list_move(&slot->mm_node,
-					  &ksm_scan.mm_slot->slot.mm_node);
-			}
-		}
+	if (!slot)
+		goto unlock;
+	mm_slot = mm_slot_entry(slot, struct ksm_mm_slot, slot);
+	if (ksm_scan.mm_slot == mm_slot)
+		goto unlock;
+	if (!mm_slot->rmap_list) {
+		hash_del(&slot->hash);
+		list_del(&slot->mm_node);
+		easy_to_free = 1;
+	} else {
+		list_move(&slot->mm_node,
+			  &ksm_scan.mm_slot->slot.mm_node);
 	}
+unlock:
 	spin_unlock(&ksm_mmlist_lock);
 
 	if (easy_to_free) {
-- 
2.34.1

[Patch mm-stable-fixup 2/2] mm/khugepaged: remove definition of struct khugepaged_mm_slot

[Wei Yang]

Current code calls mm_slot_entry() even when we don't have a valid slot,
which is not future proof. Currently, this is not a problem because
"slot" is the first member in struct khugepaged_mm_slot.

While struct khugepaged_mm_slot is just a wrapper of struct mm_slot, there
is no need to define it.

Remove the definition of struct khugepaged_mm_slot, so there is not chance
to miss use mm_slot_entry().

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Cc: Lance Yang <lance.yang@linux.dev>
Cc: David Hildenbrand <david@redhat.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Kiryl Shutsemau <kirill@shutemov.name>
Cc: xu xin <xu.xin16@zte.com.cn>
Cc: SeongJae Park <sj@kernel.org>
Cc: Nico Pache <npache@redhat.com>
Acked-by: Lance Yang <lance.yang@linux.dev>
Reviewed-by: Dev Jain <dev.jain@arm.com>
Acked-by: David Hildenbrand <david@redhat.com>
Acked-by: Zi Yan <ziy@nvidia.com>
Reviewed-by: Raghavendra K T <raghavendra.kt@amd.com>
---
 mm/khugepaged.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 7ab2d1a42df3..abe54f0043c7 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -376,10 +376,7 @@ int hugepage_madvise(struct vm_area_struct *vma,
 
 int __init khugepaged_init(void)
 {
-	mm_slot_cache = kmem_cache_create("khugepaged_mm_slot",
-					  sizeof(struct mm_slot),
-					  __alignof__(struct mm_slot),
-					  0, NULL);
+	mm_slot_cache = KMEM_CACHE(mm_slot, 0);
 	if (!mm_slot_cache)
 		return -ENOMEM;
 
-- 
2.34.1

Re: [Patch mm-stable-fixup 0/2] mm_slot: fix the usage of mm_slot_entry()

[David Hildenbrand]

On 01.10.25 03:06, Wei Yang wrote:
> This is the fixup based on mm-stable.
> 
> The base commit:
> 
>     mm: swap: check for stable address space before operating on the VMA
> 
> The reviewed version is at [1].

I'm confused: why "fixup"?

Usually fixups are applied on top of other patches (to be squashed in).

-- 
Cheers

David / dhildenb

Re: [Patch mm-stable-fixup 0/2] mm_slot: fix the usage of mm_slot_entry()

[Wei Yang]

On Wed, Oct 01, 2025 at 10:34:52AM +0200, David Hildenbrand wrote:
>On 01.10.25 03:06, Wei Yang wrote:
>> This is the fixup based on mm-stable.
>> 
>> The base commit:
>> 
>>     mm: swap: check for stable address space before operating on the VMA
>> 
>> The reviewed version is at [1].
>
>I'm confused: why "fixup"?
>
>Usually fixups are applied on top of other patches (to be squashed in).
>

Andrew mention he has merged v2 into mm-stable and ask for a fixup.

Not sure I misunderstand the process.

>-- 
>Cheers
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me

Re: [Patch mm-stable-fixup 0/2] mm_slot: fix the usage of mm_slot_entry()

[David Hildenbrand]

On 01.10.25 10:39, Wei Yang wrote:
> On Wed, Oct 01, 2025 at 10:34:52AM +0200, David Hildenbrand wrote:
>> On 01.10.25 03:06, Wei Yang wrote:
>>> This is the fixup based on mm-stable.
>>>
>>> The base commit:
>>>
>>>      mm: swap: check for stable address space before operating on the VMA
>>>
>>> The reviewed version is at [1].
>>
>> I'm confused: why "fixup"?
>>
>> Usually fixups are applied on top of other patches (to be squashed in).
>>
> 
> Andrew mention he has merged v2 into mm-stable and ask for a fixup.

Ah, now I understand, some part is already in mm-stable.

In that case the patch descriptions+subjects have to be updated to 
clarify that these are only cleanups on top of the existing commits.

Then, also mention the merged commit in mm-stable.

E.g., for Patch #1

"
mm/ksm: cleanup mm_slot_entry() invocation

We got some late review commits during review of commit XXX ("XXX"). 
Let's reduce the indentation level and make the code easier to follow by
using gotos to a new label.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
"

E.g., for Patch #2

"
mm/khugepaged: use KMEM_CACHE()

We got some late review commits during review of commit XXX ("XXX"). No 
need to keep the old cache name "khugepaged_mm_slot", let's simply
use KMEM_CACHE().

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
"

-- 
Cheers

David / dhildenb

Re: [Patch mm-stable-fixup 0/2] mm_slot: fix the usage of mm_slot_entry()

[Wei Yang]

On Wed, Oct 01, 2025 at 10:45:53AM +0200, David Hildenbrand wrote:
>On 01.10.25 10:39, Wei Yang wrote:
>> On Wed, Oct 01, 2025 at 10:34:52AM +0200, David Hildenbrand wrote:
>> > On 01.10.25 03:06, Wei Yang wrote:
>> > > This is the fixup based on mm-stable.
>> > > 
>> > > The base commit:
>> > > 
>> > >      mm: swap: check for stable address space before operating on the VMA
>> > > 
>> > > The reviewed version is at [1].
>> > 
>> > I'm confused: why "fixup"?
>> > 
>> > Usually fixups are applied on top of other patches (to be squashed in).
>> > 
>> 
>> Andrew mention he has merged v2 into mm-stable and ask for a fixup.
>
>Ah, now I understand, some part is already in mm-stable.
>
>In that case the patch descriptions+subjects have to be updated to clarify
>that these are only cleanups on top of the existing commits.
>
>Then, also mention the merged commit in mm-stable.
>

Ok, got it.

Let me update it.

>E.g., for Patch #1
>
>"
>mm/ksm: cleanup mm_slot_entry() invocation
>
>We got some late review commits during review of commit XXX ("XXX"). Let's
>reduce the indentation level and make the code easier to follow by
>using gotos to a new label.
>
>Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>"
>
>E.g., for Patch #2
>
>"
>mm/khugepaged: use KMEM_CACHE()
>
>We got some late review commits during review of commit XXX ("XXX"). No need
>to keep the old cache name "khugepaged_mm_slot", let's simply
>use KMEM_CACHE().
>
>Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>"
>

Thanks for these detailed description.


-- 
Wei Yang
Help you, Help me