[PATCH 1/1] mm/ksm: avoid missing ksmd wakeups in ksm_enter

[PATCH 1/1] mm/ksm: avoid missing ksmd wakeups in ksm_enter

[Longlong Xia]

From: Longlong Xia <xialonglong@kylinos.cn>

__ksm_enter() decides whether ksmd needs a wakeup by checking if the
mm slot list is empty before inserting the new slot.

The empty check is currently outside ksm_mmlist_lock.  Another CPU can
remove the last slot and let ksmd go back to sleep after the unlocked
check, while this CPU inserts a new slot and skips the wakeup based on
the stale result.

Take ksm_mmlist_lock before checking the list so the empty-to-nonempty
transition and the insertion are observed as one critical section.

Fixes: 6e15838425ac ("ksm: keep quiet while list empty")
Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
---
 mm/ksm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index 7d5b76478f0b..3f2f4707ca9e 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -3019,7 +3019,7 @@ int __ksm_enter(struct mm_struct *mm)
 {
 	struct ksm_mm_slot *mm_slot;
 	struct mm_slot *slot;
-	int needs_wakeup;
+	bool needs_wakeup;
 
 	mm_slot = mm_slot_alloc(mm_slot_cache);
 	if (!mm_slot)
@@ -3027,10 +3027,9 @@ int __ksm_enter(struct mm_struct *mm)
 
 	slot = &mm_slot->slot;
 
+	spin_lock(&ksm_mmlist_lock);
 	/* Check ksm_run too?  Would need tighter locking */
 	needs_wakeup = list_empty(&ksm_mm_head.slot.mm_node);
-
-	spin_lock(&ksm_mmlist_lock);
 	mm_slot_insert(mm_slots_hash, mm, slot);
 	/*
 	 * When KSM_RUN_MERGE (or KSM_RUN_STOP),
-- 
2.43.0

Re: [PATCH 1/1] mm/ksm: avoid missing ksmd wakeups in ksm_enter

[David Hildenbrand (Arm)]

On 8/3/26 17:21, Longlong Xia wrote:
> From: Longlong Xia <xialonglong@kylinos.cn>
> 
> __ksm_enter() decides whether ksmd needs a wakeup by checking if the
> mm slot list is empty before inserting the new slot.
> 
> The empty check is currently outside ksm_mmlist_lock.  Another CPU can
> remove the last slot and let ksmd go back to sleep after the unlocked
> check, while this CPU inserts a new slot and skips the wakeup based on
> the stale result.
> 
> Take ksm_mmlist_lock before checking the list so the empty-to-nonempty
> transition and the insertion are observed as one critical section.
> 
> Fixes: 6e15838425ac ("ksm: keep quiet while list empty")
> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
> ---
>  mm/ksm.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 7d5b76478f0b..3f2f4707ca9e 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -3019,7 +3019,7 @@ int __ksm_enter(struct mm_struct *mm)
>  {
>  	struct ksm_mm_slot *mm_slot;
>  	struct mm_slot *slot;
> -	int needs_wakeup;
> +	bool needs_wakeup;

Unrelated change that should not be included here.

>  
>  	mm_slot = mm_slot_alloc(mm_slot_cache);
>  	if (!mm_slot)
> @@ -3027,10 +3027,9 @@ int __ksm_enter(struct mm_struct *mm)
>  
>  	slot = &mm_slot->slot;
>  
> +	spin_lock(&ksm_mmlist_lock);
>  	/* Check ksm_run too?  Would need tighter locking */
>  	needs_wakeup = list_empty(&ksm_mm_head.slot.mm_node);
> -
> -	spin_lock(&ksm_mmlist_lock);
>  	mm_slot_insert(mm_slots_hash, mm, slot);
>  	/*
>  	 * When KSM_RUN_MERGE (or KSM_RUN_STOP),


Makes sense to me. With the above variable change removed:

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH 1/1] mm/ksm: avoid missing ksmd wakeups in ksm_enter

[Longlong Xia]

在 2026/8/5 20:26, David Hildenbrand (Arm) 写道:
> On 8/3/26 17:21, Longlong Xia wrote:
>> From: Longlong Xia <xialonglong@kylinos.cn>
>>
>> __ksm_enter() decides whether ksmd needs a wakeup by checking if the
>> mm slot list is empty before inserting the new slot.
>>
>> The empty check is currently outside ksm_mmlist_lock.  Another CPU can
>> remove the last slot and let ksmd go back to sleep after the unlocked
>> check, while this CPU inserts a new slot and skips the wakeup based on
>> the stale result.
>>
>> Take ksm_mmlist_lock before checking the list so the empty-to-nonempty
>> transition and the insertion are observed as one critical section.
>>
>> Fixes: 6e15838425ac ("ksm: keep quiet while list empty")
>> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
>> ---
>>   mm/ksm.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/ksm.c b/mm/ksm.c
>> index 7d5b76478f0b..3f2f4707ca9e 100644
>> --- a/mm/ksm.c
>> +++ b/mm/ksm.c
>> @@ -3019,7 +3019,7 @@ int __ksm_enter(struct mm_struct *mm)
>>   {
>>   	struct ksm_mm_slot *mm_slot;
>>   	struct mm_slot *slot;
>> -	int needs_wakeup;
>> +	bool needs_wakeup;
> Unrelated change that should not be included here.
>
>>   
>>   	mm_slot = mm_slot_alloc(mm_slot_cache);
>>   	if (!mm_slot)
>> @@ -3027,10 +3027,9 @@ int __ksm_enter(struct mm_struct *mm)
>>   
>>   	slot = &mm_slot->slot;
>>   
>> +	spin_lock(&ksm_mmlist_lock);
>>   	/* Check ksm_run too?  Would need tighter locking */
>>   	needs_wakeup = list_empty(&ksm_mm_head.slot.mm_node);
>> -
>> -	spin_lock(&ksm_mmlist_lock);
>>   	mm_slot_insert(mm_slots_hash, mm, slot);
>>   	/*
>>   	 * When KSM_RUN_MERGE (or KSM_RUN_STOP),
>
> Makes sense to me. With the above variable change removed:
>
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>


   Thanks for the review.  I will keep `needs_wakeup` as an `int` and send
   v2 with your Acked-by.

   Best regards,
   Longlong