Re: [PATCH v2 bpf-next] bpf: Reduce smap->elem_size

[Martin KaFai Lau <martin.lau@linux.dev>]

On 12/20/22 5:17 PM, Andrii Nakryiko wrote:
> On Tue, Dec 20, 2022 at 5:15 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>>
>> From: Martin KaFai Lau <martin.lau@kernel.org>
>>
>> 'struct bpf_local_storage_elem' has an unused 56 byte padding at the
>> end due to struct's cache-line alignment requirement. This padding
>> space is overlapped by storage value contents, so if we use sizeof()
>> to calculate the total size, we overinflate it by 56 bytes. Use
>> offsetofend() instead to calculate more exact memory use.
>>
>> Acked-by: Yonghong Song <yhs@fb.com>
>> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
>> ---
>> v2:
>>    Rephrase the commit message (Andrii and Yonghong)
>>    Use offsetofend instead of offsetof (Andrii)
>>
>>   kernel/bpf/bpf_local_storage.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c
>> index b39a46e8fb08..e73fc70071b0 100644
>> --- a/kernel/bpf/bpf_local_storage.c
>> +++ b/kernel/bpf/bpf_local_storage.c
>> @@ -580,8 +580,8 @@ static struct bpf_local_storage_map *__bpf_local_storage_map_alloc(union bpf_att
>>                  raw_spin_lock_init(&smap->buckets[i].lock);
>>          }
>>
>> -       smap->elem_size =
>> -               sizeof(struct bpf_local_storage_elem) + attr->value_size;
>> +       smap->elem_size = offsetofend(struct bpf_local_storage_elem, sdata) +
>> +               attr->value_size;
> 
> Heh, we raced down to a minute. Copy/pasting my latest reply from
> original thread.

lol.  email is more parallel than most people thought :)

> 
> it just occurred to me
> that your change can be written equivalently (but now I do think it's
> cleaner) as:
> 
> smap->elem_size = offsetof(struct bpf_local_storage_elem,
> sdata.data[attr->value_size]);

Sure. will post v3.

> 
> 
> sdata is embedded struct, no pointer dereference, so single offsetof()
> should be enough to peer inside it
> 
> 
> Whichever you prefer, both versions work for me:
> 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>

Thanks for the review.