linux-kernel - Re: [PATCH] dma-buf: refcount the attachment for cache_sgt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 12 Jun 2019 01:02:14 -0700
From:   Nicolin Chen <nicoleotsuka@...il.com>
To:     "Koenig, Christian" <Christian.Koenig@....com>
Cc:     "sumit.semwal@...aro.org" <sumit.semwal@...aro.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linaro-mm-sig@...ts.linaro.org" <linaro-mm-sig@...ts.linaro.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "daniel.vetter@...ll.ch" <daniel.vetter@...ll.ch>
Subject: Re: [PATCH] dma-buf: refcount the attachment for cache_sgt_mapping

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.

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?

Thanks
Nicolin