Re: [Intel-gfx] [PATCH 3/4] lib/scatterlist: Introduce and export __sg_alloc_table_from_pages

Andrew Morton
2017-08-02T23:01:56Z

On Wed, 2 Aug 2017 14:06:39 +0100 Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:

>
> Hi Andrew,
>
> We have a couple of small lib/scatterlist.c tidies here, plus exporting
> the new API which allows drivers to control the maximum coalesced entry
> as created by __sg_alloc_table_from_pages.
>
> I am looking for an ack to merge these three patches via the drm-intel tree.
>
>
> ...
>
> On 27/07/2017 10:05, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >
> > Drivers like i915 benefit from being able to control the maxium
> > size of the sg coallesced segment while building the scatter-

"coalesced"

> > gather list.
> >
> > Introduce and export the __sg_alloc_table_from_pages function
> > which will allow it that control.
> >
>
> ...
>
> > --- a/lib/scatterlist.c
> > +++ b/lib/scatterlist.c
> > @@ -370,14 +370,15 @@ int sg_alloc_table(struct sg_table *table, unsigned int nents, gfp_t gfp_mask)
> > EXPORT_SYMBOL(sg_alloc_table);
> >
> > /**
> > - * sg_alloc_table_from_pages - Allocate and initialize an sg table from
> > - * an array of pages
> > - * @sgt: The sg table header to use
> > - * @pages: Pointer to an array of page pointers
> > - * @n_pages: Number of pages in the pages array
> > - * @offset: Offset from start of the first page to the start of a buffer
> > - * @size: Number of valid bytes in the buffer (after offset)
> > - * @gfp_mask: GFP allocation mask
> > + * __sg_alloc_table_from_pages - Allocate and initialize an sg table from
> > + * an array of pages
> > + * @sgt: The sg table header to use
> > + * @pages: Pointer to an array of page pointers
> > + * @n_pages: Number of pages in the pages array
> > + * @offset: Offset from start of the first page to the start of a buffer
> > + * @size: Number of valid bytes in the buffer (after offset)
> > + * @max_segment: Maximum size of a scatterlist node in bytes (page aligned)
> > + * @gfp_mask: GFP allocation mask
> > *
> > * Description:
> > * Allocate and initialize an sg table from a list of pages. Contiguous

The Description doesn't describe how this differs from
sg_alloc_table_from_pages(), although it doesn't seem terribly
important.

> > +
> > +/**
> > + * sg_alloc_table_from_pages - Allocate and initialize an sg table from
> > + * an array of pages
> > + * @sgt: The sg table header to use
> > + * @pages: Pointer to an array of page pointers
> > + * @n_pages: Number of pages in the pages array
> > + * @offset: Offset from start of the first page to the start of a buffer
> > + * @size: Number of valid bytes in the buffer (after offset)
> > + * @gfp_mask: GFP allocation mask
> > + *
> > + * Description:
> > + * Allocate and initialize an sg table from a list of pages. Contiguous
> > + * ranges of the pages are squashed into a single scatterlist node. A user
> > + * may provide an offset at a start and a size of valid data in a buffer
> > + * specified by the page array. The returned sg table is released by
> > + * sg_free_table.
> > + *
> > + * Returns:
> > + * 0 on success, negative error on failure
> > + */
> > +int sg_alloc_table_from_pages(struct sg_table *sgt, struct page **pages,
> > + unsigned int n_pages, unsigned int offset,
> > + unsigned long size, gfp_t gfp_mask)
> > +{
> > + return __sg_alloc_table_from_pages(sgt, pages, n_pages, offset, size,
> > + SCATTERLIST_MAX_SEGMENT, gfp_mask);
> > +}

Making this an inline would save a bunch or stack space in the callers?

One could just add the new arg then run around and update the 10ish
callers but I guess the new interface is OK.

Otherwise it's OK by me, please go ahead as proposed.