Re: [PATCH bpf-next v3 01/16] bpf: Convert bpf_selem_unlink_map to failable

[Martin KaFai Lau <martin.lau@linux.dev>]

On 12/18/25 10:27 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c
>> index e2fe6c32822b..4e3f227fd634 100644
>> --- a/kernel/bpf/bpf_local_storage.c
>> +++ b/kernel/bpf/bpf_local_storage.c
> 
> [ ... ]
> 
>> @@ -488,9 +516,10 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
>>   {
>>   	struct bpf_local_storage_data *old_sdata = NULL;
>>   	struct bpf_local_storage_elem *alloc_selem, *selem = NULL;
>> +	struct bpf_local_storage_map_bucket *b, *old_b = NULL;
>> +	unsigned long flags, b_flags, old_b_flags;
>>   	struct bpf_local_storage *local_storage;
>>   	HLIST_HEAD(old_selem_free_list);
>> -	unsigned long flags;
>>   	int err;
> 
> [ ... ]
> 
>> @@ -574,20 +603,37 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
>>   		goto unlock;
>>   	}
>>
>> +	b = select_bucket(smap, selem);
>> +
>> +	if (old_sdata) {
>> +		old_b = select_bucket(smap, SELEM(old_sdata));
>> +		old_b = old_b == b ? NULL : old_b;
>> +	}
>> +
>> +	raw_spin_lock_irqsave(&b->lock, b_flags);
>> +
>> +	if (old_b)
>> +		raw_spin_lock_irqsave(&old_b->lock, old_b_flags);
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Could this introduce an ABBA deadlock between two bucket locks? If two
> threads concurrently update storage on the same map with different owners,
> their new and old selems could hash to different buckets with reversed
> order:
> 
>    Thread 1: new_selem -> bucket_A, old_selem -> bucket_B
>              acquires b->lock (A), then waits for old_b->lock (B)
> 
>    Thread 2: new_selem -> bucket_B, old_selem -> bucket_A
>              acquires b->lock (B), then waits for old_b->lock (A)
> 
> Since the locks are regular raw_spinlocks (not rqspinlocks yet), they
> will block indefinitely in this scenario.
> 
> Note: I see that commit 615477732ee8 ("bpf: Change local_storage->lock
> and b->lock to rqspinlock") later in this series changes to rqspinlock
> which has deadlock detection and can return -EDEADLK instead of blocking.
> However, in the current commit the locks are still raw_spinlocks.

I just catch up on this ai-review, so I made a similar comment on this 
raw_spinlock in another reply instead of here. imo, we should still 
avoid this even the rqspinlock is used in the later patch.