Re: [PATCH v3 13/26] mm: introduce freetype_t

[Yosry Ahmed]

On Thu, Sep 3, 2026 at 1:00 AM Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx> wrote:
>
> On 8/18/26 02:35, Yosry Ahmed wrote:
> >> >> +static inline
> >> >> +struct list_head *free_area_list(struct free_area *area, freetype_t type)
> >> >> +{
> >> >> + int idx = freetype_idx(type);
> >> >> +
> >> >> + VM_WARN_ON(idx < 0);
> >> >> + return &area->free_list[idx];
> >> >
> >> > Should we return NULL here if idx < 0 instead of an out of bounds
> >> > access?
> >>
> >> TBH my descending order of preference is:
> >>
> >> 1.
> >>
> >> BUG_ON(idx < 0);
> >> return &area->free_list[idx];
> >>
> >> 2.
> >>
> >> if (WARN_ON(idx < 0)) // OR VM_WARN_ON
> >>   return NULL;
> >> return &area->free_list[idx];
> >>
> >> 3.
> >>
> >> VM_WARN_ON(idx < 0);
> >> return &area->free_list[idx];
> >>
> >> 4.
> >>
> >> return &area->free_list[idx];
> >>
> >> But I suspect[0] Vlastimil (and Linus) would order it the exact opposite
> >> way.
> >>
> >> [0]: https://lore.kernel.org/all/bd36e972-8900-4476-a66b-4dc218b21a4d@xxxxxxxxxx/
> >>
> >> And I care more about making Vlastimil (and Linus) happy than this tiny
> >> detail of the code, so I'll defer to him. The current style is a
> >> compromise, but maybe it's just a compromise that makes nobody happy.
> >
> > I think I prefer #2 (regardless of whether it's WARN_ON() or
> > VM_WARN_ON()), let's see if anyone disagrees.
>
> #2 can't work with VM_WARN_ON as it returns void. Otherwise if it evaluated
> the cond with DEBUG_VM and hardcoded false otherwise, it would be ideal. I
> don't remember why it's not.
>
> Do the callers all handle NULL safely? In that case #2 has some merit (maybe
> with WARN_ON_ONCE()) otherwise #3

I don't think callers handle NULL safely, so I guess I'll keep it as-is (#3).