On Tue, Dec 20, 2022 at 5:15 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > From: Martin KaFai Lau <martin.lau@xxxxxxxxxx> > > '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@xxxxxx> > Signed-off-by: Martin KaFai Lau <martin.lau@xxxxxxxxxx> > --- > 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. 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]); 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@xxxxxxxxxx> > > return smap; > } > -- > 2.30.2 >
