zram: extend zero pages to same element pages

drivers/block/zram/zram_drv.c +5 -0

+static inline void zram_fill_page(char *ptr, unsigned long len,
+					unsigned long value)
+{
+	int i;
+	unsigned long *page = (unsigned long *)ptr;

[adding linux-arch to see if anyone there wants to do an optimised version of memfill for their CPU]

Quoted 5 lines · Minchan Kim

drivers/block/zram/zram_drv.c +5 -0

+static inline void zram_fill_page(char *ptr, unsigned long len,
+					unsigned long value)
+{
+	int i;
+	unsigned long *page = (unsigned long *)ptr;

drivers/block/zram/zram_drv.c static inline void update_used_max(struct zram *zram, · line 174

Quoted 10 lines

drivers/block/zram/zram_drv.c +10 -0

+
+	WARN_ON_ONCE(!IS_ALIGNED(len, sizeof(unsigned long)));
+
+	if (likely(value == 0)) {
+		memset(ptr, 0, len);
+	} else {
+		for (i = 0; i < len / sizeof(*page); i++)
+			page[i] = value;
+	}
+}

I would suggest:

#ifndef __HAVE_ARCH_MEMFILL
/**
* memfill - Fill a region of memory with the given value
* @s: Pointer to the start of the region.
* @v: The word to fill the region with.
* @n: The size of the region.
*
* Differs from memset() in that it fills with an unsigned long instead of
* a byte. The pointer and the size must be aligned to unsigned long. */
void memfill(unsigned long *s, unsigned long v, size_t n)

{
	WARN_ON_ONCE(!IS_ALIGNED(n, sizeof(v)));

	if (likely(v == 0)) {
		memset(s, 0, n);
	} else {
		while (n) {
			*s++ = v;
			n -= sizeof(v);
		}
	}
}

EXPORT_SYMBOL(memfill);
#endif

(I would also suggest this move to lib/string.c and architectures be given the opportunity to provide an optimised version of memfill).

.

[adding linux-arch to see if anyone there wants to do an optimised version of memfill for their CPU]

Quoted 5 lines · Minchan Kim

drivers/block/zram/zram_drv.c +5 -0

+static inline void zram_fill_page(char *ptr, unsigned long len,
+					unsigned long value)
+{
+	int i;
+	unsigned long *page = (unsigned long *)ptr;

drivers/block/zram/zram_drv.c static inline void update_used_max(struct zram *zram, · line 174

Quoted 10 lines

drivers/block/zram/zram_drv.c +10 -0

+
+	WARN_ON_ONCE(!IS_ALIGNED(len, sizeof(unsigned long)));
+
+	if (likely(value == 0)) {
+		memset(ptr, 0, len);
+	} else {
+		for (i = 0; i < len / sizeof(*page); i++)
+			page[i] = value;
+	}
+}

I would suggest:

#ifndef __HAVE_ARCH_MEMFILL
/**
* memfill - Fill a region of memory with the given value
* @s: Pointer to the start of the region.
* @v: The word to fill the region with.
* @n: The size of the region.
*
* Differs from memset() in that it fills with an unsigned long instead of
* a byte. The pointer and the size must be aligned to unsigned long. */
void memfill(unsigned long *s, unsigned long v, size_t n)

{
	WARN_ON_ONCE(!IS_ALIGNED(n, sizeof(v)));

	if (likely(v == 0)) {
		memset(s, 0, n);
	} else {
		while (n) {
			*s++ = v;
			n -= sizeof(v);
		}
	}
}

EXPORT_SYMBOL(memfill);
#endif

(I would also suggest this move to lib/string.c and architectures be given the opportunity to provide an optimised version of memfill).

[adding linux-arch to see if anyone there wants to do an optimised version of memfill for their CPU]

Quoted 2 lines · Matthew Wilcox

[adding linux-arch to see if anyone there wants to do an optimised version of memfill for their CPU]

For mn10300, this is superfluous since the memset() implementation will do optimised filling of up to 8 x 4 bytes per loop if the alignments suit.

This is also superfluous for frv as that will do up to 8 x 8 bytes per loop.

So on both those arches, memfill() should probably just wrap memset().

You've misunderstood the purpose of memfill. memfill allows the caller to specify a pattern which is not a single byte in size, eg memfill(addr, 0x12345678, 64) would result in 0x12345678 being reproduced 16 times. memset(addr, 0x12345678, 64) would result in 0x78 being reproduced 64 times.

Quoted 5 lines · Matthew Wilcox

You've misunderstood the purpose of memfill. memfill allows the caller to specify a pattern which is not a single byte in size, eg memfill(addr, 0x12345678, 64) would result in 0x12345678 being reproduced 16 times. memset(addr, 0x12345678, 64) would result in 0x78 being reproduced 64 times.

Ah. Should it take a unsigned int rather than an unsigned long?

[adding linux-arch to see if anyone there wants to do an optimised version of memfill for their CPU]

Quoted 5 lines · Minchan Kim

drivers/block/zram/zram_drv.c +5 -0

+static inline void zram_fill_page(char *ptr, unsigned long len,
+					unsigned long value)
+{
+	int i;
+	unsigned long *page = (unsigned long *)ptr;

drivers/block/zram/zram_drv.c static inline void update_used_max(struct zram *zram, · line 174

Quoted 10 lines

drivers/block/zram/zram_drv.c +10 -0

+
+	WARN_ON_ONCE(!IS_ALIGNED(len, sizeof(unsigned long)));
+
+	if (likely(value == 0)) {
+		memset(ptr, 0, len);
+	} else {
+		for (i = 0; i < len / sizeof(*page); i++)
+			page[i] = value;
+	}
+}

I would suggest:

#ifndef __HAVE_ARCH_MEMFILL
/**
* memfill - Fill a region of memory with the given value
* @s: Pointer to the start of the region.
* @v: The word to fill the region with.
* @n: The size of the region.
*
* Differs from memset() in that it fills with an unsigned long
instead of
* a byte. The pointer and the size must be aligned to unsigned
long.
*/
void memfill(unsigned long *s, unsigned long v, size_t n)

Quoted 4 lines

/**
* memfill - Fill a region of memory with the given value
* @s: Pointer to the start of the region.
* @v: The word to fill the region with.

Quoted 8 lines

* @n: The size of the region.
*
* Differs from memset() in that it fills with an unsigned long
instead of
* a byte. The pointer and the size must be aligned to unsigned
long.
*/
void memfill(unsigned long *s, unsigned long v, size_t n)

If we're going to do this, are you sure we wouldn't be wanting a string fill instead of a memfill (because filling either by byte or long looks a bit restrictive) assuming static strings that we can tell the compile time size of, it would be easy for generic code to optimise.

Quoted 13 lines · James Bottomley

Quoted 4 lines

/**
* memfill - Fill a region of memory with the given value
* @s: Pointer to the start of the region.
* @v: The word to fill the region with.

Quoted 8 lines

* @n: The size of the region.
*
* Differs from memset() in that it fills with an unsigned long
instead of
* a byte. The pointer and the size must be aligned to unsigned
long.
*/
void memfill(unsigned long *s, unsigned long v, size_t n)

Quoted 7 lines

If we're going to do this, are you sure we wouldn't be wanting a string
fill instead of a memfill (because filling either by byte or long looks
a bit restrictive) assuming static strings that we can tell the compile
time size of, it would be easy for generic code to optimise.

When you say 'string fill', do you mean this?

void memfill(void *dst, size_t dsz, void *src, size_t ssz)

{
	if (ssz == 1) {
		memset(dst, *(unsigned char *)src, dsz);
	} else if (ssz == sizeof(short)) {
		memset_s(dst, *(unsigned short *)src, dsz);
	} else if (ssz == sizeof(int)) {
		memset_i(dst, *(unsigned int *)src, dsz);
	} else if (ssz == sizeof(long)) {
		memset_l(dst, *(unsigned long *)src, dsz);
	} else {
		while (dsz >= ssz) {
			memcpy(dst, src, ssz);
			dsz -= ssz;
			dst += ssz;
		}
		if (dsz)
			memcpy(dst. src. dsz);
	}
}

(with memset_[sil] being obvious, I hope). Possibly some variation on this to optimise compile-time constant dsz.

I have no objection to that. Indeed, it would match Lars Wirzenius' memfill() definition (if not implementation) which makes me quite happy.

Yes, that's about it. My only qualm looking at the proposal was if memfill is genuinely useful to something why would it only want to fill in units of sizeof(long). On the other hand, we've been operating for decades without it, so perhaps memset_l is the only use case?

From: zhouxianrong <[email protected]>

the idea is that without doing more calculations we extend zero pages to same element pages for zram. zero page is special case of same element page with zero element.

1. the test is done under android 7.0
2. startup too many applications circularly
3. sample the zero pages, same pages (none-zero element) and total pages in function page_zero_filled

the result is listed as below:

ZERO SAME TOTAL
36214 17842 598196

ZERO/TOTAL SAME/TOTAL (ZERO+SAME)/TOTAL ZERO/SAME
AVERAGE 0.060631909 0.024990816 0.085622726 2.663825038
STDEV 0.00674612 0.005887625 0.009707034 2.115881328
MAX 0.069698422 0.030046087 0.094975336 7.56043956
MIN 0.03959586 0.007332205 0.056055193 1.928985507

from above data, the benefit is about 2.5% and up to 3% of total swapout pages.

the defect of the patch is that when we recovery a page from non-zero element the operations are low efficient for partial read.

This patch extend zero_page to same_page so if there is any user to have monitored zero_pages, he will be surprised if the number is increased but it's no harmful, I believe.

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by zhouxianrong <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

From e5a2a1dac4783f29d028170724578c0d11c80975 Mon Sep 17 00:00:00 2001

drivers/block/zram/zram_drv.c +5 -0

+static inline void zram_fill_page(char *ptr, unsigned long len,
+					unsigned long value)
+{
+	int i;
+	unsigned long *page = (unsigned long *)ptr;

Quoted 5 lines · Minchan Kim

drivers/block/zram/zram_drv.c +5 -0

+static inline void zram_fill_page(char *ptr, unsigned long len,
+					unsigned long value)
+{
+	int i;
+	unsigned long *page = (unsigned long *)ptr;

drivers/block/zram/zram_drv.c static inline void update_used_max(struct zram *zram, · line 174

Quoted 10 lines

drivers/block/zram/zram_drv.c +10 -0

+
+	WARN_ON_ONCE(!IS_ALIGNED(len, sizeof(unsigned long)));
+
+	if (likely(value == 0)) {
+		memset(ptr, 0, len);
+	} else {
+		for (i = 0; i < len / sizeof(*page); i++)
+			page[i] = value;
+	}
+}

I've hacked up memset32/memset64 for both ARM and x86 here:

http://git.infradead.org/users/willy/linux-dax.git/shortlog/refs/heads/memfill

Can you do some performance testing and see if it makes a difference?

At this point, I'd probably ask for the first 5 patches in that git branch to be included, and leave out memfill and the shoddy testsuite.

I haven't actually tested either asm implementation ... only the C fallback.