[Mesa-dev] [PATCH 2/2] i965: Reduce the frequency of madvise calls

Chris Wilson chris at chris-wilson.co.uk
Fri Oct 20 13:10:36 UTC 2017


We can reduce the frequency of calling madvise by only using it for
buffers that are inactive (busy buffers will be pinned by they use by the
GPU, so should not significantly add to mempressure, but will still be
reaped by our timed cache.)

v2: Remember about APPLE_purgeable_memory that also calls
brw_bo_madvise() and so leave the buffer as purgeable needing to be
recovered prior to use.
---
 src/mesa/drivers/dri/i965/brw_bufmgr.c | 53 +++++++++++++++++++---------------
 src/mesa/drivers/dri/i965/brw_bufmgr.h | 17 +++++++----
 2 files changed, 41 insertions(+), 29 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index 19c9ba2fa5..5035b92308 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -111,7 +111,7 @@ struct brw_bufmgr {
    /** Array of lists of cached gem objects of power-of-two sizes */
    struct bo_cache_bucket cache_bucket[14 * 4];
    int num_buckets;
-   time_t time;
+   unsigned time;
 
    struct hash_table *name_table;
    struct hash_table *handle_table;
@@ -213,7 +213,7 @@ brw_bo_busy(struct brw_bo *bo)
    return false;
 }
 
-int
+bool
 brw_bo_madvise(struct brw_bo *bo, int state)
 {
    struct drm_i915_gem_madvise madv;
@@ -221,9 +221,10 @@ brw_bo_madvise(struct brw_bo *bo, int state)
    memclear(madv);
    madv.handle = bo->gem_handle;
    madv.madv = state;
-   madv.retained = 1;
-   drmIoctl(bo->bufmgr->fd, DRM_IOCTL_I915_GEM_MADVISE, &madv);
+   if (drmIoctl(bo->bufmgr->fd, DRM_IOCTL_I915_GEM_MADVISE, &madv))
+      return false;
 
+   bo->purgeable = !madv.retained || state != I915_MADV_WILLNEED;
    return madv.retained;
 }
 
