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);
> }
