On Tue, Nov 29, 2022 at 9:32 AM Boqun Feng <boqun.feng@xxxxxxxxx> 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. Hao
