Re: [PATCH bpf-next 1/3] bpf: Add memory usage for arena

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 7/17/26 5:24 PM, Kumar Kartikeya Dwivedi wrote:
> On Fri Jul 17, 2026 at 4:08 AM CEST, Jiayuan Chen wrote:
>> On 7/16/26 10:48 PM, sashiko-bot@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>>
>>> New issues:
>>> - [Medium] The `nr_pages` counter is modified using plain writes while being read concurrently locklessly, constituting a data race.
>>>
>>> Pre-existing issues:
>>> - [High] Functions that can sleep and acquire sleepable locks are called while holding a raw spinlock with interrupts disabled.
>>> --
>>>
>>> --- Patch [1]: [PATCH bpf-next 1/3] bpf: Add memory usage for arena ---
>>> commit 68cbc4739a7a9be56d49db779d319ec002523097
>>> Author: Jiayuan Chen <jiayuan.chen@linux.dev>
>>> Subject: bpf: Add memory usage for arena
>>>
>>> This patch adds memory usage tracking for the arena map type by counting
>>> the pages mapped into the arena. It bumps a counter in apply_range_set_cb()
>>> when a page is added, drops it in apply_range_clear_cb() when removed, and
>>> returns the calculated usage in map_mem_usage().
>>>
>>>> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
>>>> index 80b7b8a694464..bc34602116040 100644
>>>> --- a/kernel/bpf/arena.c
>>>> +++ b/kernel/bpf/arena.c
>>> [ ... ]
>>>
>>>> @@ -196,6 +198,7 @@ static int apply_range_set_cb(pte_t *pte, unsigned long addr, void *data)
>>>>    	set_pte_at(&init_mm, addr, pte, pteval);
>>>>    #endif
>>>>    	d->i++;
>>>> +	d->arena->nr_pages++;
>>>>    	return 0;
>>>>    }
>>> [Severity: Medium]
>>> Is it safe to use a plain increment here since nr_pages is read concurrently?
>>>
>>> In arena_map_mem_usage(), arena->nr_pages is read locklessly via
>>> READ_ONCE(arena->nr_pages). The Linux Kernel Memory Model requires that writes
>>> pairing with a lockless READ_ONCE must be marked with WRITE_ONCE to prevent
>>> the compiler from splitting the writes or assuming no concurrent access.
>>>
>>> Could this lead to incorrect statistics or torn writes?
>>>
>>> [ ... ]
>>>
>>>> @@ -227,10 +230,11 @@ static int apply_range_clear_cb(pte_t *pte, unsigned long addr, void *data)
>>>>    	 * scratches its PTE. A later bpf_arena_free_pages() over that range walks
>>>>    	 * here. Without the skip, scratch_page would be freed.
>>>>    	 */
>>>> -	if (page == d->scratch_page)
>>>> +	if (page == d->arena->scratch_page)
>>>>    		return 0;
>>>>
>>>>    	__llist_add(&page->pcp_llist, d->free_pages);
>>>> +	d->arena->nr_pages--;
>>> [Severity: Medium]
>>> Similar to the increment above, does this plain decrement need WRITE_ONCE() to
>>> safely pair with the READ_ONCE() in arena_map_mem_usage()?
>> It's true.
>>
>>
> Overall seems ok to me, since you are going to resend, could you also split the
> changes to move remove scratch page field into a prep patch before adding
> nr_pages?
>
> Thanks!

Thanks for your review.

I will do it.