[PATCH 1/2] drm/ttm: remove some bo->mutex remains

[PATCH 1/2] drm/ttm: remove some bo->mutex remains

[Maarten Maathuis]

- A few comments existed here and there that referred to a bo->mutex.

Signed-off-by: Maarten Maathuis <madman2003@gmail.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c    |    6 +-----
 drivers/gpu/drm/ttm/ttm_bo_vm.c |    2 +-
 include/drm/ttm/ttm_bo_api.h    |    3 +--
 include/drm/ttm/ttm_bo_driver.h |    1 +
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2920f9a..f5333d9 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -46,7 +46,6 @@
 #include <linux/file.h>
 #include <linux/module.h>
 
-#define TTM_ASSERT_LOCKED(param)
 #define TTM_DEBUG(fmt, arg...)
 #define TTM_BO_HASH_ORDER 13
 
@@ -306,9 +305,6 @@ void ttm_bo_unreserve(struct ttm_buffer_object *bo)
 }
 EXPORT_SYMBOL(ttm_bo_unreserve);
 
-/*
- * Call bo->mutex locked.
- */
 static int ttm_bo_add_ttm(struct ttm_buffer_object *bo, bool zero_alloc)
 {
 	struct ttm_bo_device *bdev = bo->bdev;
@@ -316,7 +312,7 @@ static int ttm_bo_add_ttm(struct ttm_buffer_object *bo, bool zero_alloc)
 	int ret = 0;
 	uint32_t page_flags = 0;
 
-	TTM_ASSERT_LOCKED(&bo->mutex);
+	BUG_ON(!atomic_read(&bo->reserved));
 	bo->ttm = NULL;
 
 	if (bdev->need_dma32)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 668dbe8..41b0c1e 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -146,7 +146,7 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	 * since the mmap_sem is only held in read mode. However, we
 	 * modify only the caching bits of vma->vm_page_prot and
 	 * consider those bits protected by
-	 * the bo->mutex, as we should be the only writers.
+	 * bo->reserved, as we should be the only writers.
 	 * There shouldn't really be any readers of these bits except
 	 * within vm_insert_mixed()? fork?
 	 *
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 81eb9f4..c1093ae 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -35,7 +35,6 @@
 #include <linux/kref.h>
 #include <linux/list.h>
 #include <linux/wait.h>
-#include <linux/mutex.h>
 #include <linux/mm.h>
 #include <linux/rbtree.h>
 #include <linux/bitmap.h>
@@ -298,7 +297,7 @@ ttm_bo_reference(struct ttm_buffer_object *bo)
  * @interruptible:  Use interruptible wait.
  * @no_wait:  Return immediately if buffer is busy.
  *
- * This function must be called with the bo::mutex held, and makes
+ * This function must be called with bo->reserved held, and makes
  * sure any previous rendering to the buffer is completed.
  * Note: It might be necessary to block validations before the
  * wait by reserving the buffer.
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index ff7664e..d3fc5f8 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -37,6 +37,7 @@
 #include "linux/workqueue.h"
 #include "linux/fs.h"
 #include "linux/spinlock.h"
+#include <linux/mutex.h>
 
 struct ttm_backend;
 
-- 
1.7.0


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 2/2] drm/ttm: don't write to bo->reserved without holding glob->lru_lock

[Maarten Maathuis]

- The headerfile says you can't write to it without holding the lock.

Signed-off-by: Maarten Maathuis <madman2003@gmail.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index f5333d9..2104885 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -476,9 +476,9 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, bool remove_all)
 			drm_mm_put_block(bo->mem.mm_node);
 			bo->mem.mm_node = NULL;
 		}
-		spin_unlock(&glob->lru_lock);
 
 		atomic_set(&bo->reserved, 0);
+		spin_unlock(&glob->lru_lock);
 
 		while (put_count--)
 			kref_put(&bo->list_kref, ttm_bo_ref_bug);
@@ -1707,8 +1707,12 @@ EXPORT_SYMBOL(ttm_bo_wait);
 
 void ttm_bo_unblock_reservation(struct ttm_buffer_object *bo)
 {
+	struct ttm_bo_global *glob = bo->glob;
+
+	spin_lock(&glob->lru_lock);
 	atomic_set(&bo->reserved, 0);
 	wake_up_all(&bo->event_queue);
+	spin_unlock(&glob->lru_lock);
 }
 
 int ttm_bo_block_reservation(struct ttm_buffer_object *bo, bool interruptible,
@@ -1849,8 +1853,10 @@ out:
 	 * already swapped buffer.
 	 */
 
+	spin_lock(&glob->lru_lock);
 	atomic_set(&bo->reserved, 0);
 	wake_up_all(&bo->event_queue);
+	spin_unlock(&glob->lru_lock);
 	kref_put(&bo->list_kref, ttm_bo_release_list);
 	return ret;
 }
