Re: [PATCH] mm/mm_slot.h: add a helper function mm_slot_remove

[Lorenzo Stoakes]

On Mon, Jul 13, 2026 at 01:44:54PM +0800, xu.xin16@xxxxxxxxxx wrote:
> > > No functional change is intended.
> > >
> > > Signed-off-by: xu xin <xu.xin16@xxxxxxxxxx>
> >
> > In general it's fine but I want more :) see below.
> >
> > >
> > >
> > > diff --git a/mm/mm_slot.h b/mm/mm_slot.h
> > > index 83f18ed1c4bd..bc3b9ae8fdbc 100644
> > > --- a/mm/mm_slot.h
> > > +++ b/mm/mm_slot.h
> > > @@ -52,4 +52,11 @@ static inline void mm_slot_free(struct kmem_cache *cache, void *objp)
> > >  	hash_add(_hashtable, &_mm_slot->hash, (unsigned long)_mm);	       \
> > >  })
> > >
> >
> > mm_slot.h also inexplicably implements mm_slot_lookup() and mm_slot_insert() as
> > macros. Can you change these to static inline functions in the respin?
>
> Sorry, I looked into turning mm_slot_lookup() and mm_slot_insert() into
> static inline functions, but unfortunately it’s not possible because
> they rely on hash_for_each_possible() and hash_add(), which expect an
> actual array instead of pointer so that sizeof() can determine the
> hash table size. Passing a struct hlist_head * would make sizeof() return
> the pointer size instead of the array size, leading to incorrect bucket
> indexing and memory corruption.

Ah that's fair enough then, explains why they're macros there :)


>
> >
> > Also rename the horrible names from _foo -> foo too please.
> >
> > And we don't really need 'mm_slot' in the var name for a slot just 'slot' works
> > fine.
>
> Ok, I can clean up these stuff.

Thanks!

>
> >
> > > +static inline void mm_slot_remove(struct mm_slot *_mm_slot)
> >
> > Yeah this name is gross just 'slot' please.
> >
> > > +{
> > > +	hash_del(&_mm_slot->hash);
> > > +	list_del(&_mm_slot->mm_node);
> > > +}
> > > +
> > > +
> > >  #endif /* _LINUX_MM_SLOT_H */
> >
> > Thanks, Lorenzo
> >

Cheers, Lorenzo