On 11/14/23 05:50, Kees Cook wrote:
> On Mon, Nov 13, 2023 at 08:13:59PM +0100, Vlastimil Babka wrote:
>> slab_alloc() is a thin wrapper around slab_alloc_node() with only one
>> caller. Replace with direct call of slab_alloc_node().
>> __kmem_cache_alloc_lru() itself is a thin wrapper with two callers,
>> so replace it with direct calls of slab_alloc_node() and
>> trace_kmem_cache_alloc().
>
> I'd have a sense that with 2 callers a wrapper is still useful?
Well it bothered me how many layers everything went through, it makes it
harder to comprehend the code.
>>
>> This also makes sure _RET_IP_ has always the expected value and not
>> depending on inlining decisions.
And there's also this argument. We should evaluate _RET_IP_ in
kmem_cache_alloc() and kmem_cache_alloc_lru().
>> Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
>> [...]
>> void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
>> {
>> - void *ret = slab_alloc_node(s, NULL, gfpflags, node, _RET_IP_, s->object_size);
>> + void *ret = slab_alloc_node(s, NULL, gfpflags, node, _RET_IP_,
>> + s->object_size);
>>
>
> Whitespace change here isn't mentioned in the commit log.
OK, will mention.
> Regardless:
>
> Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
Thanks!
