Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1564074 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] mm: extend zero pages to same element pages for zram Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-01-21 09:50 +0100
    Re: [PATCH] mm: extend zero pages to same element pages for zram zhouxianrong <zhouxianrong@huawei.com> - 2017-01-22 04:10 +0100
      Re: [PATCH] mm: extend zero pages to same element pages for zram Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-01-22 06:00 +0100
      Re: [PATCH] mm: extend zero pages to same element pages for zram Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2017-01-23 04:00 +0100
        Re: [PATCH] mm: extend zero pages to same element pages for zram zhouxianrong <zhouxianrong@huawei.com> - 2017-01-23 04:40 +0100
        Re: [PATCH] mm: extend zero pages to same element pages for zram Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-01-23 05:10 +0100
          Re: [PATCH] mm: extend zero pages to same element pages for zram Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2017-01-23 07:30 +0100
            Re: [PATCH] mm: extend zero pages to same element pages for zram Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-01-23 08:20 +0100
              Re: [PATCH] mm: extend zero pages to same element pages for zram Minchan Kim <minchan@kernel.org> - 2017-01-23 08:50 +0100
                Re: [PATCH] mm: extend zero pages to same element pages for zram zhouxianrong <zhouxianrong@huawei.com> - 2017-01-24 09:30 +0100
                  Re: [PATCH] mm: extend zero pages to same element pages for zram Minchan Kim <minchan@kernel.org> - 2017-01-25 02:30 +0100
                    Re: [PATCH] mm: extend zero pages to same element pages for zram Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-01-25 02:40 +0100
                      Re: [PATCH] mm: extend zero pages to same element pages for zram Matthew Wilcox <willy@infradead.org> - 2017-01-25 03:50 +0100
                        Re: [PATCH] mm: extend zero pages to same element pages for zram Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-01-25 05:20 +0100
                          Re: [PATCH] mm: extend zero pages to same element pages for zram Minchan Kim <minchan@kernel.org> - 2017-01-25 06:00 +0100
                            Re: [PATCH] mm: extend zero pages to same element pages for zram Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-01-25 06:40 +0100
                              Re: [PATCH] mm: extend zero pages to same element pages for zram Minchan Kim <minchan@kernel.org> - 2017-01-25 06:50 +0100
      Re: [PATCH] mm: extend zero pages to same element pages for zram Matthew Wilcox <willy@infradead.org> - 2017-01-23 07:30 +0100
        答复: [PATCH] mm: extend zero pages to same element pages for zram zhouxianrong <zhouxianrong@huawei.com> - 2017-01-23 07:50 +0100

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

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

[toc] | [next] | [standalone]

#1564286

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
>
> .
>

[toc] | [prev] | [next] | [standalone]

#1564303

On (01/22/17 10:58), zhouxianrong wrote:
> 1. memset is just set a int value but i want to set a long value.

ah... ok. because you union it with the handle.

> 2. using clear_page rather than memset MAYBE due to in arm64 arch
>    it is a 64-bytes operations.

clear_page() basically does memset(), which is quite well optimized.
except for arm64, yes.


> 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.

thaks for the reply.

	-ss

[toc] | [prev] | [next] | [standalone]

#1564638

Hello,

On Sun, Jan 22, 2017 at 10:58:38AM +0800, zhouxianrong wrote:
> 1. memset is just set a int value but i want to set a long value.

Sorry for late review.

Do we really need to set a long value? I cannot believe that
long value is repeated in the page. Value repeatition is
usually done by value 0 or 1 and it's enough to use int. And, I heard
that value 0 or 1 is repeated in Android. Could you check the distribution
of the value in the same page?

Thanks.

[toc] | [prev] | [next] | [standalone]

#1564646

hey Joonsoo:
	i would test and give the same element type later.

On 2017/1/23 10:58, Joonsoo Kim wrote:
> Hello,
>
> On Sun, Jan 22, 2017 at 10:58:38AM +0800, zhouxianrong wrote:
>> 1. memset is just set a int value but i want to set a long value.
>
> Sorry for late review.
>
> Do we really need to set a long value? I cannot believe that
> long value is repeated in the page. Value repeatition is
> usually done by value 0 or 1 and it's enough to use int. And, I heard
> that value 0 or 1 is repeated in Android. Could you check the distribution
> of the value in the same page?
>
> Thanks.
>
>
> .
>

[toc] | [prev] | [next] | [standalone]

#1564656

