In D58851#1400172, @kib wrote:

Hm, but if ZFS does not need the exclusively locked vnode for writes, does it need the same for setattr()? May be we should lower the requirements for VOP_SETATTR() same as for VOP_WRITE()?

Well, I was able to use these helpers for read reduction,
but I did not observe any performance improvement
compared to the mechanism already in the RDMA server,
which uses M_EXTPG mbuf lists. (It was either perfomance
neutral or slightly poorer performance.)

This looks fine, although it is hard to imagine
how the malloc() could fail, since the process
runs as root and can only be used on 64bit systems.

So, in general, I'm confused w.r.t. how the interface
is used when growing a file via writing.

Yes, as markj@ notes, the change is harmless
and mostly meaningless. The NFSVNO_EXSTRICTACCESS()
is a placeholder for some site that might not want to let
the file "owner" get access despite mode bits.

I am now testing using NFS over RDMA for read RPCs
in the NFS server and it seems to be working fine.

This gets ugly when another client modifies the directory
by adding or deleting an entry.
Once that happens, all these "logical offsets" become bogus.

Yea, sure. I've seen RFC#s written both ways.

Don't forget to update the date ".Dd".

In D58851#1377474, @kib wrote:

In D58851#1377389, @rmacklem wrote:

In D58851#1377388, @kib wrote:

"pgrbwt" means that the function waits for the page busy state to pass. Often the reason for this state waiting forever is that the thread that waits already owns the busy state on the page.

Does the hang occur in the buffer strategy method?

Yes. Btw, I think the main use for this helper is in the server
where the vnode won't be an NFS one, so if this isn't easily
avoided, I wouldn't worry about it. (In other words, use the
helper in the NFS server, but not the NFS client.)

So in the strategy routine you effectively have the same algorithm applied already by the buf layer. You can use b_pages[] directly.

The main change is using b_pages directly for read/write.

In D60044#1379367, @0mp wrote:

If the mountd program can already tell if it's missing essential files, then perhaps there it makes no sense to set required_files="/etc/exports" at all. The error will show up in logs, it's just that the error will come from the mountd program instead of the mountd service. I think that this would be cleaner than a implementing a complicated if statement conditional that is expressing what's already present in mountd. Also, this approach would allow us to avoid shipping an empty /etc/exports file that might mess with existing automation scripts. Not the end of the world in itself, but also potentially unnecessary work for users.

Not to mention that required_files="/etc/exports" is probably wrong anyway, because alternative exports files can be specified via mountd_flags.

tl;dr: I'd suggest removing the required_files="/etc/exports" from the service script completely and let mountd handle its requirements on its own.

In D60044#1378989, @markj wrote:

Is there a reason we shouldn't instead ship an empty /etc/exports file as part of the distribution, just like we do with, say, /etc/sysctl.conf?

