linux-kernel - Re: [RFC PATCH v1] dma-buf: heaps: move the verification of heap

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]

Message-ID: <20240603172148.gb7txpg2ya43jyxn@oppo.com>
Date: Tue, 4 Jun 2024 01:21:48 +0800
From: Hailong Liu <hailong.liu@...o.com>
To: John Stultz <jstultz@...gle.com>
CC: Sumit Semwal <sumit.semwal@...aro.org>, Benjamin Gaignard
	<benjamin.gaignard@...labora.com>, Brian Starkey <Brian.Starkey@....com>,
	"T.J. Mercier" <tjmercier@...gle.com>, Christian König
	<christian.koenig@....com>, <21cnbao@...il.com>,
	<linux-media@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
	<linaro-mm-sig@...ts.linaro.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v1] dma-buf: heaps: move the verification of
 heap_flags to the corresponding heap

On Mon, 03. Jun 09:01, John Stultz wrote:
> On Mon, Jun 3, 2024 at 4:40 AM <hailong.liu@...o.com> wrote:
> >
> > From: "Hailong.Liu" <hailong.liu@...o.com>
> >
> > This help module use heap_flags to determine the type of dma-buf,
> > so that some mechanisms can be used to speed up allocation, such as
> > memory_pool, to optimize the allocation time of dma-buf.
>
> This feels like it's trying to introduce heap specific flags, but
> doesn't introduce any details about what those flags might be?
>
> This seems like it would re-allow the old opaque vendor specific heap
> flags that we saw in the ION days, which was problematic as different
> userspaces would use the same interface with potentially colliding
> heap flags with different meanings. Resulting in no way to properly
> move to an upstream solution.
>
> With the dma-heaps interface, we're trying to make sure it is well
> defined. One can register a number of heaps with different behaviors,
> and the heap name is used to differentiate the behavior. Any flags
> introduced will need to be well defined and behaviorally consistent
> between heaps. That way when an upstream solution lands, if necessary
> we can provide backwards compatibility via symlinks.
>
> So I don't think this is a good direction to go for dma-heaps.
>
> It would be better if you were able to clarify what flag requirements
> you need, so we can better understand how they might apply to other
> heaps, and see if it was something we would want to define as a flag
> (see the discussion here for similar thoughts:
> https://lore.kernel.org/lkml/CANDhNCoOKwtpstFE2VDcUvzdXUWkZ-Zx+fz6xrdPWTyciVXMXQ@mail.gmail.com/
> )
>
> But if your vendor heap really needs some sort of flags argument that
> you can't generalize, you can always implement your own dmabuf
> exporter driver with whatever ioctl interface you'd prefer.

Thanks for your reply. Let’s continue our discussion here instead
of on android-review. We aim to enhance memory allocation on each
all heaps. Your pointer towards heap_flags used in /dev/ion for heap
identification was helpful.

We now aim to improve priority dma-buf allocation. Consider android
animations scene:

when device is in low memory, Allocating dma-buf as animation
buffers enter direct_reclaimation, longer allocation time result in a
laggy UI. But if we know the usage of the dma-buf, we can use some
mechanisms to boost, e.g. animation-memory-pool.

However, dma-buf usage identification becomes a challenge. A potential
solution could be heap_flags. the use of heap_flags seems ugly and
contrary to the intended design as you said, How aboult extending
dma_heap_allocation_data as follows?

struct dma_heap_allocation_data {
	__u64 len;
	__u32 fd;
	__u32 fd_flags;
	__u64 heap_flags;
	__u64 buf_flags: // buf usage
};

Do you have any better suggestion?

>
> thanks
> -john

--

Best Regards,
Hailong.