On (01/23/17 11:58), Joonsoo Kim wrote:
> Hello,
> 
> On Sun, Jan 22, 2017 at 10:58:38AM +0800, zhouxianrong wrote:
> > 1. memset is just set a int value but i want to set a long value.
> 
> Sorry for late review.
> 
> Do we really need to set a long value? I cannot believe that
> long value is repeated in the page. Value repeatition is
> usually done by value 0 or 1 and it's enough to use int. And, I heard
> that value 0 or 1 is repeated in Android. Could you check the distribution
> of the value in the same page?

Hello Joonsoo,

thanks for taking a look and for bringing this question up.
so I kinda wanted to propose union of `ulong handle' with `uint element'
and switching to memset(), but I couldn't figure out if that change would
break detection of some patterns.

 /* Allocated for each disk page */
 struct zram_table_entry {
-       unsigned long handle;
+       union {
+               unsigned long handle;
+               unsigned int element;
+       };
        unsigned long value;
 };

	-ss

[toc] | [prev] | [next] | [standalone]

#1564707

On Mon, Jan 23, 2017 at 01:03:47PM +0900, Sergey Senozhatsky wrote:
> On (01/23/17 11:58), Joonsoo Kim wrote:
> > Hello,
> > 
> > On Sun, Jan 22, 2017 at 10:58:38AM +0800, zhouxianrong wrote:
> > > 1. memset is just set a int value but i want to set a long value.
> > 
> > Sorry for late review.
> > 
> > Do we really need to set a long value? I cannot believe that
> > long value is repeated in the page. Value repeatition is
> > usually done by value 0 or 1 and it's enough to use int. And, I heard
> > that value 0 or 1 is repeated in Android. Could you check the distribution
> > of the value in the same page?
> 
> Hello Joonsoo,
> 
> thanks for taking a look and for bringing this question up.
> so I kinda wanted to propose union of `ulong handle' with `uint element'
> and switching to memset(), but I couldn't figure out if that change would
> break detection of some patterns.
> 
>  /* Allocated for each disk page */
>  struct zram_table_entry {
> -       unsigned long handle;
> +       union {
> +               unsigned long handle;
> +               unsigned int element;
> +       };
>         unsigned long value;
>  };

Hello,

Think about following case in 64 bits kernel.

If value pattern in the page is like as following, we cannot detect
the same page with 'unsigned int' element.

AAAAAAAABBBBBBBBAAAAAAAABBBBBBBB...

4 bytes is 0xAAAAAAAA and next 4 bytes is 0xBBBBBBBB and so on.

However, as I said before, I think that it is uncommon case.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1564717

On (01/23/17 15:27), Joonsoo Kim wrote:
> Hello,
> 
> Think about following case in 64 bits kernel.
> 
> If value pattern in the page is like as following, we cannot detect
> the same page with 'unsigned int' element.
> 
> AAAAAAAABBBBBBBBAAAAAAAABBBBBBBB...
> 
> 4 bytes is 0xAAAAAAAA and next 4 bytes is 0xBBBBBBBB and so on.

yep, that's exactly the case that I though would be broken
with a 4-bytes pattern matching. so my conlusion was that
for 4 byte pattern we would have working detection anyway,
for 8 bytes patterns we might have some extra matching.
not sure if it matters that much though.

	-ss

[toc] | [prev] | [next] | [standalone]

#1564739

On Mon, Jan 23, 2017 at 04:13:39PM +0900, Sergey Senozhatsky wrote:
> On (01/23/17 15:27), Joonsoo Kim wrote:
> > Hello,
> > 
> > Think about following case in 64 bits kernel.
> > 
> > If value pattern in the page is like as following, we cannot detect
> > the same page with 'unsigned int' element.
> > 
> > AAAAAAAABBBBBBBBAAAAAAAABBBBBBBB...
> > 
> > 4 bytes is 0xAAAAAAAA and next 4 bytes is 0xBBBBBBBB and so on.
> 
> yep, that's exactly the case that I though would be broken
> with a 4-bytes pattern matching. so my conlusion was that
> for 4 byte pattern we would have working detection anyway,
> for 8 bytes patterns we might have some extra matching.
> not sure if it matters that much though.

It would be better for deduplication as pattern coverage is bigger
and we cannot guess all of patterns now so it would be never ending
story(i.e., someone claims 16bytes pattern matching would be better).
So, I want to make that path fast rather than increasing dedup ratio
if memset is really fast rather than open-looping. So in future,
if we can prove bigger pattern can increase dedup ratio a lot, then,
we could consider to extend it at the cost of make that path slow.

