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

[xu.xin16]

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

Maybe I misunderstood it.

> 
> > Signed-off-by: xu xin <xu.xin16@xxxxxxxxxx>
> 
> Honestly I don't think this is really adding much, but I guess adding the
> comment can't hurt.

Agreed.

Regardless, adding a comment to explain why these have to be macros would be
better. Would it be okay if I send a new patch that merely adds the comment,
without removing the underscore prefixes from the existing macros? 

> 
> > ---
> >  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() :)

Yes.