From: Tvrtko Ursulin <[email protected]>
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 <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: [email protected]
Cc: Chris Wilson <[email protected]>
Reviewed-by: Chris Wilson <[email protected]> (v2)
Cc: Joonas Lahtinen <[email protected]>
---
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,
--
2.9.4
Quoting Tvrtko Ursulin (2017-07-27 10:05:03)
> From: Tvrtko Ursulin <[email protected]>
>
> 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 <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Cc: [email protected]
> Cc: Chris Wilson <[email protected]>
> Reviewed-by: Chris Wilson <[email protected]> (v2)
> Cc: Joonas Lahtinen <[email protected]>
Sill happy and checked the external user,
Reviewed-by: Chris Wilson <[email protected]>
-Chris
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 <[email protected]>
>
> 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 <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Cc: [email protected]
> Cc: Chris Wilson <[email protected]>
> Reviewed-by: Chris Wilson <[email protected]> (v2)
> Cc: Joonas Lahtinen <[email protected]>
> ---
> 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,
>
On Wed, 2 Aug 2017 14:06:39 +0100 Tvrtko Ursulin <[email protected]> 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 <[email protected]>
> >
> > 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.
On 03/08/2017 00:01, Andrew Morton wrote:
> On Wed, 2 Aug 2017 14:06:39 +0100 Tvrtko Ursulin <[email protected]> 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 <[email protected]>
>>>
>>> Drivers like i915 benefit from being able to control the maxium
>>> size of the sg coallesced segment while building the scatter-
>
> "coalesced"
Oops, I've had this in other patches in the series. Fixed now.
>>> 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.
Well spotted, I've fixed this as well.
>>> +
>>> +/**
>>> + * 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.
On the first suggestion - there are other trivial wrappers in
lib/scatterlist.c which could benefit from the same treatment (move to
inline) so I opted not to do this in this patch but will send a follow up.
> Otherwise it's OK by me, please go ahead as proposed.
Thanks a lot!
Regards,
Tvrtko