Messages in this thread | | | Date | Thu, 11 Apr 2024 16:43:46 +0800 | Subject | Re: [PATCH] dma-buf: add DMA_BUF_IOCTL_SYNC_PARTIAL support | From | Rong Qianfeng <> |
| |
在 2024/4/10 23:07, T.J. Mercier 写道: > [You don't often get email from tjmercier@google.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > On Wed, Apr 10, 2024 at 7:22 AM Christian König > <christian.koenig@amd.com> wrote: >> Am 09.04.24 um 18:37 schrieb T.J. Mercier: >>> On Tue, Apr 9, 2024 at 12:34 AM Rong Qianfeng <11065417@vivo.com> wrote: >>>> 在 2024/4/8 15:58, Christian König 写道: >>>>> Am 07.04.24 um 09:50 schrieb Rong Qianfeng: >>>>>> [SNIP] >>>>>>> Am 13.11.21 um 07:22 schrieb Jianqun Xu: >>>>>>>> Add DMA_BUF_IOCTL_SYNC_PARTIAL support for user to sync dma-buf with >>>>>>>> offset and len. >>>>>>> You have not given an use case for this so it is a bit hard to >>>>>>> review. And from the existing use cases I don't see why this should >>>>>>> be necessary. >>>>>>> >>>>>>> Even worse from the existing backend implementation I don't even see >>>>>>> how drivers should be able to fulfill this semantics. >>>>>>> >>>>>>> Please explain further, >>>>>>> Christian. >>>>>> Here is a practical case: >>>>>> The user space can allocate a large chunk of dma-buf for >>>>>> self-management, used as a shared memory pool. >>>>>> Small dma-buf can be allocated from this shared memory pool and >>>>>> released back to it after use, thus improving the speed of dma-buf >>>>>> allocation and release. >>>>>> Additionally, custom functionalities such as memory statistics and >>>>>> boundary checking can be implemented in the user space. >>>>>> Of course, the above-mentioned functionalities require the >>>>>> implementation of a partial cache sync interface. >>>>> Well that is obvious, but where is the code doing that? >>>>> >>>>> You can't send out code without an actual user of it. That will >>>>> obviously be rejected. >>>>> >>>>> Regards, >>>>> Christian. >>>> In fact, we have already used the user-level dma-buf memory pool in the >>>> camera shooting scenario on the phone. >>>> >>>> From the test results, The execution time of the photo shooting >>>> algorithm has been reduced from 3.8s to 3s. >>>> >>> For phones, the (out of tree) Android version of the system heap has a >>> page pool connected to a shrinker. >> Well, it should be obvious but I'm going to repeat it here: Submitting >> kernel patches for our of tree code is a rather *extreme* no-go. >> > Sorry I think my comment led to some confusion. I wasn't suggesting > you should take the patch; it's clearly incomplete. I was pointing out > another option to Rong. It appears Rong is creating a single oversized > dma-buf, and subdividing it in userspace to avoid multiple allocations > for multiple buffers. That would lead to a need for a partial sync of > the buffer from userspace. Instead of that, using a heap with a page > pool gets you kind of the same thing with a lot less headache in > userspace, and no need for partial sync. So I wanted to share that > option, and that can go in just Android if necessary without this > patch.
Hi T.J.
If there is a chance to use this patch on Android, I can explain to you in detail
why the user layer needs the dma-buf memory pool.
Thanks
Rong Qianfeng
> >> That in kernel code *must* have an in kernel user is a number one rule. >> >> When somebody violates this rule he pretty much disqualifying himself as >> reliable source of patches since maintainers then have to assume that >> this person tries to submit code which doesn't have a justification to >> be upstream. >> >> So while this actually looks useful from the technical side as long as >> nobody does an implementation based on an upstream driver I absolutely >> have to reject it from the organizational side. >> >> Regards, >> Christian. >> >>> That allows you to skip page >>> allocation without fully pinning the memory like you get when >>> allocating a dma-buf that's way larger than necessary. If it's for a >>> camera maybe you need physically contiguous memory, but it's also >>> possible to set that up. >>> >>> https://android.googlesource.com/kernel/common/+/refs/heads/android14-6.1/drivers/dma-buf/heaps/system_heap.c#377 >>> >>> >>>> To be honest, I didn't understand your concern "...how drivers should be >>>> able to fulfill this semantics." Can you please help explain it in more >>>> detail? >>>> >>>> Thanks, >>>> >>>> Rong Qianfeng. >>>> >>>>>> Thanks >>>>>> Rong Qianfeng.
| |