On Wed, Jun 23, 2021 at 10:00:29PM +0300, Oded Gabbay wrote:
I understand the argument and I agree that for the generic case, the top of the stack can't assume anything. Having said that, in this case the SGL is encapsulated inside a dma-buf object.
But the scatterlist is defined to have a valid page. If in dma-bufs you can't do that dmabufs are completely broken. Apparently the gpu folks can somehow live with that and deal with the pitfals, but for dma-buf users outside of their little fiefdom were they arbitrarily break rules it simply is not acceptable.
Am 24.06.21 um 07:34 schrieb Christoph Hellwig:
On Wed, Jun 23, 2021 at 10:00:29PM +0300, Oded Gabbay wrote:
I understand the argument and I agree that for the generic case, the top of the stack can't assume anything. Having said that, in this case the SGL is encapsulated inside a dma-buf object.
But the scatterlist is defined to have a valid page. If in dma-bufs you can't do that dmabufs are completely broken. Apparently the gpu folks can somehow live with that and deal with the pitfals, but for dma-buf users outside of their little fiefdom were they arbitrarily break rules it simply is not acceptable.
The key point is that accessing the underlying pages even when DMA-bufs are backed by system memory is illegal. Daniel even created a patch which mangles the page pointers in sg_tables used by DMA-buf to make sure that people don't try to use them.
So the conclusion is that using sg_table in the DMA-buf framework was just the wrong data structure and we should have invented a new one.
But then people would have complained that we have a duplicated infrastructure (which is essentially true).
My best plan to get out of this mess is that we change the DMA-buf interface to use an array of dma_addresses instead of the sg_table object and I have already been working on this actively the last few month.
Regards, Christian.
On Thu, Jun 24, 2021 at 10:07:14AM +0200, Christian König wrote:
The key point is that accessing the underlying pages even when DMA-bufs are backed by system memory is illegal. Daniel even created a patch which mangles the page pointers in sg_tables used by DMA-buf to make sure that people don't try to use them.
Which is another goddamn layering violation of a subsystem that has no business at all poking into the scatterlist structure, yes.
So the conclusion is that using sg_table in the DMA-buf framework was just the wrong data structure and we should have invented a new one.
I think so.
But then people would have complained that we have a duplicated infrastructure (which is essentially true).
I doubt it. At least if you had actually talked to the relevant people. Which seems to be a major issue with what is going on GPU land.
My best plan to get out of this mess is that we change the DMA-buf interface to use an array of dma_addresses instead of the sg_table object and I have already been working on this actively the last few month.
Awesome! I have a bit of related work on the DMA mapping subsystems, so let's sync up as soon as you have some first sketches.
Btw, one thing I noticed when looking over the dma-buf instances is that there is a lot of duplicated code for creating a sg_table from pages, and then mapping it. It would be good if we could move toward common helpers instead of duplicating that all over again.
Am 24.06.21 um 10:12 schrieb Christoph Hellwig:
On Thu, Jun 24, 2021 at 10:07:14AM +0200, Christian König wrote:
The key point is that accessing the underlying pages even when DMA-bufs are backed by system memory is illegal. Daniel even created a patch which mangles the page pointers in sg_tables used by DMA-buf to make sure that people don't try to use them.
Which is another goddamn layering violation of a subsystem that has no business at all poking into the scatterlist structure, yes.
Completely agree, but it is also the easiest way to get away from the scatterlist as trasnport vehicle for the dma_addresses.
[SNIP]
My best plan to get out of this mess is that we change the DMA-buf interface to use an array of dma_addresses instead of the sg_table object and I have already been working on this actively the last few month.
Awesome! I have a bit of related work on the DMA mapping subsystems, so let's sync up as soon as you have some first sketches.
Don't start cheering to fast.
I've already converted a bunch of the GPU drivers, but there are at least 6 GPU still needing to be fixed and on top of that comes VA-API and a few others.
What are your plans for the DMA mapping subsystem?
Btw, one thing I noticed when looking over the dma-buf instances is that there is a lot of duplicated code for creating a sg_table from pages, and then mapping it. It would be good if we could move toward common helpers instead of duplicating that all over again.
Can you give an example?
Thanks, Christian.
On Thu, Jun 24, 2021 at 11:52:47AM +0200, Christian König wrote:
I've already converted a bunch of the GPU drivers, but there are at least 6 GPU still needing to be fixed and on top of that comes VA-API and a few others.
What are your plans for the DMA mapping subsystem?
Building a new API that allows batched DMA mapping without the scatterlist. The main input for my use case would be bio_vecs, but I plan to make it a little flexible, and the output would be a list of [dma_addr,len] tuples, with the API being flexible enough to just return a single [dma_addr,len] for the common IOMMU coalescing case.
Btw, one thing I noticed when looking over the dma-buf instances is that there is a lot of duplicated code for creating a sg_table from pages, and then mapping it. It would be good if we could move toward common helpers instead of duplicating that all over again.
Can you give an example?
Take a look at the get_sg_table and put_sg_table helpers in udmabuf. Those would also be useful in armda, i915, tegra, gntdev-dmabuf, mbochs in one form or another.
Similar for variants that use a contigous regions.
linaro-mm-sig@lists.linaro.org
-
Christian König -
Christoph Hellwig