[OPW,kernel,v4,2/2] Staging: zram: Fix decrement of variable by calling bdput()
diff mbox

Message ID 4936b93e4ea449b898a966969c6638c4f9f9f2df.1382448451.git.rashika.kheria@gmail.com
State Changes Requested
Headers show

Commit Message

Rashika Oct. 22, 2013, 1:34 p.m. UTC
As suggested by Jerome Marchand "The code in reset_store get the block device
(bdget_disk()) but it does not put it (bdput()) when it's done using it.
The usage count is therefor incremented but never decremented."

Hence, this patch introduces a call to bdput() to decrement the variable after usage.

Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>
---
 drivers/staging/zram/zram_drv.c |    1 +
 1 file changed, 1 insertion(+)

Comments

Minchan Kim Oct. 25, 2013, 10:13 a.m. UTC | #1
On Tue, Oct 22, 2013 at 07:04:37PM +0530, Rashika Kheria wrote:
> As suggested by Jerome Marchand "The code in reset_store get the block device
> (bdget_disk()) but it does not put it (bdput()) when it's done using it.
> The usage count is therefor incremented but never decremented."
> 
> Hence, this patch introduces a call to bdput() to decrement the variable after usage.
> 
> Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>
> ---
>  drivers/staging/zram/zram_drv.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
> index 8679a06..2cb33ab 100644
> --- a/drivers/staging/zram/zram_drv.c
> +++ b/drivers/staging/zram/zram_drv.c
> @@ -664,6 +664,7 @@ static ssize_t reset_store(struct device *dev,
>  
>  	/* Make sure all pending I/O is finished */
>  	fsync_bdev(bdev);
> +	bdput(bdev);

We should handle error case, too.
And please put this bug fix patch ahead of [1/2] because this patch is
more real bug fix than [1/2] which is just warning of smatch.

Thanks.
Minchan Kim Oct. 25, 2013, 10:26 a.m. UTC | #2
Hey Rashika,


And please Cc LKML in next.

Thanks!

On Fri, Oct 25, 2013 at 7:13 PM, Minchan Kim <minchan@kernel.org> wrote:
> On Tue, Oct 22, 2013 at 07:04:37PM +0530, Rashika Kheria wrote:
>> As suggested by Jerome Marchand "The code in reset_store get the block device
>> (bdget_disk()) but it does not put it (bdput()) when it's done using it.
>> The usage count is therefor incremented but never decremented."
>>
>> Hence, this patch introduces a call to bdput() to decrement the variable after usage.
>>
>> Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>
>> ---
>>  drivers/staging/zram/zram_drv.c |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
>> index 8679a06..2cb33ab 100644
>> --- a/drivers/staging/zram/zram_drv.c
>> +++ b/drivers/staging/zram/zram_drv.c
>> @@ -664,6 +664,7 @@ static ssize_t reset_store(struct device *dev,
>>
>>       /* Make sure all pending I/O is finished */
>>       fsync_bdev(bdev);
>> +     bdput(bdev);
>
> We should handle error case, too.
> And please put this bug fix patch ahead of [1/2] because this patch is
> more real bug fix than [1/2] which is just warning of smatch.
>
> Thanks.
>
> --
> Kind regards,
> Minchan Kim
Rashika Oct. 25, 2013, 11:09 a.m. UTC | #3
On Fri, Oct 25, 2013 at 3:56 PM, Minchan Kim <minchan@kernel.org> wrote:

> Hey Rashika,
>
>
> And please Cc LKML in next.
>
> Thanks!
>
>
> 
Thanks Minchan for the review.

Kindly guide me on how to handle the error cases. Moreover, does LKML
stands for linux kernel mailing list?

Thanks,
Minchan Kim Oct. 25, 2013, 11:19 a.m. UTC | #4
2013. 10. 25. ?? 12:09? "Rashika Kheria" <rashika.kheria@gmail.com>?? ??:
>
>
>
>
> On Fri, Oct 25, 2013 at 3:56 PM, Minchan Kim <minchan@kernel.org> wrote:
>>
>> Hey Rashika,
>>
>>
>> And please Cc LKML in next.
>>
>> Thanks!
>>
>>
>
> Thanks Minchan for the review.
>
> Kindly guide me on how to handle the error cases. Moreover, does LKML
stands for linux kernel mailing list?

Yes. :)

>
> Thanks,
>
> --
> Rashika Kheria
> B.Tech CSE
> IIIT Hyderabad

Patch
diff mbox

diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
index 8679a06..2cb33ab 100644
--- a/drivers/staging/zram/zram_drv.c
+++ b/drivers/staging/zram/zram_drv.c
@@ -664,6 +664,7 @@  static ssize_t reset_store(struct device *dev,
 
 	/* Make sure all pending I/O is finished */
 	fsync_bdev(bdev);
+	bdput(bdev);
 
 	zram_reset_device(zram, true);
 	return len;