Details
- Reviewers
- None
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Small fixes; the version worked in my testing.
This diff includes the test driver I used to verify the code, it is not intended for upstream.
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?)
For our purposes, there is no point in doing this if it requires a bounce
buffer or such. (The fallback is pretty good. It just ends up doing
copying in VOP_READ().)
Otherwise, it looks good to me. I'll email you and Vinicius a rough partial
patch for using it in the NFS server.
Otherwise, the interface looks
IOMMU has nothing to do with this function. It does not even look into the bus address space.
Also, any decent network card would handle the reasonably sized rdma s/g list fine, without IOMMU assistance.I think you should not worry about bounce fallbacks at all, they normally would not happen.
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.
Also, any decent network card would handle the reasonably sized rdma s/g list fine, without IOMMU assistance.I think you should not worry about bounce fallbacks at all, they normally would not happen.
For Mellanox, max_sg == 31 and max_sg_rd == 30 and RFC8267 recommends a limit of 16.
So, if pages aren't mapped into contiguous bus dma space, that means 64K.
For Vinicius's server code, I think he loops around doing multiple RDMA Writes to transfer
it all, so it isn't a big issue.
However, for the client, the entire transfer has to be set up before doing the RPC,
so that limits the Read RPC to 64K. I am hoping your _bus_dmamap_load_ma[]
function will allow larger RPC sizes, so it is for that that I need to know if there
is an IOMMU?
| sys/vm/vm_object.c | ||
|---|---|---|
| 2617 | Can we use an iterator here? Note that there is a vm_page_grab_valid_iter(). | |
| 2623 | Are we leaking the PIP counter here? And, why exactly is it necessary to bump the PIP counter in the first place? | |
| 2643 | Do you need to use vm_page_try_remove_{all,write}()? That is, suppose one of the pages is wired; the holder of the wiring reference may map it. Does vm_object_take_pages_for_io() provide some guarantee that this won't happen? | |
| sys/vm/vm_object.c | ||
|---|---|---|
| 2623 | What would prevent freeing and reusing of the vm_object otherwise? For instance, everything that could is dropped during vm_page_busy_sleep() call from vm_page_busy_acquire(). | |
| 2643 | We definitely need pmap_remove_write() to ensure that io op is atomic. I am not sure what troubles pmap_remove_write() would cause for wired mappings. For the VM_PROT_WRITE case, it is highly desirable to not allow userspace to observe the intermediate state during the write, I believe. For wired pages, pmap_remove_all() only causes soft faults. I think it is fine on principle, e.g. there is no guarantee that CPUs would not cause spurious page faults on the present PTEs. At least, some generations of Intel processors were known to do that. | |
| sys/vm/vm_object.c | ||
|---|---|---|
| 2623 | I would presume that the caller is responsible for this. The vm_page_grab_* helpers do not try to handle this AFAICS. | |
| 2643 |
I mean, if some kernel consumer holds a wiring of a page, then it implicitly has permission to modify the page. That is, what prevents vm_object_take_pages_for_io() from racing with a thread calling vm_fault_quick_hold_pages()? If pmap_extract_and_hold() sees a writeable mapping of the page, it will wire the page and return it, and the consumer is allowed to modify that page. To avoid this race, vm_object_take_pages_for_io() must not return wired pages. | |
| sys/vm/vm_object.c | ||
|---|---|---|
| 2623 | vm_page_grab() is somewhat internal to the VM subsystem. This interface is a convenient wrapper around the subsystem primitives. From this PoV, it is desirable to not expose consumers to such details. If you think that the interface might be useful without pip increment, I can create yet another wrapper around this function, which would e.g. take a locked vnode, lock the object, and increment the pip. | |
| 2643 | Not allowing to return wired pages is not feasible, it means that the filesystem cannot have a VMIO buffer owning the range of the page for this function to succeed. You probably pointed to vm_page_try_remove_all/write(). I do not think it is usable there due to the above. And, we do have the same issue in vfs_busy_pages(), for instance? I think this problem should be handled globally, might be by using an shared sx around vm_fault_quick(), and taking it exclusively in specific places. It might cause ordering issues, I did not thought about it hard. | |
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.
Yes, I recognized that it didn't have anything to do with range locking.
I was just stating exactly what the NFS server does now, in case VOP_READ()/VOP_WRITE()
behaviour (what you call buffered I/O) was not affected by this issue the same
way as your page based system is.
(As I think you know, I do not understand the vm side of fs caching, but I can follow
what VOP_READ()/VOP_WRITE() does.)
It is not that simple. There might be IOMMUs in the system, but dma might still use bounce (or any other platform-specific mechanism) for the given device. I think the only reliable method would be to try bus_dmamap_load_ma() and do something else if the load cannot be performed.