This patch adds a new constructor for an sg table. The table is constructed from an array of struct pages. All contiguous chunks of the pages are merged into a single sg nodes. A user may provide an offset and a size of a buffer if the buffer is not page-aligned.
The function is dedicated for DMABUF exporters which often perform conversion from an page array to a scatterlist. Moreover the scatterlist should be squashed in order to save memory and to speed-up the process of DMA mapping using dma_map_sg.
The code is based on the patch 'v4l: vb2-dma-contig: add support for scatterlist in userptr mode' and hints from Laurent Pinchart.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com --- include/linux/scatterlist.h | 4 +++ lib/scatterlist.c | 64 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index ac9586d..7b600da 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -214,6 +214,10 @@ void sg_free_table(struct sg_table *); int __sg_alloc_table(struct sg_table *, unsigned int, unsigned int, 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 long offset, unsigned long size, + gfp_t gfp_mask);
size_t sg_copy_from_buffer(struct scatterlist *sgl, unsigned int nents, void *buf, size_t buflen); diff --git a/lib/scatterlist.c b/lib/scatterlist.c index 6096e89..85868a1 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -319,6 +319,70 @@ 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 + * + * Description: + * Allocate and initialize an sg table from a list of pages. Continuous + * 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 long offset, unsigned long size, + gfp_t gfp_mask) +{ + unsigned int chunks; + unsigned int i; + unsigned int cur_page; + int ret; + struct scatterlist *s; + + /* compute number of contiguous chunks */ + chunks = 1; + for (i = 1; i < n_pages; ++i) + if (page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1) + ++chunks; + + ret = sg_alloc_table(sgt, chunks, gfp_mask); + if (unlikely(ret)) + return ret; + + /* merging chunks and putting them into the scatterlist */ + cur_page = 0; + for_each_sg(sgt->sgl, s, sgt->orig_nents, i) { + unsigned long chunk_size; + unsigned int j; + + /* looking for the end of the current chunk */ + for (j = cur_page + 1; j < n_pages; ++j) + if (page_to_pfn(pages[j]) != + page_to_pfn(pages[j - 1]) + 1) + break; + + chunk_size = ((j - cur_page) << PAGE_SHIFT) - offset; + sg_set_page(s, pages[cur_page], min(size, chunk_size), offset); + size -= chunk_size; + offset = 0; + cur_page = j; + } + + return 0; +} +EXPORT_SYMBOL(sg_alloc_table_from_pages); + +/** * sg_miter_start - start mapping iteration over a sg list * @miter: sg mapping iter to be started * @sgl: sg list to iterate over
On Tue, May 08, 2012 at 11:50:33AM +0200, Tomasz Stanislawski wrote:
This patch adds a new constructor for an sg table. The table is constructed from an array of struct pages. All contiguous chunks of the pages are merged into a single sg nodes. A user may provide an offset and a size of a buffer if the buffer is not page-aligned.
The function is dedicated for DMABUF exporters which often perform conversion from an page array to a scatterlist. Moreover the scatterlist should be squashed in order to save memory and to speed-up the process of DMA mapping using dma_map_sg.
The code is based on the patch 'v4l: vb2-dma-contig: add support for scatterlist in userptr mode' and hints from Laurent Pinchart.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
Looks neat and useful and we could use this to kill the copy in drm_prime.c. So
Acked-by: Daniel Vetter daniel.vetter@ffwll.ch
include/linux/scatterlist.h | 4 +++ lib/scatterlist.c | 64 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index ac9586d..7b600da 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -214,6 +214,10 @@ void sg_free_table(struct sg_table *); int __sg_alloc_table(struct sg_table *, unsigned int, unsigned int, 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 long offset, unsigned long size,
- gfp_t gfp_mask);
size_t sg_copy_from_buffer(struct scatterlist *sgl, unsigned int nents, void *buf, size_t buflen); diff --git a/lib/scatterlist.c b/lib/scatterlist.c index 6096e89..85868a1 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -319,6 +319,70 @@ 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
- Description:
- Allocate and initialize an sg table from a list of pages. Continuous
- 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 long offset, unsigned long size,
- gfp_t gfp_mask)
+{
- unsigned int chunks;
- unsigned int i;
- unsigned int cur_page;
- int ret;
- struct scatterlist *s;
- /* compute number of contiguous chunks */
- chunks = 1;
- for (i = 1; i < n_pages; ++i)
if (page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1)++chunks;- ret = sg_alloc_table(sgt, chunks, gfp_mask);
- if (unlikely(ret))
return ret;- /* merging chunks and putting them into the scatterlist */
- cur_page = 0;
- for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
unsigned long chunk_size;unsigned int j;/* looking for the end of the current chunk */for (j = cur_page + 1; j < n_pages; ++j)if (page_to_pfn(pages[j]) !=page_to_pfn(pages[j - 1]) + 1)break;chunk_size = ((j - cur_page) << PAGE_SHIFT) - offset;sg_set_page(s, pages[cur_page], min(size, chunk_size), offset);size -= chunk_size;offset = 0;cur_page = j;- }
- return 0;
+} +EXPORT_SYMBOL(sg_alloc_table_from_pages);
+/**
- sg_miter_start - start mapping iteration over a sg list
- @miter: sg mapping iter to be started
- @sgl: sg list to iterate over
-- 1.7.5.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Tomasz,
Thank you for the patch.
On Tuesday 08 May 2012 11:50:33 Tomasz Stanislawski wrote:
This patch adds a new constructor for an sg table. The table is constructed from an array of struct pages. All contiguous chunks of the pages are merged into a single sg nodes. A user may provide an offset and a size of a buffer if the buffer is not page-aligned.
The function is dedicated for DMABUF exporters which often perform conversion from an page array to a scatterlist. Moreover the scatterlist should be squashed in order to save memory and to speed-up the process of DMA mapping using dma_map_sg.
The code is based on the patch 'v4l: vb2-dma-contig: add support for scatterlist in userptr mode' and hints from Laurent Pinchart.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
include/linux/scatterlist.h | 4 +++ lib/scatterlist.c | 64 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index ac9586d..7b600da 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -214,6 +214,10 @@ void sg_free_table(struct sg_table *); int __sg_alloc_table(struct sg_table *, unsigned int, unsigned int, 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 long offset, unsigned long size,
- gfp_t gfp_mask);
size_t sg_copy_from_buffer(struct scatterlist *sgl, unsigned int nents, void *buf, size_t buflen); diff --git a/lib/scatterlist.c b/lib/scatterlist.c index 6096e89..85868a1 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -319,6 +319,70 @@ 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
- Description:
- Allocate and initialize an sg table from a list of pages. Continuous
- 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 long offset, unsigned long size,
- gfp_t gfp_mask)
+{
- unsigned int chunks;
- unsigned int i;
- unsigned int cur_page;
- int ret;
- struct scatterlist *s;
- /* compute number of contiguous chunks */
- chunks = 1;
- for (i = 1; i < n_pages; ++i)
if (page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1)++chunks;- ret = sg_alloc_table(sgt, chunks, gfp_mask);
- if (unlikely(ret))
return ret;- /* merging chunks and putting them into the scatterlist */
- cur_page = 0;
- for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
unsigned long chunk_size;unsigned int j;/* looking for the end of the current chunk */for (j = cur_page + 1; j < n_pages; ++j)if (page_to_pfn(pages[j]) !=page_to_pfn(pages[j - 1]) + 1)break;chunk_size = ((j - cur_page) << PAGE_SHIFT) - offset;sg_set_page(s, pages[cur_page], min(size, chunk_size), offset);size -= chunk_size;offset = 0;cur_page = j;- }
- return 0;
+} +EXPORT_SYMBOL(sg_alloc_table_from_pages);
+/**
- sg_miter_start - start mapping iteration over a sg list
- @miter: sg mapping iter to be started
- @sgl: sg list to iterate over
On Tue, 08 May 2012 11:50:33 +0200 Tomasz Stanislawski t.stanislaws@samsung.com wrote:
This patch adds a new constructor for an sg table. The table is constructed from an array of struct pages. All contiguous chunks of the pages are merged into a single sg nodes. A user may provide an offset and a size of a buffer if the buffer is not page-aligned.
The function is dedicated for DMABUF exporters which often perform conversion from an page array to a scatterlist. Moreover the scatterlist should be squashed in order to save memory and to speed-up the process of DMA mapping using dma_map_sg.
The code is based on the patch 'v4l: vb2-dma-contig: add support for scatterlist in userptr mode' and hints from Laurent Pinchart.
...
/**
- 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. Continuous
s/Continuous/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
- **/
nit: Use */, not **/ here.
+int sg_alloc_table_from_pages(struct sg_table *sgt,
- struct page **pages, unsigned int n_pages,
- unsigned long offset, unsigned long size,
- gfp_t gfp_mask)
I guess a 32-bit n_pages is OK. A 16TB IO seems enough ;)
+{
- unsigned int chunks;
- unsigned int i;
erk, please choose a different name for this. When a C programmer sees "i", he very much assumes it has type "int". Making it unsigned causes surprise.
And don't rename it to "u"! Let's give it a nice meaningful name. pageno?
- unsigned int cur_page;
- int ret;
- struct scatterlist *s;
- /* compute number of contiguous chunks */
- chunks = 1;
- for (i = 1; i < n_pages; ++i)
if (page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1)
This assumes that if two pages have contiguous pfn's then they are physically contiguous. Is that true for all architectures and memory models, including sparsemem? See sparse_encode_mem_map().
++chunks;- ret = sg_alloc_table(sgt, chunks, gfp_mask);
- if (unlikely(ret))
return ret;- /* merging chunks and putting them into the scatterlist */
- cur_page = 0;
- for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
unsigned long chunk_size;unsigned int j;
"j" is an "int", too.
/* looking for the end of the current chunk */
s/looking/look/
for (j = cur_page + 1; j < n_pages; ++j)if (page_to_pfn(pages[j]) !=page_to_pfn(pages[j - 1]) + 1)break;chunk_size = ((j - cur_page) << PAGE_SHIFT) - offset;sg_set_page(s, pages[cur_page], min(size, chunk_size), offset);size -= chunk_size;offset = 0;cur_page = j;- }
- return 0;
+} +EXPORT_SYMBOL(sg_alloc_table_from_pages);
Hi Andrew, Thank you for your review, Please refer to the comments below.
On 05/18/2012 01:56 AM, Andrew Morton wrote:
On Tue, 08 May 2012 11:50:33 +0200 Tomasz Stanislawski t.stanislaws@samsung.com wrote:
This patch adds a new constructor for an sg table. The table is constructed from an array of struct pages. All contiguous chunks of the pages are merged into a single sg nodes. A user may provide an offset and a size of a buffer if the buffer is not page-aligned.
The function is dedicated for DMABUF exporters which often perform conversion from an page array to a scatterlist. Moreover the scatterlist should be squashed in order to save memory and to speed-up the process of DMA mapping using dma_map_sg.
The code is based on the patch 'v4l: vb2-dma-contig: add support for scatterlist in userptr mode' and hints from Laurent Pinchart.
...
/**
- 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. Continuous
s/Continuous/Contiguous/
Ok. Thanks for noticing it.
- 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
- **/
nit: Use */, not **/ here.
ok
+int sg_alloc_table_from_pages(struct sg_table *sgt,
- struct page **pages, unsigned int n_pages,
- unsigned long offset, unsigned long size,
- gfp_t gfp_mask)
I guess a 32-bit n_pages is OK. A 16TB IO seems enough ;)
Do you think that 'unsigned long' for offset is too big?
Ad n_pages. Assuming that Moore's law holds it will take circa 25 years before the limit of 16 TB is reached :) for high-end scatterlist operations. Or I can change the type of n_pages to 'unsigned long' now at no cost :).
+{
- unsigned int chunks;
- unsigned int i;
erk, please choose a different name for this. When a C programmer sees "i", he very much assumes it has type "int". Making it unsigned causes surprise.
And don't rename it to "u"! Let's give it a nice meaningful name. pageno?
The problem is that 'i' is a natural name for a loop counter. This exactly how 'i' is used in this function. The type 'int' was used in the initial version of the code. It was changed to avoid 'unsigned vs signed' comparisons in the loop condition.
AFAIK, in the kernel code developers try to avoid Hungarian notation. A name of a variable should reflect its purpose, not its type. I can change the name of 'i' to 'pageno' and 'j' to 'pageno2' (?) but I think it will make the code less reliable.
- unsigned int cur_page;
- int ret;
- struct scatterlist *s;
- /* compute number of contiguous chunks */
- chunks = 1;
- for (i = 1; i < n_pages; ++i)
if (page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1)This assumes that if two pages have contiguous pfn's then they are physically contiguous. Is that true for all architectures and memory models, including sparsemem? See sparse_encode_mem_map().
This is a very good questions. I did some research and I had looked for all pfn_to_phys implementations in the kernel code. I found that all conversions are performed by bit shifting. Therefore I expect that assumption that contiguous PFNs imply contiguous physical addresses is true for all architectures supported by Linux kernel.
++chunks;- ret = sg_alloc_table(sgt, chunks, gfp_mask);
- if (unlikely(ret))
return ret;- /* merging chunks and putting them into the scatterlist */
- cur_page = 0;
- for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
unsigned long chunk_size;unsigned int j;"j" is an "int", too.
Please refer to 'i'-arguments above.
/* looking for the end of the current chunk */s/looking/look/
ok
for (j = cur_page + 1; j < n_pages; ++j)if (page_to_pfn(pages[j]) !=page_to_pfn(pages[j - 1]) + 1)break;chunk_size = ((j - cur_page) << PAGE_SHIFT) - offset;sg_set_page(s, pages[cur_page], min(size, chunk_size), offset);size -= chunk_size;offset = 0;cur_page = j;- }
- return 0;
+} +EXPORT_SYMBOL(sg_alloc_table_from_pages);
Regards, Tomasz Stanislawski
On Mon, 21 May 2012 16:01:50 +0200 Tomasz Stanislawski t.stanislaws@samsung.com wrote:
+int sg_alloc_table_from_pages(struct sg_table *sgt,
- struct page **pages, unsigned int n_pages,
- unsigned long offset, unsigned long size,
- gfp_t gfp_mask)
I guess a 32-bit n_pages is OK. A 16TB IO seems enough ;)
Do you think that 'unsigned long' for offset is too big?
Ad n_pages. Assuming that Moore's law holds it will take circa 25 years before the limit of 16 TB is reached :) for high-end scatterlist operations. Or I can change the type of n_pages to 'unsigned long' now at no cost :).
By then it will be Someone Else's Problem ;)
+{
- unsigned int chunks;
- unsigned int i;
erk, please choose a different name for this. When a C programmer sees "i", he very much assumes it has type "int". Making it unsigned causes surprise.
And don't rename it to "u"! Let's give it a nice meaningful name. pageno?
The problem is that 'i' is a natural name for a loop counter.
It's also the natural name for an integer. If a C programmer sees "i", he thinks "int". It's a Fortran thing ;)
AFAIK, in the kernel code developers try to avoid Hungarian notation. A name of a variable should reflect its purpose, not its type. I can change the name of 'i' to 'pageno' and 'j' to 'pageno2' (?) but I think it will make the code less reliable.
Well, one could do something radical such as using "p".
On 05/22/2012 10:10 PM, Andrew Morton wrote:
On Mon, 21 May 2012 16:01:50 +0200 Tomasz Stanislawski t.stanislaws@samsung.com wrote:
+int sg_alloc_table_from_pages(struct sg_table *sgt,
- struct page **pages, unsigned int n_pages,
- unsigned long offset, unsigned long size,
- gfp_t gfp_mask)
I guess a 32-bit n_pages is OK. A 16TB IO seems enough ;)
Do you think that 'unsigned long' for offset is too big?
Ad n_pages. Assuming that Moore's law holds it will take circa 25 years before the limit of 16 TB is reached :) for high-end scatterlist operations. Or I can change the type of n_pages to 'unsigned long' now at no cost :).
By then it will be Someone Else's Problem ;)
Ok. So let's keep to 'unsigned int n_pages'.
+{
- unsigned int chunks;
- unsigned int i;
erk, please choose a different name for this. When a C programmer sees "i", he very much assumes it has type "int". Making it unsigned causes surprise.
And don't rename it to "u"! Let's give it a nice meaningful name. pageno?
The problem is that 'i' is a natural name for a loop counter.
It's also the natural name for an integer. If a C programmer sees "i", he thinks "int". It's a Fortran thing ;)
AFAIK, in the kernel code developers try to avoid Hungarian notation. A name of a variable should reflect its purpose, not its type. I can change the name of 'i' to 'pageno' and 'j' to 'pageno2' (?) but I think it will make the code less reliable.
Well, one could do something radical such as using "p".
I can not change the type to 'int' due to 'signed vs unsigned' comparisons in the loop condition. What do you think about changing the names 'i' -> 'p' and 'j' -> 'q'?
Regards, Tomasz Stanislawski
-- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
dri-devel@lists.freedesktop.org
-
Andrew Morton -
Daniel Vetter -
Laurent Pinchart -
Tomasz Stanislawski