There is rwlock_padalign too.

Wouldn't it be better to add a standalone macro, lock2lock_object() or whatever, to sys/lock.h which maps a lock pointer to its lock object? This is pretty similar to the macro in D59456.

I see that some lock types are excluded from this list, but it's not clear why. buf_ring just asserts that the lock is held. It's fine to use a sleepable lock AFAICS. (And rmlocks can be sleepable anyway.)

Note that macros in this revision and D59456 are different. buf_ring supports lockless initialization, and thus this macro has extra casts. I really don't like that, but this is problem of the _Generic() feature. Alternative would be to provide a NULLLOCK define that would be a lock-typed NULL pointer. That won't require a big sweep over the kernel, there is only one consumer that passes NULL.

I see that some lock types are excluded from this list, but it's not clear why. buf_ring just asserts that the lock is held. It's fine to use a sleepable lock AFAICS. (And rmlocks can be sleepable anyway.)

IMHO, it is better not to throw unused and untested types into the macro. I agree that speculatively there is nothing that would prevent them working. However, I'd prefer to see them added upon first use case.

One other side effect of this support advertisement is that we actually encourage to use locks that are already known to have better alternatives. For example, for rmlock it is already clear that probably every its use in the network stack can be substituted by the net epoch and change is going to be beneficial. Adding rmlock here will also increase grep count of rmlock across kernel, skewing rough estimates on its usage.

IMO it is not worth fighting for the !KERNEL case to save one pointer.

Could BR_LOCK_ASSERT() include the check for br->lock not being NULL?

It is not about saving a pointer, it is about keeping it compilable.

If it is NULL, of course it would panic on NULL dereference and line number will point right here. We will lose only the diagnostic message. IMHO, not worth overloading the INVARIANTS kernel with extra branches just to provide a diagnostic message on a stupid mistake.

And how it prevents compilation?
You can include sys/_lock.h unconditionally. Or you could forward-declare struct lock_object.

What do we gain having unused pointer? The BR_LOCK_ASSERT() still needs to be nothing to get it compilable. IMHO, easier to just remove the unused member, rather then cruft extra stuff to make it compilable.

Note that this header is not supposed to be used by userland programs to read kernel buf_rings via kvm(3)! It is userland mode today is merely for testing, potentially if someone wants to have a buf_ring in a userland program.

We get less confusion at the debugging time for userspace. Also we get less #ifdefs in the sources, because for instance BR_LOCK_ASSERT() can be used without #ifdef _KERNEL around it.

The use of buf_ring as a userspace tool was my main reasoning for this suggestion.