-- 
1.7.0


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: [PATCH 2/2] drm/ttm: don't write to bo->reserved without holding glob->lru_lock

[Jerome Glisse]

On Mon, Mar 01, 2010 at 07:34:40PM +0100, Maarten Maathuis wrote:
> - The headerfile says you can't write to it without holding the lock.
> 
> Signed-off-by: Maarten Maathuis <madman2003@gmail.com>

NAK, from my POV as we always use atomic_* on reserved it's useless
to protect it with spinlock.

Cheers,
Jerome

> ---
>  drivers/gpu/drm/ttm/ttm_bo.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index f5333d9..2104885 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -476,9 +476,9 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, bool remove_all)
>  			drm_mm_put_block(bo->mem.mm_node);
>  			bo->mem.mm_node = NULL;
>  		}
> -		spin_unlock(&glob->lru_lock);
>  
>  		atomic_set(&bo->reserved, 0);
> +		spin_unlock(&glob->lru_lock);
>  
>  		while (put_count--)
>  			kref_put(&bo->list_kref, ttm_bo_ref_bug);
> @@ -1707,8 +1707,12 @@ EXPORT_SYMBOL(ttm_bo_wait);
>  
>  void ttm_bo_unblock_reservation(struct ttm_buffer_object *bo)
>  {
> +	struct ttm_bo_global *glob = bo->glob;
> +
> +	spin_lock(&glob->lru_lock);
>  	atomic_set(&bo->reserved, 0);
>  	wake_up_all(&bo->event_queue);
> +	spin_unlock(&glob->lru_lock);
>  }
>  
>  int ttm_bo_block_reservation(struct ttm_buffer_object *bo, bool interruptible,
> @@ -1849,8 +1853,10 @@ out:
>  	 * already swapped buffer.
>  	 */
>  
> +	spin_lock(&glob->lru_lock);
>  	atomic_set(&bo->reserved, 0);
>  	wake_up_all(&bo->event_queue);
> +	spin_unlock(&glob->lru_lock);
>  	kref_put(&bo->list_kref, ttm_bo_release_list);
>  	return ret;
>  }
> -- 
> 1.7.0
> 
> 
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> --
> _______________________________________________
> Dri-devel mailing list
> Dri-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/dri-devel
> 

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: [PATCH 2/2] drm/ttm: don't write to bo->reserved without holding glob->lru_lock

[Thomas Hellstrom]

Maarten Maathuis wrote:
> - The headerfile says you can't write to it without holding the lock.
>   
NAK.

The header-file is incorrect. It should say that the lru_lock needs to 
be held only when we transition from 0 to 1.

That guarantees that if the lru lock is held, and we read 0, we can 
safely assume that we're the only ones trying to lock.

/Thomas










> Signed-off-by: Maarten Maathuis <madman2003@gmail.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index f5333d9..2104885 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -476,9 +476,9 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, bool remove_all)
>  			drm_mm_put_block(bo->mem.mm_node);
>  			bo->mem.mm_node = NULL;
>  		}
> -		spin_unlock(&glob->lru_lock);
>  
>  		atomic_set(&bo->reserved, 0);
> +		spin_unlock(&glob->lru_lock);
>  
>  		while (put_count--)
>  			kref_put(&bo->list_kref, ttm_bo_ref_bug);
> @@ -1707,8 +1707,12 @@ EXPORT_SYMBOL(ttm_bo_wait);
>  
>  void ttm_bo_unblock_reservation(struct ttm_buffer_object *bo)
>  {
> +	struct ttm_bo_global *glob = bo->glob;
> +
> +	spin_lock(&glob->lru_lock);
>  	atomic_set(&bo->reserved, 0);
>  	wake_up_all(&bo->event_queue);
> +	spin_unlock(&glob->lru_lock);
>  }
>  
>  int ttm_bo_block_reservation(struct ttm_buffer_object *bo, bool interruptible,
> @@ -1849,8 +1853,10 @@ out:
>  	 * already swapped buffer.
>  	 */
>  
> +	spin_lock(&glob->lru_lock);
>  	atomic_set(&bo->reserved, 0);
>  	wake_up_all(&bo->event_queue);
> +	spin_unlock(&glob->lru_lock);
>  	kref_put(&bo->list_kref, ttm_bo_release_list);
>  	return ret;
>  }
>   


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: [PATCH 1/2] drm/ttm: remove some bo->mutex remains

