* [PATCH 1/6] intel: Defer setting madv on the bo cache
@ 2015-05-05 8:53 Chris Wilson
2015-05-05 8:53 ` [PATCH 2/6] intel: Use WAIT for wait-rendering Chris Wilson
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: Chris Wilson @ 2015-05-05 8:53 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, Ben Widawsky
Convert the bo-cache into 2 phases:
1. A two second cache of recently used buffers, keep untouched.
2. A two second cache of older buffers, marked for eviction.
This helps reduce ioctl traffic on a rapid turnover in working sets. The
issue is that even a 2 second window is enough for an application to
fill all of memory with inactive buffers (and we would rely on the
oom-killer identifying the right culprit).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
---
intel/intel_bufmgr_gem.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 60c06fc..eeb9acf 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -216,6 +216,11 @@ struct _drm_intel_bo_gem {
bool used_as_reloc_target;
/**
+ * Are we keeping this buffer around in semi-permenant cache?
+ */
+ bool dontneed;
+
+ /**
* Boolean of whether we have encountered an error whilst building the relocation tree.
*/
bool has_error;
@@ -719,7 +724,8 @@ retry:
}
if (alloc_from_cache) {
- if (!drm_intel_gem_bo_madvise_internal
+ if (bo_gem->dontneed &&
+ !drm_intel_gem_bo_madvise_internal
(bufmgr_gem, bo_gem, I915_MADV_WILLNEED)) {
drm_intel_gem_bo_free(&bo_gem->bo);
drm_intel_gem_bo_cache_purge_bucket(bufmgr_gem,
@@ -1166,19 +1172,28 @@ drm_intel_gem_cleanup_bo_cache(drm_intel_bufmgr_gem *bufmgr_gem, time_t time)
for (i = 0; i < bufmgr_gem->num_buckets; i++) {
struct drm_intel_gem_bo_bucket *bucket =
&bufmgr_gem->cache_bucket[i];
+ drmMMListHead madv;
+ DRMINITLISTHEAD(&madv);
while (!DRMLISTEMPTY(&bucket->head)) {
drm_intel_bo_gem *bo_gem;
bo_gem = DRMLISTENTRY(drm_intel_bo_gem,
bucket->head.next, head);
- if (time - bo_gem->free_time <= 1)
+ if (time - bo_gem->free_time < 2*(1+bo_gem->dontneed))
break;
DRMLISTDEL(&bo_gem->head);
-
- drm_intel_gem_bo_free(&bo_gem->bo);
+ if (bo_gem->dontneed) {
+ drm_intel_gem_bo_free(&bo_gem->bo);
+ } else {
+ drm_intel_gem_bo_madvise_internal(bufmgr_gem, bo_gem,
+ I915_MADV_DONTNEED);
+ bo_gem->dontneed = 1;
+ DRMLISTADDTAIL(&bo_gem->head, &madv);
+ }
}
+ DRMLISTJOIN(&madv, &bucket->head);
}
bufmgr_gem->time = time;
@@ -1289,9 +1304,7 @@ drm_intel_gem_bo_unreference_final(drm_intel_bo *bo, time_t time)
bucket = drm_intel_gem_bo_bucket_for_size(bufmgr_gem, bo->size);
/* Put the buffer into our internal cache for reuse if we can. */
- if (bufmgr_gem->bo_reuse && bo_gem->reusable && bucket != NULL &&
- drm_intel_gem_bo_madvise_internal(bufmgr_gem, bo_gem,
- I915_MADV_DONTNEED)) {
+ if (bufmgr_gem->bo_reuse && bo_gem->reusable && bucket != NULL) {
bo_gem->free_time = time;
bo_gem->name = NULL;
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/6] intel: Use WAIT for wait-rendering 2015-05-05 8:53 [PATCH 1/6] intel: Defer setting madv on the bo cache Chris Wilson @ 2015-05-05 8:53 ` Chris Wilson 2015-05-05 8:53 ` [PATCH 3/6] intel: Export raw GEM mmap interfaces Chris Wilson ` (4 subsequent siblings) 5 siblings, 0 replies; 8+ messages in thread From: Chris Wilson @ 2015-05-05 8:53 UTC (permalink / raw) To: intel-gfx; +Cc: Daniel Vetter Using WAIT is preferrable to SET_DOMAIN as it doesn't have any domain management side-effects - but has the same flushing semantics. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- intel/intel_bufmgr_gem.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index eeb9acf..43cbae5 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -1729,6 +1729,24 @@ drm_intel_gem_bo_get_subdata(drm_intel_bo *bo, unsigned long offset, static void drm_intel_gem_bo_wait_rendering(drm_intel_bo *bo) { + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr; + + /* Using WAIT is preferrable to SET_DOMAIN as it doesn't have + * any domain management side-effects - but has the same flushing + * semantics. + */ + if (bufmgr_gem->has_wait_timeout) { + struct drm_i915_gem_wait wait; + + memclear(wait); + wait.bo_handle = bo->handle; + wait.timeout_ns = -1; + if (drmIoctl(bufmgr_gem->fd, + DRM_IOCTL_I915_GEM_WAIT, + &wait) == 0) + return; + } + drm_intel_gem_bo_start_gtt_access(bo, 1); } -- 2.1.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/6] intel: Export raw GEM mmap interfaces 2015-05-05 8:53 [PATCH 1/6] intel: Defer setting madv on the bo cache Chris Wilson 2015-05-05 8:53 ` [PATCH 2/6] intel: Use WAIT for wait-rendering Chris Wilson @ 2015-05-05 8:53 ` Chris Wilson 2015-05-05 8:53 ` [PATCH 4/6] intel: Keep the caches in loose active order Chris Wilson ` (3 subsequent siblings) 5 siblings, 0 replies; 8+ messages in thread From: Chris Wilson @ 2015-05-05 8:53 UTC (permalink / raw) To: intel-gfx Export a set of interfaces to allow the caller to have precise control over mapping the buffer - but still provide caching of the mmaps between callers. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- intel/intel_bufmgr.h | 4 ++ intel/intel_bufmgr_gem.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+) diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h index 285919e..f35c59b 100644 --- a/intel/intel_bufmgr.h +++ b/intel/intel_bufmgr.h @@ -178,6 +178,10 @@ int drm_intel_gem_bo_map_unsynchronized(drm_intel_bo *bo); int drm_intel_gem_bo_map_gtt(drm_intel_bo *bo); int drm_intel_gem_bo_unmap_gtt(drm_intel_bo *bo); +void *drm_intel_gem_bo_map__cpu(drm_intel_bo *bo); +void *drm_intel_gem_bo_map__gtt(drm_intel_bo *bo); +void *drm_intel_gem_bo_map__wc(drm_intel_bo *bo); + int drm_intel_gem_bo_get_reloc_count(drm_intel_bo *bo); void drm_intel_gem_bo_clear_relocs(drm_intel_bo *bo, int start); void drm_intel_gem_bo_start_gtt_access(drm_intel_bo *bo, int write_enable); diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 43cbae5..61a248f 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -191,6 +191,8 @@ struct _drm_intel_bo_gem { void *mem_virtual; /** GTT virtual address for the buffer, saved across map/unmap cycles */ void *gtt_virtual; + /** WC CPU address for the buffer, saved across map/unmap cycles */ + void *wc_virtual; /** * Virtual address of the buffer allocated by user, used for userptr * objects only. @@ -1129,6 +1131,11 @@ drm_intel_gem_bo_free(drm_intel_bo *bo) drm_munmap(bo_gem->mem_virtual, bo_gem->bo.size); bufmgr_gem->vma_count--; } + if (bo_gem->wc_virtual) { + VG(VALGRIND_FREELIKE_BLOCK(bo_gem->wc_virtual, 0)); + drm_munmap(bo_gem->wc_virtual, bo_gem->bo.size); + bufmgr_gem->vma_count--; + } if (bo_gem->gtt_virtual) { drm_munmap(bo_gem->gtt_virtual, bo_gem->bo.size); bufmgr_gem->vma_count--; @@ -1155,6 +1162,9 @@ drm_intel_gem_bo_mark_mmaps_incoherent(drm_intel_bo *bo) if (bo_gem->mem_virtual) VALGRIND_MAKE_MEM_NOACCESS(bo_gem->mem_virtual, bo->size); + if (bo_gem->wc_virtual) + VALGRIND_MAKE_MEM_NOACCESS(bo_gem->wc_virtual, bo->size); + if (bo_gem->gtt_virtual) VALGRIND_MAKE_MEM_NOACCESS(bo_gem->gtt_virtual, bo->size); #endif @@ -1228,6 +1238,11 @@ static void drm_intel_gem_bo_purge_vma_cache(drm_intel_bufmgr_gem *bufmgr_gem) bo_gem->mem_virtual = NULL; bufmgr_gem->vma_count--; } + if (bo_gem->wc_virtual) { + drm_munmap(bo_gem->wc_virtual, bo_gem->bo.size); + bo_gem->wc_virtual = NULL; + bufmgr_gem->vma_count--; + } if (bo_gem->gtt_virtual) { drm_munmap(bo_gem->gtt_virtual, bo_gem->bo.size); bo_gem->gtt_virtual = NULL; @@ -1243,6 +1258,8 @@ static void drm_intel_gem_bo_close_vma(drm_intel_bufmgr_gem *bufmgr_gem, DRMLISTADDTAIL(&bo_gem->vma_list, &bufmgr_gem->vma_cache); if (bo_gem->mem_virtual) bufmgr_gem->vma_count++; + if (bo_gem->wc_virtual) + bufmgr_gem->vma_count++; if (bo_gem->gtt_virtual) bufmgr_gem->vma_count++; drm_intel_gem_bo_purge_vma_cache(bufmgr_gem); @@ -1255,6 +1272,8 @@ static void drm_intel_gem_bo_open_vma(drm_intel_bufmgr_gem *bufmgr_gem, DRMLISTDEL(&bo_gem->vma_list); if (bo_gem->mem_virtual) bufmgr_gem->vma_count--; + if (bo_gem->wc_virtual) + bufmgr_gem->vma_count--; if (bo_gem->gtt_virtual) bufmgr_gem->vma_count--; drm_intel_gem_bo_purge_vma_cache(bufmgr_gem); @@ -3516,6 +3535,135 @@ drm_intel_bufmgr_gem_unref(drm_intel_bufmgr *bufmgr) } } +void *drm_intel_gem_bo_map__gtt(drm_intel_bo *bo) +{ + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr; + drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; + + if (bo_gem->is_userptr) + return NULL; + + pthread_mutex_lock(&bufmgr_gem->lock); + if (bo_gem->map_count++ == 0) + drm_intel_gem_bo_open_vma(bufmgr_gem, bo_gem); + + if (bo_gem->gtt_virtual == NULL) { + struct drm_i915_gem_mmap_gtt mmap_arg; + + DBG("bo_map_gtt: mmap %d (%s), map_count=%d\n", + bo_gem->gem_handle, bo_gem->name, bo_gem->map_count); + + memclear(mmap_arg); + mmap_arg.handle = bo_gem->gem_handle; + + /* Get the fake offset back... */ + bo_gem->gtt_virtual = MAP_FAILED; + if (drmIoctl(bufmgr_gem->fd, + DRM_IOCTL_I915_GEM_MMAP_GTT, + &mmap_arg) == 0) { + /* and mmap it */ + bo_gem->gtt_virtual = drm_mmap(0, bo->size, PROT_READ | PROT_WRITE, + MAP_SHARED, bufmgr_gem->fd, + mmap_arg.offset); + } + if (bo_gem->gtt_virtual == MAP_FAILED) { + if (--bo_gem->map_count == 0) + drm_intel_gem_bo_close_vma(bufmgr_gem, bo_gem); + bo_gem->gtt_virtual = NULL; + } + } + pthread_mutex_unlock(&bufmgr_gem->lock); + + return bo_gem->gtt_virtual; +} + +void *drm_intel_gem_bo_map__cpu(drm_intel_bo *bo) +{ + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr; + drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; + + if (bo_gem->is_userptr) { + /* Return the same user ptr */ + return bo_gem->user_virtual; + } + + pthread_mutex_lock(&bufmgr_gem->lock); + + if (bo_gem->map_count++ == 0) + drm_intel_gem_bo_open_vma(bufmgr_gem, bo_gem); + + if (!bo_gem->mem_virtual) { + struct drm_i915_gem_mmap mmap_arg; + + DBG("bo_map: %d (%s), map_count=%d\n", + bo_gem->gem_handle, bo_gem->name, bo_gem->map_count); + + memclear(mmap_arg); + mmap_arg.handle = bo_gem->gem_handle; + mmap_arg.size = bo->size; + if (drmIoctl(bufmgr_gem->fd, + DRM_IOCTL_I915_GEM_MMAP, + &mmap_arg)) { + DBG("%s:%d: Error mapping buffer %d (%s): %s .\n", + __FILE__, __LINE__, bo_gem->gem_handle, + bo_gem->name, strerror(errno)); + if (--bo_gem->map_count == 0) + drm_intel_gem_bo_close_vma(bufmgr_gem, bo_gem); + } else { + VG(VALGRIND_MALLOCLIKE_BLOCK(mmap_arg.addr_ptr, mmap_arg.size, 0, 1)); + bo_gem->mem_virtual = (void *)(uintptr_t) mmap_arg.addr_ptr; + } + } + DBG("bo_map: %d (%s) -> %p\n", bo_gem->gem_handle, bo_gem->name, + bo_gem->mem_virtual); + pthread_mutex_unlock(&bufmgr_gem->lock); + + return bo_gem->mem_virtual; +} + +void *drm_intel_gem_bo_map__wc(drm_intel_bo *bo) +{ + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr; + drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; + + if (bo_gem->is_userptr) + return NULL; + + pthread_mutex_lock(&bufmgr_gem->lock); + + if (bo_gem->map_count++ == 0) + drm_intel_gem_bo_open_vma(bufmgr_gem, bo_gem); + + if (!bo_gem->wc_virtual) { + struct drm_i915_gem_mmap mmap_arg; + + DBG("bo_map: %d (%s), map_count=%d\n", + bo_gem->gem_handle, bo_gem->name, bo_gem->map_count); + + memclear(mmap_arg); + mmap_arg.handle = bo_gem->gem_handle; + mmap_arg.size = bo->size; + mmap_arg.flags = I915_MMAP_WC; + if (drmIoctl(bufmgr_gem->fd, + DRM_IOCTL_I915_GEM_MMAP, + &mmap_arg)) { + DBG("%s:%d: Error mapping buffer %d (%s): %s .\n", + __FILE__, __LINE__, bo_gem->gem_handle, + bo_gem->name, strerror(errno)); + if (--bo_gem->map_count == 0) + drm_intel_gem_bo_close_vma(bufmgr_gem, bo_gem); + } else { + VG(VALGRIND_MALLOCLIKE_BLOCK(mmap_arg.addr_ptr, mmap_arg.size, 0, 1)); + bo_gem->wc_virtual = (void *)(uintptr_t) mmap_arg.addr_ptr; + } + } + DBG("bo_map: %d (%s) -> %p\n", bo_gem->gem_handle, bo_gem->name, + bo_gem->wc_virtual); + pthread_mutex_unlock(&bufmgr_gem->lock); + + return bo_gem->wc_virtual; +} + /** * Initializes the GEM buffer manager, which uses the kernel to allocate, map, * and manage map buffer objections. -- 2.1.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/6] intel: Keep the caches in loose active order 2015-05-05 8:53 [PATCH 1/6] intel: Defer setting madv on the bo cache Chris Wilson 2015-05-05 8:53 ` [PATCH 2/6] intel: Use WAIT for wait-rendering Chris Wilson 2015-05-05 8:53 ` [PATCH 3/6] intel: Export raw GEM mmap interfaces Chris Wilson @ 2015-05-05 8:53 ` Chris Wilson 2015-05-05 8:53 ` [PATCH 5/6] intel: Round large allocations upto PAGE_SIZE Chris Wilson ` (2 subsequent siblings) 5 siblings, 0 replies; 8+ messages in thread From: Chris Wilson @ 2015-05-05 8:53 UTC (permalink / raw) To: intel-gfx Though we don't have precise tracking for when buffers in the cache list become idle (we don't have request tracking at this layer), we do have some basic assumptions that the head is inactive and the tail active. So when inserting buffers in the cache, it would be good not to completely ignore those assumptions. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- intel/intel_bufmgr_gem.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 61a248f..2cbe973 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -1329,7 +1329,10 @@ drm_intel_gem_bo_unreference_final(drm_intel_bo *bo, time_t time) bo_gem->name = NULL; bo_gem->validate_index = -1; - DRMLISTADDTAIL(&bo_gem->head, &bucket->head); + if (drm_intel_gem_bo_busy(&bo_gem->bo)) + DRMLISTADDTAIL(&bo_gem->head, &bucket->head); + else + DRMLISTADD(&bo_gem->head, &bucket->head); } else { drm_intel_gem_bo_free(bo); } -- 2.1.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/6] intel: Round large allocations upto PAGE_SIZE 2015-05-05 8:53 [PATCH 1/6] intel: Defer setting madv on the bo cache Chris Wilson ` (2 preceding siblings ...) 2015-05-05 8:53 ` [PATCH 4/6] intel: Keep the caches in loose active order Chris Wilson @ 2015-05-05 8:53 ` Chris Wilson 2015-05-05 8:54 ` [PATCH 6/6] intel: Keep live buffers in the cache Chris Wilson 2015-06-05 16:34 ` [PATCH 1/6] intel: Defer setting madv on the bo cache Ben Widawsky 5 siblings, 0 replies; 8+ messages in thread From: Chris Wilson @ 2015-05-05 8:53 UTC (permalink / raw) To: intel-gfx The kernel interface requires us to request only PAGE_SIZE aligned buffers, so make sure that all allocation requests are indeed sized correctly (and in the process remove the excess calls to getpagesize()). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- intel/intel_bufmgr_gem.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 2cbe973..22f7501 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -671,15 +671,10 @@ drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr, { drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bufmgr; drm_intel_bo_gem *bo_gem; - unsigned int page_size = getpagesize(); - int ret; struct drm_intel_gem_bo_bucket *bucket; bool alloc_from_cache; unsigned long bo_size; - bool for_render = false; - - if (flags & BO_ALLOC_FOR_RENDER) - for_render = true; + int ret; /* Round the allocated size up to a power of two number of pages. */ bucket = drm_intel_gem_bo_bucket_for_size(bufmgr_gem, size); @@ -688,9 +683,8 @@ drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr, * allocation up. */ if (bucket == NULL) { - bo_size = size; - if (bo_size < page_size) - bo_size = page_size; + unsigned int page_size = getpagesize(); + bo_size = ALIGN(size, page_size); } else { bo_size = bucket->size; } @@ -700,7 +694,7 @@ drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr, retry: alloc_from_cache = false; if (bucket != NULL && !DRMLISTEMPTY(&bucket->head)) { - if (for_render) { + if (flags & BO_ALLOC_FOR_RENDER) { /* Allocate new render-target BOs from the tail (MRU) * of the list, as it will likely be hot in the GPU * cache and in the aperture for us. -- 2.1.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 6/6] intel: Keep live buffers in the cache 2015-05-05 8:53 [PATCH 1/6] intel: Defer setting madv on the bo cache Chris Wilson ` (3 preceding siblings ...) 2015-05-05 8:53 ` [PATCH 5/6] intel: Round large allocations upto PAGE_SIZE Chris Wilson @ 2015-05-05 8:54 ` Chris Wilson 2015-06-05 16:34 ` [PATCH 1/6] intel: Defer setting madv on the bo cache Ben Widawsky 5 siblings, 0 replies; 8+ messages in thread From: Chris Wilson @ 2015-05-05 8:54 UTC (permalink / raw) To: intel-gfx If the GPU is still processing the buffers, then keep them alive as the pages are still pinned and hot - ripe for reuse. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- intel/intel_bufmgr_gem.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 22f7501..53457cd 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -1187,6 +1187,9 @@ drm_intel_gem_cleanup_bo_cache(drm_intel_bufmgr_gem *bufmgr_gem, time_t time) if (time - bo_gem->free_time < 2*(1+bo_gem->dontneed)) break; + if (drm_intel_gem_bo_busy(&bo_gem->bo)) + break; + DRMLISTDEL(&bo_gem->head); if (bo_gem->dontneed) { drm_intel_gem_bo_free(&bo_gem->bo); -- 2.1.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/6] intel: Defer setting madv on the bo cache 2015-05-05 8:53 [PATCH 1/6] intel: Defer setting madv on the bo cache Chris Wilson ` (4 preceding siblings ...) 2015-05-05 8:54 ` [PATCH 6/6] intel: Keep live buffers in the cache Chris Wilson @ 2015-06-05 16:34 ` Ben Widawsky 2015-06-05 16:52 ` Chris Wilson 5 siblings, 1 reply; 8+ messages in thread From: Ben Widawsky @ 2015-06-05 16:34 UTC (permalink / raw) To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx Sorry. I know I asked some questions last time around, but I am having trouble finding the response. On Tue, May 05, 2015 at 09:53:55AM +0100, Chris Wilson wrote: > Convert the bo-cache into 2 phases: > > 1. A two second cache of recently used buffers, keep untouched. > 2. A two second cache of older buffers, marked for eviction. > > This helps reduce ioctl traffic on a rapid turnover in working sets. The > issue is that even a 2 second window is enough for an application to > fill all of memory with inactive buffers (and we would rely on the > oom-killer identifying the right culprit). > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Ben Widawsky <benjamin.widawsky@intel.com> > --- > intel/intel_bufmgr_gem.c | 27 ++++++++++++++++++++------- > 1 file changed, 20 insertions(+), 7 deletions(-) > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > index 60c06fc..eeb9acf 100644 > --- a/intel/intel_bufmgr_gem.c > +++ b/intel/intel_bufmgr_gem.c > @@ -216,6 +216,11 @@ struct _drm_intel_bo_gem { > bool used_as_reloc_target; > > /** > + * Are we keeping this buffer around in semi-permenant cache? > + */ > + bool dontneed; > + > + /** > * Boolean of whether we have encountered an error whilst building the relocation tree. > */ > bool has_error; > @@ -719,7 +724,8 @@ retry: > } > > if (alloc_from_cache) { > - if (!drm_intel_gem_bo_madvise_internal > + if (bo_gem->dontneed && > + !drm_intel_gem_bo_madvise_internal Why wouldn't you set dontneed = false? Just a thought, but it seems like a nicer solution here would just be a state variable tracking what we last sent with the madv ioctl. enum { WILLNEED DONTNEED } With that, it might be cool for the curious to put a histogram/hysteresis thing. Every time you hit this condition to see how many times we flip flop. > (bufmgr_gem, bo_gem, I915_MADV_WILLNEED)) { > drm_intel_gem_bo_free(&bo_gem->bo); > drm_intel_gem_bo_cache_purge_bucket(bufmgr_gem, > @@ -1166,19 +1172,28 @@ drm_intel_gem_cleanup_bo_cache(drm_intel_bufmgr_gem *bufmgr_gem, time_t time) > for (i = 0; i < bufmgr_gem->num_buckets; i++) { > struct drm_intel_gem_bo_bucket *bucket = > &bufmgr_gem->cache_bucket[i]; > + drmMMListHead madv; > > + DRMINITLISTHEAD(&madv); > while (!DRMLISTEMPTY(&bucket->head)) { > drm_intel_bo_gem *bo_gem; > > bo_gem = DRMLISTENTRY(drm_intel_bo_gem, > bucket->head.next, head); > - if (time - bo_gem->free_time <= 1) > + if (time - bo_gem->free_time < 2*(1+bo_gem->dontneed)) > break; > > DRMLISTDEL(&bo_gem->head); > - > - drm_intel_gem_bo_free(&bo_gem->bo); > + if (bo_gem->dontneed) { > + drm_intel_gem_bo_free(&bo_gem->bo); > + } else { > + drm_intel_gem_bo_madvise_internal(bufmgr_gem, bo_gem, > + I915_MADV_DONTNEED); > + bo_gem->dontneed = 1; > + DRMLISTADDTAIL(&bo_gem->head, &madv); > + } > } Again with above, how about? assert(!bo_gem->dontneed); bo_gem->dontneed = true; Also I think some comments here explaining what you're doing overall would bce nice since I surely won't remember it in >= 1 minutes. > + DRMLISTJOIN(&madv, &bucket->head); > } > > bufmgr_gem->time = time; > @@ -1289,9 +1304,7 @@ drm_intel_gem_bo_unreference_final(drm_intel_bo *bo, time_t time) > > bucket = drm_intel_gem_bo_bucket_for_size(bufmgr_gem, bo->size); > /* Put the buffer into our internal cache for reuse if we can. */ > - if (bufmgr_gem->bo_reuse && bo_gem->reusable && bucket != NULL && > - drm_intel_gem_bo_madvise_internal(bufmgr_gem, bo_gem, > - I915_MADV_DONTNEED)) { > + if (bufmgr_gem->bo_reuse && bo_gem->reusable && bucket != NULL) { > bo_gem->free_time = time; > > bo_gem->name = NULL; Just to be clear, you're reducing IOCTL traffic by deferring an IOCTL until you cleanup the BO cache. I think I said it last time, overall the idea seems fine to me. -- Ben Widawsky, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/6] intel: Defer setting madv on the bo cache 2015-06-05 16:34 ` [PATCH 1/6] intel: Defer setting madv on the bo cache Ben Widawsky @ 2015-06-05 16:52 ` Chris Wilson 0 siblings, 0 replies; 8+ messages in thread From: Chris Wilson @ 2015-06-05 16:52 UTC (permalink / raw) To: Ben Widawsky; +Cc: Daniel Vetter, intel-gfx On Fri, Jun 05, 2015 at 09:34:23AM -0700, Ben Widawsky wrote: > Sorry. I know I asked some questions last time around, but I am having trouble > finding the response. You thought the existing code used a one second timeout and so wanted a separated patch to convert from age <= 1 to age < 2. Other than that you should have pointed out that dontneed should be moved into madvise_internal() so that it is kept in sync with the kernel value to avoid EFAULT embarrassment. > Just to be clear, you're reducing IOCTL traffic by deferring an IOCTL until you > cleanup the BO cache. Right and thereby reducing ioctls. > I think I said it last time, overall the idea seems fine to me. The change is marginal except when there is a lot of mutex contention (e.g. other clients clflushing) and then the effect is to push the contention to until it is unavoidable. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-06-05 16:52 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-05 8:53 [PATCH 1/6] intel: Defer setting madv on the bo cache Chris Wilson 2015-05-05 8:53 ` [PATCH 2/6] intel: Use WAIT for wait-rendering Chris Wilson 2015-05-05 8:53 ` [PATCH 3/6] intel: Export raw GEM mmap interfaces Chris Wilson 2015-05-05 8:53 ` [PATCH 4/6] intel: Keep the caches in loose active order Chris Wilson 2015-05-05 8:53 ` [PATCH 5/6] intel: Round large allocations upto PAGE_SIZE Chris Wilson 2015-05-05 8:54 ` [PATCH 6/6] intel: Keep live buffers in the cache Chris Wilson 2015-06-05 16:34 ` [PATCH 1/6] intel: Defer setting madv on the bo cache Ben Widawsky 2015-06-05 16:52 ` Chris Wilson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.