kqueue: add knlist_init_sx() for shared/exclusive locks

[rew]

Summary

The intended consumer for this patch is https://reviews.freebsd.org/D33401.

[pauamma_gundo.com]

Other than my question about "may", the grammar and spelling LGTM. I'll leave reviewing the source and the consistency between them to domain experts.

share/man/man9/kqueue.9

328

Something for a content expert to assess; is "may" right here? That is, are both options about equally appropriate? Or would be "should", "must", or "needs to" more appropriate?

[kevans]

share/man/man9/kqueue.9

328

Right, the strength of 'may' is fine for this. It's a good idea and you'll probably prefer it if you have an sx, but there's nothing inherently wrong with not using it for whatever reason.

[rew]

• bump manpage date

• use LA_LOCKED to be consistent with the other knlist assert routines

[pauamma_gundo.com]

Manual page LGTM, but holding approval until someone/somegroup is added for kernel code review.

[kib]

Is it for single consumer? Did you tried to look in the source tree for any more uses (I doubt that we have that)?

[rew]

In D33400#818876, @kib wrote:

Is it for single consumer?

Yes, OpenZFS would be the only consumer. I went in this direction because ZFS uses an sx lock to protect the zvol state. Most, if not all, users of the knlist_init() routines use a mutex to protect their device state. Using the global knlist lock instead of the zvol state lock didn't seem like the correct approach though?

Did you tried to look in the source tree for any more uses (I doubt that we have that)?

I didn't do an exhaustive audit but, I didn't see anything right off.

As a side note - knlist_init_rw_reader() was last used in afa85850e79c1839ec33efa1138206687b952cfa, I'm guessing it can be removed now.

[kib]

In D33400#818950, @rew wrote:

In D33400#818876, @kib wrote:

Is it for single consumer?

Yes, OpenZFS would be the only consumer. I went in this direction because ZFS uses an sx lock to protect the zvol state. Most, if not all, users of the knlist_init() routines use a mutex to protect their device state. Using the global knlist lock instead of the zvol state lock didn't seem like the correct approach though?

I meant something different. There is a very little need to add knlist_init_sx(9), when we only have single consumer. Put the code into ZFS OS-specific file.

Did you tried to look in the source tree for any more uses (I doubt that we have that)?

I didn't do an exhaustive audit but, I didn't see anything right off.

Right.

As a side note - knlist_init_rw_reader() was last used in afa85850e79c1839ec33efa1138206687b952cfa, I'm guessing it can be removed now.

Sure, this should be a separate commit.

[pauamma_gundo.com]

Assuming consistent with source.

share/man/man9/kqueue.9

27

Will need another bump.

[rew]

In D33400#818953, @kib wrote:

I meant something different. There is a very little need to add knlist_init_sx(9), when we only have single consumer. Put the code into ZFS OS-specific file.

Will do - it did feel slightly odd to drop it in here since it's likely to only be used by ZFS.

Thanks for looking at this.