Re: [PATCH 3/4] Simplify zram disk resizing interface

[Nitin Gupta]

On 08/30/2011 06:11 AM, Jerome Marchand wrote:
> On 08/24/2011 03:34 AM, Nitin Gupta wrote:
> > Also remove unnecessary messages.
> >
> > Signed-off-by: Nitin Gupta<ngupta@xxxxxxxxxx>
> > ---
> >   drivers/staging/zram/zram_drv.c |   42 +++++++++++---------------------------
> >   drivers/staging/zram/zram_drv.h |    2 +-
> >   2 files changed, 13 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
> > index 81d6c43..d7fb207 100644
> > --- a/drivers/staging/zram/zram_drv.c
> > +++ b/drivers/staging/zram/zram_drv.c
> > @@ -104,33 +104,16 @@ static int page_zero_filled(void *ptr)
> >   	return 1;
> >   }
> >
> > -static void zram_set_disksize(struct zram *zram, size_t totalram_bytes)
> > +static u64 zram_default_disksize_bytes(void)
> >   {
> > -	if (!zram->disksize) {
> > -		pr_info(
> > -		"disk size not provided. You can use disksize_kb module "
> > -		"param to specify size.\nUsing default: (%u%% of RAM).\n",
> > -		default_disksize_perc_ram
> > -		);
> > -		zram->disksize = default_disksize_perc_ram *
> > -					(totalram_bytes / 100);
> > -	}
> > -
> > -	if (zram->disksize>  2 * (totalram_bytes)) {
> > -		pr_info(
> > -		"There is little point creating a zram of greater than "
> > -		"twice the size of memory since we expect a 2:1 compression "
> > -		"ratio. Note that zram uses about 0.1%% of the size of "
> > -		"the disk when not in use so a huge zram is "
> > -		"wasteful.\n"
> > -		"\tMemory Size: %zu kB\n"
> > -		"\tSize you selected: %llu kB\n"
> > -		"Continuing anyway ...\n",
> > -		totalram_bytes>>  10, zram->disksize
> > -		);
> > -	}
> > -
> > -	zram->disksize&= PAGE_MASK;
> > +	return ((totalram_pages<<  PAGE_SHIFT) *
> > +		default_disksize_perc_ram / 100)&  PAGE_MASK;
> > +}
> > +
> > +static void zram_set_disksize(struct zram *zram, u64 size_bytes)
> > +{
> > +	zram->disksize = size_bytes;
> > +	set_capacity(zram->disk, size_bytes>>  SECTOR_SHIFT);
> >   }
> >
> >   static void zram_free_page(struct zram *zram, size_t index)
> > @@ -632,7 +615,8 @@ int zram_init_device(struct zram *zram)
> >   		return 0;
> >   	}
> >
> > -	zram_set_disksize(zram, totalram_pages<<  PAGE_SHIFT);
> > +	if (!zram->disksize)
> > +		zram_set_disksize(zram, zram_default_disksize_bytes());
>
> With your next patch, this will not happen anymore, unless someone explicitly sets
> the disk size to zero. If zero means default, it should be documented. It looks weird
> anyway: if something like that should be done, it probably should be done in
> disksize_store() for clarity.
> Otherwise, your next patch should remove this chunk of code.

Thanks for the review.  I have now removed that check in v2 patches 
since we now set some default value during initialization.

Thanks,
Nitin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/