I'd suggest instead to give up on this new implementation as sx locks can directly provide this information, e.g., you can implement rwsem_is_contended() simply as:

static inline int
rwsem_is_contended(struct rw_semaphore *sem)
{
	return (SX_READ_VALUE(&sem->sx) & SX_LOCK_WAITERS);
}

(perhaps adding !! if Linux consumers expect 0 or 1)

In D56443#1298661, @olce wrote:

I'd suggest instead to give up on this new implementation as sx locks can directly provide this information, e.g., you can implement rwsem_is_contended() simply as:

static inline int
rwsem_is_contended(struct rw_semaphore *sem)
{
	return (SX_READ_VALUE(&sem->sx) & SX_LOCK_WAITERS);
}

(perhaps adding !! if Linux consumers expect 0 or 1)

I would highly advise against this. Don't start splattering lock internals without KPI over the kernel. Seems not a good idea to me.

The SX_READ_VALUE define should likely be moved to kern_sx.c, its only consumer so far--or at least seen as such. If you really want this, add a tiny (inline) wrapper to the sx implementation and then use that as a proper KPI from here in LinuxKPI.

According to style the check is not !! but != 0

In D56443#1298756, @bz wrote:

I would highly advise against this. Don't start splattering lock internals without KPI over the kernel. Seems not a good idea to me.

Sorry if that wasn't clear, but that reaction is completely missing my point, which is that sx(9) locks can readily provide the information of whether there are waiters, and there's no need to graft a machinery just for that purpose on top of it. I stand firm by it.

The snippet was just to illustrate how to implement retrieving the information (hence the e.g.). It goes without saying that sx internals should not be leaked, and that the best place for this code is in fact a sx(9) routine. As I was in a hurry when commenting, I just didn't make it explicit, but I'm pretty sure this point is obvious to everyone here.

The SX_READ_VALUE define should likely be moved to kern_sx.c, its only consumer so far--or at least seen as such.

Even kern_sx.c does not use SX_READ_VALUE() everywhere it should (sc_lock direct accesses), and conversely internals of struct sx, including sc_lock, will stay exposed to the outside to allow for some inline implementations, regardless of whether SX_READ_VALUE() is hidden, and they are currently in use by LinuxKPI and ZFS. So moving SX_READ_VALUE() is not nearly enough.

That said, I agree this kind of leakage is a sorry state of affairs that should be fixed going forward.

If you really want this, add a tiny (inline) wrapper to the sx implementation and then use that as a proper KPI from here in LinuxKPI.

There's not the slightest doubt that leveraging sx(9) information is the best way.

According to style the check is not !! but != 0

Yes. !! is indeed a Linuxism (also used elsewhere).

In D56443#1298877, @olce wrote:

In D56443#1298756, @bz wrote:

I would highly advise against this. Don't start splattering lock internals without KPI over the kernel. Seems not a good idea to me.

Sorry if that wasn't clear, but that reaction is completely missing my point, which is that sx(9) locks can readily provide the information of whether there are waiters, and there's no need to graft a machinery just for that purpose on top of it. I stand firm by it.

The snippet was just to illustrate how to implement retrieving the information (hence the e.g.). It goes without saying that sx internals should not be leaked, and that the best place for this code is in fact a sx(9) routine. As I was in a hurry when commenting, I just didn't make it explicit, but I'm pretty sure this point is obvious to everyone here.

The SX_READ_VALUE define should likely be moved to kern_sx.c, its only consumer so far--or at least seen as such.

Even kern_sx.c does not use SX_READ_VALUE() everywhere it should (sc_lock direct accesses), and conversely internals of struct sx, including sc_lock, will stay exposed to the outside to allow for some inline implementations, regardless of whether SX_READ_VALUE() is hidden, and they are currently in use by LinuxKPI and ZFS. So moving SX_READ_VALUE() is not nearly enough.

That said, I agree this kind of leakage is a sorry state of affairs that should be fixed going forward.

If you really want this, add a tiny (inline) wrapper to the sx implementation and then use that as a proper KPI from here in LinuxKPI.

There's not the slightest doubt that leveraging sx(9) information is the best way.

According to style the check is not !! but != 0

Yes. !! is indeed a Linuxism (also used elsewhere).

Sorry for the firm comment; I believe we agree on the overall solution. Thank you for the more detailed explanation!

Thank you for the great suggestion! I implement it and just uploaded the patch.

Ah yes, this is nicer.