Re: [PATCH bpf 2/2] bpf: Use __llist_del_all() whenever possbile during memory draining

[sdf@google.com]

On 10/19, Hou Tao wrote:
> From: Hou Tao <houtao1@huawei.com>

> Except for waiting_for_gp list, there are no concurrent operations on
> free_by_rcu, free_llist and free_llist_extra lists, so use
> __llist_del_all() instead of llist_del_all(). waiting_for_gp list can be
> deleted by RCU callback concurrently, so still use llist_del_all().

> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>   kernel/bpf/memalloc.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)

> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
> index 48e606aaacf0..7f45744a09f7 100644
> --- a/kernel/bpf/memalloc.c
> +++ b/kernel/bpf/memalloc.c
> @@ -422,14 +422,17 @@ static void drain_mem_cache(struct bpf_mem_cache *c)
>   	/* No progs are using this bpf_mem_cache, but htab_map_free() called
>   	 * bpf_mem_cache_free() for all remaining elements and they can be in
>   	 * free_by_rcu or in waiting_for_gp lists, so drain those lists now.
> +	 *
> +	 * Except for waiting_for_gp list, there are no concurrent operations
> +	 * on these lists, so it is safe to use __llist_del_all().
>   	 */
>   	llist_for_each_safe(llnode, t, __llist_del_all(&c->free_by_rcu))
>   		free_one(c, llnode);
>   	llist_for_each_safe(llnode, t, llist_del_all(&c->waiting_for_gp))
>   		free_one(c, llnode);
> -	llist_for_each_safe(llnode, t, llist_del_all(&c->free_llist))
> +	llist_for_each_safe(llnode, t, __llist_del_all(&c->free_llist))
>   		free_one(c, llnode);
> -	llist_for_each_safe(llnode, t, llist_del_all(&c->free_llist_extra))
> +	llist_for_each_safe(llnode, t, __llist_del_all(&c->free_llist_extra))
>   		free_one(c, llnode);

Acked-by: Stanislav Fomichev <sdf@google.com>

Seems safe even without the previous patch? OTOH, do we really care
about __lllist vs llist in the cleanup path? Might be safer to always
do llist_del_all everywhere?

>   }

> --
> 2.29.2