I thought there was such a thing installed for a while?
(If I recall correctly, it was about a screenfull of comment that tried to explain
what needed to be in /etc/exports, but maybe I don't recall correctly.)

In D60044#1377996, @christos wrote:

In D60044#1377730, @rmacklem wrote:

In D60044#1377567, @rmacklem wrote:

I suppose this is ok. It would be nice to note
somewhere that /etc/exports is always needed
for NFSv4. (A /etc/zfs/exports only setup can
be done for NFSv3 only.)

It might be better to require /etc/exports when either
nfsv4_server_enable or nfsv4_server_only are "yes".

I'm not really experienced with NFS, so we could get more people to weigh in on this. @markj what do you think?

In D60044#1377567, @rmacklem wrote:

I suppose this is ok. It would be nice to note
somewhere that /etc/exports is always needed
for NFSv4. (A /etc/zfs/exports only setup can
be done for NFSv3 only.)

I suppose this is ok. It would be nice to note
somewhere that /etc/exports is always needed
for NFSv4. (A /etc/zfs/exports only setup can
be done for NFSv3 only.)

In D58851#1377388, @kib wrote:

"pgrbwt" means that the function waits for the page busy state to pass. Often the reason for this state waiting forever is that the thread that waits already owns the busy state on the page.

Does the hang occur in the buffer strategy method?

So, I just tried to use the helper function and the process gets
stuck waiting on"pgrbwt".
Any ideas?
Or is there more info I can try and get you?

I finally got around to debugging the backchannel
stuff. I ended up creating separate svc_rdma_backchannel_xxx()
functions instead of trying to overload the svc_vc_backchannel()
ones.

The main changes are using pages for the inline send/recv
buffers instead of malloc'd buffers, which google claims is
more efficient. I have not yet implemented RFC8797, which
should allow the use of the full 4K for servers that will allow it.

Added a 2nd argument to rpc_remove_mreduce() so
that it can, optionally, not free the mbuf.

Add a couple of mxxx = NULL statements to avoid
a double free of the mbuf lists, as suggested by Vinicius.

These definitions should be taken verbatim from the
RFC (8881). To be honest, the only folk who actually
know what they mean work for Microsoft.
(NFSv4 ACLs are based on Windows ACLs and what
is in the RFC is taken verbatim from a Microsoft
document.)

Use a separate variable called reduce_pos for the
reduction position, which is now calculated by the
NFS glue code, as suggested by Vinicius.

Recent changes to "fix" handling of retries introduced
the bug Vinicius spotted. It actually didn't break anything
for the non-retry case, since it just meant the "reduce" mbuf
wasn't removed from the copy of the mbuf request list.

This update fixes a bunch of issues.

Looks good, but I'm no shell script guy, so hopefully
someone else can review it.

Oh, I should have noted that I either wrote the code
or cribbed it from Linux sources that carry the same
dual license as the OFED sources.

In D58851#1352085, @kib wrote:

In D58851#1351887, @rmacklem wrote:

In D58851#1351758, @kib wrote:

I looked more carefully at the issue of the interaction between vm_fault_quickXXX() and map_remove_YYY(). It could be implemented using the observation that when pmap_extract_and_hold() succeeds but the resulting page is busy, we should fall back to the vm_fault_hold() path for that page. This requires some kind of interlock between the busy state and wiring.

I even started implementing it, but then I considered what would be the consequence of not implementing. Basically, to get advantage of that situation, userspace needs to construct some situation. For instance, it needs to map a file shared, and then do read(2) from pipe into the mapping. Similarly, for physio. IMO if userspace is so much dedicated to cause this inconsistency, we might allow it. The VM code is simpler, and user is causing only self-inflicted damage. I do not see a problem with it.

On the other hand, the pmap_remove_all/write() calls make the regular file io with read(2) or write(2), or normal userspace writes through the mapping, atomic with regard to the NFS over RDMA io RPCs. I believe it is good enough.

Well, if this interface is not used, the NFS server does a..
VOP_READ() with a shared vnode lock
or
VOP_WRITE() with either a shared or exclusively locked vnode

(depending on whether the fs sets MNTK_SHARED_WRITES)

No range locking.

So, ideally, this interface would provide the same semantics.
If not, someone, someday, will go "when I mount rdma this weird.. happens".
I don't see that when I do a tcp mount.

What I discussed above is not about the range locking. It is about some very specific user buffer configurations that probably cannot be created by real applications, and cannot be utilized for any useful work. Also, I believe that the issue is not limited to the proposed interface, but also exists e.g. for buffered reads. My point is that it does not corrupt the kernel state, and is irrelevant practically for correctness.

In D58851#1351758, @kib wrote:

I looked more carefully at the issue of the interaction between vm_fault_quickXXX() and map_remove_YYY(). It could be implemented using the observation that when pmap_extract_and_hold() succeeds but the resulting page is busy, we should fall back to the vm_fault_hold() path for that page. This requires some kind of interlock between the busy state and wiring.

I even started implementing it, but then I considered what would be the consequence of not implementing. Basically, to get advantage of that situation, userspace needs to construct some situation. For instance, it needs to map a file shared, and then do read(2) from pipe into the mapping. Similarly, for physio. IMO if userspace is so much dedicated to cause this inconsistency, we might allow it. The VM code is simpler, and user is causing only self-inflicted damage. I do not see a problem with it.

On the other hand, the pmap_remove_all/write() calls make the regular file io with read(2) or write(2), or normal userspace writes through the mapping, atomic with regard to the NFS over RDMA io RPCs. I believe it is good enough.

In D58851#1351260, @kib wrote:

IOMMU has nothing to do with this function. It does not even look into the bus address space.

Oops, I confused this with what I need for the client, which is your _bus_dmamap_load_ma[] that maps pages to
configuous dma space.

If it isn't hard to do, it would be nice if vm_object_take_pages_for_io()
had a flag option that indicated if it should return failure if there is
no IOMMU hardware. (Or is there an easy way for the caller to check
before calling this function?)