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

[Muhammad Usama Anjum <MUsamaAnjum@gmail.com>]

On 9/27/25 5:45 AM, Wei Yang wrote:
> 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: Muhammad Usama Anjum <usama.anjum@collabora.com>

> 
> ---
> v3:
>   * adjust subject and changelog based on David's comment
>   * use invert style for coding suggested by Kiryl
>   * drop RB and Ack-by
> v2:
>   * fix uninitialized mm_slot
> ---
>  mm/ksm.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 2dbe92e3dd52..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,17 +2936,20 @@ void __ksm_exit(struct mm_struct *mm)
>  
>  	spin_lock(&ksm_mmlist_lock);
>  	slot = mm_slot_lookup(mm_slots_hash, mm);
> +	if (!slot)
> +		goto unlock;
>  	mm_slot = mm_slot_entry(slot, struct ksm_mm_slot, slot);
> -	if (mm_slot && 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 (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) {