On 05/18/2018 05:00 AM, Christoph Hellwig wrote:
On Thu, May 17, 2018 at 07:01:30PM -0400, Don Dutile wrote:
Additionally, I believe dma-debug has a bug:
-- doing a check on an op when there is no op in the dma-ops struct is not correct:
No, that is a feature.
Note: above: debug_dma_sync is called indep of whether an op function exists.
For hfi1 & qib & rxe -- which use dma-virt-ops for non-IOMMU-enabled configs,
the sync_syngle_for_cpu ops does not exist, yet debug_dma_sync is still called.
Yes, if you call dma ops you better make sure you respect the invariants.
This is the only way to get the code right for the 90% case where people
develop on x86 with cache coherent DMA but do calls that only exist
on non-coherent implementations.
So, if I re-state the above correctly, it's not an issue on x86 (a cache-coherent arch, where we saw
this message occurred), but it would be on a non-cache-coherent arch.
The 'feature' warns the caller even when run on an arch that its not a true issue/warning.
So, to Jason's question as for-rc, no, unless someone has a case of running RDMA on a non-cache-coherent arch... which seems counter-intuitive -- high speed RDMA on low-speed/cache-incorehent arch.
Christoph:
Would you object to a patch that more clearly states the above, i.e., still generates the warning but delineates that it's not an error on the given arch, but potentially other arch's?
I'm getting a fairly decent set of lib/dma-debug bz's atm (since I backported latest dma-map refactoring
& all of lib/dma-debug to RHEL-7), and I'd like to easily discern what needs to be fixed asap, e.g., not checking return value of dma_map(), vs warnings (that will still be bz'd) I don't have to prioritize over more pressing bz's.
Having the warning provide a bit more info in that area, as suggested above, would help.
Thanks for review & feedback.
- Don