@@ -233,7 +234,7 @@ brw_bo_cache_purge_bucket(struct brw_bufmgr *bufmgr,
                           struct bo_cache_bucket *bucket)
 {
    list_for_each_entry_safe(struct brw_bo, bo, &bucket->head, head) {
-      if (brw_bo_madvise(bo, I915_MADV_DONTNEED))
+      if (!bo->purgeable || brw_bo_madvise(bo, I915_MADV_DONTNEED))
          break;
 
       list_del(&bo->head);
@@ -316,7 +317,7 @@ retry:
       }
 
       if (alloc_from_cache) {
-         if (!brw_bo_madvise(bo, I915_MADV_WILLNEED)) {
+         if (bo->purgeable && !brw_bo_madvise(bo, I915_MADV_WILLNEED)) {
             bo_free(bo);
             brw_bo_cache_purge_bucket(bufmgr, bucket);
             goto retry;
@@ -388,7 +389,7 @@ retry:
 
    bo->name = name;
    p_atomic_set(&bo->refcount, 1);
-   bo->reusable = true;
+   bo->reusable = bufmgr->bo_reuse;
    bo->cache_coherent = bufmgr->has_llc;
    bo->index = -1;
 
@@ -593,10 +594,17 @@ bo_free(struct brw_bo *bo)
 
 /** Frees all cached buffers significantly older than @time. */
 static void
-cleanup_bo_cache(struct brw_bufmgr *bufmgr, time_t time)
+cleanup_bo_cache(struct brw_bufmgr *bufmgr)
 {
+   struct timespec tv;
+   unsigned time;
    int i;
 
+   clock_gettime(CLOCK_MONOTONIC, &tv);
+   time = tv.tv_sec;
+   if (unlikely(!time)) /* 0 is reserved for unset */
+      time = 1;
+
    if (bufmgr->time == time)
       return;
 
@@ -604,12 +612,16 @@ cleanup_bo_cache(struct brw_bufmgr *bufmgr, time_t time)
       struct bo_cache_bucket *bucket = &bufmgr->cache_bucket[i];
 
       list_for_each_entry_safe(struct brw_bo, bo, &bucket->head, head) {
-         if (time - bo->free_time <= 1)
-            break;
+         if (!bo->free_time) {
+            if (brw_bo_busy(bo))
+               break;
 
-         list_del(&bo->head);
-
-         bo_free(bo);
+            brw_bo_madvise(bo, I915_MADV_DONTNEED);
+            bo->free_time = time;
+         } else if (time - bo->free_time > 1) {
+            list_del(&bo->head);
+            bo_free(bo);
+         }
       }
    }
 
@@ -617,7 +629,7 @@ cleanup_bo_cache(struct brw_bufmgr *bufmgr, time_t time)
 }
 
 static void
-bo_unreference_final(struct brw_bo *bo, time_t time)
+bo_unreference_final(struct brw_bo *bo)
 {
    struct brw_bufmgr *bufmgr = bo->bufmgr;
    struct bo_cache_bucket *bucket;
@@ -626,13 +638,11 @@ bo_unreference_final(struct brw_bo *bo, time_t time)
 
    bucket = bucket_for_size(bufmgr, bo->size);
    /* Put the buffer into our internal cache for reuse if we can. */
-   if (bufmgr->bo_reuse && bo->reusable && bucket != NULL &&
-       brw_bo_madvise(bo, I915_MADV_DONTNEED)) {
-      bo->free_time = time;
-
+   if (bo->reusable && bucket != NULL) {
       bo->name = NULL;
       bo->kflags = 0;
 
+      bo->free_time = 0;
       if (brw_bo_busy(bo))
          list_addtail(&bo->head, &bucket->head);
       else
@@ -652,15 +662,12 @@ brw_bo_unreference(struct brw_bo *bo)
 
    if (atomic_add_unless(&bo->refcount, -1, 1)) {
       struct brw_bufmgr *bufmgr = bo->bufmgr;
-      struct timespec time;
-
-      clock_gettime(CLOCK_MONOTONIC, &time);
 
       mtx_lock(&bufmgr->lock);
 
       if (p_atomic_dec_zero(&bo->refcount)) {
-         bo_unreference_final(bo, time.tv_sec);
-         cleanup_bo_cache(bufmgr, time.tv_sec);
+         cleanup_bo_cache(bufmgr);
+         bo_unreference_final(bo);
       }
 
       mtx_unlock(&bufmgr->lock);
diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h b/src/mesa/drivers/dri/i965/brw_bufmgr.h
index ee91324043..d8f34fac48 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
@@ -154,17 +154,22 @@ struct brw_bo {
    /**
     * Boolean of whether this buffer can be re-used
     */
-   bool reusable;
+   bool reusable:1;
+
+   /**
+    * Boolean of whether this buffer has been marked as purgeable
+    */
+   bool purgeable:1;
 
    /**
     * Boolean of whether this buffer has been shared with an external client.
     */
-   bool external;
+   bool external:1;
 
    /**
     * Boolean of whether this buffer is cache coherent
     */
-   bool cache_coherent;
+   bool cache_coherent:1;
 };
 
 #define BO_ALLOC_BUSY       (1<<0)
@@ -307,10 +312,10 @@ int brw_bo_busy(struct brw_bo *bo);
  * reclaimed under memory pressure. If you subsequently require the buffer,
  * then you must pass I915_MADV_WILLNEED to mark the buffer as required.
  *
- * Returns 1 if the buffer was retained, or 0 if it was discarded whilst
- * marked as I915_MADV_DONTNEED.
+ * Returns true if the buffer was retained, or false if it was discarded whilst
+ * marked as I915_MADV_DONTNEED. Once discarded, the buffer should not be used!
  */
-int brw_bo_madvise(struct brw_bo *bo, int madv);
+bool brw_bo_madvise(struct brw_bo *bo, int madv);
 
 /* drm_bacon_bufmgr_gem.c */
 struct brw_bufmgr *brw_bufmgr_init(struct gen_device_info *devinfo, int fd);
-- 
2.15.0.rc1



More information about the mesa-dev mailing list