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

[Yosry Ahmed]

> >> +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.