* Re: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting
2018-01-09 10:43 [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting Zeng Tao
@ 2018-01-09 3:30 ` Chen Feng
2018-01-09 9:14 ` Dan Carpenter
0 siblings, 1 reply; 7+ messages in thread
From: Chen Feng @ 2018-01-09 3:30 UTC (permalink / raw)
To: Zeng Tao, labbott, sumit.semwal, gregkh, arve, tkjos, maco
Cc: devel, linux-kernel
On 2018/1/9 18:43, Zeng Tao wrote:
> This issue is introduced by the commit <e7f63771b60e> ("ION: Sys_heap:
> Add cached pool to spead up cached buffer alloc"), 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.zeng@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"
>
> if (orders[i] > 4)
> gfp_flags = high_order_gfp_flags;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting
2018-01-09 3:30 ` Chen Feng
@ 2018-01-09 9:14 ` Dan Carpenter
2018-01-09 12:06 ` 答复: " Zengtao (B)
0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2018-01-09 9:14 UTC (permalink / raw)
To: Chen Feng
Cc: Zeng Tao, labbott, sumit.semwal, gregkh, arve, tkjos, maco,
devel, linux-kernel
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 <e7f63771b60e> ("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")
> 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.zeng@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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting
@ 2018-01-09 10:43 Zeng Tao
2018-01-09 3:30 ` Chen Feng
0 siblings, 1 reply; 7+ messages in thread
From: Zeng Tao @ 2018-01-09 10:43 UTC (permalink / raw)
To: labbott, sumit.semwal, gregkh, arve, tkjos, maco
Cc: devel, linux-kernel, puck.chen
This issue is introduced by the commit <e7f63771b60e> ("ION: Sys_heap:
Add cached pool to spead up cached buffer alloc"), 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.
Signed-off-by: Zeng Tao <prime.zeng@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;
if (orders[i] > 4)
gfp_flags = high_order_gfp_flags;
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* 答复: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting
2018-01-09 9:14 ` Dan Carpenter
@ 2018-01-09 12:06 ` Zengtao (B)
2018-01-11 0:00 ` Laura Abbott
0 siblings, 1 reply; 7+ messages in thread
From: Zengtao (B) @ 2018-01-09 12:06 UTC (permalink / raw)
To: Dan Carpenter, Chenfeng (puck)
Cc: labbott, sumit.semwal, gregkh, arve, tkjos, maco, devel, linux-kernel
>-----邮件原件-----
>发件人: Dan Carpenter [mailto:dan.carpenter@oracle.com]
>发送时间: 2018年1月9日 17:14
>收件人: Chenfeng (puck) <puck.chen@hisilicon.com>
>抄送: Zengtao (B) <prime.zeng@hisilicon.com>; labbott@redhat.com;
>sumit.semwal@linaro.org; gregkh@linuxfoundation.org; arve@android.com;
>tkjos@android.com; maco@android.com; devel@driverdev.osuosl.org;
>linux-kernel@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 <e7f63771b60e> ("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.zeng@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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 答复: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting
2018-01-09 12:06 ` 答复: " Zengtao (B)
@ 2018-01-11 0:00 ` Laura Abbott
2018-01-11 2:06 ` 答复: " Zengtao (B)
0 siblings, 1 reply; 7+ messages in thread
From: Laura Abbott @ 2018-01-11 0:00 UTC (permalink / raw)
To: Zengtao (B), Dan Carpenter, Chenfeng (puck)
Cc: sumit.semwal, gregkh, arve, tkjos, maco, devel, linux-kernel
On 01/09/2018 04:06 AM, Zengtao (B) wrote:
>> -----邮件原件-----
>> 发件人: Dan Carpenter [mailto:dan.carpenter@oracle.com]
>> 发送时间: 2018年1月9日 17:14
>> 收件人: Chenfeng (puck) <puck.chen@hisilicon.com>
>> 抄送: Zengtao (B) <prime.zeng@hisilicon.com>; labbott@redhat.com;
>> sumit.semwal@linaro.org; gregkh@linuxfoundation.org; arve@android.com;
>> tkjos@android.com; maco@android.com; devel@driverdev.osuosl.org;
>> linux-kernel@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 <e7f63771b60e> ("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).
Thanks,
Laura
^ permalink raw reply [flat|nested] 7+ messages in thread
* 答复: 答复: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting
2018-01-11 0:00 ` Laura Abbott
@ 2018-01-11 2:06 ` Zengtao (B)
2018-01-11 18:29 ` Laura Abbott
0 siblings, 1 reply; 7+ messages in thread
From: Zengtao (B) @ 2018-01-11 2:06 UTC (permalink / raw)
To: Laura Abbott, Dan Carpenter, Chenfeng (puck)
Cc: sumit.semwal, gregkh, arve, tkjos, maco, devel, linux-kernel
>-----邮件原件-----
>发件人: Laura Abbott [mailto:labbott@redhat.com]
>发送时间: 2018年1月11日 8:01
>收件人: Zengtao (B) <prime.zeng@hisilicon.com>; Dan Carpenter
><dan.carpenter@oracle.com>; Chenfeng (puck) <puck.chen@hisilicon.com>
>抄送: sumit.semwal@linaro.org; gregkh@linuxfoundation.org;
>arve@android.com; tkjos@android.com; maco@android.com;
>devel@driverdev.osuosl.org; linux-kernel@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.carpenter@oracle.com]
>>> 发送时间: 2018年1月9日 17:14
>>> 收件人: Chenfeng (puck) <puck.chen@hisilicon.com>
>>> 抄送: Zengtao (B) <prime.zeng@hisilicon.com>; labbott@redhat.com;
>>> sumit.semwal@linaro.org; gregkh@linuxfoundation.org;
>>> arve@android.com; tkjos@android.com; maco@android.com;
>>> devel@driverdev.osuosl.org; linux-kernel@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 <e7f63771b60e> ("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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 答复: 答复: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting
2018-01-11 2:06 ` 答复: " Zengtao (B)
@ 2018-01-11 18:29 ` Laura Abbott
0 siblings, 0 replies; 7+ messages in thread
From: Laura Abbott @ 2018-01-11 18:29 UTC (permalink / raw)
To: Zengtao (B), Dan Carpenter, Chenfeng (puck)
Cc: sumit.semwal, gregkh, arve, tkjos, maco, devel, linux-kernel
On 01/10/2018 06:06 PM, Zengtao (B) wrote:
>> -----邮件原件-----
>> 发件人: Laura Abbott [mailto:labbott@redhat.com]
>> 发送时间: 2018年1月11日 8:01
>> 收件人: Zengtao (B) <prime.zeng@hisilicon.com>; Dan Carpenter
>> <dan.carpenter@oracle.com>; Chenfeng (puck) <puck.chen@hisilicon.com>
>> 抄送: sumit.semwal@linaro.org; gregkh@linuxfoundation.org;
>> arve@android.com; tkjos@android.com; maco@android.com;
>> devel@driverdev.osuosl.org; linux-kernel@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.carpenter@oracle.com]
>>>> 发送时间: 2018年1月9日 17:14
>>>> 收件人: Chenfeng (puck) <puck.chen@hisilicon.com>
>>>> 抄送: Zengtao (B) <prime.zeng@hisilicon.com>; labbott@redhat.com;
>>>> sumit.semwal@linaro.org; gregkh@linuxfoundation.org;
>>>> arve@android.com; tkjos@android.com; maco@android.com;
>>>> devel@driverdev.osuosl.org; linux-kernel@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 <e7f63771b60e> ("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;
>
I prefer #1, but please make sure you are following the
suggestions in https://marc.info/?l=linux-kernel&m=151518104214191&w=2
> 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.
>
Yes, go ahead and fix that in a separate patch too.
> Thanks
> Zengtao
>
Thanks,
Laura
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-01-11 18:29 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-09 10:43 [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting Zeng Tao
2018-01-09 3:30 ` Chen Feng
2018-01-09 9:14 ` Dan Carpenter
2018-01-09 12:06 ` 答复: " Zengtao (B)
2018-01-11 0:00 ` Laura Abbott
2018-01-11 2:06 ` 答复: " Zengtao (B)
2018-01-11 18:29 ` Laura Abbott
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).