Definitely my miss. Thanks for fixing!

This looks good to me. Thanks a lot! Please commit if it passes tests.

spl calls is not something that gives any maintenance burden. They have been defined to nop for years. Of course it makes sense to cleanup those that aren't mentioned anywhere just like rS364918 did. However, I don't think just removing these nop defines is a holy grail we should aim for.

In D26027#577448, @mjg wrote:

I remember UMA warning about relevant allocations without even attempting to sleep, but maybe my memory is off here.

Add same check to UMA.

In D26027#577390, @kib wrote:

Why it is not WITNESS_WARN() ? Perhaps witness_warn should take td_no_sleeping as arbitrary non-sleepable lock as well.

In D22691#569217, @kevans wrote:

In D22691#569148, @melifaro wrote:

The resulting if_busy() / if_unbusy() / if_busy_wait() semantics looks very similar to sx locking. I'm wondering why not simply use sx locks as @glebius suggested?

I think, through the course of this, I became convinced that sx usage complicates some possible future busy'ers that would no longer be able to hold a mutex across calls here.

Why don't you just remove call to bind() and listen on unbound socket? Then you get its address with getsockname() and use it in the client side?

I called Cronyx and here is information I got. All the devices from http://cronyx.ru/hardware/wan.html can be ordered and purchased, even the ISA ones. The FreeBSD drivers were discontinued back when rik@ quit them, and couple years later the Linux drivers were discontinued, too. Right now there are no driver developers at Cronyx at all. However, they got small but constant demand for all of the devices they produce. They can't disclose who buys them and what OS do they run. The tech support guy was responsive and nice to talk to. We discussed a hypothetical situation when someone buys for example Tau-PCI, runs FreeBSD and encounters a problem. If I would offer my help to this user, would Cronyx offer their help to me? The guy on the other side of the phone repeated that they don't have driver developers now, but they would be able to provide some information on hardware to a FreeBSD person who is maintaining the drivers.

Why do these functions logically belong to ffs_subr? The ffs_load_inode() definitely belongs to ffs_vfsops since it is used only there and with the change it becomes static. Other moved functions are associated with unmounting, which IMHO also belongs to vfsops rather than to subr. What else outside of real FFS code is going to use them? AFAIU, the code that is ffs_subr is something that is shared between real FFS, non-kernel code and GEOM_LABEL.

This looks correct to me.

In D24552#540209, @hselasky wrote:

Can we use a time-based expiry, like ticks?

In D24408#536859, @rmacklem wrote:

Oh, one other thing.
I can never remember without doing a build, but maybe a
MODULE_DEPEND(xxx, xdr, 1, 1, 1)
needs to be added to all the modules that currently have
MODULE_DEPEND(xxx, krpc, 1, 1, 1)?

In D24408#537610, @freqlabs wrote:

Out of curiosity, how does options XDR (in a kernel config) avoid a name collision with the XDR typedef?

Since separating malloc types goes more and more complicated, follow
Rick's suggestion and abandon that idea. Move MALLOC_DECLARE(M_RPC)
into XDR. Three commits remain:

In D24408#537268, @mmacy wrote:

I don't see anything wrong with this review, but FYI there is now a live upstream https://github.com/openzfs/zfs/commit/9f0a21e6411aa0bac23fba0ddb220342a48c7cc7 and the code in HEAD will some time later this year be replaced with vendor code. Most of the changes to ZFS in HEAD at this point will only live on through MFCs to stable/12 and stable/11.

In D24408#537225, @rmacklem wrote:

It seems that a lot of the complexity of this patch is a result of defining
a new malloc() type.
Is a new type really needed?
I'll leave it up to you, but I'd just stick with M_RPC everywhere and
that would keep the patch a lot simpler.

Address Rick's comment and split into more commits.

In D24408#536854, @rmacklem wrote:

My only concern is that there is a mem_free() call done
somewhere in the rpc code for one of these malloc()s,
but I couldn't find one.
If this happens, it will be easily spotted during testing
when "vmstat -m" generates crap for "rpc" and "xdr".

In D24408#536859, @rmacklem wrote:

Oh, one other thing.
I can never remember without doing a build, but maybe a
MODULE_DEPEND(xxx, xdr, 1, 1, 1)
needs to be added to all the modules that currently have
MODULE_DEPEND(xxx, krpc, 1, 1, 1)?

Sorry, totally forgot about this. Will call them today/tomorrow.

Thanks a lot for the change! I'd really wish that scopes of local variables isn't increased. If it is possible to avoid doing that, I'd really appreciate.

Just a general note, that I really dislike moving of local variables to a greater scope. Yes, I know that a document from the previous century named style(9) suggests to do that. But doing it this way makes code harder to read! Change my mind.

These cards seem to be still produced and sold http://cronyx.ru/hardware/wan.html

In D23788#523247, @neel_neelc.org wrote:

I still get "ngctl: send msg: Invalid argument" errors with the commands in the Test Plan. Am I doing anything wrong?

This can make sense in certain setups. However, since originally node provided writable copies to each of "many" hooks, we can't change that. This can be configured as a node option, if sysadmin is sure that nodes downstream of "many" hooks are fine with read only mbufs.