Re: [PATCH] mm: extend zero pages to same element pages for zram

[zhouxianrong <zhouxianrong@huawei.com>]

1. memset is just set a int value but i want to set a long value.
2. using clear_page rather than memset MAYBE due to in arm64 arch
    it is a 64-bytes operations.

6.6.4. Data Cache Zero

The ARMv8-A architecture introduces a Data Cache Zero by Virtual Address (DC ZVA) instruction. This enables a block of 64
bytes in memory, aligned to 64 bytes in size, to be set to zero. If the DC ZVA instruction misses in the cache, it clears main
memory, without causing an L1 or L2 cache allocation.

but i only consider the arm64 arch, other archs need to be reviewed.

On 2017/1/21 16:43, Sergey Senozhatsky wrote:
> Hello,
>
> On (01/13/17 16:29), zhouxianrong@huawei.com wrote:
> [..]
>> --- a/Documentation/ABI/testing/sysfs-block-zram
>> +++ b/Documentation/ABI/testing/sysfs-block-zram
>> @@ -86,21 +86,21 @@ Description:
>>  		ones are sent by filesystem mounted with discard option,
>>  		whenever some data blocks are getting discarded.
>>
>> -What:		/sys/block/zram<id>/zero_pages
>> +What:		/sys/block/zram<id>/same_pages
> [..]
>> -zero_pages        RO    the number of zero filled pages written to this disk
>> +same_pages        RO    the number of same element filled pages written to this disk
> [..]
>> -	zero_pages
>> +	same_pages
>>  	num_migrated
>> +}
>
> we removed deprecated sysfs attrs. zero_pages does not exist anymore.
>
>>  static size_t zram_get_obj_size(struct zram_meta *meta, u32 index)
>>  {
>>  	return meta->table[index].value & (BIT(ZRAM_FLAG_SHIFT) - 1);
>> @@ -158,31 +169,76 @@ static inline void update_used_max(struct zram *zram,
>>  	} while (old_max != cur_max);
>>  }
>>
>> -static bool page_zero_filled(void *ptr)
>> +static inline void zram_fill_page(char *ptr, unsigned long value)
>> +{
>> +	int i;
>> +	unsigned long *page = (unsigned long *)ptr;
>> +
>> +	if (likely(value == 0)) {
>> +		clear_page(ptr);
>> +	} else {
>> +		for (i = PAGE_SIZE / sizeof(unsigned long) - 1; i >= 0; i--)
>> +			page[i] = value;
>> +	}
>
> any particular reason not to use memset() here?
> memset() can be faster that that, right?
>
>
> [..]
>>  /* Flags for zram pages (table[page_no].value) */
>>  enum zram_pageflags {
>> -	/* Page consists entirely of zeros */
>> -	ZRAM_ZERO = ZRAM_FLAG_SHIFT,
>> +	/* Page consists entirely of same elements */
>> +	ZRAM_SAME = ZRAM_FLAG_SHIFT,
>>  	ZRAM_ACCESS,	/* page is now accessed */
> [..]
>> @@ -83,7 +86,7 @@ struct zram_stats {
>>  	atomic64_t failed_writes;	/* can happen when memory is too low */
>>  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
>>  	atomic64_t notify_free;	/* no. of swap slot free notifications */
>> -	atomic64_t zero_pages;		/* no. of zero filled pages */
>> +	atomic64_t same_pages;		/* no. of same element filled pages */
>
> not like this rename is particularity important, but ok. works for me.
>
> 	-ss
>
> .
>