Re: [RFC PATCH bpf-next v3 07/10] bpf: Add warnings for internal bugs in map_create

[Leon Hwang <leon.hwang@linux.dev>]

On 3/10/25 08:06, Alexei Starovoitov wrote:
> On Thu, Oct 2, 2025 at 8:49 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> In next commit, it will report users the reason of -EINVAL in
>> map_create.
>>
>> However, as for the check of '!ops' and '!ops->map_mem_usage', it
>> shouldn't report the reason as they would be internal bugs.
>>
>> Instead, add WARN_ON_ONCE to them. Then, it is able to check dmesg to get
>> the error details.
>>
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>> ---
>>  kernel/bpf/syscall.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index fc1b5c8c5e82f..49db250a2f5da 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -1406,7 +1406,7 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr)
>>                 return -EINVAL;
>>         map_type = array_index_nospec(map_type, ARRAY_SIZE(bpf_map_types));
>>         ops = bpf_map_types[map_type];
>> -       if (!ops)
>> +       if (WARN_ON_ONCE(!ops))
>>                 return -EINVAL;
> 
> It was a strong recommendation for a long time to avoid WARN*() at all cost.
> In the verifier we removed majority of them and replaced with verifier_bug()
> which WARNS only when DEBUG_KERNEL.
> 
> Here there is no reason to warn at all. Keep it as-is.

Got it, thanks for clarifying.

I understand now that WARN*() should generally be avoided, since they
add noise and aren't justified here. The existing 'return -EINVAL;' is
sufficient.

I'll keep it as-is in the next revision.

Thanks,
Leon