vmwgfx: add missing mutex_unlocks
diff mbox

Message ID 1344058420-18329-1-git-send-email-develkernel412222@gmail.com
State New, archived
Headers show

Commit Message

Devendra Naga Aug. 4, 2012, 5:33 a.m. UTC
we have done a proper mutex_unlock in the error cases of the vmw_fifo_reserve, but
there are missing mutex unlocks where we return a valid pointer in vmw_fifo_reserve.

Signed-off-by: Devendra Naga <develkernel412222@gmail.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Thomas Hellstrom Aug. 6, 2012, 8:07 a.m. UTC | #1
NAK,

The mutex unlock is done in vmw_fifo_commit. The mutex is protecting the 
fifo memory against multiple simultaneous reservers.

/Thomas



On 08/04/2012 07:33 AM, Devendra Naga wrote:
> we have done a proper mutex_unlock in the error cases of the vmw_fifo_reserve, but
> there are missing mutex unlocks where we return a valid pointer in vmw_fifo_reserve.
>
> Signed-off-by: Devendra Naga <develkernel412222@gmail.com>
> ---
>   drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c |    7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
> index a0c2f12..e3abd7a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
> @@ -355,6 +355,7 @@ void *vmw_fifo_reserve(struct vmw_private *dev_priv, uint32_t bytes)
>   				if (reserveable)
>   					iowrite32(bytes, fifo_mem +
>   						  SVGA_FIFO_RESERVED);
> +				mutex_unlock(&fifo_state->fifo_mutex);
>   				return fifo_mem + (next_cmd >> 2);
>   			} else {
>   				need_bounce = true;
> @@ -363,10 +364,12 @@ void *vmw_fifo_reserve(struct vmw_private *dev_priv, uint32_t bytes)
>   
>   		if (need_bounce) {
>   			fifo_state->using_bounce_buffer = true;
> -			if (bytes < fifo_state->static_buffer_size)
> +			if (bytes < fifo_state->static_buffer_size) {
> +				mutex_unlock(&fifo_state->fifo_mutex);
>   				return fifo_state->static_buffer;
> -			else {
> +			} else {
>   				fifo_state->dynamic_buffer = vmalloc(bytes);
> +				mutex_unlock(&fifo_state->fifo_mutex);
>   				return fifo_state->dynamic_buffer;
>   			}
>   		}
Devendra Naga Aug. 6, 2012, 8:12 a.m. UTC | #2
Hi Thomas,

On Mon, Aug 6, 2012 at 1:52 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
> NAK,
>
> The mutex unlock is done in vmw_fifo_commit. The mutex is protecting the
> fifo memory against multiple simultaneous reservers.
>

Thanks for the answer :-). sorry for your time... i will definitely
try more reading and analyse the code and then send patch if it needs
really.
> /Thomas
>
>
>
>
> On 08/04/2012 07:33 AM, Devendra Naga wrote:
>>
>> we have done a proper mutex_unlock in the error cases of the
>> vmw_fifo_reserve, but
>> there are missing mutex unlocks where we return a valid pointer in
>> vmw_fifo_reserve.
>>
>> Signed-off-by: Devendra Naga <develkernel412222@gmail.com>
>> ---
>>   drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c |    7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
>> b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
>> index a0c2f12..e3abd7a 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
>> @@ -355,6 +355,7 @@ void *vmw_fifo_reserve(struct vmw_private *dev_priv,
>> uint32_t bytes)
>>                                 if (reserveable)
>>                                         iowrite32(bytes, fifo_mem +
>>                                                   SVGA_FIFO_RESERVED);
>> +                               mutex_unlock(&fifo_state->fifo_mutex);
>>                                 return fifo_mem + (next_cmd >> 2);
>>                         } else {
>>                                 need_bounce = true;
>> @@ -363,10 +364,12 @@ void *vmw_fifo_reserve(struct vmw_private *dev_priv,
>> uint32_t bytes)
>>                 if (need_bounce) {
>>                         fifo_state->using_bounce_buffer = true;
>> -                       if (bytes < fifo_state->static_buffer_size)
>> +                       if (bytes < fifo_state->static_buffer_size) {
>> +                               mutex_unlock(&fifo_state->fifo_mutex);
>>                                 return fifo_state->static_buffer;
>> -                       else {
>> +                       } else {
>>                                 fifo_state->dynamic_buffer =
>> vmalloc(bytes);
>> +                               mutex_unlock(&fifo_state->fifo_mutex);
>>                                 return fifo_state->dynamic_buffer;
>>                         }
>>                 }
>
>

Patch
diff mbox

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
index a0c2f12..e3abd7a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
@@ -355,6 +355,7 @@  void *vmw_fifo_reserve(struct vmw_private *dev_priv, uint32_t bytes)
 				if (reserveable)
 					iowrite32(bytes, fifo_mem +
 						  SVGA_FIFO_RESERVED);
+				mutex_unlock(&fifo_state->fifo_mutex);
 				return fifo_mem + (next_cmd >> 2);
 			} else {
 				need_bounce = true;
@@ -363,10 +364,12 @@  void *vmw_fifo_reserve(struct vmw_private *dev_priv, uint32_t bytes)
 
 		if (need_bounce) {
 			fifo_state->using_bounce_buffer = true;
-			if (bytes < fifo_state->static_buffer_size)
+			if (bytes < fifo_state->static_buffer_size) {
+				mutex_unlock(&fifo_state->fifo_mutex);
 				return fifo_state->static_buffer;
-			else {
+			} else {
 				fifo_state->dynamic_buffer = vmalloc(bytes);
+				mutex_unlock(&fifo_state->fifo_mutex);
 				return fifo_state->dynamic_buffer;
 			}
 		}