Am 12.06.19 um 10:02 schrieb Nicolin Chen:

Hi Christian,

Thanks for the quick reply.

On Wed, Jun 12, 2019 at 07:45:38AM +0000, Koenig, Christian wrote:

...

Am 12.06.19 um 03:22 schrieb Nicolin Chen:

...

Commit f13e143e7444 ("dma-buf: start caching of sg_table objects v2") added a support of caching the sgt pointer into an attach pointer to let users reuse the sgt pointer without another mapping. However, it might not totally work as most of dma-buf callers are doing attach() and map_attachment() back-to-back, using drm_prime.c for example: drm_gem_prime_import_dev() { attach = dma_buf_attach() { /* Allocating a new attach */ attach = kzalloc(); /* .... */ return attach; } dma_buf_map_attachment(attach, direction) { /* attach->sgt would be always empty as attach is new */ if (attach->sgt) { /* Reuse attach->sgt */ } /* Otherwise, map it */ attach->sgt = map(); } }

So, for a cache_sgt_mapping use case, it would need to get the same attachment pointer in order to reuse its sgt pointer. So this patch adds a refcount to the attach() function and lets it search for the existing attach pointer by matching the dev pointer.

I don't think that this is a good idea.

We use sgt caching as workaround for locking order problems and want to remove it again in the long term.

Oh. I thought it was for a performance improving purpose. It may be a misunderstanding then.

...

So what is the actual use case of this?

We have some similar downstream changes at dma_buf to reduce the overhead from multiple clients of the same device doing attach() and map_attachment() calls for the same dma_buf.

I don't think that this is a good idea over all. A driver calling attach for the same buffer is doing something wrong in the first place and we should not work around this in the DMA-buf handling.

We haven't used DRM/GRM_PRIME yet but I am also curious would it benefit DRM also if we reduce this overhead in the dma_buf?

No, not at all.

Regards, Christian.

Thanks Nicolin