* [PATCH 0/5] Avoid pessimistic scatter-gather allocation
@ 2016-10-21 14:11 ` Tvrtko Ursulin
0 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2016-10-21 14:11 UTC (permalink / raw)
To: Intel-gfx; +Cc: linux-kernel, linux-media
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
We can decrease the i915 kernel memory usage by doing more sg list
coallescing and avoiding the pessimistic list allocation.
At the moment we got two places in our code, the main shmemfs backed
object allocator, and the userptr object allocator, which both can
allocate sg list size pessimistically, and in the latter case also do
not exploit entry coallescing when it is possible.
This results in between one to six megabytes of memory wasted on unused
sg list entries under some common workloads:
* Logging into KDE there is 1-2 MiB of unused sg entries.
* Running the T-Rex benchamrk aroun 3 Mib.
* Similarly for Manhattan 5-6 MiB.
To remove this wastage this series starts with some cleanups in the
sg_alloc_table_from_pages implementation and then adds and exports a new
__sg_alloc_table_from_pages function.
This then gets used by the i915 driver to achieve the described savings.
Tvrtko Ursulin (5):
lib/scatterlist: Fix offset type in sg_alloc_table_from_pages
lib/scatterlist: Avoid potential scatterlist entry overflow
lib/scatterlist: Introduce and export __sg_alloc_table_from_pages
drm/i915: Use __sg_alloc_table_from_pages for allocating object
backing store
drm/i915: Use __sg_alloc_table_from_pages for userptr allocations
drivers/gpu/drm/i915/i915_drv.h | 9 +++
drivers/gpu/drm/i915/i915_gem.c | 77 +++++++++++--------------
drivers/gpu/drm/i915/i915_gem_userptr.c | 29 +++-------
drivers/media/v4l2-core/videobuf2-dma-contig.c | 4 +-
drivers/rapidio/devices/rio_mport_cdev.c | 4 +-
include/linux/scatterlist.h | 11 ++--
lib/scatterlist.c | 78 ++++++++++++++++++++------
7 files changed, 120 insertions(+), 92 deletions(-)
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/5] lib/scatterlist: Fix offset type in sg_alloc_table_from_pages
2016-10-21 14:11 ` Tvrtko Ursulin
@ 2016-10-21 14:11 ` Tvrtko Ursulin
-1 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2016-10-21 14:11 UTC (permalink / raw)
To: Intel-gfx
Cc: linux-kernel, linux-media, Chris Wilson, Tvrtko Ursulin,
Masahiro Yamada, Pawel Osciak, Marek Szyprowski, Kyungmin Park,
Tomasz Stanislawski, Matt Porter, Alexandre Bounine
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Scatterlist entries have an unsigned int for the offset so
correct the sg_alloc_table_from_pages function accordingly.
Since these are offsets withing a page, unsigned int is
wide enough.
Also converts callers which were using unsigned long locally
with the lower_32_bits annotation to make it explicitly
clear what is happening.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Pawel Osciak <pawel@osciak.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Tomasz Stanislawski <t.stanislaws@samsung.com>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Alexandre Bounine <alexandre.bounine@idt.com>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/media/v4l2-core/videobuf2-dma-contig.c | 4 ++--
drivers/rapidio/devices/rio_mport_cdev.c | 4 ++--
include/linux/scatterlist.h | 2 +-
lib/scatterlist.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index fb6a177be461..a3aac7533241 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -478,7 +478,7 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr,
{
struct vb2_dc_buf *buf;
struct frame_vector *vec;
- unsigned long offset;
+ unsigned int offset;
int n_pages, i;
int ret = 0;
struct sg_table *sgt;
@@ -506,7 +506,7 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr,
buf->dev = dev;
buf->dma_dir = dma_dir;
- offset = vaddr & ~PAGE_MASK;
+ offset = lower_32_bits(vaddr & ~PAGE_MASK);
vec = vb2_create_framevec(vaddr, size, dma_dir == DMA_FROM_DEVICE);
if (IS_ERR(vec)) {
ret = PTR_ERR(vec);
diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
index 436dfe871d32..f545cf20561f 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -876,10 +876,10 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
* offset within the internal buffer specified by handle parameter.
*/
if (xfer->loc_addr) {
- unsigned long offset;
+ unsigned int offset;
long pinned;
- offset = (unsigned long)(uintptr_t)xfer->loc_addr & ~PAGE_MASK;
+ offset = lower_32_bits(xfer->loc_addr & ~PAGE_MASK);
nr_pages = PAGE_ALIGN(xfer->length + offset) >> PAGE_SHIFT;
page_list = kmalloc_array(nr_pages,
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index cb3c8fe6acd7..c981bee1a3ae 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -263,7 +263,7 @@ int __sg_alloc_table(struct sg_table *, unsigned int, unsigned int,
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,
+ unsigned int offset, unsigned long size,
gfp_t gfp_mask);
size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, void *buf,
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 004fc70fc56a..e05e7fc98892 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -391,7 +391,7 @@ EXPORT_SYMBOL(sg_alloc_table);
*/
int sg_alloc_table_from_pages(struct sg_table *sgt,
struct page **pages, unsigned int n_pages,
- unsigned long offset, unsigned long size,
+ unsigned int offset, unsigned long size,
gfp_t gfp_mask)
{
unsigned int chunks;
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 1/5] lib/scatterlist: Fix offset type in sg_alloc_table_from_pages
@ 2016-10-21 14:11 ` Tvrtko Ursulin
0 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2016-10-21 14:11 UTC (permalink / raw)
To: Intel-gfx
Cc: Tomasz Stanislawski, Pawel Osciak, linux-kernel, Masahiro Yamada,
Kyungmin Park, Matt Porter, Marek Szyprowski, Alexandre Bounine,
linux-media
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Scatterlist entries have an unsigned int for the offset so
correct the sg_alloc_table_from_pages function accordingly.
Since these are offsets withing a page, unsigned int is
wide enough.
Also converts callers which were using unsigned long locally
with the lower_32_bits annotation to make it explicitly
clear what is happening.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Pawel Osciak <pawel@osciak.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Tomasz Stanislawski <t.stanislaws@samsung.com>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Alexandre Bounine <alexandre.bounine@idt.com>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/media/v4l2-core/videobuf2-dma-contig.c | 4 ++--
drivers/rapidio/devices/rio_mport_cdev.c | 4 ++--
include/linux/scatterlist.h | 2 +-
lib/scatterlist.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index fb6a177be461..a3aac7533241 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -478,7 +478,7 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr,
{
struct vb2_dc_buf *buf;
struct frame_vector *vec;
- unsigned long offset;
+ unsigned int offset;
int n_pages, i;
int ret = 0;
struct sg_table *sgt;
@@ -506,7 +506,7 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr,
buf->dev = dev;
buf->dma_dir = dma_dir;
- offset = vaddr & ~PAGE_MASK;
+ offset = lower_32_bits(vaddr & ~PAGE_MASK);
vec = vb2_create_framevec(vaddr, size, dma_dir == DMA_FROM_DEVICE);
if (IS_ERR(vec)) {
ret = PTR_ERR(vec);
diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
index 436dfe871d32..f545cf20561f 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -876,10 +876,10 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
* offset within the internal buffer specified by handle parameter.
*/
if (xfer->loc_addr) {
- unsigned long offset;
+ unsigned int offset;
long pinned;
- offset = (unsigned long)(uintptr_t)xfer->loc_addr & ~PAGE_MASK;
+ offset = lower_32_bits(xfer->loc_addr & ~PAGE_MASK);
nr_pages = PAGE_ALIGN(xfer->length + offset) >> PAGE_SHIFT;
page_list = kmalloc_array(nr_pages,
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index cb3c8fe6acd7..c981bee1a3ae 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -263,7 +263,7 @@ int __sg_alloc_table(struct sg_table *, unsigned int, unsigned int,
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,
+ unsigned int offset, unsigned long size,
gfp_t gfp_mask);
size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, void *buf,
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 004fc70fc56a..e05e7fc98892 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -391,7 +391,7 @@ EXPORT_SYMBOL(sg_alloc_table);
*/
int sg_alloc_table_from_pages(struct sg_table *sgt,
struct page **pages, unsigned int n_pages,
- unsigned long offset, unsigned long size,
+ unsigned int offset, unsigned long size,
gfp_t gfp_mask)
{
unsigned int chunks;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 1/5] lib/scatterlist: Fix offset type in sg_alloc_table_from_pages
2016-10-21 14:11 ` Tvrtko Ursulin
(?)
@ 2016-10-24 7:21 ` Marek Szyprowski
-1 siblings, 0 replies; 16+ messages in thread
From: Marek Szyprowski @ 2016-10-24 7:21 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
Cc: linux-kernel, linux-media, Chris Wilson, Tvrtko Ursulin,
Masahiro Yamada, Pawel Osciak, Kyungmin Park, Matt Porter,
Alexandre Bounine
Hi Tvrtko,
On 2016-10-21 16:11, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Scatterlist entries have an unsigned int for the offset so
> correct the sg_alloc_table_from_pages function accordingly.
>
> Since these are offsets withing a page, unsigned int is
> wide enough.
>
> Also converts callers which were using unsigned long locally
> with the lower_32_bits annotation to make it explicitly
> clear what is happening.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Pawel Osciak <pawel@osciak.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Cc: Matt Porter <mporter@kernel.crashing.org>
> Cc: Alexandre Bounine <alexandre.bounine@idt.com>
> Cc: linux-media@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/media/v4l2-core/videobuf2-dma-contig.c | 4 ++--
> drivers/rapidio/devices/rio_mport_cdev.c | 4 ++--
> include/linux/scatterlist.h | 2 +-
> lib/scatterlist.c | 2 +-
> 4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> index fb6a177be461..a3aac7533241 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -478,7 +478,7 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr,
> {
> struct vb2_dc_buf *buf;
> struct frame_vector *vec;
> - unsigned long offset;
> + unsigned int offset;
> int n_pages, i;
> int ret = 0;
> struct sg_table *sgt;
> @@ -506,7 +506,7 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr,
> buf->dev = dev;
> buf->dma_dir = dma_dir;
>
> - offset = vaddr & ~PAGE_MASK;
> + offset = lower_32_bits(vaddr & ~PAGE_MASK);
> vec = vb2_create_framevec(vaddr, size, dma_dir == DMA_FROM_DEVICE);
> if (IS_ERR(vec)) {
> ret = PTR_ERR(vec);
> diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
> index 436dfe871d32..f545cf20561f 100644
> --- a/drivers/rapidio/devices/rio_mport_cdev.c
> +++ b/drivers/rapidio/devices/rio_mport_cdev.c
> @@ -876,10 +876,10 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
> * offset within the internal buffer specified by handle parameter.
> */
> if (xfer->loc_addr) {
> - unsigned long offset;
> + unsigned int offset;
> long pinned;
>
> - offset = (unsigned long)(uintptr_t)xfer->loc_addr & ~PAGE_MASK;
> + offset = lower_32_bits(xfer->loc_addr & ~PAGE_MASK);
> nr_pages = PAGE_ALIGN(xfer->length + offset) >> PAGE_SHIFT;
>
> page_list = kmalloc_array(nr_pages,
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index cb3c8fe6acd7..c981bee1a3ae 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -263,7 +263,7 @@ int __sg_alloc_table(struct sg_table *, unsigned int, unsigned int,
> 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,
> + unsigned int offset, unsigned long size,
> gfp_t gfp_mask);
>
> size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, void *buf,
> diff --git a/lib/scatterlist.c b/lib/scatterlist.c
> index 004fc70fc56a..e05e7fc98892 100644
> --- a/lib/scatterlist.c
> +++ b/lib/scatterlist.c
> @@ -391,7 +391,7 @@ EXPORT_SYMBOL(sg_alloc_table);
> */
> int sg_alloc_table_from_pages(struct sg_table *sgt,
> struct page **pages, unsigned int n_pages,
> - unsigned long offset, unsigned long size,
> + unsigned int offset, unsigned long size,
> gfp_t gfp_mask)
> {
> unsigned int chunks;
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/5] lib/scatterlist: Avoid potential scatterlist entry overflow
2016-10-21 14:11 ` Tvrtko Ursulin
(?)
(?)
@ 2016-10-21 14:11 ` Tvrtko Ursulin
-1 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2016-10-21 14:11 UTC (permalink / raw)
To: Intel-gfx
Cc: linux-kernel, linux-media, Chris Wilson, Tvrtko Ursulin, Masahiro Yamada
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Since the scatterlist length field is an unsigned int, make
sure that sg_alloc_table_from_pages does not overflow it while
coallescing pages to a single entry.
It is I think only a theoretical possibility at the moment,
but the ability to limit the coallesced size will have
another use in following patches.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: linux-kernel@vger.kernel.org
---
lib/scatterlist.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index e05e7fc98892..d928fa04aee3 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -394,7 +394,8 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
unsigned int offset, unsigned long size,
gfp_t gfp_mask)
{
- unsigned int chunks;
+ const unsigned int max_segment = ~0;
+ unsigned int seg_len, chunks;
unsigned int i;
unsigned int cur_page;
int ret;
@@ -402,9 +403,16 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
/* 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)
+ seg_len = PAGE_SIZE;
+ for (i = 1; i < n_pages; ++i) {
+ if (seg_len >= max_segment ||
+ page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1) {
++chunks;
+ seg_len = PAGE_SIZE;
+ } else {
+ seg_len += PAGE_SIZE;
+ }
+ }
ret = sg_alloc_table(sgt, chunks, gfp_mask);
if (unlikely(ret))
@@ -413,17 +421,22 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
/* 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 chunk_size;
unsigned int j;
/* look for the end of the current chunk */
+ seg_len = PAGE_SIZE;
for (j = cur_page + 1; j < n_pages; ++j)
- if (page_to_pfn(pages[j]) !=
+ if (seg_len >= max_segment ||
+ page_to_pfn(pages[j]) !=
page_to_pfn(pages[j - 1]) + 1)
break;
+ else
+ seg_len += PAGE_SIZE;
chunk_size = ((j - cur_page) << PAGE_SHIFT) - offset;
- sg_set_page(s, pages[cur_page], min(size, chunk_size), offset);
+ sg_set_page(s, pages[cur_page],
+ min_t(unsigned long, size, chunk_size), offset);
size -= chunk_size;
offset = 0;
cur_page = j;
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 3/5] lib/scatterlist: Introduce and export __sg_alloc_table_from_pages
2016-10-21 14:11 ` Tvrtko Ursulin
` (2 preceding siblings ...)
(?)
@ 2016-10-21 14:11 ` Tvrtko Ursulin
-1 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2016-10-21 14:11 UTC (permalink / raw)
To: Intel-gfx
Cc: linux-kernel, linux-media, Chris Wilson, Tvrtko Ursulin, Masahiro Yamada
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.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: linux-kernel@vger.kernel.org
---
include/linux/scatterlist.h | 11 +++++----
lib/scatterlist.c | 55 ++++++++++++++++++++++++++++++++++-----------
2 files changed, 49 insertions(+), 17 deletions(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index c981bee1a3ae..29591dbb20fd 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -261,10 +261,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, gfp_t gfp_mask,
+ unsigned int max_segment);
+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 d928fa04aee3..0378c5fd7caa 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)
+ * @gfp_mask: GFP allocation mask
+ * @max_segment: Maximum size of a single scatterlist node in bytes
*
* Description:
* Allocate and initialize an sg table from a list of pages. Contiguous
@@ -389,12 +390,11 @@ 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, gfp_t gfp_mask,
+ unsigned int max_segment)
{
- const unsigned int max_segment = ~0;
unsigned int seg_len, chunks;
unsigned int i;
unsigned int cur_page;
@@ -444,6 +444,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, gfp_mask, ~0);
+}
EXPORT_SYMBOL(sg_alloc_table_from_pages);
void __sg_page_iter_start(struct sg_page_iter *piter,
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 4/5] drm/i915: Use __sg_alloc_table_from_pages for allocating object backing store
2016-10-21 14:11 ` Tvrtko Ursulin
@ 2016-10-21 14:11 ` Tvrtko Ursulin
-1 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2016-10-21 14:11 UTC (permalink / raw)
To: Intel-gfx; +Cc: linux-kernel, linux-media, Chris Wilson, Tvrtko Ursulin
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
With the current way of allocating backing store which over-estimates
the number of sg entries required we typically waste around 1-6 MiB of
memory on unused sg entries at runtime.
We can instead have the intermediate step of storing our pages in an
array and use __sg_alloc_table_from_pages which will create us the
most compact list possible.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 72 ++++++++++++++++++++---------------------
1 file changed, 35 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8ed8e24025ac..4bf675568a37 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2208,9 +2208,9 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
static unsigned int swiotlb_max_size(void)
{
#if IS_ENABLED(CONFIG_SWIOTLB)
- return rounddown(swiotlb_nr_tbl() << IO_TLB_SHIFT, PAGE_SIZE);
+ return swiotlb_nr_tbl() << IO_TLB_SHIFT;
#else
- return 0;
+ return UINT_MAX;
#endif
}
@@ -2221,11 +2221,8 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
int page_count, i;
struct address_space *mapping;
struct sg_table *st;
- struct scatterlist *sg;
- struct sgt_iter sgt_iter;
- struct page *page;
- unsigned long last_pfn = 0; /* suppress gcc warning */
- unsigned int max_segment;
+ struct page *page, **pages;
+ unsigned int max_segment = swiotlb_max_size();
int ret;
gfp_t gfp;
@@ -2236,18 +2233,16 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS);
BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS);
- max_segment = swiotlb_max_size();
- if (!max_segment)
- max_segment = rounddown(UINT_MAX, PAGE_SIZE);
-
- st = kmalloc(sizeof(*st), GFP_KERNEL);
- if (st == NULL)
- return -ENOMEM;
-
page_count = obj->base.size / PAGE_SIZE;
- if (sg_alloc_table(st, page_count, GFP_KERNEL)) {
- kfree(st);
+ pages = drm_malloc_gfp(page_count, sizeof(struct page *),
+ GFP_TEMPORARY | __GFP_ZERO);
+ if (!pages)
return -ENOMEM;
+
+ st = kmalloc(sizeof(*st), GFP_KERNEL);
+ if (st == NULL) {
+ ret = -ENOMEM;
+ goto err_st;
}
/* Get the list of pages out of our struct file. They'll be pinned
@@ -2258,8 +2253,6 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
mapping = obj->base.filp->f_mapping;
gfp = mapping_gfp_constraint(mapping, ~(__GFP_IO | __GFP_RECLAIM));
gfp |= __GFP_NORETRY | __GFP_NOWARN;
- sg = st->sgl;
- st->nents = 0;
for (i = 0; i < page_count; i++) {
page = shmem_read_mapping_page_gfp(mapping, i, gfp);
if (IS_ERR(page)) {
@@ -2281,29 +2274,28 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
goto err_pages;
}
}
- if (!i ||
- sg->length >= max_segment ||
- page_to_pfn(page) != last_pfn + 1) {
- if (i)
- sg = sg_next(sg);
- st->nents++;
- sg_set_page(sg, page, PAGE_SIZE, 0);
- } else {
- sg->length += PAGE_SIZE;
- }
- last_pfn = page_to_pfn(page);
+
+ pages[i] = page;
/* Check that the i965g/gm workaround works. */
- WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x00100000UL));
+ WARN_ON((gfp & __GFP_DMA32) &&
+ (page_to_pfn(page) >= 0x00100000UL));
}
- if (sg) /* loop terminated early; short sg table */
- sg_mark_end(sg);
+
+ ret = __sg_alloc_table_from_pages(st, pages, page_count, 0,
+ obj->base.size, GFP_KERNEL,
+ max_segment);
+ if (ret)
+ goto err_pages;
+
obj->pages = st;
ret = i915_gem_gtt_prepare_object(obj);
if (ret)
goto err_pages;
+ drm_free_large(pages);
+
if (i915_gem_object_needs_bit17_swizzle(obj))
i915_gem_object_do_bit_17_swizzle(obj);
@@ -2314,10 +2306,13 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
return 0;
err_pages:
- sg_mark_end(sg);
- for_each_sgt_page(page, sgt_iter, st)
- put_page(page);
- sg_free_table(st);
+ for (i = 0; i < page_count; i++) {
+ if (pages[i])
+ put_page(pages[i]);
+ else
+ break;
+ }
+
kfree(st);
/* shmemfs first checks if there is enough memory to allocate the page
@@ -2331,6 +2326,9 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
if (ret == -ENOSPC)
ret = -ENOMEM;
+err_st:
+ drm_free_large(pages);
+
return ret;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 4/5] drm/i915: Use __sg_alloc_table_from_pages for allocating object backing store
@ 2016-10-21 14:11 ` Tvrtko Ursulin
0 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2016-10-21 14:11 UTC (permalink / raw)
To: Intel-gfx; +Cc: linux-kernel, linux-media
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
With the current way of allocating backing store which over-estimates
the number of sg entries required we typically waste around 1-6 MiB of
memory on unused sg entries at runtime.
We can instead have the intermediate step of storing our pages in an
array and use __sg_alloc_table_from_pages which will create us the
most compact list possible.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 72 ++++++++++++++++++++---------------------
1 file changed, 35 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8ed8e24025ac..4bf675568a37 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2208,9 +2208,9 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
static unsigned int swiotlb_max_size(void)
{
#if IS_ENABLED(CONFIG_SWIOTLB)
- return rounddown(swiotlb_nr_tbl() << IO_TLB_SHIFT, PAGE_SIZE);
+ return swiotlb_nr_tbl() << IO_TLB_SHIFT;
#else
- return 0;
+ return UINT_MAX;
#endif
}
@@ -2221,11 +2221,8 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
int page_count, i;
struct address_space *mapping;
struct sg_table *st;
- struct scatterlist *sg;
- struct sgt_iter sgt_iter;
- struct page *page;
- unsigned long last_pfn = 0; /* suppress gcc warning */
- unsigned int max_segment;
+ struct page *page, **pages;
+ unsigned int max_segment = swiotlb_max_size();
int ret;
gfp_t gfp;
@@ -2236,18 +2233,16 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS);
BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS);
- max_segment = swiotlb_max_size();
- if (!max_segment)
- max_segment = rounddown(UINT_MAX, PAGE_SIZE);
-
- st = kmalloc(sizeof(*st), GFP_KERNEL);
- if (st == NULL)
- return -ENOMEM;
-
page_count = obj->base.size / PAGE_SIZE;
- if (sg_alloc_table(st, page_count, GFP_KERNEL)) {
- kfree(st);
+ pages = drm_malloc_gfp(page_count, sizeof(struct page *),
+ GFP_TEMPORARY | __GFP_ZERO);
+ if (!pages)
return -ENOMEM;
+
+ st = kmalloc(sizeof(*st), GFP_KERNEL);
+ if (st == NULL) {
+ ret = -ENOMEM;
+ goto err_st;
}
/* Get the list of pages out of our struct file. They'll be pinned
@@ -2258,8 +2253,6 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
mapping = obj->base.filp->f_mapping;
gfp = mapping_gfp_constraint(mapping, ~(__GFP_IO | __GFP_RECLAIM));
gfp |= __GFP_NORETRY | __GFP_NOWARN;
- sg = st->sgl;
- st->nents = 0;
for (i = 0; i < page_count; i++) {
page = shmem_read_mapping_page_gfp(mapping, i, gfp);
if (IS_ERR(page)) {
@@ -2281,29 +2274,28 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
goto err_pages;
}
}
- if (!i ||
- sg->length >= max_segment ||
- page_to_pfn(page) != last_pfn + 1) {
- if (i)
- sg = sg_next(sg);
- st->nents++;
- sg_set_page(sg, page, PAGE_SIZE, 0);
- } else {
- sg->length += PAGE_SIZE;
- }
- last_pfn = page_to_pfn(page);
+
+ pages[i] = page;
/* Check that the i965g/gm workaround works. */
- WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x00100000UL));
+ WARN_ON((gfp & __GFP_DMA32) &&
+ (page_to_pfn(page) >= 0x00100000UL));
}
- if (sg) /* loop terminated early; short sg table */
- sg_mark_end(sg);
+
+ ret = __sg_alloc_table_from_pages(st, pages, page_count, 0,
+ obj->base.size, GFP_KERNEL,
+ max_segment);
+ if (ret)
+ goto err_pages;
+
obj->pages = st;
ret = i915_gem_gtt_prepare_object(obj);
if (ret)
goto err_pages;
+ drm_free_large(pages);
+
if (i915_gem_object_needs_bit17_swizzle(obj))
i915_gem_object_do_bit_17_swizzle(obj);
@@ -2314,10 +2306,13 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
return 0;
err_pages:
- sg_mark_end(sg);
- for_each_sgt_page(page, sgt_iter, st)
- put_page(page);
- sg_free_table(st);
+ for (i = 0; i < page_count; i++) {
+ if (pages[i])
+ put_page(pages[i]);
+ else
+ break;
+ }
+
kfree(st);
/* shmemfs first checks if there is enough memory to allocate the page
@@ -2331,6 +2326,9 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
if (ret == -ENOSPC)
ret = -ENOMEM;
+err_st:
+ drm_free_large(pages);
+
return ret;
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 4/5] drm/i915: Use __sg_alloc_table_from_pages for allocating object backing store
2016-10-21 14:11 ` Tvrtko Ursulin
(?)
@ 2016-10-21 14:27 ` Chris Wilson
2016-10-21 14:55 ` Tvrtko Ursulin
-1 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2016-10-21 14:27 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx, linux-kernel, linux-media, Tvrtko Ursulin
On Fri, Oct 21, 2016 at 03:11:22PM +0100, Tvrtko Ursulin wrote:
> @@ -2236,18 +2233,16 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
> BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS);
> BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS);
>
> - max_segment = swiotlb_max_size();
> - if (!max_segment)
> - max_segment = rounddown(UINT_MAX, PAGE_SIZE);
> -
> - st = kmalloc(sizeof(*st), GFP_KERNEL);
> - if (st == NULL)
> - return -ENOMEM;
> -
> page_count = obj->base.size / PAGE_SIZE;
> - if (sg_alloc_table(st, page_count, GFP_KERNEL)) {
> - kfree(st);
> + pages = drm_malloc_gfp(page_count, sizeof(struct page *),
> + GFP_TEMPORARY | __GFP_ZERO);
> + if (!pages)
> return -ENOMEM;
Full circle! The whole reason this exists was to avoid that vmalloc. I
don't really want it back...
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH 4/5] drm/i915: Use __sg_alloc_table_from_pages for allocating object backing store
2016-10-21 14:27 ` Chris Wilson
@ 2016-10-21 14:55 ` Tvrtko Ursulin
0 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2016-10-21 14:55 UTC (permalink / raw)
To: Chris Wilson, Tvrtko Ursulin, Intel-gfx, linux-kernel,
linux-media, Tvrtko Ursulin
On 21/10/2016 15:27, Chris Wilson wrote:
> On Fri, Oct 21, 2016 at 03:11:22PM +0100, Tvrtko Ursulin wrote:
>> @@ -2236,18 +2233,16 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
>> BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS);
>> BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS);
>>
>> - max_segment = swiotlb_max_size();
>> - if (!max_segment)
>> - max_segment = rounddown(UINT_MAX, PAGE_SIZE);
>> -
>> - st = kmalloc(sizeof(*st), GFP_KERNEL);
>> - if (st == NULL)
>> - return -ENOMEM;
>> -
>> page_count = obj->base.size / PAGE_SIZE;
>> - if (sg_alloc_table(st, page_count, GFP_KERNEL)) {
>> - kfree(st);
>> + pages = drm_malloc_gfp(page_count, sizeof(struct page *),
>> + GFP_TEMPORARY | __GFP_ZERO);
>> + if (!pages)
>> return -ENOMEM;
>
> Full circle! The whole reason this exists was to avoid that vmalloc. I
> don't really want it back...
Yes, it is not ideal.
However all objects under 4 MiB should fall under the kmalloc fast path
(8 KiB of struct page pointers, which should always be available), and
possibly bigger ones as well if there is room.
It only fallbacks to vmalloc for objects larger than 4 MiB, when it also
fails to get the page pointer array from the SLAB (GFP_TEMPORARY).
So perhaps SLAB would most of the time have some nice chunks for us to
pretty much limit vmalloc apart for the huge objects? And then, is
creation time for those so performance critical?
I came up with this because I started to dislike my previous
sg_trim_table approach as too ugly. It had an advantage of simplicity
after fixing the theoretical chunk overflow in sg_alloc_table_from_pages.
If we choose none of the two, only third option I can think of is to
allocate the sg table as we add entries to it. I don't think it would be
hard to do that.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 4/5] drm/i915: Use __sg_alloc_table_from_pages for allocating object backing store
@ 2016-10-21 14:55 ` Tvrtko Ursulin
0 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2016-10-21 14:55 UTC (permalink / raw)
To: Chris Wilson, Tvrtko Ursulin, Intel-gfx, linux-kernel,
linux-media, Tvrtko Ursulin
On 21/10/2016 15:27, Chris Wilson wrote:
> On Fri, Oct 21, 2016 at 03:11:22PM +0100, Tvrtko Ursulin wrote:
>> @@ -2236,18 +2233,16 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
>> BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS);
>> BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS);
>>
>> - max_segment = swiotlb_max_size();
>> - if (!max_segment)
>> - max_segment = rounddown(UINT_MAX, PAGE_SIZE);
>> -
>> - st = kmalloc(sizeof(*st), GFP_KERNEL);
>> - if (st == NULL)
>> - return -ENOMEM;
>> -
>> page_count = obj->base.size / PAGE_SIZE;
>> - if (sg_alloc_table(st, page_count, GFP_KERNEL)) {
>> - kfree(st);
>> + pages = drm_malloc_gfp(page_count, sizeof(struct page *),
>> + GFP_TEMPORARY | __GFP_ZERO);
>> + if (!pages)
>> return -ENOMEM;
>
> Full circle! The whole reason this exists was to avoid that vmalloc. I
> don't really want it back...
Yes, it is not ideal.
However all objects under 4 MiB should fall under the kmalloc fast path
(8 KiB of struct page pointers, which should always be available), and
possibly bigger ones as well if there is room.
It only fallbacks to vmalloc for objects larger than 4 MiB, when it also
fails to get the page pointer array from the SLAB (GFP_TEMPORARY).
So perhaps SLAB would most of the time have some nice chunks for us to
pretty much limit vmalloc apart for the huge objects? And then, is
creation time for those so performance critical?
I came up with this because I started to dislike my previous
sg_trim_table approach as too ugly. It had an advantage of simplicity
after fixing the theoretical chunk overflow in sg_alloc_table_from_pages.
If we choose none of the two, only third option I can think of is to
allocate the sg table as we add entries to it. I don't think it would be
hard to do that.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/5] drm/i915: Use __sg_alloc_table_from_pages for userptr allocations
2016-10-21 14:11 ` Tvrtko Ursulin
@ 2016-10-21 14:11 ` Tvrtko Ursulin
-1 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2016-10-21 14:11 UTC (permalink / raw)
To: Intel-gfx; +Cc: linux-kernel, linux-media, Chris Wilson, Tvrtko Ursulin
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
With the addition of __sg_alloc_table_from_pages we can control
the maximum coallescing size and eliminate a separate path for
allocating backing store here.
This also makes the tables as compact as possible in all cases.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 9 +++++++++
drivers/gpu/drm/i915/i915_gem.c | 11 +----------
drivers/gpu/drm/i915/i915_gem_userptr.c | 29 +++++++----------------------
3 files changed, 17 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5b2b7f3c6e76..577a3a87f680 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -4001,4 +4001,13 @@ int remap_io_mapping(struct vm_area_struct *vma,
__T; \
})
+static inline unsigned int i915_swiotlb_max_size(void)
+{
+#if IS_ENABLED(CONFIG_SWIOTLB)
+ return swiotlb_nr_tbl() << IO_TLB_SHIFT;
+#else
+ return UINT_MAX;
+#endif
+}
+
#endif
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4bf675568a37..18125d7279c6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2205,15 +2205,6 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
return 0;
}
-static unsigned int swiotlb_max_size(void)
-{
-#if IS_ENABLED(CONFIG_SWIOTLB)
- return swiotlb_nr_tbl() << IO_TLB_SHIFT;
-#else
- return UINT_MAX;
-#endif
-}
-
static int
i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
{
@@ -2222,7 +2213,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
struct address_space *mapping;
struct sg_table *st;
struct page *page, **pages;
- unsigned int max_segment = swiotlb_max_size();
+ unsigned int max_segment = i915_swiotlb_max_size();
int ret;
gfp_t gfp;
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index e537930c64b5..17dca225a3e0 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -397,36 +397,21 @@ struct get_pages_work {
struct task_struct *task;
};
-#if IS_ENABLED(CONFIG_SWIOTLB)
-#define swiotlb_active() swiotlb_nr_tbl()
-#else
-#define swiotlb_active() 0
-#endif
-
static int
st_set_pages(struct sg_table **st, struct page **pvec, int num_pages)
{
- struct scatterlist *sg;
- int ret, n;
+ unsigned int max_segment = i915_swiotlb_max_size();
+ int ret;
*st = kmalloc(sizeof(**st), GFP_KERNEL);
if (*st == NULL)
return -ENOMEM;
- if (swiotlb_active()) {
- ret = sg_alloc_table(*st, num_pages, GFP_KERNEL);
- if (ret)
- goto err;
-
- for_each_sg((*st)->sgl, sg, num_pages, n)
- sg_set_page(sg, pvec[n], PAGE_SIZE, 0);
- } else {
- ret = sg_alloc_table_from_pages(*st, pvec, num_pages,
- 0, num_pages << PAGE_SHIFT,
- GFP_KERNEL);
- if (ret)
- goto err;
- }
+ ret = __sg_alloc_table_from_pages(*st, pvec, num_pages, 0,
+ num_pages << PAGE_SHIFT,
+ GFP_KERNEL, max_segment);
+ if (ret)
+ goto err;
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 5/5] drm/i915: Use __sg_alloc_table_from_pages for userptr allocations
@ 2016-10-21 14:11 ` Tvrtko Ursulin
0 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2016-10-21 14:11 UTC (permalink / raw)
To: Intel-gfx; +Cc: linux-kernel, linux-media
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
With the addition of __sg_alloc_table_from_pages we can control
the maximum coallescing size and eliminate a separate path for
allocating backing store here.
This also makes the tables as compact as possible in all cases.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 9 +++++++++
drivers/gpu/drm/i915/i915_gem.c | 11 +----------
drivers/gpu/drm/i915/i915_gem_userptr.c | 29 +++++++----------------------
3 files changed, 17 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5b2b7f3c6e76..577a3a87f680 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -4001,4 +4001,13 @@ int remap_io_mapping(struct vm_area_struct *vma,
__T; \
})
+static inline unsigned int i915_swiotlb_max_size(void)
+{
+#if IS_ENABLED(CONFIG_SWIOTLB)
+ return swiotlb_nr_tbl() << IO_TLB_SHIFT;
+#else
+ return UINT_MAX;
+#endif
+}
+
#endif
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4bf675568a37..18125d7279c6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2205,15 +2205,6 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
return 0;
}
-static unsigned int swiotlb_max_size(void)
-{
-#if IS_ENABLED(CONFIG_SWIOTLB)
- return swiotlb_nr_tbl() << IO_TLB_SHIFT;
-#else
- return UINT_MAX;
-#endif
-}
-
static int
i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
{
@@ -2222,7 +2213,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
struct address_space *mapping;
struct sg_table *st;
struct page *page, **pages;
- unsigned int max_segment = swiotlb_max_size();
+ unsigned int max_segment = i915_swiotlb_max_size();
int ret;
gfp_t gfp;
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index e537930c64b5..17dca225a3e0 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -397,36 +397,21 @@ struct get_pages_work {
struct task_struct *task;
};
-#if IS_ENABLED(CONFIG_SWIOTLB)
-#define swiotlb_active() swiotlb_nr_tbl()
-#else
-#define swiotlb_active() 0
-#endif
-
static int
st_set_pages(struct sg_table **st, struct page **pvec, int num_pages)
{
- struct scatterlist *sg;
- int ret, n;
+ unsigned int max_segment = i915_swiotlb_max_size();
+ int ret;
*st = kmalloc(sizeof(**st), GFP_KERNEL);
if (*st == NULL)
return -ENOMEM;
- if (swiotlb_active()) {
- ret = sg_alloc_table(*st, num_pages, GFP_KERNEL);
- if (ret)
- goto err;
-
- for_each_sg((*st)->sgl, sg, num_pages, n)
- sg_set_page(sg, pvec[n], PAGE_SIZE, 0);
- } else {
- ret = sg_alloc_table_from_pages(*st, pvec, num_pages,
- 0, num_pages << PAGE_SHIFT,
- GFP_KERNEL);
- if (ret)
- goto err;
- }
+ ret = __sg_alloc_table_from_pages(*st, pvec, num_pages, 0,
+ num_pages << PAGE_SHIFT,
+ GFP_KERNEL, max_segment);
+ if (ret)
+ goto err;
return 0;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* ✗ Fi.CI.BAT: failure for Avoid pessimistic scatter-gather allocation
2016-10-21 14:11 ` Tvrtko Ursulin
` (5 preceding siblings ...)
(?)
@ 2016-10-21 15:53 ` Patchwork
-1 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2016-10-21 15:53 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: Avoid pessimistic scatter-gather allocation
URL : https://patchwork.freedesktop.org/series/14166/
State : failure
== Summary ==
Series 14166v1 Avoid pessimistic scatter-gather allocation
https://patchwork.freedesktop.org/api/1.0/series/14166/revisions/1/mbox/
Test drv_module_reload_basic:
dmesg-warn -> PASS (fi-skl-6770hq)
dmesg-warn -> PASS (fi-skl-6700hq)
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> DMESG-WARN (fi-skl-6700hq)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-c:
pass -> INCOMPLETE (fi-hsw-4770)
Subgroup suspend-read-crc-pipe-a:
pass -> DMESG-WARN (fi-skl-6700hq)
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (fi-skl-6700hq)
Subgroup suspend-read-crc-pipe-c:
pass -> DMESG-WARN (fi-skl-6700hq)
fi-bdw-5557u total:246 pass:231 dwarn:0 dfail:0 fail:0 skip:15
fi-bsw-n3050 total:246 pass:204 dwarn:0 dfail:0 fail:0 skip:42
fi-bxt-t5700 total:246 pass:216 dwarn:0 dfail:0 fail:0 skip:30
fi-byt-j1900 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31
fi-byt-n2820 total:246 pass:211 dwarn:0 dfail:0 fail:0 skip:35
fi-hsw-4770 total:195 pass:174 dwarn:0 dfail:0 fail:0 skip:20
fi-hsw-4770r total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22
fi-ilk-650 total:246 pass:185 dwarn:0 dfail:0 fail:1 skip:60
fi-ivb-3520m total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25
fi-ivb-3770 total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25
fi-kbl-7200u total:246 pass:222 dwarn:0 dfail:0 fail:0 skip:24
fi-skl-6260u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14
fi-skl-6700hq total:246 pass:219 dwarn:4 dfail:0 fail:0 skip:23
fi-skl-6700k total:246 pass:222 dwarn:1 dfail:0 fail:0 skip:23
fi-skl-6770hq total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14
fi-snb-2520m total:246 pass:210 dwarn:0 dfail:0 fail:0 skip:36
fi-snb-2600 total:246 pass:209 dwarn:0 dfail:0 fail:0 skip:37
Results at /archive/results/CI_IGT_test/Patchwork_2789/
45ce7992e2d3211009c035db41745e1f12bbf6e8 drm-intel-nightly: 2016y-10m-21d-14h-52m-49s UTC integration manifest
11af57e drm/i915: Use __sg_alloc_table_from_pages for userptr allocations
3688d80 drm/i915: Use __sg_alloc_table_from_pages for allocating object backing store
1118c12 lib/scatterlist: Introduce and export __sg_alloc_table_from_pages
ee42408 lib/scatterlist: Avoid potential scatterlist entry overflow
52c395b lib/scatterlist: Fix offset type in sg_alloc_table_from_pages
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread