RE: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
>-Original Message- >From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] >Sent: Saturday, March 30, 2019 12:04 AM >To: Zengtao (B) >Cc: labb...@redhat.com; sumit.sem...@linaro.org; >de...@driverdev.osuosl.org; Todd Kjos ; >linux-ker...@vger.kernel.org; dri-de...@lists.freedesktop.org; >linaro-mm-...@lists.linaro.org; Arve Hjønnevåg ; >Joel Fernandes ; Martijn Coenen >; Christian Brauner >Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel >driver use > >On Sat, Mar 30, 2019 at 02:40:16AM +0800, Zeng Tao wrote: >> There are two reasons for this patch: >> 1. There are some potential requirements for ion_alloc in kernel >> space, some media drivers need to allocate media buffers from ion >> instead of buddy or dma framework, this is more convient and clean >> very for media drivers. And In that case, ion is the only media buffer >> provider, it's more easier to maintain. > >As this really is just DMA, what is wrong with the existing dma framework >that makes it hard to use? You have seen all of the changes recently to it, >right? The current dma framework is powerful enough(to me, and more complex ^_^) , CMA, IOMMU are all integrated, it's good. But buffer sharing, statistics, debug, are not so friendly for media drivers(each driver has to do all, but duplicate jobs). > >thanks, > >greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
>-Original Message-
>From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
>Sent: Friday, March 29, 2019 7:03 PM
>To: Zengtao (B)
>Cc: labb...@redhat.com; sumit.sem...@linaro.org;
>de...@driverdev.osuosl.org; Todd Kjos ; Greg
>Kroah-Hartman ;
>linux-ker...@vger.kernel.org; dri-de...@lists.freedesktop.org;
>linaro-mm-...@lists.linaro.org; Arve Hjønnevåg ;
>Joel Fernandes ; Martijn Coenen
>; Christian Brauner
>Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel
>driver use
>
>If we're going to export ion_alloc() for other drivers to use then let's make
>an ion_free() helper function as well.
>
Good catch, thanks.
Regards
Zengtao
>void ion_free(struct dma_buf *dmabuf)
>{
> dma_buf_put(dmabuf);
>}
>
>regards,
>dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
>-Original Message-
>From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
>Sent: Saturday, March 30, 2019 12:02 AM
>To: Zengtao (B)
>Cc: labb...@redhat.com; sumit.sem...@linaro.org;
>de...@driverdev.osuosl.org; Todd Kjos ;
>linux-ker...@vger.kernel.org; dri-de...@lists.freedesktop.org;
>linaro-mm-...@lists.linaro.org; Arve Hjønnevåg ;
>Joel Fernandes ; Martijn Coenen
>; Christian Brauner
>Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel
>driver use
>
>On Sat, Mar 30, 2019 at 02:40:16AM +0800, Zeng Tao wrote:
>> There are two reasons for this patch:
>> 1. There are some potential requirements for ion_alloc in kernel
>> space, some media drivers need to allocate media buffers from ion
>> instead of buddy or dma framework, this is more convient and clean
>> very for media drivers. And In that case, ion is the only media buffer
>> provider, it's more easier to maintain.
>> 2. Fd is only needed by user processes, not the kernel space, so
>> dma_buf should be returned instead of fd for kernel space, and
>> dma_buf_fd should be called only for userspace api.
>>
>> Signed-off-by: Zeng Tao
>> ---
>> drivers/staging/android/ion/ion.c | 32
>> +---
>> 1 file changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 92c2914..e93fb49 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -387,13 +387,13 @@ static const struct dma_buf_ops
>dma_buf_ops = {
>> .unmap = ion_dma_buf_kunmap,
>> };
>>
>> -static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned
>> int flags)
>> +struct dma_buf *ion_alloc(size_t len, unsigned int heap_id_mask,
>> + unsigned int flags)
>> {
>> struct ion_device *dev = internal_dev;
>> struct ion_buffer *buffer = NULL;
>> struct ion_heap *heap;
>> DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>> -int fd;
>> struct dma_buf *dmabuf;
>>
>> pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
>@@
>> -407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int
>heap_id_mask, unsigned int flags)
>> len = PAGE_ALIGN(len);
>>
>> if (!len)
>> -return -EINVAL;
>> +return ERR_PTR(-EINVAL);
>>
>> down_read(>lock);
>> plist_for_each_entry(heap, >heaps, node) { @@ -421,10
>+421,10
>> @@ static int ion_alloc(size_t len, unsigned int heap_id_mask,
>unsigned int flags)
>> up_read(>lock);
>>
>> if (!buffer)
>> -return -ENODEV;
>> +return ERR_PTR(-ENODEV);
>>
>> if (IS_ERR(buffer))
>> -return PTR_ERR(buffer);
>> +return ERR_PTR(PTR_ERR(buffer));
>>
>> exp_info.ops = _buf_ops;
>> exp_info.size = buffer->size;
>> @@ -432,17 +432,12 @@ static int ion_alloc(size_t len, unsigned int
>heap_id_mask, unsigned int flags)
>> exp_info.priv = buffer;
>>
>> dmabuf = dma_buf_export(_info);
>> -if (IS_ERR(dmabuf)) {
>> +if (IS_ERR(dmabuf))
>> _ion_buffer_destroy(buffer);
>> -return PTR_ERR(dmabuf);
>> -}
>>
>> -fd = dma_buf_fd(dmabuf, O_CLOEXEC);
>> -if (fd < 0)
>> -dma_buf_put(dmabuf);
>> -
>> -return fd;
>> +return dmabuf;
>> }
>> +EXPORT_SYMBOL(ion_alloc);
>
>If you are going to do this (and personally I'm with Laura in that I don't
>think you need it) this should be EXPORT_SYMBOL_GPL() please.
>
Agree, thanks
Regards
Zengtao
>thanks,
>
>greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
Hi laura:
>-Original Message-
>From: Laura Abbott [mailto:labb...@redhat.com]
>Sent: Friday, March 29, 2019 9:27 PM
>To: Zengtao (B) ; sumit.sem...@linaro.org
>Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
>; Todd Kjos ; Martijn Coenen
>; Joel Fernandes ;
>Christian Brauner ; de...@driverdev.osuosl.org;
>dri-de...@lists.freedesktop.org; linaro-mm-...@lists.linaro.org;
>linux-ker...@vger.kernel.org
>Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel
>driver use
>
>On 3/29/19 11:40 AM, Zeng Tao wrote:
>> There are two reasons for this patch:
>> 1. There are some potential requirements for ion_alloc in kernel
>> space, some media drivers need to allocate media buffers from ion
>> instead of buddy or dma framework, this is more convient and clean
>> very for media drivers. And In that case, ion is the only media buffer
>> provider, it's more easier to maintain.
>> 2. Fd is only needed by user processes, not the kernel space, so
>> dma_buf should be returned instead of fd for kernel space, and
>> dma_buf_fd should be called only for userspace api.
>>
>
>I really want to just NAK this because it doesn't seem like something
>that's necessary. The purpose of Ion is to provide buffers to userspace
>because there's no other way for userspace to get access to the memory.
>The kernel already has other APIs to access the memory. This also
>complicates the re-work that's been happening where the requirement is
>only userspace.
>
>Can you be more detailed about which media drivers you are referring to
>and why they can't just use other APIs?
>
I think I 've got your point, the ION is designed for usespace, but for kernel
space, we are really lacking of someone which plays the same role,(allocate
media memory, share the memory using dma_buf, provide debug and statistics
for media memory).
In fact, for kernel space, we have the dma framework, dma-buf, etc..
And we can work on top of such apis, but some duplicate jobs(everyone has
to maintain its own buffer sharing, debug and statistics).
So we need to have some to do the common things(ION's the best choice now)
When the ION was introduced, a lot of media memory frameworks existed, the
dma framework was not so good, so ION heaps, integrated buffer sharing,
statistics
and usespace api were the required features, but now dma framework is more
powerful,
we don't even need ION heaps now, but the userspace api, buffer sharing,
statistics are
still needed, and the buffer sharing, statistics can be re-worked and export to
kernel space,
not only used by userspace, , and that is my point.
>
>> Signed-off-by: Zeng Tao
>> ---
>> drivers/staging/android/ion/ion.c | 32
>+---
>> 1 file changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 92c2914..e93fb49 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -387,13 +387,13 @@ static const struct dma_buf_ops
>dma_buf_ops = {
>> .unmap = ion_dma_buf_kunmap,
>> };
>>
>> -static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned
>> int flags)
>> +struct dma_buf *ion_alloc(size_t len, unsigned int heap_id_mask,
>> + unsigned int flags)
>> {
>> struct ion_device *dev = internal_dev;
>> struct ion_buffer *buffer = NULL;
>> struct ion_heap *heap;
>> DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>> -int fd;
>> struct dma_buf *dmabuf;
>>
>> pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
>@@
>> -407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int
>heap_id_mask, unsigned int flags)
>> len = PAGE_ALIGN(len);
>>
>> if (!len)
>> -return -EINVAL;
>> +return ERR_PTR(-EINVAL);
>>
>> down_read(>lock);
>> plist_for_each_entry(heap, >heaps, node) { @@ -421,10
>+421,10
>> @@ static int ion_alloc(size_t len, unsigned int heap_id_mask,
>unsigned int flags)
>> up_read(>lock);
>>
>> if (!buffer)
>> -return -ENODEV;
>> +return ERR_PTR(-ENODEV);
>>
>> if (IS_ERR(buffer))
>> -return PTR_ERR(buffer);
>> +return ERR_PTR(PTR_ERR(buffer));
>>
>> exp_info.ops = _buf_ops;
>> exp_info.size = buffer->size;
>> @@ -432,17 +432,12 @@ static int ion_alloc(size_t len, unsigned int
>heap_id_mask, unsigned int flags)
>> exp_info.priv = buffer;
>>
>> dmabuf = dma_buf_exp
RE: [PATCH] staging: android: ion: add buffer flag update ioctl
>-Original Message- >From: Laura Abbott [mailto:labb...@redhat.com] >Sent: Friday, January 04, 2019 9:53 AM >To: Zengtao (B) ; sumit.sem...@linaro.org >Cc: Greg Kroah-Hartman ; Arve Hjønnevåg >; Todd Kjos ; Martijn Coenen >; Joel Fernandes ; >de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org; >linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org >Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl > >On 1/3/19 5:42 PM, Zengtao (B) wrote: >>> -Original Message- >>> From: Laura Abbott [mailto:labb...@redhat.com] >>> Sent: Thursday, January 03, 2019 6:31 AM >>> To: Zengtao (B) ; >sumit.sem...@linaro.org >>> Cc: Greg Kroah-Hartman ; Arve >Hjønnevåg >>> ; Todd Kjos ; Martijn >Coenen >>> ; Joel Fernandes ; >>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org; >>> linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org >>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update >>> ioctl >>> >>> On 12/23/18 6:47 PM, Zengtao (B) wrote: >>>> Hi laura: >>>> >>>>> -Original Message- >>>>> From: Laura Abbott [mailto:labb...@redhat.com] >>>>> Sent: Friday, December 21, 2018 4:50 AM >>>>> To: Zengtao (B) ; >>> sumit.sem...@linaro.org >>>>> Cc: Greg Kroah-Hartman ; Arve >>> Hjønnevåg >>>>> ; Todd Kjos ; Martijn >>> Coenen >>>>> ; Joel Fernandes ; >>>>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org; >>>>> linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org >>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update >>>>> ioctl >>>>> >>>>> On 12/19/18 5:39 PM, Zengtao (B) wrote: >>>>>> Hi laura: >>>>>> >>>>>>> -Original Message- >>>>>>> From: Laura Abbott [mailto:labb...@redhat.com] >>>>>>> Sent: Thursday, December 20, 2018 2:10 AM >>>>>>> To: Zengtao (B) ; >>>>> sumit.sem...@linaro.org >>>>>>> Cc: Greg Kroah-Hartman ; Arve >>>>> Hjønnevåg >>>>>>> ; Todd Kjos ; Martijn >>>>> Coenen >>>>>>> ; Joel Fernandes ; >>>>>>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org; >>>>>>> linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org >>>>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag >>>>>>> update ioctl >>>>>>> >>>>>>> On 12/19/18 9:19 AM, Zeng Tao wrote: >>>>>>>> In some usecases, the buffer cached attribute is not determined >>>>>>>> at allocation time, it's determined just before the real cpu >mapping. >>>>>>>> And from the memory view of point, a buffer should not have >the >>>>>>> cached >>>>>>>> attribute util is really mapped by the cpu. So in this patch, we >>>>>>>> introduced the new ioctl command to target the requirement. >>>>>>>> >>>>>>> >>>>>>> This is racy and error prone. Can you explain more what problem >>> you >>>>>>> are trying to solve? >>>>>> >>>>>> My use case is like this: >>>>>> 1. There are two process A and B, A takes case of ion buffer >>>>>> allocation, >>>>> and >>>>>> pass the buffer fd to B, then B maps and uses it. >>>>>> 2. Process B need to map the buffer with different cached >>>>>> attribute for different use case, for example, if the buffer is >>>>>> used for pure software algorithm, then we need to map it as >>>>>> cached, otherwise non-cached, and B needs to deal with both >cases. >>>>>> And unfortunately the mmap syscall takes no cached flags and we >>>>>> can't decide the cache attribute when we are doing the mmap, so I >>>>>> introduce new the ioctl even though I think the solution is not as >>> good. >>>>>> >>>>>> >>>>> >>>>> Thanks for the explanation, this was about the use case I expected. >>>>> I'm pretty sure I had this exact problem once upon a time and we >>>>> didn't come up with a sol
RE: [PATCH] staging: android: ion: add buffer flag update ioctl
>-Original Message- >From: Laura Abbott [mailto:labb...@redhat.com] >Sent: Thursday, January 03, 2019 6:31 AM >To: Zengtao (B) ; sumit.sem...@linaro.org >Cc: Greg Kroah-Hartman ; Arve Hjønnevåg >; Todd Kjos ; Martijn Coenen >; Joel Fernandes ; >de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org; >linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org >Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl > >On 12/23/18 6:47 PM, Zengtao (B) wrote: >> Hi laura: >> >>> -Original Message- >>> From: Laura Abbott [mailto:labb...@redhat.com] >>> Sent: Friday, December 21, 2018 4:50 AM >>> To: Zengtao (B) ; >sumit.sem...@linaro.org >>> Cc: Greg Kroah-Hartman ; Arve >Hjønnevåg >>> ; Todd Kjos ; Martijn >Coenen >>> ; Joel Fernandes ; >>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org; >>> linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org >>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update >>> ioctl >>> >>> On 12/19/18 5:39 PM, Zengtao (B) wrote: >>>> Hi laura: >>>> >>>>> -Original Message- >>>>> From: Laura Abbott [mailto:labb...@redhat.com] >>>>> Sent: Thursday, December 20, 2018 2:10 AM >>>>> To: Zengtao (B) ; >>> sumit.sem...@linaro.org >>>>> Cc: Greg Kroah-Hartman ; Arve >>> Hjønnevåg >>>>> ; Todd Kjos ; Martijn >>> Coenen >>>>> ; Joel Fernandes ; >>>>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org; >>>>> linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org >>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update >>>>> ioctl >>>>> >>>>> On 12/19/18 9:19 AM, Zeng Tao wrote: >>>>>> In some usecases, the buffer cached attribute is not determined at >>>>>> allocation time, it's determined just before the real cpu mapping. >>>>>> And from the memory view of point, a buffer should not have the >>>>> cached >>>>>> attribute util is really mapped by the cpu. So in this patch, we >>>>>> introduced the new ioctl command to target the requirement. >>>>>> >>>>> >>>>> This is racy and error prone. Can you explain more what problem >you >>>>> are trying to solve? >>>> >>>> My use case is like this: >>>> 1. There are two process A and B, A takes case of ion buffer >>>> allocation, >>> and >>>>pass the buffer fd to B, then B maps and uses it. >>>> 2. Process B need to map the buffer with different cached attribute >>>> for different use case, for example, if the buffer is used for pure >>>> software algorithm, then we need to map it as cached, otherwise >>>> non-cached, and B needs to deal with both cases. >>>> And unfortunately the mmap syscall takes no cached flags and we >>>> can't decide the cache attribute when we are doing the mmap, so I >>>> introduce new the ioctl even though I think the solution is not as >good. >>>> >>>> >>> >>> Thanks for the explanation, this was about the use case I expected. >>> I'm pretty sure I had this exact problem once upon a time and we >>> didn't come up with a solution. I'd still like to get rid of uncached >>> buffers in general and just use cached buffers (see >>> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/201 >>> 8-N >>> ovember/128842.html) >>> What's your usecase for uncached buffers? >> >> Some buffers are only used by HW, in this case, we tend to use >uncached buffers. >> But sometimes the SW need to read few buffer contents for debug >> purpose and no synchronization is needed. >> > >If this is primarily for debug, that's not really a compelling reason to >support uncached buffers. It's incredibly difficult to do uncached buffers >correctly I'd rather spend effort on making the cached use cases work >better. > No, not only for debug, I meant if the buffers are uncached, the SW will only mmap them for debug or even don't need to mmap them. So for the two kinds of buffers: 1. uncached: only the HW need to access it, the SW don't need to mmap it or just for debug. 2. cached: both the HW and the SW need to access it. The ION is designed as a generalized memory manager, I think we should cover as many requi
RE: [PATCH] staging: android: ion: add buffer flag update ioctl
Hi laura:
>-Original Message-
>From: Laura Abbott [mailto:labb...@redhat.com]
>Sent: Friday, December 21, 2018 4:50 AM
>To: Zengtao (B) ; sumit.sem...@linaro.org
>Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
>; Todd Kjos ; Martijn Coenen
>; Joel Fernandes ;
>de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org
>Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>
>On 12/19/18 5:39 PM, Zengtao (B) wrote:
>> Hi laura:
>>
>>> -Original Message-
>>> From: Laura Abbott [mailto:labb...@redhat.com]
>>> Sent: Thursday, December 20, 2018 2:10 AM
>>> To: Zengtao (B) ;
>sumit.sem...@linaro.org
>>> Cc: Greg Kroah-Hartman ; Arve
>Hjønnevåg
>>> ; Todd Kjos ; Martijn
>Coenen
>>> ; Joel Fernandes ;
>>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>>> linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org
>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>> ioctl
>>>
>>> On 12/19/18 9:19 AM, Zeng Tao wrote:
>>>> In some usecases, the buffer cached attribute is not determined at
>>>> allocation time, it's determined just before the real cpu mapping.
>>>> And from the memory view of point, a buffer should not have the
>>> cached
>>>> attribute util is really mapped by the cpu. So in this patch, we
>>>> introduced the new ioctl command to target the requirement.
>>>>
>>>
>>> This is racy and error prone. Can you explain more what problem you
>>> are trying to solve?
>>
>> My use case is like this:
>> 1. There are two process A and B, A takes case of ion buffer allocation,
>and
>> pass the buffer fd to B, then B maps and uses it.
>> 2. Process B need to map the buffer with different cached attribute
>> for different use case, for example, if the buffer is used for pure
>> software algorithm, then we need to map it as cached, otherwise
>> non-cached, and B needs to deal with both cases.
>> And unfortunately the mmap syscall takes no cached flags and we can't
>> decide the cache attribute when we are doing the mmap, so I introduce
>> new the ioctl even though I think the solution is not as good.
>>
>>
>
>Thanks for the explanation, this was about the use case I expected.
>I'm pretty sure I had this exact problem once upon a time and we didn't
>come up with a solution. I'd still like to get rid of uncached buffers in
>general and just use cached buffers (see
>http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-N
>ovember/128842.html)
>What's your usecase for uncached buffers?
Some buffers are only used by HW, in this case, we tend to use uncached buffers.
But sometimes the SW need to read few buffer contents for debug purpose and
no synchronization is needed.
>
>>>
>>>> Signed-off-by: Zeng Tao
>>>> ---
>>>>drivers/staging/android/ion/ion-ioctl.c | 4
>>>>drivers/staging/android/ion/ion.c | 17
>+
>>>>drivers/staging/android/ion/ion.h | 1 +
>>>>drivers/staging/android/uapi/ion.h | 22
>>> ++
>>>>4 files changed, 44 insertions(+)
>>>>
>>>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>>>> b/drivers/staging/android/ion/ion-ioctl.c
>>>> index a8d3cc4..60bb702 100644
>>>> --- a/drivers/staging/android/ion/ion-ioctl.c
>>>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>>>> @@ -12,6 +12,7 @@
>>>>
>>>>union ion_ioctl_arg {
>>>>struct ion_allocation_data allocation;
>>>> + struct ion_buffer_flag_data update;
>>>>struct ion_heap_query query;
>>>>};
>>>>
>>>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int
>>>> cmd, unsigned long arg)
>>>>
>>>>break;
>>>>}
>>>> + case ION_IOC_BUFFER_UPDATE:
>>>> + ret = ion_buffer_update(data.update.fd, data.update.flags);
>>>> + break;
>>>>case ION_IOC_HEAP_QUERY:
>>>>ret = ion_query_heaps();
>>>>break;
>>>> diff --git a/drivers/staging/android/ion/ion.c
>>>> b/drivers/staging/android/ion/ion.c
>>>> index 9907332..f1404dc 100644
>>>> --- a/drivers/staging/android/ion/ion
RE: [PATCH] staging: android: ion: add buffer flag update ioctl
Hi laura:
>-Original Message-
>From: Laura Abbott [mailto:labb...@redhat.com]
>Sent: Thursday, December 20, 2018 2:10 AM
>To: Zengtao (B) ; sumit.sem...@linaro.org
>Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
>; Todd Kjos ; Martijn Coenen
>; Joel Fernandes ;
>de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org
>Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>
>On 12/19/18 9:19 AM, Zeng Tao wrote:
>> In some usecases, the buffer cached attribute is not determined at
>> allocation time, it's determined just before the real cpu mapping.
>> And from the memory view of point, a buffer should not have the
>cached
>> attribute util is really mapped by the cpu. So in this patch, we
>> introduced the new ioctl command to target the requirement.
>>
>
>This is racy and error prone. Can you explain more what problem you are
>trying to solve?
My use case is like this:
1. There are two process A and B, A takes case of ion buffer allocation, and
pass the buffer fd to B, then B maps and uses it.
2. Process B need to map the buffer with different cached attribute for
different use case, for example, if the buffer is used for pure software
algorithm,
then we need to map it as cached, otherwise non-cached, and B needs to deal
with both cases.
And unfortunately the mmap syscall takes no cached flags and we can't decide
the cache attribute when we are doing the mmap, so I introduce new the ioctl
even though I think the solution is not as good.
>
>> Signed-off-by: Zeng Tao
>> ---
>> drivers/staging/android/ion/ion-ioctl.c | 4
>> drivers/staging/android/ion/ion.c | 17 +
>> drivers/staging/android/ion/ion.h | 1 +
>> drivers/staging/android/uapi/ion.h | 22
>++
>> 4 files changed, 44 insertions(+)
>>
>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>> b/drivers/staging/android/ion/ion-ioctl.c
>> index a8d3cc4..60bb702 100644
>> --- a/drivers/staging/android/ion/ion-ioctl.c
>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>> @@ -12,6 +12,7 @@
>>
>> union ion_ioctl_arg {
>> struct ion_allocation_data allocation;
>> +struct ion_buffer_flag_data update;
>> struct ion_heap_query query;
>> };
>>
>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
>> unsigned long arg)
>>
>> break;
>> }
>> +case ION_IOC_BUFFER_UPDATE:
>> +ret = ion_buffer_update(data.update.fd, data.update.flags);
>> +break;
>> case ION_IOC_HEAP_QUERY:
>> ret = ion_query_heaps();
>> break;
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 9907332..f1404dc 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
>heap_id_mask, unsigned int flags)
>> return fd;
>> }
>>
>> +int ion_buffer_update(unsigned int fd, unsigned int flags) {
>> +struct dma_buf *dmabuf;
>> +struct ion_buffer *buffer;
>> +
>> +dmabuf = dma_buf_get(fd);
>> +
>> +if (!dmabuf)
>> +return -EINVAL;
>> +
>> +buffer = dmabuf->priv;
>> +buffer->flags = flags;
>> +dma_buf_put(dmabuf);
>> +
>> +return 0;
>> +}
>> +
>> int ion_query_heaps(struct ion_heap_query *query)
>> {
>> struct ion_device *dev = internal_dev; diff --git
>> a/drivers/staging/android/ion/ion.h
>> b/drivers/staging/android/ion/ion.h
>> index c006fc1..99bf9ab 100644
>> --- a/drivers/staging/android/ion/ion.h
>> +++ b/drivers/staging/android/ion/ion.h
>> @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page,
>size_t size, pgprot_t pgprot);
>> int ion_alloc(size_t len,
>>unsigned int heap_id_mask,
>>unsigned int flags);
>> +int ion_buffer_update(unsigned int fd, unsigned int flags);
>>
>> /**
>>* ion_heap_init_shrinker
>> diff --git a/drivers/staging/android/uapi/ion.h
>> b/drivers/staging/android/uapi/ion.h
>> index 5d70098..99753fc 100644
>> --- a/drivers/staging/android/uapi/ion.h
>> +++ b/drivers/staging/android/uapi/ion.h
>> @@ -74,6 +74,20 @@ struct ion_allocation_data {
>> __u32 unused;
>> };
>>
>> +/**
>> + * struct ion_buffer_flag_data - metadata passed from userspace for
>> +up
答复: 答复: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting
>-邮件原件-
>发件人: Laura Abbott [mailto:labb...@redhat.com]
>发送时间: 2018年1月11日 8:01
>收件人: Zengtao (B) <prime.z...@hisilicon.com>; Dan Carpenter
><dan.carpen...@oracle.com>; Chenfeng (puck) <puck.c...@hisilicon.com>
>抄送: sumit.sem...@linaro.org; gre...@linuxfoundation.org;
>a...@android.com; tk...@android.com; m...@android.com;
>de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org
>主题: Re: 答复: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask
>setting
>
>On 01/09/2018 04:06 AM, Zengtao (B) wrote:
>>> -邮件原件-
>>> 发件人: Dan Carpenter [mailto:dan.carpen...@oracle.com]
>>> 发送时间: 2018年1月9日 17:14
>>> 收件人: Chenfeng (puck) <puck.c...@hisilicon.com>
>>> 抄送: Zengtao (B) <prime.z...@hisilicon.com>; labb...@redhat.com;
>>> sumit.sem...@linaro.org; gre...@linuxfoundation.org;
>>> a...@android.com; tk...@android.com; m...@android.com;
>>> de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org
>>> 主题: Re: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask
>>> setting
>>>
>>> On Tue, Jan 09, 2018 at 11:30:09AM +0800, Chen Feng wrote:
>>>>
>>>>
>>>> On 2018/1/9 18:43, Zeng Tao wrote:
>>>>> This issue is introduced by the commit ("ION: Sys_heap:
>>>>> Add cached pool to spead up cached buffer alloc"),
>>>
>>> Use the Fixes tag.
>>>
>>> Fixes: e7f63771b60e ("ION: Sys_heap: Add cached pool to spead up
>>> cached buffer alloc")
>>>
>> Agree, thanks.
>>
>
>If you're going to be fixing this, it would be good to fix the other problems
>pointed out (stop with the #define of the flags).
>
It is OK, I will fix in the new version fix.
And to make the code more explicit, I have to choices of fixes:
Choice 1:
if (orders[i] > 4)
gfp_flags = high_order_gfp_flags;
else
gfp_flags = low_order_gfp_flags;
Choice 2:
gfp_flags = (orders[i] > 4) ? high_order_gfp_flags : low_order_gfp_flags;
Any suggestion ?
BTW, I found another problem related:
Currently the order 4 and order 0 allocation flag haven't got the __GFP_NOWARN
set,
if the order 4 allocation failed but the allocation of order 0 success, it will
print warning
message which is useless.
Of course, this is not related to this fix, but this is what I have met when
test this fix.
Thanks
Zengtao
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
答复: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting
>-邮件原件-
>发件人: Dan Carpenter [mailto:dan.carpen...@oracle.com]
>发送时间: 2018年1月9日 17:14
>收件人: Chenfeng (puck) <puck.c...@hisilicon.com>
>抄送: Zengtao (B) <prime.z...@hisilicon.com>; labb...@redhat.com;
>sumit.sem...@linaro.org; gre...@linuxfoundation.org; a...@android.com;
>tk...@android.com; m...@android.com; de...@driverdev.osuosl.org;
>linux-ker...@vger.kernel.org
>主题: Re: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting
>
>On Tue, Jan 09, 2018 at 11:30:09AM +0800, Chen Feng wrote:
>>
>>
>> On 2018/1/9 18:43, Zeng Tao wrote:
>> > This issue is introduced by the commit ("ION: Sys_heap:
>> > Add cached pool to spead up cached buffer alloc"),
>
>Use the Fixes tag.
>
>Fixes: e7f63771b60e ("ION: Sys_heap: Add cached pool to spead up cached
>buffer alloc")
>
Agree, thanks.
>> the gfp_mask low
>> > order pool is overlapped by the high order inside the loop, so the
>> > gfp_mask of all pools are set to high_order_gfp_flags.
>> >
>>
>> Thanks
>> > Signed-off-by: Zeng Tao <prime.z...@hisilicon.com>
>> > ---
>> > drivers/staging/android/ion/ion_system_heap.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/staging/android/ion/ion_system_heap.c
>> > b/drivers/staging/android/ion/ion_system_heap.c
>> > index 4dc5d7a..b6386be 100644
>> > --- a/drivers/staging/android/ion/ion_system_heap.c
>> > +++ b/drivers/staging/android/ion/ion_system_heap.c
>> > @@ -298,10 +298,10 @@ static int ion_system_heap_create_pools(struct
>ion_page_pool **pools,
>> >bool cached)
>> > {
>> >int i;
>> > - gfp_t gfp_flags = low_order_gfp_flags;
>> >
>> >for (i = 0; i < NUM_ORDERS; i++) {
>> >struct ion_page_pool *pool;
>> > + gfp_t gfp_flags = low_order_gfp_flags;
>>
>> Not define here. Better "gfp_flags = low_order_gfp_flags"
>
>Either way is fine...
>
>regards,
>dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
答复: 答复: Using ion memory for direct-io
>-邮件原件- >发件人: Laura Abbott [mailto:labb...@redhat.com] >发送时间: 2017年4月18日 23:56 >收件人: Zengtao (B) <prime.z...@hisilicon.com>; sumit.sem...@linaro.org >抄送: gre...@linuxfoundation.org; a...@android.com; >riandr...@android.com; de...@driverdev.osuosl.org; >linux-ker...@vger.kernel.org >主题: Re: 答复: Using ion memory for direct-io > >On 04/17/2017 07:05 PM, Zengtao (B) wrote: >> Hi Laura: >> >>> -邮件原件- >>> 发件人: Laura Abbott [mailto:labb...@redhat.com] >>> 发送时间: 2017年4月18日 0:14 >>> 收件人: Zengtao (B) <prime.z...@hisilicon.com>; >sumit.sem...@linaro.org >>> 抄送: gre...@linuxfoundation.org; a...@android.com; >>> riandr...@android.com; de...@driverdev.osuosl.org; >>> linux-ker...@vger.kernel.org >>> 主题: Re: Using ion memory for direct-io >>> >>> On 04/14/2017 02:18 AM, Zengtao (B) wrote: >>>> Hi >>>> >>>> Currently, the ion mapped to userspace will be forced with VM_IO and >>> VM_PFNMAP flags. >>>> When I use the ion memory to do the direct-io, it will fail when >>>> reaching the get_user_pages, >>>> >>>> Back to the VM_IO and VM_PFNMAP flag, there two flags are introduced >>>> by the remap_pfn_range called by the ion_heap_mmap_user. >>>> >>>> From my point of view, all ion memory(cma/vmalloc/system heap) are >>>> managed by linux vm, it is not reasonable to have the VM_IO and >>>> VM_PFNMAP flag, but I don't any suitable function to replace the >>> remap_pfn_range, any suggestions? >>>> >>>> Thanks && Regards >>>> >>>> Zengtao >>>> >>> >>> The carveout heap is omitted from your list of 'all ion memory'. At >>> one time, carveout memory was not backed by struct pages so I suspect >>> this is a holdover from then. This would probably be better served >> Yes, you are right, I missed the carveout heap which needs the VM_IO >> and VM_PFNMAP, and I think the carveout heap can implement its own >> map_user rather then using the common ion_heap_map_user. >> > >The carveout heap only uses memory with struct pages these days. My point >was that the VM_IO and VFM_PFNMAP shouldn't need to be used at all >anymore. Sorry for the confusion. > >>> by using vm_insert_page and handling higher order pages properly. >> Your latest patch has remove the the page faulting support, I didn't >> deep into the reason, but I think this conflicts with the vm_insert_page. >> > >vm_insert_page should be able to be used outside the fault handler. > >>> >>> Thanks, >>> Laura >> >> I tried to use the similar way as the dma framework do(split the page >> and map_vm_area), but the split will break the ion sg design, maybe we >> need a new lowlevel map function instead of directly using the >remap_pfn_range. > >Yes, I don't think we can directly copy what the dma framework does. I think we >should be okay with allocating pages with __GFP_COMP and then just using >vm_insert_page but I'm not 100% sure this would work with CMA. > >For completeness sake, I know the request is for direct-io but can you share a >few more details about the driver/use case where you want to use direct-io with >Ion buffers? For example, in the video recorder application, the hardware video encoder will use ion memory to encode the raw video data, and then store it on disk, if we can use direct-io , an extra memcpy is saved, and we can get a better performance and a lower cpu usage. > >> >> Thanks >> Zengtao >> > >Thanks, >Laura Thanks ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
答复: Using ion memory for direct-io
Hi Laura: >-邮件原件- >发件人: Laura Abbott [mailto:labb...@redhat.com] >发送时间: 2017年4月18日 0:14 >收件人: Zengtao (B) <prime.z...@hisilicon.com>; sumit.sem...@linaro.org >抄送: gre...@linuxfoundation.org; a...@android.com; >riandr...@android.com; de...@driverdev.osuosl.org; >linux-ker...@vger.kernel.org >主题: Re: Using ion memory for direct-io > >On 04/14/2017 02:18 AM, Zengtao (B) wrote: >> Hi >> >> Currently, the ion mapped to userspace will be forced with VM_IO and >VM_PFNMAP flags. >> When I use the ion memory to do the direct-io, it will fail when >> reaching the get_user_pages, >> >> Back to the VM_IO and VM_PFNMAP flag, there two flags are introduced >> by the remap_pfn_range called by the ion_heap_mmap_user. >> >> From my point of view, all ion memory(cma/vmalloc/system heap) are >> managed by linux vm, it is not reasonable to have the VM_IO and >> VM_PFNMAP flag, but I don't any suitable function to replace the >remap_pfn_range, any suggestions? >> >> Thanks && Regards >> >> Zengtao >> > >The carveout heap is omitted from your list of 'all ion memory'. At one >time, carveout memory was not backed by struct pages so I suspect >this is a holdover from then. This would probably be better served Yes, you are right, I missed the carveout heap which needs the VM_IO and VM_PFNMAP, and I think the carveout heap can implement its own map_user rather then using the common ion_heap_map_user. >by using vm_insert_page and handling higher order pages properly. Your latest patch has remove the the page faulting support, I didn't deep into the reason, but I think this conflicts with the vm_insert_page. > >Thanks, >Laura I tried to use the similar way as the dma framework do(split the page and map_vm_area), but the split will break the ion sg design, maybe we need a new lowlevel map function instead of directly using the remap_pfn_range. Thanks Zengtao ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Using ion memory for direct-io
Hi Currently, the ion mapped to userspace will be forced with VM_IO and VM_PFNMAP flags. When I use the ion memory to do the direct-io, it will fail when reaching the get_user_pages, Back to the VM_IO and VM_PFNMAP flag, there two flags are introduced by the remap_pfn_range called by the ion_heap_mmap_user. >From my point of view, all ion memory(cma/vmalloc/system heap) are managed by >linux vm, it is not reasonable to have the VM_IO and VM_PFNMAP flag, but I don't any suitable function to replace the remap_pfn_range, any suggestions? Thanks && Regards Zengtao ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] drivers: android: fix watermark when cma enabled
From: Zeng Tao prime.z...@hisilicon.com Date: Tue, 5 Aug 2014 17:58:10 +0800 Subject: [PATCH] drivers: android: fix watermark when cma enabled when cma is enabled, the watermark caculation will include the cma pages, while it can't use cma pages. this will cause oom while lowmemory killer don't work. Signed-off-by: Zeng Tao prime.z...@hisilicon.com --- drivers/staging/android/lowmemorykiller.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c index 12f0a13..12cfd4f 100644 --- a/drivers/staging/android/lowmemorykiller.c +++ b/drivers/staging/android/lowmemorykiller.c @@ -74,7 +74,9 @@ static int lowmem_shrink(struct shrinker *s, struct shrink_control *sc) int selected_tasksize = 0; int selected_oom_score_adj; int array_size = ARRAY_SIZE(lowmem_adj); - int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages; + int other_free = global_page_state(NR_FREE_PAGES) - + global_page_state(NR_FREE_CMA_PAGES) - + totalreserve_pages; int other_file = global_page_state(NR_FILE_PAGES) - global_page_state(NR_SHMEM); -- 1.7.0.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Fix the Signature 答复: [PATCH] drivers: android: fix watermark when cma enabled
From: Zeng Tao prime.z...@hisilicon.com Date: Tue, 5 Aug 2014 17:58:10 +0800 Subject: [PATCH] drivers: android: fix watermark when cma enabled when cma is enabled, the watermark caculation will include the cma pages, while it can't use cma pages. this will cause oom while lowmemory killer don't work. Signed-off-by: Zeng Tao prime.z...@hisilicon.com --- drivers/staging/android/lowmemorykiller.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c index 12f0a13..12cfd4f 100644 --- a/drivers/staging/android/lowmemorykiller.c +++ b/drivers/staging/android/lowmemorykiller.c @@ -74,7 +74,9 @@ static int lowmem_shrink(struct shrinker *s, struct shrink_control *sc) int selected_tasksize = 0; int selected_oom_score_adj; int array_size = ARRAY_SIZE(lowmem_adj); - int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages; + int other_free = global_page_state(NR_FREE_PAGES) - + global_page_state(NR_FREE_CMA_PAGES) - + totalreserve_pages; int other_file = global_page_state(NR_FILE_PAGES) - global_page_state(NR_SHMEM); -- 1.7.0.4 -邮件原件- 发件人: Dan Carpenter [mailto:dan.carpen...@oracle.com] 发送时间: 2014年8月5日 18:36 收件人: Zengtao (B) 抄送: de...@driverdev.osuosl.org; gre...@linuxfoundation.org 主题: Re: [PATCH] drivers: android: fix watermark when cma enabled On Tue, Aug 05, 2014 at 09:43:48AM +, Zengtao (B) wrote: From: zengtao prime.z...@hisilicon.com This is for when you are sending patches on behalf of someone else. Just fix your from header. Use your real name. Zeng Tao? when cma is enabled, the watermark caculation will include the cma pages, while it can't use cma pages. this will cause oom while lowmemory killer don't work. Signed-off-by: zengtao prime.z...@hisilicon.com Real name again. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] drivers: android: fix watermark when cma enabled
From: zengtao prime.z...@hisilicon.com when cma is enabled, the watermark caculation will include the cma pages, while it can't use cma pages. this will cause oom while lowmemory killer don't work. Signed-off-by: zengtao prime.z...@hisilicon.com --- drivers/staging/android/lowmemorykiller.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c index 12f0a13..12cfd4f 100644 --- a/drivers/staging/android/lowmemorykiller.c +++ b/drivers/staging/android/lowmemorykiller.c @@ -74,7 +74,9 @@ static int lowmem_shrink(struct shrinker *s, struct shrink_control *sc) int selected_tasksize = 0; int selected_oom_score_adj; int array_size = ARRAY_SIZE(lowmem_adj); - int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages; + int other_free = global_page_state(NR_FREE_PAGES) - + global_page_state(NR_FREE_CMA_PAGES) - + totalreserve_pages; int other_file = global_page_state(NR_FILE_PAGES) - global_page_state(NR_SHMEM); -- 1.7.0.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
