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?
(These Netperf systems use a NFS root fs and I'm not sure if
there is a swap set up, so I'm not sure what it would take to
get a dump?)

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?)

In D58181#1334837, @fuz wrote:

This would have been nicer if the flag was named "noucache" with inverse
semantics as with nodump. (so "ucache" to turn it off)
nodontcache is a weird double negation.

Maybe this can still be revised?

In the NFS game there are two things...

• The RFCs. They don't define exactly when an NFS server should return ESTALE, so I don't think either the current behaviour or the behaviour with this patch is incorrect.
• Linux. It has become the "defacto standard" for NFS. (Solaris once was the "defacto standard" but??)

So, if this patch results in behaviour consistent with
Linux, I think it is reasonable to do.

I also updated the comment in kgss_load().