On Tue, Apr 15, 2025 at 7:28 PM 김재원 <jaewon31....@samsung.com> wrote:
>
>
>
> > -----Original Message-----
> > From: T.J. Mercier [mailto:tjmerc...@google.com]
> > Sent: Wednesday, April 16, 2025 5:57 AM
> > To: Juan Yescas <jyes...@google.com>
> > Cc: Sumit Semwal <sumit.sem...@linaro.org>; Benjamin Gaignard
> > <benjamin.gaign...@collabora.com>; Brian Starkey <brian.star...@arm.com>;
> > John Stultz <jstu...@google.com>; Christian König
> > <christian.koe...@amd.com>; linux-me...@vger.kernel.org; dri-
> > de...@lists.freedesktop.org; linaro-mm-...@lists.linaro.org; linux-
> > ker...@vger.kernel.org; bao...@kernel.org; dmitry.osipe...@collabora.com;
> > jaewon31....@samsung.com; guangming....@mediatek.com; sur...@google.com;
> > kaleshsi...@google.com
> > Subject: Re: [PATCH] dma-buf: heaps: Set allocation orders for larger page
> > sizes
> >
> > On Tue, Apr 15, 2025 at 10:20 AM Juan Yescas <jyes...@google.com> wrote:
> > >
> > > This change sets the allocation orders for the different page sizes
> > > (4k, 16k, 64k) based on PAGE_SHIFT. Before this change, the orders for
> > > large page sizes were calculated incorrectly, this caused system heap
> > > to allocate from 2% to 4% more memory on 16KiB page size kernels.
> > >
> > > This change was tested on 4k/16k page size kernels.
> > >
> > > Signed-off-by: Juan Yescas <jyes...@google.com>
> >
> > I think "dma-buf: system_heap:" would be better for the subject since this
> > is specific to the system heap.
> >
> > Would you mind cleaning up the extra space on line 321 too?
> > @@ -318,7 +318,7 @@ static struct page
> > *alloc_largest_available(unsigned long size,
> > int i;
> >
> > for (i = 0; i < NUM_ORDERS; i++) {
> > - if (size < (PAGE_SIZE << orders[i]))
> > + if (size < (PAGE_SIZE << orders[i]))
> >
> > With that,
> > Reviewed-by: T.J. Mercier <tjmerc...@google.com>
> >
> > Fixes: d963ab0f15fb ("dma-buf: system_heap: Allocate higher order pages if
> > available") is also probably a good idea.
> >
>
>
> Hi Juan.
>
> Yes. This system_heap change should be changed for 16KB page. Actually,
> we may need to check other drivers using page order number. I guess
> gpu drivers may be one of them.
>
Thanks Jaewon for pointing it out. We'll take a look at the GPU drivers to make
sure that they are using the proper page order.
> > > ---
> > > drivers/dma-buf/heaps/system_heap.c | 9 ++++++++-
> > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/dma-buf/heaps/system_heap.c
> > > b/drivers/dma-buf/heaps/system_heap.c
> > > index 26d5dc89ea16..54674c02dcb4 100644
> > > --- a/drivers/dma-buf/heaps/system_heap.c
> > > +++ b/drivers/dma-buf/heaps/system_heap.c
> > > @@ -50,8 +50,15 @@ static gfp_t order_flags[] = {HIGH_ORDER_GFP,
> > HIGH_ORDER_GFP, LOW_ORDER_GFP};
> > > * to match with the sizes often found in IOMMUs. Using order 4 pages
> > instead
> > > * of order 0 pages can significantly improve the performance of many
> > IOMMUs
> > > * by reducing TLB pressure and time spent updating page tables.
> > > + *
> > > + * Note: When the order is 0, the minimum allocation is PAGE_SIZE.
> > > + The possible
> > > + * page sizes for ARM devices could be 4K, 16K and 64K.
> > > */
> > > -static const unsigned int orders[] = {8, 4, 0};
> > > +#define ORDER_1M (20 - PAGE_SHIFT)
> > > +#define ORDER_64K (16 - PAGE_SHIFT)
> > > +#define ORDER_FOR_PAGE_SIZE (0)
> > > +static const unsigned int orders[] = {ORDER_1M, ORDER_64K,
> > > +ORDER_FOR_PAGE_SIZE};
> > > +
> > > #define NUM_ORDERS ARRAY_SIZE(orders)
> > >
> > > static struct sg_table *dup_sg_table(struct sg_table *table)
> > > --
> > > 2.49.0.604.gff1f9ca942-goog
> > >
>
>
