Messages in this thread | | | Date | Tue, 29 Nov 2022 16:13:35 -0500 | Subject | Re: [net-next] bpf: avoid hashtab deadlock with try_lock | From | Waiman Long <> |
| |
On 11/29/22 14:36, Hao Luo wrote: > On Tue, Nov 29, 2022 at 9:32 AM Boqun Feng <boqun.feng@gmail.com> wrote: >> Just to be clear, I meant to refactor htab_lock_bucket() into a try >> lock pattern. Also after a second thought, the below suggestion doesn't >> work. I think the proper way is to make htab_lock_bucket() as a >> raw_spin_trylock_irqsave(). >> >> Regards, >> Boqun >> > The potential deadlock happens when the lock is contended from the > same cpu. When the lock is contended from a remote cpu, we would like > the remote cpu to spin and wait, instead of giving up immediately. As > this gives better throughput. So replacing the current > raw_spin_lock_irqsave() with trylock sacrifices this performance gain. > > I suspect the source of the problem is the 'hash' that we used in > htab_lock_bucket(). The 'hash' is derived from the 'key', I wonder > whether we should use a hash derived from 'bucket' rather than from > 'key'. For example, from the memory address of the 'bucket'. Because, > different keys may fall into the same bucket, but yield different > hashes. If the same bucket can never have two different 'hashes' here, > the map_locked check should behave as intended. Also because > ->map_locked is per-cpu, execution flows from two different cpus can > both pass.
I would suggest that you add a in_nmi() check and if true use trylock to get the lock. You can continue to use raw_spin_lock_irqsave() in all other cases.
Cheers, Longman
| |