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

[Christian König]

On 01.05.2012 19:19, j.glisse at gmail.com wrote:
> From: Christian König<deathsimple at 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önig<deathsimple at vodafone.de>
> Signed-off-by: Jerome Glisse<jglisse at 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);
>   }