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

[Nitin Gupta <ngupta@vflare.org>]

On 09/09/2011 07:11 PM, Greg KH wrote:

> "simplify" it in what way?
> 
> What just got changed, and why, and did it just break any documentation?
> 


Its simplification in the sense that earlier we had to be pass total RAM
size in bytes when setting zram disksize using zram_set_disksize()
which is not intuitive.  The replaced version instead just requires
the new size to be set which seems to make more sense.

> 
> On Fri, Sep 09, 2011 at 07:01:03PM -0400, Nitin Gupta wrote:
>> Also remove unnecessary messages.
>>
>> Signed-off-by: Nitin Gupta <ngupta@vflare.org>
>> Reviewed-by: Jerome Marchand <jmarchan@redhat.com>
>> ---
>>  drivers/staging/zram/zram_drv.c |   42 +++++++++++---------------------------
>>  1 files changed, 12 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
>> index b2e29dd..b1c4abd 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
>> -		);
>> -	}
> 
> Why are these print messages removed, aren't they useful to people?
> 


The first message is no longer required since we now set some default
value (25% of RAM) instead of zero. So, if user doesn't provide any
disksize value, we use the default.

The second message really doesn't belong in a kernel driver. That's more
of a documentation or user commonsense thing.  If a user really thinks
that the data to be written is going to be highly compressible, setting
zram to such large value makes some sense and there is little point in
throwing out this big warning in system logs.

Thanks,
Nitin