Re: [PATCH v2 2/2] mm/mm_slot.h: clean up macro parameter names in mm_slot_lookup() and mm_slot_insert()

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Mon, Jul 13, 2026 at 02:41:53PM +0800, xu.xin16@zte.com.cn wrote:
> From: xu xin <xu.xin16@zte.com.cn>
>

Obv. as others have noted, you're missing a commit message here :)

> Suggested-by: Lorenzo Stoakes <ljs@kernel.org>

Hmm not sure I suggested adding a comment to explain it...  or removing the
prefix _ from the existing macros :P

> Signed-off-by: xu xin <xu.xin16@zte.com.cn>

Honestly I don't think this is really adding much, but I guess adding the
comment can't hurt.

> ---
>  mm/mm_slot.h | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/mm/mm_slot.h b/mm/mm_slot.h
> index 5de3e91d86b4..710c70166c79 100644
> --- a/mm/mm_slot.h
> +++ b/mm/mm_slot.h
> @@ -33,12 +33,17 @@ static inline void mm_slot_free(struct kmem_cache *cache, void *objp)
>  	kmem_cache_free(cache, objp);
>  }
>
> -#define mm_slot_lookup(_hashtable, _mm) 				       \
> +/*
> + * Note: mm_slot_lookup and mm_slot_insert cannot be converted to static inline
> + * functions because hash_for_each_possible relys on the actual array argument
> + * 'hashtable' for sizeof() instead of pointers.
> + */

I didn't say that you should change the _macros_ to drop the underscores
though, there could be some nasty macro hygiene issue without (though
probably something's broken if that does happen).

So let's not do that.

Also you'd probably want this comment above both with a newline after so it
doesn't just seem attached to mm_slot_lookup() :)

> +#define mm_slot_lookup(hashtable, mm)					       \
>  ({									       \
>  	struct mm_slot *tmp_slot, *mm_slot = NULL;			       \
>  									       \
> -	hash_for_each_possible(_hashtable, tmp_slot, hash, (unsigned long)_mm) \
> -		if (_mm == tmp_slot->mm) {				       \
> +	hash_for_each_possible(hashtable, tmp_slot, hash, (unsigned long)mm)   \
> +		if (mm == tmp_slot->mm) {				       \
>  			mm_slot = tmp_slot;				       \
>  			break;						       \
>  		}							       \
> @@ -46,10 +51,10 @@ static inline void mm_slot_free(struct kmem_cache *cache, void *objp)
>  	mm_slot;							       \
>  })
>
> -#define mm_slot_insert(_hashtable, _mm, _mm_slot)			       \
> +#define mm_slot_insert(hashtable, mm, mm_slot)				       \
>  ({									       \
> -	_mm_slot->mm = _mm;						       \
> -	hash_add(_hashtable, &_mm_slot->hash, (unsigned long)_mm);	       \
> +	mm_slot->mm = mm;						       \
> +	hash_add(hashtable, &mm_slot->hash, (unsigned long)mm);		       \
>  })
>
>  static inline void mm_slot_remove(struct mm_slot *slot)
> --
> 2.25.1

I think maybe we should just drop this patch altogether and stick with the
first patch only.

Cheers, Lorenzo