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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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.

Regards,

Tvrtko

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-
> gather list.
> 
> Introduce and export the __sg_alloc_table_from_pages function
> which will allow it that control.
> 
> v2: Reorder parameters. (Chris Wilson)
> v3: Fix incomplete reordering in v2.
> v4: max_segment needs to be page aligned.
> v5: Rebase.
> v6: Rebase.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v2)
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   include/linux/scatterlist.h | 11 +++++----
>   lib/scatterlist.c           | 58 +++++++++++++++++++++++++++++++++++----------
>   2 files changed, 52 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index 6dd2ddbc6230..874b50c232de 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -267,10 +267,13 @@ void sg_free_table(struct sg_table *);
>   int __sg_alloc_table(struct sg_table *, unsigned int, unsigned int,
>   		     struct scatterlist *, gfp_t, sg_alloc_fn *);
>   int sg_alloc_table(struct sg_table *, unsigned int, gfp_t);
> -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);
> +int __sg_alloc_table_from_pages(struct sg_table *sgt, struct page **pages,
> +				unsigned int n_pages, unsigned int offset,
> +				unsigned long size, unsigned int max_segment,
> +				gfp_t gfp_mask);
> +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);
>   
>   size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, void *buf,
>   		      size_t buflen, off_t skip, bool to_buffer);
> diff --git a/lib/scatterlist.c b/lib/scatterlist.c
> index 7b2e74da2c44..1a5900f9a057 100644
> --- 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
> @@ -389,16 +390,18 @@ EXPORT_SYMBOL(sg_alloc_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)
> +int __sg_alloc_table_from_pages(struct sg_table *sgt, struct page **pages,
> +				unsigned int n_pages, unsigned int offset,
> +				unsigned long size, unsigned int max_segment,
> +				gfp_t gfp_mask)
>   {
> -	const unsigned int max_segment = SCATTERLIST_MAX_SEGMENT;
>   	unsigned int chunks, cur_page, seg_len, i;
>   	int ret;
>   	struct scatterlist *s;
>   
> +	if (WARN_ON(!max_segment || offset_in_page(max_segment)))
> +		return -EINVAL;
> +
>   	/* compute number of contiguous chunks */
>   	chunks = 1;
>   	seg_len = 0;
> @@ -440,6 +443,35 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
>   
>   	return 0;
>   }
> +EXPORT_SYMBOL(__sg_alloc_table_from_pages);
> +
> +/**
> + * 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);
> +}
>   EXPORT_SYMBOL(sg_alloc_table_from_pages);
>   
>   void __sg_page_iter_start(struct sg_page_iter *piter,
>