I fixed this in D2642 by modifying the rwlock probes to pass the correct arguments as documented in Solaris' lockstat provider page: https://wikis.oracle.com/display/DTrace/lockstat+Provider

Do you have a preference for either approach?

I think you can get rid of spin_cnt and sleep_cnt entirely here: spin_cnt > sleep_cnt is always true at this point since spin_cnt is incremented at least once per loop iteration, and sleep_cnt is incremented at most once per loop iteration.

Same as above - I believe spin_cnt and sleep_cnt can be removed.

I like your approach better.
So, it seems that with your change we conform to the Solaris documentation except for rw-spin extension?

Ah, another thing is that we may report both the spinning and sleeping events for adaptive locks while on Solaris it's either-or.
I guess this results from different implementations of adaptive behavior.

BTW, it would probably make sense for rw-spin to provide exactly the same arguments as rw-block?

Can't there be a case where spin_cnt == sleep_cnt ?
In that case we would fire the spin event unnecessarily.

ditto :-)

Yup, I believe so. My plan is to next write a lockstat provider man page so that these differences can be documented (as was done with some other providers).

I think it makes sense to reuse rw-block's arguments; the one difference is that args[1] should be the time spent spinning rather than the time spent blocked.

Oh, oops. :(
I somehow convinced myself that that was ok (i.e. firing the spin event with spin_cnt == sleep_cnt), since lockstat(1)'s event predicates would filter them out, but that's not true - in fact there is no predicate on the mtx spin event.

I made this same mistake in my lockstat code review, I'll fix it in the next upload.

This should be lockstat:::sx-spin?

These probes should also use arguments to determine whether a reader or writer is taking or releasing the lock.

Shouldn't this be LOCKSTAT_READER?

I don't think the parens are needed here.