| Message ID | 1515494623-8383-1-git-send-email-prime.zeng@hisilicon.com |
|---|---|
| State | New |
| Headers | show |
| Series | ION: Sys_heap: fix the incorrect pool->gfp_mask setting | expand |
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; >
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
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
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
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;
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(-) -- 2.7.4