[ Cc: the bpf list back. I dropped it by mistake in my last reply. ]
On 12/20/22 3:43 PM, Andrii Nakryiko wrote:
On Mon, Dec 19, 2022 at 11:47 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
On 12/16/22 5:23 PM, Yonghong Song wrote:
On 12/16/22 3:29 PM, Martin KaFai Lau wrote:
From: Martin KaFai Lau <martin.lau@xxxxxxxxxx>
'struct bpf_local_storage_elem' has a 56 bytes padding at the end
which can be used for attr->value_size. The current smap->elem_size
'can be' => 'will be'?
I used 'can be' to describe the current situation that the padding is not used
for the map's value. I may have used the wrong tense?
I can rephrase it to something like,
'struct bpf_local_storage_elem' has a 56 bytes padding at the end which is
currently not used for the attr->value_size.
I actually found the use of attr->value_size to mean "value content"
more confusing than can vs will be.
As a suggestion something like the below?
'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
offsetof() instead to calculate more exact memory use.
SGTM.
btw, I think you can calculate the same arguably a bit more
straightforwardly as:
smap->elem_size = offsetofend(struct bpf_local_storage_elem, sdata) +
attr->value_size;
Sure. will change.
right?
but TIL that offsetof(struct bpf_local_storage_data,
data[attr->value_size]) does the right thing
Yeah, I think I have seen other places using it also. I found it more intuitive
to read with array[size] to tell there is a flexible array at the end. I am not
super attached to it and will change to the way above.