In summary, zhouxianrong, please test pattern as Joonsoo asked.
So if there are not much benefit with 'long', let's go to the
'int' with memset. And Please resend patch if anyone dosn't oppose
strongly by the time.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1565673

@@ -161,15 +161,55 @@ static bool page_zero_filled(void *ptr)
  {
  	unsigned int pos;
  	unsigned long *page;
+	static unsigned long total;
+	static unsigned long zero;
+	static unsigned long pattern_char;
+	static unsigned long pattern_short;
+	static unsigned long pattern_int;
+	static unsigned long pattern_long;
+	unsigned char *p_char;
+	unsigned short *p_short;
+	unsigned int *p_int;
+	bool retval = false;
+
+	++total;

  	page = (unsigned long *)ptr;

-	for (pos = 0; pos != PAGE_SIZE / sizeof(*page); pos++) {
-		if (page[pos])
-			return false;
+	for (pos = 0; pos < PAGE_SIZE / sizeof(unsigned long) - 1; ++pos) {
+	       if (page[pos] != page[pos + 1])
+	                return false;
  	}

-	return true;
+	p_char = (unsigned char *)ptr;
+	p_short = (unsigned short *)ptr;
+	p_int = (unsigned int *)ptr;
+
+	if (page[0] == 0) {
+		++zero;
+		retval = true;
+	} else if (p_char[0] == p_char[1] &&
+		       p_char[1] == p_char[2] &&
+		       p_char[2] == p_char[3] &&
+		       p_char[3] == p_char[4] &&
+		       p_char[4] == p_char[5] &&
+		       p_char[5] == p_char[6] &&
+		       p_char[6] == p_char[7])
+		++pattern_char;
+	else if (p_short[0] == p_short[1] &&
+		       p_short[1] == p_short[2] &&
+		       p_short[2] == p_short[3])
+		++pattern_short;
+	else if (p_int[0] == p_int[1] &&
+		       p_int[1] == p_int[2])
+		++pattern_int;
+	else {
+		++pattern_long;
+	}
+
+	pr_err("%lld %lld %lld %lld %lld %lld\n", zero, pattern_char, pattern_short, pattern_int, pattern_long, total);
+
+	return retval;
  }

the result as listed below:

zero    pattern_char   pattern_short   pattern_int   pattern_long   total      (unit)
162989  14454          3534            23516         2769           3294399    (page)

statistics for the result:

          pattern zero  pattern char  pattern short  pattern int  pattern long
AVERAGE  0.745696298   0.085937175   0.015957701    0.131874915  0.020533911
STDEV    0.035623777   0.016892402   0.004454534    0.021657123  0.019420072
MAX      0.973813421   0.222222222   0.021409518    0.211812245  0.176512625
MIN      0.645431905   0.004634398   0              0            0


On 2017/1/23 15:40, Minchan Kim wrote:
> On Mon, Jan 23, 2017 at 04:13:39PM +0900, Sergey Senozhatsky wrote:
>> On (01/23/17 15:27), Joonsoo Kim wrote:
>>> Hello,
>>>
>>> Think about following case in 64 bits kernel.
>>>
>>> If value pattern in the page is like as following, we cannot detect
>>> the same page with 'unsigned int' element.
>>>
>>> AAAAAAAABBBBBBBBAAAAAAAABBBBBBBB...
>>>
>>> 4 bytes is 0xAAAAAAAA and next 4 bytes is 0xBBBBBBBB and so on.
>>
>> yep, that's exactly the case that I though would be broken
>> with a 4-bytes pattern matching. so my conlusion was that
>> for 4 byte pattern we would have working detection anyway,
>> for 8 bytes patterns we might have some extra matching.
>> not sure if it matters that much though.
>
> It would be better for deduplication as pattern coverage is bigger
> and we cannot guess all of patterns now so it would be never ending
> story(i.e., someone claims 16bytes pattern matching would be better).
> So, I want to make that path fast rather than increasing dedup ratio
> if memset is really fast rather than open-looping. So in future,
> if we can prove bigger pattern can increase dedup ratio a lot, then,
> we could consider to extend it at the cost of make that path slow.
>
> In summary, zhouxianrong, please test pattern as Joonsoo asked.
> So if there are not much benefit with 'long', let's go to the
> 'int' with memset. And Please resend patch if anyone dosn't oppose
> strongly by the time.
>
> Thanks.
>
>
> .
>

[toc] | [prev] | [next] | [standalone]

#1566265

Hi zhouxianrong,

On Tue, Jan 24, 2017 at 03:58:02PM +0800, zhouxianrong wrote:
> @@ -161,15 +161,55 @@ static bool page_zero_filled(void *ptr)
>  {
>  	unsigned int pos;
>  	unsigned long *page;
> +	static unsigned long total;
> +	static unsigned long zero;
> +	static unsigned long pattern_char;
> +	static unsigned long pattern_short;
> +	static unsigned long pattern_int;
> +	static unsigned long pattern_long;
> +	unsigned char *p_char;
> +	unsigned short *p_short;
> +	unsigned int *p_int;
> +	bool retval = false;
> +
> +	++total;
> 
>  	page = (unsigned long *)ptr;
> 
> -	for (pos = 0; pos != PAGE_SIZE / sizeof(*page); pos++) {
> -		if (page[pos])
> -			return false;
> +	for (pos = 0; pos < PAGE_SIZE / sizeof(unsigned long) - 1; ++pos) {
> +	       if (page[pos] != page[pos + 1])
> +	                return false;
>  	}
> 
> -	return true;
> +	p_char = (unsigned char *)ptr;
> +	p_short = (unsigned short *)ptr;
> +	p_int = (unsigned int *)ptr;
> +
> +	if (page[0] == 0) {
> +		++zero;
> +		retval = true;
> +	} else if (p_char[0] == p_char[1] &&
> +		       p_char[1] == p_char[2] &&
> +		       p_char[2] == p_char[3] &&
> +		       p_char[3] == p_char[4] &&
> +		       p_char[4] == p_char[5] &&
> +		       p_char[5] == p_char[6] &&
> +		       p_char[6] == p_char[7])
> +		++pattern_char;
> +	else if (p_short[0] == p_short[1] &&
> +		       p_short[1] == p_short[2] &&
> +		       p_short[2] == p_short[3])
> +		++pattern_short;
> +	else if (p_int[0] == p_int[1] &&
> +		       p_int[1] == p_int[2])
> +		++pattern_int;
> +	else {
> +		++pattern_long;
> +	}
> +
> +	pr_err("%lld %lld %lld %lld %lld %lld\n", zero, pattern_char, pattern_short, pattern_int, pattern_long, total);
> +
> +	return retval;
>  }
> 
> the result as listed below:
> 
> zero    pattern_char   pattern_short   pattern_int   pattern_long   total      (unit)
> 162989  14454          3534            23516         2769           3294399    (page)
> 

so, int covers 93%. As considering non-zero dedup hit ratio is low, I think *int* is
enough if memset is really fast. So, I'd like to go with 'int' if Sergey doesn't mind.

Please include the number in description and resend patch, zhouxianrong. :)

Thanks.

> statistics for the result:
> 
>          pattern zero  pattern char  pattern short  pattern int  pattern long
> AVERAGE  0.745696298   0.085937175   0.015957701    0.131874915  0.020533911
> STDEV    0.035623777   0.016892402   0.004454534    0.021657123  0.019420072
> MAX      0.973813421   0.222222222   0.021409518    0.211812245  0.176512625
> MIN      0.645431905   0.004634398   0              0            0
> 
> 
> On 2017/1/23 15:40, Minchan Kim wrote:
> >On Mon, Jan 23, 2017 at 04:13:39PM +0900, Sergey Senozhatsky wrote:
> >>On (01/23/17 15:27), Joonsoo Kim wrote:
> >>>Hello,
> >>>
> >>>Think about following case in 64 bits kernel.
> >>>
> >>>If value pattern in the page is like as following, we cannot detect
> >>>the same page with 'unsigned int' element.
> >>>
> >>>AAAAAAAABBBBBBBBAAAAAAAABBBBBBBB...
> >>>
> >>>4 bytes is 0xAAAAAAAA and next 4 bytes is 0xBBBBBBBB and so on.
> >>
> >>yep, that's exactly the case that I though would be broken
> >>with a 4-bytes pattern matching. so my conlusion was that
> >>for 4 byte pattern we would have working detection anyway,
> >>for 8 bytes patterns we might have some extra matching.
> >>not sure if it matters that much though.
> >
> >It would be better for deduplication as pattern coverage is bigger
> >and we cannot guess all of patterns now so it would be never ending
> >story(i.e., someone claims 16bytes pattern matching would be better).
> >So, I want to make that path fast rather than increasing dedup ratio
> >if memset is really fast rather than open-looping. So in future,
> >if we can prove bigger pattern can increase dedup ratio a lot, then,
> >we could consider to extend it at the cost of make that path slow.
> >
> >In summary, zhouxianrong, please test pattern as Joonsoo asked.
> >So if there are not much benefit with 'long', let's go to the
> >'int' with memset. And Please resend patch if anyone dosn't oppose
> >strongly by the time.
> >
> >Thanks.
> >
> >
> >.
> >
> 
> --
> 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>

[toc] | [prev] | [next] | [standalone]

#1566267

Hello,

On (01/25/17 10:29), Minchan Kim wrote:
[..]
> > the result as listed below:
> > 
> > zero    pattern_char   pattern_short   pattern_int   pattern_long   total      (unit)
> > 162989  14454          3534            23516         2769           3294399    (page)
> > 
>
> so, int covers 93%. As considering non-zero dedup hit ratio is low, I think *int* is
> enough if memset is really fast. So, I'd like to go with 'int' if Sergey doesn't mind.

yep, 4 byte pattern matching and memset() sounds like a good plan to me

	-ss

[toc] | [prev] | [next] | [standalone]

#1566296

On Wed, Jan 25, 2017 at 10:32:44AM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> On (01/25/17 10:29), Minchan Kim wrote:
> [..]
> > > the result as listed below:
> > > 
> > > zero    pattern_char   pattern_short   pattern_int   pattern_long   total      (unit)
> > > 162989  14454          3534            23516         2769           3294399    (page)
> > > 
> >
> > so, int covers 93%. As considering non-zero dedup hit ratio is low, I think *int* is
> > enough if memset is really fast. So, I'd like to go with 'int' if Sergey doesn't mind.
> 
> yep, 4 byte pattern matching and memset() sounds like a good plan to me

what?  memset ONLY HANDLES BYTES.

I pointed this out earlier, but you don't seem to be listening.  Let me
try it again.

MEMSET ONLY HANDLES BYTES.

[toc] | [prev] | [next] | [standalone]

#1566315

On (01/24/17 18:48), Matthew Wilcox wrote:
> On Wed, Jan 25, 2017 at 10:32:44AM +0900, Sergey Senozhatsky wrote:
> > Hello,
> > 
> > On (01/25/17 10:29), Minchan Kim wrote:
> > [..]
> > > > the result as listed below:
> > > > 
> > > > zero    pattern_char   pattern_short   pattern_int   pattern_long   total      (unit)
> > > > 162989  14454          3534            23516         2769           3294399    (page)
> > > > 
> > >
> > > so, int covers 93%. As considering non-zero dedup hit ratio is low, I think *int* is
> > > enough if memset is really fast. So, I'd like to go with 'int' if Sergey doesn't mind.
> > 
> > yep, 4 byte pattern matching and memset() sounds like a good plan to me
> 
> what?  memset ONLY HANDLES BYTES.
> 
> I pointed this out earlier, but you don't seem to be listening.  Let me
> try it again.
> 
> MEMSET ONLY HANDLES BYTES.

dammit... how did that happen...


Matthew, you are absolute right. and, yes, I missed out your previous
mail, indeed. sorry. and thanks for "re-pointing" that out.


Minchan, zhouxianrong, I was completely wrong. we can't
do memset(). d'oh, I did not know it truncates 4 bytes to
one byte only (doesn't make too much sense to me).

my apologies.

	-ss

[toc] | [prev] | [next] | [standalone]

#1566322

On Wed, Jan 25, 2017 at 01:18:58PM +0900, Sergey Senozhatsky wrote:
> On (01/24/17 18:48), Matthew Wilcox wrote:
> > On Wed, Jan 25, 2017 at 10:32:44AM +0900, Sergey Senozhatsky wrote:
> > > Hello,
> > > 
> > > On (01/25/17 10:29), Minchan Kim wrote:
> > > [..]
> > > > > the result as listed below:
> > > > > 
> > > > > zero    pattern_char   pattern_short   pattern_int   pattern_long   total      (unit)
> > > > > 162989  14454          3534            23516         2769           3294399    (page)
> > > > > 
> > > >
> > > > so, int covers 93%. As considering non-zero dedup hit ratio is low, I think *int* is
> > > > enough if memset is really fast. So, I'd like to go with 'int' if Sergey doesn't mind.
> > > 
> > > yep, 4 byte pattern matching and memset() sounds like a good plan to me
> > 
> > what?  memset ONLY HANDLES BYTES.
> > 
> > I pointed this out earlier, but you don't seem to be listening.  Let me
> > try it again.
> > 
> > MEMSET ONLY HANDLES BYTES.
> 
> dammit... how did that happen...
> 
> 
> Matthew, you are absolute right. and, yes, I missed out your previous
> mail, indeed. sorry. and thanks for "re-pointing" that out.
> 
> 
> Minchan, zhouxianrong, I was completely wrong. we can't
> do memset(). d'oh, I did not know it truncates 4 bytes to
> one byte only (doesn't make too much sense to me).

Now, I read Matthew's comment and understood. Thanks.
It means zhouxianrong's patch I sent recently is okay?

Thanks.

[toc] | [prev] | [next] | [standalone]

#1566329

On (01/25/17 13:51), Minchan Kim wrote:
[..]
> > Minchan, zhouxianrong, I was completely wrong. we can't
> > do memset(). d'oh, I did not know it truncates 4 bytes to
> > one byte only (doesn't make too much sense to me).
> 
> Now, I read Matthew's comment and understood. Thanks.
> It means zhouxianrong's patch I sent recently is okay?

this one looks OK to me
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1316290.html


I'd agree with Joonsoo that doing forward prefetching is _probably_ better
than backwards prefetching. not that it necessarily should confuse the CPU
(need to google if ARM handles it normally), but still.

	-ss

[toc] | [prev] | [next] | [standalone]

#1566333

On Wed, Jan 25, 2017 at 02:38:49PM +0900, Sergey Senozhatsky wrote:
> On (01/25/17 13:51), Minchan Kim wrote:
> [..]
> > > Minchan, zhouxianrong, I was completely wrong. we can't
> > > do memset(). d'oh, I did not know it truncates 4 bytes to
> > > one byte only (doesn't make too much sense to me).
> > 
> > Now, I read Matthew's comment and understood. Thanks.
> > It means zhouxianrong's patch I sent recently is okay?
> 
> this one looks OK to me
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1316290.html
> 
> 
> I'd agree with Joonsoo that doing forward prefetching is _probably_ better
> than backwards prefetching. not that it necessarily should confuse the CPU
> (need to google if ARM handles it normally), but still.

Okay, let's settle down.

zhouxianrong, please resend one Sergey pointed out with changing to
forward loop. though, sorry for a lot confusion!

> 
> 	-ss
> 
> --
> 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>

[toc] | [prev] | [next] | [standalone]

#1564705

On Sun, Jan 22, 2017 at 10:58:38AM +0800, zhouxianrong wrote:
> 1. memset is just set a int value but i want to set a long value.

memset doesn't set an int value.

DESCRIPTION
       The  memset()  function  fills  the  first  n  bytes of the memory area
       pointed to by s with the constant byte c.

It sets a byte value.  K&R just happened to choose 'int' as the type
to store that "unsigned char" in.  Probably for very good reasons which
make absolutely no sense today.

[toc] | [prev] | [next] | [standalone]

#1564711 — 答复: [PATCH] mm: extend zero pages to same element pages for zram

Yes, memset's prototype is int but
the implement of arch is unsigned char; for example, in arm64

	.weak memset
ENTRY(__memset)
ENTRY(memset)
	mov	dst, dstin	/* Preserve return value.  */
	and	A_lw, val, #255
	orr	A_lw, A_lw, A_lw, lsl #8
	orr	A_lw, A_lw, A_lw, lsl #16
	orr	A_l, A_l, A_l, lsl #32

-----邮件原件-----
发件人: Matthew Wilcox [mailto:willy@infradead.org] 
发送时间: 2017年1月23日 14:26
收件人: zhouxianrong
抄送: Sergey Senozhatsky; linux-mm@kvack.org; linux-kernel@vger.kernel.org; akpm@linux-foundation.org; sergey.senozhatsky@gmail.com; minchan@kernel.org; ngupta@vflare.org; Mi Sophia Wang; Zhouxiyu; Duwei (Device OS); Zhangshiming (Simon, Device OS); Won Ho Park
主题: Re: [PATCH] mm: extend zero pages to same element pages for zram

On Sun, Jan 22, 2017 at 10:58:38AM +0800, zhouxianrong wrote:
> 1. memset is just set a int value but i want to set a long value.

memset doesn't set an int value.

DESCRIPTION
       The  memset()  function  fills  the  first  n  bytes of the memory area
       pointed to by s with the constant byte c.

It sets a byte value.  K&R just happened to choose 'int' as the type to store that "unsigned char" in.  Probably for very good reasons which make absolutely no sense today.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web