Messages in this thread Patch in this message | | | Date | Wed, 10 Feb 2021 09:48:45 -0800 | From | Minchan Kim <> | Subject | Re: [PATCH] dma-buf: system_heap: do not warn for costly allocation |
| |
On Wed, Feb 10, 2021 at 09:32:09AM -0800, John Stultz wrote: > On Wed, Feb 10, 2021 at 8:26 AM Minchan Kim <minchan@kernel.org> wrote: > > > > Linux VM is not hard to support PAGE_ALLOC_COSTLY_ODER allocation > > so normally expects driver passes __GFP_NOWARN in that case > > if they has fallback options. > > > > system_heap in dmabuf is the case so do not flood into demsg > > with the warning for recording more precious information logs. > > (below is ION warning example I got but dmabuf system heap is > > nothing different). > > > > [ 1233.911533][ T460] warn_alloc: 11 callbacks suppressed > > [ 1233.911539][ T460] allocator@2.0-s: page allocation failure: order:4, mode:0x140dc2(GFP_HIGHUSER|__GFP_COMP|__GFP_ZERO), nodemask=(null),cpuset=/,mems_allowed=0 > > [ 1233.926235][ T460] Call trace: > > [ 1233.929370][ T460] dump_backtrace+0x0/0x1d8 > > [ 1233.933704][ T460] show_stack+0x18/0x24 > > [ 1233.937701][ T460] dump_stack+0xc0/0x140 > > [ 1233.941783][ T460] warn_alloc+0xf4/0x148 > > [ 1233.945862][ T460] __alloc_pages_slowpath+0x9fc/0xa10 > > [ 1233.951101][ T460] __alloc_pages_nodemask+0x278/0x2c0 > > [ 1233.956285][ T460] ion_page_pool_alloc+0xd8/0x100 > > [ 1233.961144][ T460] ion_system_heap_allocate+0xbc/0x2f0 > > [ 1233.966440][ T460] ion_buffer_create+0x68/0x274 > > [ 1233.971130][ T460] ion_buffer_alloc+0x8c/0x110 > > [ 1233.975733][ T460] ion_dmabuf_alloc+0x44/0xe8 > > [ 1233.980248][ T460] ion_ioctl+0x100/0x320 > > [ 1233.984332][ T460] __arm64_sys_ioctl+0x90/0xc8 > > [ 1233.988934][ T460] el0_svc_common+0x9c/0x168 > > [ 1233.993360][ T460] do_el0_svc+0x1c/0x28 > > [ 1233.997358][ T460] el0_sync_handler+0xd8/0x250 > > [ 1234.001989][ T460] el0_sync+0x148/0x180 > > > > Signed-off-by: Minchan Kim <minchan@kernel.org> > > --- > > drivers/dma-buf/heaps/system_heap.c | 9 +++++++-- > > 1 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c > > index 29e49ac17251..33c25a5e06f9 100644 > > --- a/drivers/dma-buf/heaps/system_heap.c > > +++ b/drivers/dma-buf/heaps/system_heap.c > > @@ -40,7 +40,7 @@ struct dma_heap_attachment { > > bool mapped; > > }; > > > > -#define HIGH_ORDER_GFP (((GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN \ > > +#define HIGH_ORDER_GFP (((GFP_HIGHUSER | __GFP_ZERO \ > > | __GFP_NORETRY) & ~__GFP_RECLAIM) \ > > | __GFP_COMP) > > #define LOW_ORDER_GFP (GFP_HIGHUSER | __GFP_ZERO | __GFP_COMP) > > @@ -315,6 +315,7 @@ static struct page *alloc_largest_available(unsigned long size, > > unsigned int max_order) > > { > > struct page *page; > > + unsigned long gfp_flags; > > int i; > > > > for (i = 0; i < NUM_ORDERS; i++) { > > @@ -323,7 +324,11 @@ static struct page *alloc_largest_available(unsigned long size, > > if (max_order < orders[i]) > > continue; > > > > - page = alloc_pages(order_flags[i], orders[i]); > > + gfp_flags = order_flags[i]; > > + if (orders[i] > PAGE_ALLOC_COSTLY_ORDER) > > + gfp_flags |= __GFP_NOWARN; > > + > > + page = alloc_pages(gfp_flags, orders[i]); > > Would it be cleaner to just set up the flags properly in the > order_flags array? I'm not sure I understand why your patch does it > dynamically?
That's exactly I had in my branch for aosp fix but I wanted to hear it explicitly from dmabuf maintainer since I was worried chaninging order-4 allocation behavior, especially, __GFP_NORETRY and &~__GFP_RECLAIM. (It will make allocation failure easier than old and that's not thing my patch is addressing).
If you want this, I am happy to change it. Shall I?
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index 29e49ac17251..865ec847013d 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -44,7 +44,7 @@ struct dma_heap_attachment { | __GFP_NORETRY) & ~__GFP_RECLAIM) \ | __GFP_COMP) #define LOW_ORDER_GFP (GFP_HIGHUSER | __GFP_ZERO | __GFP_COMP) -static gfp_t order_flags[] = {HIGH_ORDER_GFP, LOW_ORDER_GFP, LOW_ORDER_GFP}; +static gfp_t order_flags[] = {HIGH_ORDER_GFP, HIGH_ORDER_GFP, LOW_ORDER_GFP}; /* * The selection of the orders used for allocation (1MB, 64K, 4K) is designed * to match with the sizes often found in IOMMUs. Using order 4 pages instead
| |