Re: [PATCH 07/27] drm/radeon: add proper locking to the SA v3

On 01.05.2012 19:19, j.glisse@gmail.com wrote:

From: Christian Königdeathsimple@vodafone.de

Make the suballocator self containing to locking.

v2: split the bugfix into a seperate patch. v3: Jerome Glisse use mutex, no reason to use spinlock that are more heavyweight than mutex

NAK, radeon_sa_bo_free is called from interrupt context, so a mutex won't work here.

Also because of the short amount of time the lock is (or should be) hold a spinlock seems to be more appropriate here.

Christian.

Signed-off-by: Christian Königdeathsimple@vodafone.de Signed-off-by: Jerome Glissejglisse@redhat.com

---

drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_sa.c | 15 +++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 85a3aa9..34c1041 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -381,6 +381,7 @@ struct radeon_bo_list {

• alignment).

*/ struct radeon_sa_manager {

• struct mutex mutex; struct radeon_bo *bo; struct list_head sa_bo; unsigned size;

diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c index 8fbfe69..8c76c2e 100644 --- a/drivers/gpu/drm/radeon/radeon_sa.c +++ b/drivers/gpu/drm/radeon/radeon_sa.c @@ -37,6 +37,7 @@ int radeon_sa_bo_manager_init(struct radeon_device *rdev, { int r;

• mutex_init(&sa_manager->mutex); sa_manager->bo = NULL; sa_manager->size = size; sa_manager->domain = domain;

@@ -139,15 +140,15 @@ int radeon_sa_bo_new(struct radeon_device *rdev,

BUG_ON(align> RADEON_GPU_PAGE_SIZE); BUG_ON(size> sa_manager->size);

• mutex_lock(&sa_manager->mutex);

  /* no one ? */

• head = sa_manager->sa_bo.prev; if (list_empty(&sa_manager->sa_bo)) {

• head =&sa_manager->sa_bo;
  

  goto out; }

  /* look for a hole big enough */

• offset = 0; list_for_each_entry(tmp,&sa_manager->sa_bo, list) { /* room before this object ? */ if (offset< tmp->offset&& (tmp->offset - offset)>= size) {

@@ -157,9 +158,8 @@ int radeon_sa_bo_new(struct radeon_device *rdev, offset = tmp->offset + tmp->size; wasted = offset % align; if (wasted) {

• 	wasted = align - wasted;
  

• 	offset += align - wasted;
  

  }

• offset += wasted;
  

  } /* room at the end ? */ head = sa_manager->sa_bo.prev;

@@ -167,11 +167,11 @@ int radeon_sa_bo_new(struct radeon_device *rdev, offset = tmp->offset + tmp->size; wasted = offset % align; if (wasted) {

• wasted = align - wasted;
  

• offset += wasted = align - wasted;
  

  }

• offset += wasted; if ((sa_manager->size - offset)< size) { /* failed to find somethings big enough */

• mutex_unlock(&sa_manager->mutex);
  

  return -ENOMEM; }

@@ -180,10 +180,13 @@ out: sa_bo->offset = offset; sa_bo->size = size; list_add(&sa_bo->list, head);

• mutex_unlock(&sa_manager->mutex); return 0; }

  void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo) {

• mutex_lock(&sa_bo->manager->mutex); list_del_init(&sa_bo->list);

• mutex_unlock(&sa_bo->manager->mutex); }