From: <ic...@ke...> - 2009-12-02 14:30:44
|
intel/intel_bufmgr_gem.c | 111 ++++++++++++++++++++++++++++++----------------- 1 file changed, 72 insertions(+), 39 deletions(-) New commits: commit 57473c7f523f476ffa54c34e0c6312ffa66dcc5c Author: Chris Wilson <ch...@ch...> Date: Wed Dec 2 13:36:22 2009 +0000 intel: Free memory before inserting bo into cache. This has the unfortunate behaviour of releasing our malloc cache, but the alternative is for X to consume a couple of gigabytes of ram and die during testing. Fortunately the extra mallocs have little impact on performance whereas avoiding swap and death, lots. Signed-off-by: Chris Wilson <ch...@ch...> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index ac7297b..571ab5c 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -727,9 +727,6 @@ drm_intel_gem_bo_free(drm_intel_bo *bo) if (bo_gem->gtt_virtual) munmap(bo_gem->gtt_virtual, bo_gem->bo.size); - free(bo_gem->reloc_target_bo); - free(bo_gem->relocs); - /* Close this object */ memset(&close, 0, sizeof(close)); close.handle = bo_gem->gem_handle; @@ -788,6 +785,16 @@ drm_intel_gem_bo_unreference_final(drm_intel_bo *bo, time_t time) DBG("bo_unreference final: %d (%s)\n", bo_gem->gem_handle, bo_gem->name); + /* release memory associated with this object */ + if (bo_gem->reloc_target_bo) { + free(bo_gem->reloc_target_bo); + bo_gem->reloc_target_bo = NULL; + } + if (bo_gem->relocs) { + free(bo_gem->relocs); + bo_gem->relocs = NULL; + } + bucket = drm_intel_gem_bo_bucket_for_size(bufmgr_gem, bo->size); /* Put the buffer into our internal cache for reuse if we can. */ tiling_mode = I915_TILING_NONE; commit 792fed1e2460f96459141b5a628dd5ab4fbb87db Author: Chris Wilson <ch...@ch...> Date: Wed Dec 2 13:12:39 2009 +0000 intel: Check and propagate errors from building reloc-tree Instead of forcing the caller to check after every emit_reloc(), we can flag the object as being in error, propagating that error upwards through the relocation tree, and failing the eventual batch buffer execution. Signed-off-by: Chris Wilson <ch...@ch...> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 239f36d..ac7297b 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -154,6 +154,11 @@ struct _drm_intel_bo_gem { char used_as_reloc_target; /** + * Boolean of whether we have encountered an error whilst building the relocation tree. + */ + char has_error; + + /** * Boolean of whether this buffer can be re-used */ char reusable; @@ -396,6 +401,17 @@ drm_intel_setup_reloc_list(drm_intel_bo *bo) bo_gem->relocs = malloc(max_relocs * sizeof(struct drm_i915_gem_relocation_entry)); bo_gem->reloc_target_bo = malloc(max_relocs * sizeof(drm_intel_bo *)); + if (bo_gem->relocs == NULL || bo_gem->reloc_target_bo == NULL) { + bo_gem->has_error = 1; + + free (bo_gem->relocs); + bo_gem->relocs = NULL; + + free (bo_gem->reloc_target_bo); + bo_gem->reloc_target_bo = NULL; + + return 1; + } return 0; } @@ -564,6 +580,7 @@ retry: bo_gem->validate_index = -1; bo_gem->reloc_tree_fences = 0; bo_gem->used_as_reloc_target = 0; + bo_gem->has_error = 0; bo_gem->tiling_mode = I915_TILING_NONE; bo_gem->swizzle_mode = I915_BIT_6_SWIZZLE_NONE; bo_gem->reusable = 1; @@ -1178,10 +1195,22 @@ drm_intel_gem_bo_emit_reloc(drm_intel_bo *bo, uint32_t offset, drm_intel_bo_gem *target_bo_gem = (drm_intel_bo_gem *) target_bo; pthread_mutex_lock(&bufmgr_gem->lock); + if (bo_gem->has_error) { + pthread_mutex_unlock(&bufmgr_gem->lock); + return -ENOMEM; + } + + if (target_bo_gem->has_error) { + bo_gem->has_error = 1; + pthread_mutex_unlock(&bufmgr_gem->lock); + return -ENOMEM; + } /* Create a new relocation list if needed */ - if (bo_gem->relocs == NULL) - drm_intel_setup_reloc_list(bo); + if (bo_gem->relocs == NULL && drm_intel_setup_reloc_list(bo)) { + pthread_mutex_unlock(&bufmgr_gem->lock); + return -ENOMEM; + } /* Check overflow */ assert(bo_gem->reloc_count < bufmgr_gem->max_relocs); @@ -1268,9 +1297,13 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used, drm_clip_rect_t * cliprects, int num_cliprects, int DR4) { drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr; + drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; struct drm_i915_gem_execbuffer execbuf; int ret, i; + if (bo_gem->has_error) + return -ENOMEM; + pthread_mutex_lock(&bufmgr_gem->lock); /* Update indices and set up the validate list. */ drm_intel_gem_bo_process_reloc(bo); commit b73612e4fd69565aa2c5c2e9677f3e0af1945f7d Author: Chris Wilson <ch...@ch...> Date: Wed Dec 2 12:58:00 2009 +0000 intel: Repeat execbuffer after EINTR EAGAIN cannot be raised by the current code, but the system call maybe interrupted and so return EINTR. Signed-off-by: Chris Wilson <ch...@ch...> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index bb20f43..239f36d 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -1293,7 +1293,7 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used, ret = ioctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_EXECBUFFER, &execbuf); - } while (ret != 0 && errno == EAGAIN); + } while (ret != 0 && errno == EINTR); if (ret != 0) { ret = -errno; commit acb4aa671507aa181b3ff50ccf26a1c0d705a309 Author: Chris Wilson <ch...@ch...> Date: Wed Dec 2 12:40:26 2009 +0000 intel: Review use of errno. Hitting this error lead to a segfault: intel_bufmgr_gem.c:919: Error mapping buffer 48607 (pixmap): Cannot allocate memory. because the errno was reused as the function return value after being reset by the fprintf(), so caller thought the mapping had succeeded. The convention established by libdrm is that the return value is the negative errno and that uses of libdrm cannot trust the value of errno afterwards, but must use the return code. Signed-off-by: Chris Wilson <ch...@ch...> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index e9896e7..bb20f43 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -846,6 +846,7 @@ static int drm_intel_gem_bo_map(drm_intel_bo *bo, int write_enable) &mmap_arg); } while (ret == -1 && errno == EINTR); if (ret != 0) { + ret = -errno; fprintf(stderr, "%s:%d: Error mapping buffer %d (%s): %s .\n", __FILE__, __LINE__, bo_gem->gem_handle, @@ -871,6 +872,7 @@ static int drm_intel_gem_bo_map(drm_intel_bo *bo, int write_enable) &set_domain); } while (ret == -1 && errno == EINTR); if (ret != 0) { + ret = -errno; fprintf(stderr, "%s:%d: Error setting to CPU domain %d: %s\n", __FILE__, __LINE__, bo_gem->gem_handle, strerror(errno)); @@ -909,6 +911,7 @@ int drm_intel_gem_bo_map_gtt(drm_intel_bo *bo) &mmap_arg); } while (ret == -1 && errno == EINTR); if (ret != 0) { + ret = -errno; fprintf(stderr, "%s:%d: Error preparing buffer map %d (%s): %s .\n", __FILE__, __LINE__, @@ -923,13 +926,14 @@ int drm_intel_gem_bo_map_gtt(drm_intel_bo *bo) MAP_SHARED, bufmgr_gem->fd, mmap_arg.offset); if (bo_gem->gtt_virtual == MAP_FAILED) { + ret = -errno; fprintf(stderr, "%s:%d: Error mapping buffer %d (%s): %s .\n", __FILE__, __LINE__, bo_gem->gem_handle, bo_gem->name, strerror(errno)); pthread_mutex_unlock(&bufmgr_gem->lock); - return errno; + return ret; } } @@ -949,6 +953,7 @@ int drm_intel_gem_bo_map_gtt(drm_intel_bo *bo) } while (ret == -1 && errno == EINTR); if (ret != 0) { + ret = -errno; fprintf(stderr, "%s:%d: Error setting domain %d: %s\n", __FILE__, __LINE__, bo_gem->gem_handle, strerror(errno)); @@ -1077,12 +1082,13 @@ drm_intel_gem_bo_get_subdata(drm_intel_bo *bo, unsigned long offset, &pread); } while (ret == -1 && errno == EINTR); if (ret != 0) { + ret = -errno; fprintf(stderr, "%s:%d: Error reading data from buffer %d: (%d %d) %s .\n", __FILE__, __LINE__, bo_gem->gem_handle, (int)offset, (int)size, strerror(errno)); } - return 0; + return ret; } /** Waits for all GPU rendering to the object to have completed. */ @@ -1289,17 +1295,20 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used, &execbuf); } while (ret != 0 && errno == EAGAIN); - if (ret != 0 && errno == ENOMEM) { - fprintf(stderr, - "Execbuffer fails to pin. " - "Estimate: %u. Actual: %u. Available: %u\n", - drm_intel_gem_estimate_batch_space(bufmgr_gem->exec_bos, - bufmgr_gem-> - exec_count), - drm_intel_gem_compute_batch_space(bufmgr_gem->exec_bos, - bufmgr_gem-> - exec_count), - (unsigned int)bufmgr_gem->gtt_size); + if (ret != 0) { + ret = -errno; + if (errno == ENOSPC) { + fprintf(stderr, + "Execbuffer fails to pin. " + "Estimate: %u. Actual: %u. Available: %u\n", + drm_intel_gem_estimate_batch_space(bufmgr_gem->exec_bos, + bufmgr_gem-> + exec_count), + drm_intel_gem_compute_batch_space(bufmgr_gem->exec_bos, + bufmgr_gem-> + exec_count), + (unsigned int)bufmgr_gem->gtt_size); + } } drm_intel_update_buffer_offsets(bufmgr_gem); @@ -1317,7 +1326,7 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used, bufmgr_gem->exec_count = 0; pthread_mutex_unlock(&bufmgr_gem->lock); - return 0; + return ret; } static int @@ -1606,7 +1615,7 @@ drm_intel_gem_check_aperture_space(drm_intel_bo **bo_array, int count) if (bufmgr_gem->available_fences) { total_fences = drm_intel_gem_total_fences(bo_array, count); if (total_fences > bufmgr_gem->available_fences) - return -1; + return -ENOSPC; } total = drm_intel_gem_estimate_batch_space(bo_array, count); @@ -1618,7 +1627,7 @@ drm_intel_gem_check_aperture_space(drm_intel_bo **bo_array, int count) DBG("check_space: overflowed available aperture, " "%dkb vs %dkb\n", total / 1024, (int)bufmgr_gem->gtt_size / 1024); - return -1; + return -ENOSPC; } else { DBG("drm_check_space: total %dkb vs bufgr %dkb\n", total / 1024, (int)bufmgr_gem->gtt_size / 1024); commit 9fec2a8cb28d814da4fdd97b25e9cc5c10768c87 Author: Chris Wilson <ch...@ch...> Date: Wed Dec 2 10:42:51 2009 +0000 intel: Make bo_reference() inline for internal use. Signed-off-by: Chris Wilson <ch...@ch...> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index cf3943c..e9896e7 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -304,7 +304,7 @@ drm_intel_gem_dump_validation_list(drm_intel_bufmgr_gem *bufmgr_gem) } } -static void +static inline void drm_intel_gem_bo_reference(drm_intel_bo *bo) { drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; commit 9c8ad05e8bb1c954b804e40f2f975fed23c24550 Author: Chris Wilson <ch...@ch...> Date: Wed Dec 2 10:41:39 2009 +0000 intel: Remove the extra reference while validating the reloc tree Buffers on the relocation tree are guarded by the reference to the batch object and so do not need an extra reference whilst constructing the list of execution buffer objects. Signed-off-by: Chris Wilson <ch...@ch...> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 3b4d3cf..cf3943c 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -187,7 +187,6 @@ static int drm_intel_gem_bo_set_tiling(drm_intel_bo *bo, uint32_t * tiling_mode, uint32_t stride); -static void drm_intel_gem_bo_unreference_locked(drm_intel_bo *bo); static void drm_intel_gem_bo_unreference_locked_timed(drm_intel_bo *bo, time_t time); @@ -357,7 +356,6 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo) bufmgr_gem->exec_objects[index].alignment = 0; bufmgr_gem->exec_objects[index].offset = 0; bufmgr_gem->exec_bos[index] = bo; - drm_intel_gem_bo_reference(bo); bufmgr_gem->exec_count++; } @@ -793,19 +791,6 @@ drm_intel_gem_bo_unreference_final(drm_intel_bo *bo, time_t time) } } -static void drm_intel_gem_bo_unreference_locked(drm_intel_bo *bo) -{ - drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; - - assert(atomic_read(&bo_gem->refcount) > 0); - if (atomic_dec_and_test(&bo_gem->refcount)) { - struct timespec time; - - clock_gettime(CLOCK_MONOTONIC, &time); - drm_intel_gem_bo_unreference_final(bo, time.tv_sec); - } -} - static void drm_intel_gem_bo_unreference_locked_timed(drm_intel_bo *bo, time_t time) { @@ -1327,7 +1312,6 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used, /* Disconnect the buffer from the validate list */ bo_gem->validate_index = -1; - drm_intel_gem_bo_unreference_locked(bo); bufmgr_gem->exec_bos[i] = NULL; } bufmgr_gem->exec_count = 0; |