PeterZ, may I summon you. On 2025-01-28 10:50:33 [-0800], Alexei Starovoitov wrote: > On Tue, Jan 28, 2025 at 9:21 AM Sebastian Andrzej Siewior > <bigeasy@xxxxxxxxxxxxx> wrote: > > > > On 2025-01-23 19:56:52 [-0800], Alexei Starovoitov wrote: > > > Usage: > > > > > > local_lock_t lock; // sizeof(lock) == 0 in !RT > > > local_lock_irqsave(&lock, ...); // irqsave as before > > > if (local_trylock_irqsave(&lock, ...)) // compilation error > > > > > > local_trylock_t lock; // sizeof(lock) == 4 in !RT > > > local_lock_irqsave(&lock, ...); // irqsave and active = 1 > > > if (local_trylock_irqsave(&lock, ...)) // if (!active) irqsave > > > > so I've been looking at this for a while and I don't like the part where > > the type is hidden away. It is then casted back. So I tried something > > with _Generics but then the existing guard implementation complained. > > Then I asked myself why do we want to hide much of the implementation > > and not make it obvious. > > Well, the idea of hiding extra field with _Generic is to avoid > the churn: > > git grep -E 'local_.*lock_irq'|wc -l > 42 This could be also hidden with a macro defining the general body and having a place holder for "lock primitive". > I think the api is clean enough and _Generic part is not exposed > to users. > Misuse or accidental usage is not possible either. > See the point: > if (local_trylock_irqsave(&lock, ...)) // compilation error > > So imo it's a better tradeoff. > > > is this anywhere near possible to accept? > > Other than churn it's fine. > I can go with it if you insist, > but casting and _Generic() I think is cleaner. > Certainly a bit unusual pattern. > Could you sleep on it? The cast there is somehow… We could have BUILD_BUG_ON() to ensure a stable the layout of the structs… However all this is not my call. PeterZ, do you have any preferences or an outline what you would like to see here? > I can do s/local_trylock_t/localtry_lock_t/. > That part is trivial. Sebastian
