On 7/11/25 17:17, Sebastian Andrzej Siewior wrote: > On 2025-07-11 11:55:22 [+0200], Vlastimil Babka wrote: >> On 7/11/25 09:50, Sebastian Andrzej Siewior wrote: >> > On 2025-07-08 18:53:00 [-0700], Alexei Starovoitov wrote: >> >> From: Alexei Starovoitov <ast@xxxxxxxxxx> >> >> >> >> Introduce local_lock_lockdep_start/end() pair to teach lockdep >> >> about a region of execution where per-cpu local_lock is not taken >> >> and lockdep should consider such local_lock() as "trylock" to >> >> avoid multiple false-positives: >> >> - lockdep doesn't like when the same lock is taken in normal and >> >> in NMI context >> >> - lockdep cannot recognize that local_locks that protect kmalloc >> >> buckets are different local_locks and not taken together >> >> >> >> This pair of lockdep aid is used by slab in the following way: >> >> >> >> if (local_lock_is_locked(&s->cpu_slab->lock)) >> >> goto out; >> >> local_lock_lockdep_start(&s->cpu_slab->lock); >> >> p = ___slab_alloc(s, gfpflags, node, addr, c, orig_size); >> >> local_lock_lockdep_end(&s->cpu_slab->lock); >> >> >> >> Where ___slab_alloc() is calling >> >> local_lock_irqsave(&s->cpu_slab->lock, ...) many times, >> >> and all of them will not deadlock since this lock is not taken. >> > >> > So you prefer this instead of using a trylock variant in ___slab_alloc() >> > which would simply return in case the trylock fails? >> >> The code isn't always in a position to "simply return". On !RT I think we >> can at least assume that if we succeeded once, it means we're not a irq/nmi >> interrupting a locked context so we'll succeed the following attempts too. >> On RT IIUC the lock might be taken by someone else, so a trylock might fail >> (even if it should also mean we're in a context that can do a non-try lock). > > There is this parent check. If the parent check "allows" the allocation > then on !RT the trylock should always succeed. So the return "empty > handed" would be there but should not happen kind of thing. > > On RT this is different so local_lock_is_locked() will return false but > the trylock might fail if the lock is acquired by another task. > > But then with this change we do trylock from lockdep's point of view > while in reality we do the full locking including possible context > switch. > > That is why I don't like the part where we trick lockdep. > > If we the parent check we could trylock for !RT and normal lock for RT > what we actual do. > If there is no parent check then we could do "normal lock" on both > sides. So you mean the approach of v1 with local_lock_irqsave_check()? https://lore.kernel.org/bpf/20250501032718.65476-5-alexei.starovoitov@xxxxxxxxx/#t > Sebastian
