Hi,
On 12/15/2023 3:38 PM, Yonghong Song wrote:
>
> On 12/14/23 7:33 PM, Hou Tao wrote:
>> Hi,
>>
>> On 12/15/2023 8:12 AM, Yonghong Song wrote:
>>> In the previous patch, the maximum data size for bpf_global_percpu_ma
>>> is 512 bytes. This breaks selftest test_bpf_ma. Let us adjust it
>>> accordingly. Also added a selftest to capture the verification failure
>>> when the allocation size is greater than 512.
>>>
>>> Signed-off-by: Yonghong Song <yonghong.song@xxxxxxxxx>
>>>
SNIP
>>> DEFINE_ARRAY_WITH_PERCPU_KPTR(192);
>>> DEFINE_ARRAY_WITH_PERCPU_KPTR(256);
>>> DEFINE_ARRAY_WITH_PERCPU_KPTR(512);
>>> -DEFINE_ARRAY_WITH_PERCPU_KPTR(1024);
>>> -DEFINE_ARRAY_WITH_PERCPU_KPTR(2048);
>>> -DEFINE_ARRAY_WITH_PERCPU_KPTR(4096);
>> Considering the update in patch "bpf: Avoid unnecessary extra percpu
>> memory allocation", the definition of DEFINE_ARRAY_WITH_PERCPU_KPTR()
>> needs update as well, because for 512-sized per-cpu kptr, the tests only
>> allocate for (512 - sizeof(void *)) bytes. And we could do
>> DEFINE_ARRAY_WITH_PERCPU_KPTR(8) test after the update. I could do that
>> after the patch-set is landed if you don't have time to do that.
>>
>> A bit of off-topic, but it is still relevant. I have a question about
>> how to forcibly generate BTF info for struct definition in the test ?
>> Currently, I have to include bin_data_xx in the definition of
>> map_value, but I don't want to increase the size of map_value. I had
>> tried to use BTF_TYPE_EMIT() in prog just like in linux kernel, but it
>> didn't work.
>
> Since you mentioned the btf generation issue, I did some investigation.
> To workaround btf generation issue, we can use the method in
> prog_tests/local_kptr_stash.c:
>
> ====
> /* This is necessary so that LLVM generates BTF for node_data struct
> * If it's not included, a fwd reference for node_data will be
> generated but
> * no struct. Example BTF of "node" field in map_value when not included:
> *
> * [10] PTR '(anon)' type_id=35
> * [34] FWD 'node_data' fwd_kind=struct
> * [35] TYPE_TAG 'kptr_ref' type_id=34
> *
> * (with no node_data struct defined)
> * Had to do the same w/ bpf_kfunc_call_test_release below
> */
> struct node_data *just_here_because_btf_bug;
> struct refcounted_node *just_here_because_btf_bug2;
> ====
Totally missed it. Thanks for pointing it out to me.
>
> I have hacked the test_bpf_ma.c files and something like below
> should work to generate btf types:
>
> struct bin_data_##_size { \
> char data[_size - sizeof(void *)]; \
> }; \
> + /* See Commit 5d8d6634ccc, force btf generation for type
> bin_data_##_size */ \
> + struct bin_data_##_size *__bin_data_##_size; \
> struct map_value_##_size { \
> struct bin_data_##_size __kptr * data; \
> - /* To emit BTF info for bin_data_xx */ \
> - struct bin_data_##_size not_used; \
> }; \
> struct { \
> __uint(type, BPF_MAP_TYPE_ARRAY); \
> @@ -40,8 +43,12 @@ int pid = 0;
> } array_##_size SEC(".maps")
>
> #define DEFINE_ARRAY_WITH_PERCPU_KPTR(_size) \
> + struct percpu_bin_data_##_size { \
> + char data[_size]; \
> + }; \
> + struct percpu_bin_data_##_size *__percpu_bin_data_##_size; \
> struct map_value_percpu_##_size { \
> - struct bin_data_##_size __percpu_kptr * data; \
> + struct percpu_bin_data_##_size __percpu_kptr * data; \
> }; \
> struct { \
> __uint(type, BPF_MAP_TYPE_ARRAY); \
>
> I have a prototype to ensure the type (for percpu kptr) removing these
> '- sizeof(void *)' and enabling DEFINE_ARRAY_WITH_PERCPU_KPTR().
> Once we resolved the check_obj_size() issue, I can then post v3.
Thanks for the update and it looks fine to me. Looking forwards to v3.
>
>>> SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
>>> int test_batch_alloc_free(void *ctx)
>>> @@ -259,9 +256,6 @@ int test_batch_percpu_alloc_free(void *ctx)
>>> CALL_BATCH_PERCPU_ALLOC_FREE(192, 128, 6);
>>> CALL_BATCH_PERCPU_ALLOC_FREE(256, 128, 7);
>>> CALL_BATCH_PERCPU_ALLOC_FREE(512, 64, 8);
>>> - CALL_BATCH_PERCPU_ALLOC_FREE(1024, 32, 9);
>>> - CALL_BATCH_PERCPU_ALLOC_FREE(2048, 16, 10);
>>> - CALL_BATCH_PERCPU_ALLOC_FREE(4096, 8, 11);
>>> return 0;
>>> }
>>> @@ -283,9 +277,6 @@ int test_percpu_free_through_map_free(void *ctx)
>>> CALL_BATCH_PERCPU_ALLOC(192, 128, 6);
>>> CALL_BATCH_PERCPU_ALLOC(256, 128, 7);
>>> CALL_BATCH_PERCPU_ALLOC(512, 64, 8);
>>> - CALL_BATCH_PERCPU_ALLOC(1024, 32, 9);
>>> - CALL_BATCH_PERCPU_ALLOC(2048, 16, 10);
>>> - CALL_BATCH_PERCPU_ALLOC(4096, 8, 11);
>>> return 0;
>>> }
