Re: [PATCH 2/2] bpf: Migrate dynptr file to kmalloc_nolock

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:

> On Wed, 25 Mar 2026 at 22:12, Mykyta Yatsenko
> <mykyta.yatsenko5@gmail.com> wrote:
>>
>> From: Mykyta Yatsenko <yatsenko@meta.com>
>>
>> Replace bpf_mem_alloc/bpf_mem_free with kmalloc_nolock/kfree_rcu for
>> bpf_dynptr_file_impl, continuing the migration away from bpf_mem_alloc
>> now that kmalloc can be used from NMI context.
>>
>> freader_cleanup() runs before kfree_rcu() while the dynptr still holds
>> exclusive access. kfree_rcu() then defers the actual free until after
>> a grace period.
>>
>> Add struct rcu_head to bpf_dynptr_file_impl for kfree_rcu().
>> ---
>>  kernel/bpf/helpers.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index b197b6978f1a..b349c8a34e50 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -1736,7 +1736,10 @@ static const struct bpf_func_proto bpf_kptr_xchg_proto = {
>>  };
>>
>>  struct bpf_dynptr_file_impl {
>> -       struct freader freader;
>> +       union {
>> +               struct freader freader;
>> +               struct rcu_head rcu;
>> +       };
>
> Sorry, this is confusing to me. Why do we need RCU gp wait before freeing here?
> bpf_mem_free() didn't do any RCU gp before.
>
Double checked, I think you are right, there's no point to do
kfree_rcu() just kfree() should do, as concurrent access to dynptr
should not be possible and after discard() nothing should be able to
access it anyway. Thanks.
>>         /* 64 bit offset and size overriding 32 bit ones in bpf_dynptr_kern */
>>         u64 offset;
>>         u64 size;
>> @@ -4427,7 +4430,7 @@ static int make_file_dynptr(struct file *file, u32 flags, bool may_sleep,
>>                 return -EINVAL;
>>         }
>>
>> -       state = bpf_mem_alloc(&bpf_global_ma, sizeof(struct bpf_dynptr_file_impl));
>> +       state = kmalloc_nolock(sizeof(*state), 0, NUMA_NO_NODE);
>>         if (!state) {
>>                 bpf_dynptr_set_null(ptr);
>>                 return -ENOMEM;
>> @@ -4459,7 +4462,7 @@ __bpf_kfunc int bpf_dynptr_file_discard(struct bpf_dynptr *dynptr)
>>                 return 0;
>>
>>         freader_cleanup(&df->freader);
>> -       bpf_mem_free(&bpf_global_ma, df);
>> +       kfree_rcu(df, rcu);
>>         bpf_dynptr_set_null(ptr);
>>         return 0;
>>  }
>>
>> --
>> 2.52.0
>>