Re: çå: çå: [PATCH] ION: Sys_heap: fix the incorrect pool->gfp_mask setting

[Laura Abbott]

On 01/10/2018 06:06 PM, Zengtao (B) wrote:
> > -----邮件原件-----
> > 发件人: Laura Abbott [mailto:labbott@xxxxxxxxxx]
> > 发送时间: 2018年1月11日 8:01
> > 收件人: Zengtao (B) <prime.zeng@xxxxxxxxxxxxx>; Dan Carpenter
> > <dan.carpenter@xxxxxxxxxx>; Chenfeng (puck) <puck.chen@xxxxxxxxxxxxx>
> > 抄送: sumit.semwal@xxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx;
> > arve@xxxxxxxxxxx; tkjos@xxxxxxxxxxx; maco@xxxxxxxxxxx;
> > devel@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > 主题: 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@xxxxxxxxxx]
> > > > 发送时间: 2018年1月9日 17:14
> > > > 收件人: Chenfeng (puck) <puck.chen@xxxxxxxxxxxxx>
> > > > 抄送: Zengtao (B) <prime.zeng@xxxxxxxxxxxxx>; labbott@xxxxxxxxxx;
> > > > sumit.semwal@xxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx;
> > > > arve@xxxxxxxxxxx; tkjos@xxxxxxxxxxx; maco@xxxxxxxxxxx;
> > > > devel@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > > > 主题: 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