+cc Christoph & Marek On 2024/11/6 4:11, Jesper Dangaard Brouer wrote: ... >>> >>>> I am not sure if I understand the reasoning behind the above suggestion to 'wait >>>> and see if this actually turns out to be a problem' when we already know that there >>>> are some cases which need cache kicking/flushing for the waiting to work and those >>>> kicking/flushing may not be easy and may take indefinite time too, not to mention >>>> there might be other cases that need kicking/flushing that we don't know yet. >>>> >>>> Is there any reason not to consider recording the inflight pages so that unmapping >>>> can be done for inflight pages before driver unbound supposing dynamic number of >>>> inflight pages can be supported? >>>> >>>> IOW, Is there any reason you and jesper taking it as axiomatic that recording the >>>> inflight pages is bad supposing the inflight pages can be unlimited and recording >>>> can be done with least performance overhead? >>> >>> Well, page pool is a memory allocator, and it already has a mechanism to >>> handle returning of memory to it. You're proposing to add a second, >>> orthogonal, mechanism to do this, one that adds both overhead and >> >> I would call it as a replacement/improvement for the old one instead of >> 'a second, orthogonal' as the old one doesn't really exist after this patch. >> > > Yes, are proposing doing a very radical change to the page_pool design. > And this is getting proposed as a fix patch for IOMMU. > > It is a very radical change that page_pool needs to keep track of *ALL* in-flight pages. I am agreed that it is a radical change, that is why it is targetting net-next tree instead of net tree even when there is a Fixes tag for it. If there is a proper and non-radical way to fix that, I would prefer the non-radical way too. > > The DMA issue is a life-time issue of DMA object associated with the > struct device. Then, why are you not looking at extending the life-time It seems it is not really about the life-time of DMA object associated with the life-time of 'struct device', it seems to be the life-time of DMA API associated with the life-time of the driver for the 'struct device' from the the opinion of experts from IOMMU/DMA subsystem in [1] & [2]. I am not sure what is reasoning behind the above, but the implementation seems to be the case as mentioned in [3]: __device_release_driver -> device_unbind_cleanup -> arch_teardown_dma_ops 1. https://lkml.org/lkml/2024/8/6/632 2. https://lore.kernel.org/all/20240923175226.GC9634@xxxxxxxx/ 3. https://lkml.org/lkml/2024/10/15/686 > of the DMA object, or at least detect when DMA object goes away, such > that we can change a setting in page_pool to stop calling DMA unmap for > the pages in-flight once they get returned (which we have en existing > mechanism for). To be honest, I was mostly depending on the opinion of the experts from IOMMU/DMA subsystem for the correct DMA API usage as mentioned above. So I am not sure if skipping DMA unmapping for the inflight pages is the correct DMA API usage? If it is the correct DMA API usage, how to detect that if DMA unmapping can be skipped?