[Jerome Glisse]

On Mon, Mar 01, 2010 at 07:34:39PM +0100, Maarten Maathuis wrote:
> - A few comments existed here and there that referred to a bo->mutex.
> 
> Signed-off-by: Maarten Maathuis <madman2003@gmail.com>

Reviewed-by: Jerome Glisse <jglisse@redhat.com>

> ---
>  drivers/gpu/drm/ttm/ttm_bo.c    |    6 +-----
>  drivers/gpu/drm/ttm/ttm_bo_vm.c |    2 +-
>  include/drm/ttm/ttm_bo_api.h    |    3 +--
>  include/drm/ttm/ttm_bo_driver.h |    1 +
>  4 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 2920f9a..f5333d9 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -46,7 +46,6 @@
>  #include <linux/file.h>
>  #include <linux/module.h>
>  
> -#define TTM_ASSERT_LOCKED(param)
>  #define TTM_DEBUG(fmt, arg...)
>  #define TTM_BO_HASH_ORDER 13
>  
> @@ -306,9 +305,6 @@ void ttm_bo_unreserve(struct ttm_buffer_object *bo)
>  }
>  EXPORT_SYMBOL(ttm_bo_unreserve);
>  
> -/*
> - * Call bo->mutex locked.
> - */
>  static int ttm_bo_add_ttm(struct ttm_buffer_object *bo, bool zero_alloc)
>  {
>  	struct ttm_bo_device *bdev = bo->bdev;
> @@ -316,7 +312,7 @@ static int ttm_bo_add_ttm(struct ttm_buffer_object *bo, bool zero_alloc)
>  	int ret = 0;
>  	uint32_t page_flags = 0;
>  
> -	TTM_ASSERT_LOCKED(&bo->mutex);
> +	BUG_ON(!atomic_read(&bo->reserved));
>  	bo->ttm = NULL;
>  
>  	if (bdev->need_dma32)
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index 668dbe8..41b0c1e 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -146,7 +146,7 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	 * since the mmap_sem is only held in read mode. However, we
>  	 * modify only the caching bits of vma->vm_page_prot and
>  	 * consider those bits protected by
> -	 * the bo->mutex, as we should be the only writers.
> +	 * bo->reserved, as we should be the only writers.
>  	 * There shouldn't really be any readers of these bits except
>  	 * within vm_insert_mixed()? fork?
>  	 *
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 81eb9f4..c1093ae 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -35,7 +35,6 @@
>  #include <linux/kref.h>
>  #include <linux/list.h>
>  #include <linux/wait.h>
> -#include <linux/mutex.h>
>  #include <linux/mm.h>
>  #include <linux/rbtree.h>
>  #include <linux/bitmap.h>
> @@ -298,7 +297,7 @@ ttm_bo_reference(struct ttm_buffer_object *bo)
>   * @interruptible:  Use interruptible wait.
>   * @no_wait:  Return immediately if buffer is busy.
>   *
> - * This function must be called with the bo::mutex held, and makes
> + * This function must be called with bo->reserved held, and makes
>   * sure any previous rendering to the buffer is completed.
>   * Note: It might be necessary to block validations before the
>   * wait by reserving the buffer.
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index ff7664e..d3fc5f8 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -37,6 +37,7 @@
>  #include "linux/workqueue.h"
>  #include "linux/fs.h"
>  #include "linux/spinlock.h"
> +#include <linux/mutex.h>
>  
>  struct ttm_backend;
>  
> -- 
> 1.7.0
> 
> 
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> --
> _______________________________________________
> Dri-devel mailing list
> Dri-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/dri-devel
> 

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: [PATCH 1/2] drm/ttm: remove some bo->mutex remains

[Maarten Maathuis]

Ping?

On Mon, Mar 1, 2010 at 9:55 PM, Jerome Glisse <glisse@freedesktop.org> wrote:
> On Mon, Mar 01, 2010 at 07:34:39PM +0100, Maarten Maathuis wrote:
>> - A few comments existed here and there that referred to a bo->mutex.
>>
>> Signed-off-by: Maarten Maathuis <madman2003@gmail.com>
>
> Reviewed-by: Jerome Glisse <jglisse@redhat.com>
>
>> ---
>>  drivers/gpu/drm/ttm/ttm_bo.c    |    6 +-----
>>  drivers/gpu/drm/ttm/ttm_bo_vm.c |    2 +-
>>  include/drm/ttm/ttm_bo_api.h    |    3 +--
>>  include/drm/ttm/ttm_bo_driver.h |    1 +
>>  4 files changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 2920f9a..f5333d9 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -46,7 +46,6 @@
>>  #include <linux/file.h>
>>  #include <linux/module.h>
>>
>> -#define TTM_ASSERT_LOCKED(param)
>>  #define TTM_DEBUG(fmt, arg...)
>>  #define TTM_BO_HASH_ORDER 13
>>
>> @@ -306,9 +305,6 @@ void ttm_bo_unreserve(struct ttm_buffer_object *bo)
>>  }
>>  EXPORT_SYMBOL(ttm_bo_unreserve);
>>
>> -/*
>> - * Call bo->mutex locked.
>> - */
>>  static int ttm_bo_add_ttm(struct ttm_buffer_object *bo, bool zero_alloc)
>>  {
>>       struct ttm_bo_device *bdev = bo->bdev;
>> @@ -316,7 +312,7 @@ static int ttm_bo_add_ttm(struct ttm_buffer_object *bo, bool zero_alloc)
>>       int ret = 0;
>>       uint32_t page_flags = 0;
>>
>> -     TTM_ASSERT_LOCKED(&bo->mutex);
>> +     BUG_ON(!atomic_read(&bo->reserved));
>>       bo->ttm = NULL;
>>
>>       if (bdev->need_dma32)
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> index 668dbe8..41b0c1e 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> @@ -146,7 +146,7 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>>        * since the mmap_sem is only held in read mode. However, we
>>        * modify only the caching bits of vma->vm_page_prot and
>>        * consider those bits protected by
>> -      * the bo->mutex, as we should be the only writers.
>> +      * bo->reserved, as we should be the only writers.
>>        * There shouldn't really be any readers of these bits except
>>        * within vm_insert_mixed()? fork?
>>        *
>> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
>> index 81eb9f4..c1093ae 100644
>> --- a/include/drm/ttm/ttm_bo_api.h
>> +++ b/include/drm/ttm/ttm_bo_api.h
>> @@ -35,7 +35,6 @@
>>  #include <linux/kref.h>
>>  #include <linux/list.h>
>>  #include <linux/wait.h>
>> -#include <linux/mutex.h>
>>  #include <linux/mm.h>
>>  #include <linux/rbtree.h>
>>  #include <linux/bitmap.h>
>> @@ -298,7 +297,7 @@ ttm_bo_reference(struct ttm_buffer_object *bo)
>>   * @interruptible:  Use interruptible wait.
>>   * @no_wait:  Return immediately if buffer is busy.
>>   *
>> - * This function must be called with the bo::mutex held, and makes
>> + * This function must be called with bo->reserved held, and makes
>>   * sure any previous rendering to the buffer is completed.
>>   * Note: It might be necessary to block validations before the
>>   * wait by reserving the buffer.
>> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
>> index ff7664e..d3fc5f8 100644
>> --- a/include/drm/ttm/ttm_bo_driver.h
>> +++ b/include/drm/ttm/ttm_bo_driver.h
>> @@ -37,6 +37,7 @@
>>  #include "linux/workqueue.h"
>>  #include "linux/fs.h"
>>  #include "linux/spinlock.h"
>> +#include <linux/mutex.h>
>>
>>  struct ttm_backend;
>>
>> --
>> 1.7.0
>>
>>
>> ------------------------------------------------------------------------------
>> Download Intel&#174; Parallel Studio Eval
>> Try the new software tools for yourself. Speed compiling, find bugs
>> proactively, and fine-tune applications for parallel performance.
>> See why Intel Parallel Studio got high marks during beta.
>> http://p.sf.net/sfu/intel-sw-dev
>> --
>> _______________________________________________
>> Dri-devel mailing list
>> Dri-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/dri-devel
>>
>

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--