[PATCH] Fix reset of ramzswap

[PATCH] Fix reset of ramzswap

[minchan.kim]

ioctl(cmd=reset)
	-> bd_holder check (if whoever hold bdev, return -EBUSY)
	-> ramzswap_ioctl_reset_device
		-> reset_device
			-> bd_release

bd_release is called by reset_device.
but ramzswap_ioctl always checks bd_holder before
reset_device. it means reset ioctl always fails.

This patch fixes it.

This patch is based on mmotm-2010-01-06-14-34 + 
[PATCH] Free memory when create_device is failed.

Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
Cc: Nitin Gupta <ngupta@vflare.org>
---
 drivers/staging/ramzswap/ramzswap_drv.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ramzswap/ramzswap_drv.c b/drivers/staging/ramzswap/ramzswap_drv.c
index 18196f3..42531bd 100644
--- a/drivers/staging/ramzswap/ramzswap_drv.c
+++ b/drivers/staging/ramzswap/ramzswap_drv.c
@@ -1270,11 +1270,6 @@ static int ramzswap_ioctl(struct block_device *bdev, fmode_t mode,
 		break;
 
 	case RZSIO_RESET:
-		/* Do not reset an active device! */
-		if (bdev->bd_holders) {
-			ret = -EBUSY;
-			goto out;
-		}
 		ret = ramzswap_ioctl_reset_device(rzs);
 		break;
 
-- 
1.5.6.3


-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Fix reset of ramzswap

[Nitin Gupta]

On Tue, Jan 12, 2010 at 10:06 AM, minchan.kim <minchan.kim@gmail.com> wrote:
> ioctl(cmd=reset)
>        -> bd_holder check (if whoever hold bdev, return -EBUSY)
>        -> ramzswap_ioctl_reset_device
>                -> reset_device
>                        -> bd_release
>
> bd_release is called by reset_device.
> but ramzswap_ioctl always checks bd_holder before
> reset_device. it means reset ioctl always fails.

Are you sure you checked this patch?

This check makes sure that you cannot reset an active swap device.
When device in swapoff'ed the ioctl works as expected.

Greg: Can you please exclude earlier 'Free memory when create_device
is failed' patch?
That patch is correct however, my pending patch series conflicts with
that. So, I will
instead include that fix with this patch series (and add appropriate
signed-off-by)

Thanks,
Nitin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Fix reset of ramzswap

[Minchan Kim]

On Tue, 2010-01-12 at 11:00 +0530, Nitin Gupta wrote:
> On Tue, Jan 12, 2010 at 10:06 AM, minchan.kim <minchan.kim@gmail.com> wrote:
> > ioctl(cmd=reset)
> >        -> bd_holder check (if whoever hold bdev, return -EBUSY)
> >        -> ramzswap_ioctl_reset_device
> >                -> reset_device
> >                        -> bd_release
> >
> > bd_release is called by reset_device.
> > but ramzswap_ioctl always checks bd_holder before
> > reset_device. it means reset ioctl always fails.
> 
> Are you sure you checked this patch?

> This check makes sure that you cannot reset an active swap device.
> When device in swapoff'ed the ioctl works as expected.
> 
It seems my test was wrong. 
Maybe my test case don't swapoff swap device. 
Sorry. Ignore this patch, pz.
Thanks for the reivew, Nitin. 

I have one more patch. But I don't want to conflict your pending
patches. If it is right, pz, merge this patch with your pending series.

>From bf810ec09761b0f37eca7ba22d72fb2b1f2cba50 Mon Sep 17 00:00:00 2001
From: Minchan Kim <minchan.kim@gmail.com>
Date: Tue, 12 Jan 2010 14:46:46 +0900
Subject: [PATCH] Remove unnecessary check of ramzswap_write

Nitin already implement swap slot free callback.
So, we don't need this test any more.

Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
---
 drivers/staging/ramzswap/ramzswap_drv.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/ramzswap/ramzswap_drv.c
b/drivers/staging/ramzswap/ramzswap_drv.c
index 18196f3..575a147 100644
--- a/drivers/staging/ramzswap/ramzswap_drv.c
+++ b/drivers/staging/ramzswap/ramzswap_drv.c
@@ -784,14 +784,6 @@ static int ramzswap_write(struct ramzswap *rzs,
struct bio *bio)
 	src = rzs->compress_buffer;
 
 	/*
-	 * System swaps to same sector again when the stored page
-	 * is no longer referenced by any process. So, its now safe
-	 * to free the memory that was allocated for this page.
-	 */
-	if (rzs->table[index].page)
-		ramzswap_free_page(rzs, index);
-
-	/*
 	 * No memory ia allocated for zero filled pages.
 	 * Simply clear zero page flag.
 	 */
-- 
1.5.6.3




-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Fix reset of ramzswap

[Nitin Gupta]

On Tue, Jan 12, 2010 at 11:18 AM, Minchan Kim <minchan.kim@gmail.com> wrote:
> On Tue, 2010-01-12 at 11:00 +0530, Nitin Gupta wrote:
>> On Tue, Jan 12, 2010 at 10:06 AM, minchan.kim <minchan.kim@gmail.com> wrote:

>>
>> Are you sure you checked this patch?
>
>> This check makes sure that you cannot reset an active swap device.
>> When device in swapoff'ed the ioctl works as expected.
>>
> It seems my test was wrong.
> Maybe my test case don't swapoff swap device.
> Sorry. Ignore this patch, pz.
> Thanks for the reivew, Nitin.
>
> I have one more patch. But I don't want to conflict your pending
> patches. If it is right, pz, merge this patch with your pending series.
>

I will merge your patches with my pending series and add appropriate
signed-off-by lines.


> >From bf810ec09761b0f37eca7ba22d72fb2b1f2cba50 Mon Sep 17 00:00:00 2001
> From: Minchan Kim <minchan.kim@gmail.com>
> Date: Tue, 12 Jan 2010 14:46:46 +0900
> Subject: [PATCH] Remove unnecessary check of ramzswap_write
>
> Nitin already implement swap slot free callback.
> So, we don't need this test any more.
>
> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>


Great catch! thanks.

I think, we should avoid adding linux-mm to CC list (unless its about
xvmalloc allocator).
LKML alone should be sufficient.

Thanks,
Nitin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>