* [PATCHv2 0/2] zsmalloc/zram: drop zram's max_zpage_size @ 2018-03-06 7:06 Sergey Senozhatsky 2018-03-06 7:06 ` [PATCHv2 1/2] zsmalloc: introduce zs_huge_class_size() function Sergey Senozhatsky 2018-03-06 7:06 ` [PATCHv2 2/2] zram: drop max_zpage_size and use zs_huge_class_size() Sergey Senozhatsky 0 siblings, 2 replies; 9+ messages in thread From: Sergey Senozhatsky @ 2018-03-06 7:06 UTC (permalink / raw) To: Minchan Kim, Andrew Morton Cc: linux-kernel, linux-mm, Sergey Senozhatsky, Sergey Senozhatsky, Mike Rapoport Hello, ZRAM's max_zpage_size is a bad thing. It forces zsmalloc to store normal objects as huge ones, which results in bigger zsmalloc memory usage. Drop it and use actual zsmalloc huge-class value when decide if the object is huge or not. Sergey Senozhatsky (2): zsmalloc: introduce zs_huge_class_size() function zram: drop max_zpage_size and use zs_huge_class_size() drivers/block/zram/zram_drv.c | 9 ++++++++- drivers/block/zram/zram_drv.h | 16 ---------------- include/linux/zsmalloc.h | 2 ++ mm/zsmalloc.c | 40 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 17 deletions(-) -- 2.16.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCHv2 1/2] zsmalloc: introduce zs_huge_class_size() function 2018-03-06 7:06 [PATCHv2 0/2] zsmalloc/zram: drop zram's max_zpage_size Sergey Senozhatsky @ 2018-03-06 7:06 ` Sergey Senozhatsky 2018-03-06 7:06 ` [PATCHv2 2/2] zram: drop max_zpage_size and use zs_huge_class_size() Sergey Senozhatsky 1 sibling, 0 replies; 9+ messages in thread From: Sergey Senozhatsky @ 2018-03-06 7:06 UTC (permalink / raw) To: Minchan Kim, Andrew Morton Cc: linux-kernel, linux-mm, Sergey Senozhatsky, Sergey Senozhatsky, Mike Rapoport Not every object can be share its zspage with other objects, e.g. when the object is as big as zspage or nearly as big a zspage. For such objects zsmalloc has a so called huge class - every object which belongs to huge class consumes the entire zspage (which consists of a physical page). On x86_64, PAGE_SHIFT 12 box, the first non-huge class size is 3264, so starting down from size 3264, objects can share page(-s) and thus minimize memory wastage. ZRAM, however, has its own statically defined watermark for huge objects - "3 * PAGE_SIZE / 4 = 3072", and forcibly stores every object larger than this watermark (3072) as a PAGE_SIZE object, in other words, to a huge class, while zsmalloc can keep some of those objects in non-huge classes. This results in increased memory consumption. zsmalloc knows better if the object is huge or not. Introduce zs_huge_class_size() function which tells if the given object can be stored in one of non-huge classes or not. This will let us to drop ZRAM's huge object watermark and fully rely on zsmalloc when we decide if the object is huge. Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> --- include/linux/zsmalloc.h | 2 ++ mm/zsmalloc.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h index 57a8e98f2708..753c1af4d2cb 100644 --- a/include/linux/zsmalloc.h +++ b/include/linux/zsmalloc.h @@ -47,6 +47,8 @@ void zs_destroy_pool(struct zs_pool *pool); unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t flags); void zs_free(struct zs_pool *pool, unsigned long obj); +size_t zs_huge_class_size(void); + void *zs_map_object(struct zs_pool *pool, unsigned long handle, enum zs_mapmode mm); void zs_unmap_object(struct zs_pool *pool, unsigned long handle); diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c index a583ab111a43..63422cf35b94 100644 --- a/mm/zsmalloc.c +++ b/mm/zsmalloc.c @@ -193,6 +193,7 @@ static struct vfsmount *zsmalloc_mnt; * (see: fix_fullness_group()) */ static const int fullness_threshold_frac = 4; +static size_t huge_class_size; struct size_class { spinlock_t lock; @@ -1407,6 +1408,24 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle) } EXPORT_SYMBOL_GPL(zs_unmap_object); +/** + * zs_huge_class_size() - Returns the size (in bytes) of the first huge + * zsmalloc &size_class. + * + * The function returns the size of the first huge class - any object of equal + * or bigger size will be stored in zspage consisting of a single physical + * page. + * + * Context: Any context. + * + * Return: the size (in bytes) of the first huge zsmalloc &size_class. + */ +size_t zs_huge_class_size(void) +{ + return huge_class_size; +} +EXPORT_SYMBOL_GPL(zs_huge_class_size); + static unsigned long obj_malloc(struct size_class *class, struct zspage *zspage, unsigned long handle) { @@ -2363,6 +2382,27 @@ struct zs_pool *zs_create_pool(const char *name) pages_per_zspage = get_pages_per_zspage(size); objs_per_zspage = pages_per_zspage * PAGE_SIZE / size; + /* + * We iterate from biggest down to smallest classes, + * so huge_class_size holds the size of the first huge + * class. Any object bigger than or equal to that will + * endup in the huge class. + */ + if (pages_per_zspage != 1 && objs_per_zspage != 1 && + !huge_class_size) { + huge_class_size = size; + /* + * The object uses ZS_HANDLE_SIZE bytes to store the + * handle. We need to subtract it, because zs_malloc() + * unconditionally adds handle size before it performs + * size class search - so object may be smaller than + * huge class size, yet it still can end up in the huge + * class because it grows by ZS_HANDLE_SIZE extra bytes + * right before class lookup. + */ + huge_class_size -= (ZS_HANDLE_SIZE - 1); + } + /* * size_class is used for normal zsmalloc operation such * as alloc/free for that size. Although it is natural that we -- 2.16.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCHv2 2/2] zram: drop max_zpage_size and use zs_huge_class_size() 2018-03-06 7:06 [PATCHv2 0/2] zsmalloc/zram: drop zram's max_zpage_size Sergey Senozhatsky 2018-03-06 7:06 ` [PATCHv2 1/2] zsmalloc: introduce zs_huge_class_size() function Sergey Senozhatsky @ 2018-03-06 7:06 ` Sergey Senozhatsky 2018-03-13 9:02 ` Minchan Kim 1 sibling, 1 reply; 9+ messages in thread From: Sergey Senozhatsky @ 2018-03-06 7:06 UTC (permalink / raw) To: Minchan Kim, Andrew Morton Cc: linux-kernel, linux-mm, Sergey Senozhatsky, Sergey Senozhatsky, Mike Rapoport This patch removes ZRAM's enforced "huge object" value and uses zsmalloc huge-class watermark instead, which makes more sense. TEST - I used a 1G zram device, LZO compression back-end, original data set size was 444MB. Looking at zsmalloc classes stats the test ended up to be pretty fair. BASE ZRAM/ZSMALLOC ===================== zram mm_stat 498978816 191482495 199831552 0 199831552 15634 0 zsmalloc classes class size almost_full almost_empty obj_allocated obj_used pages_used pages_per_zspage freeable ... 151 2448 0 0 1240 1240 744 3 0 168 2720 0 0 4200 4200 2800 2 0 190 3072 0 0 10100 10100 7575 3 0 202 3264 0 0 380 380 304 4 0 254 4096 0 0 10620 10620 10620 1 0 Total 7 46 106982 106187 48787 0 PATCHED ZRAM/ZSMALLOC ===================== zram mm_stat 498978816 182579184 194248704 0 194248704 15628 0 zsmalloc classes class size almost_full almost_empty obj_allocated obj_used pages_used pages_per_zspage freeable ... 151 2448 0 0 1240 1240 744 3 0 168 2720 0 0 4200 4200 2800 2 0 190 3072 0 0 10100 10100 7575 3 0 202 3264 0 0 7180 7180 5744 4 0 254 4096 0 0 3820 3820 3820 1 0 Total 8 45 106959 106193 47424 0 As we can see, we reduced the number of objects stored in class-4096, because a huge number of objects which we previously forcibly stored in class-4096 now stored in non-huge class-3264. This results in lower memory consumption: - zsmalloc now uses 47424 physical pages, which is less than 48787 pages zsmalloc used before. - objects that we store in class-3264 share zspages. That's why overall the number of pages that both class-4096 and class-3264 consumed went down from 10924 to 9564. Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> --- drivers/block/zram/zram_drv.c | 9 ++++++++- drivers/block/zram/zram_drv.h | 16 ---------------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 85110e7931e5..1b8082e6d2f5 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -44,6 +44,11 @@ static const char *default_compressor = "lzo"; /* Module params (documentation at end) */ static unsigned int num_devices = 1; +/* + * Pages that compress to sizes equals or greater than this are stored + * uncompressed in memory. + */ +static size_t huge_class_size; static void zram_free_page(struct zram *zram, size_t index); @@ -786,6 +791,8 @@ static bool zram_meta_alloc(struct zram *zram, u64 disksize) return false; } + if (!huge_class_size) + huge_class_size = zs_huge_class_size(); return true; } @@ -965,7 +972,7 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec, return ret; } - if (unlikely(comp_len > max_zpage_size)) { + if (unlikely(comp_len >= huge_class_size)) { if (zram_wb_enabled(zram) && allow_wb) { zcomp_stream_put(zram->comp); ret = write_to_bdev(zram, bvec, index, bio, &element); diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h index 31762db861e3..d71c8000a964 100644 --- a/drivers/block/zram/zram_drv.h +++ b/drivers/block/zram/zram_drv.h @@ -21,22 +21,6 @@ #include "zcomp.h" -/*-- Configurable parameters */ - -/* - * Pages that compress to size greater than this are stored - * uncompressed in memory. - */ -static const size_t max_zpage_size = PAGE_SIZE / 4 * 3; - -/* - * NOTE: max_zpage_size must be less than or equal to: - * ZS_MAX_ALLOC_SIZE. Otherwise, zs_malloc() would - * always return failure. - */ - -/*-- End of configurable params */ - #define SECTOR_SHIFT 9 #define SECTORS_PER_PAGE_SHIFT (PAGE_SHIFT - SECTOR_SHIFT) #define SECTORS_PER_PAGE (1 << SECTORS_PER_PAGE_SHIFT) -- 2.16.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCHv2 2/2] zram: drop max_zpage_size and use zs_huge_class_size() 2018-03-06 7:06 ` [PATCHv2 2/2] zram: drop max_zpage_size and use zs_huge_class_size() Sergey Senozhatsky @ 2018-03-13 9:02 ` Minchan Kim 2018-03-13 10:24 ` Sergey Senozhatsky 0 siblings, 1 reply; 9+ messages in thread From: Minchan Kim @ 2018-03-13 9:02 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, linux-kernel, linux-mm, Sergey Senozhatsky, Mike Rapoport Hi Sergey, Sorry for being late. I love this patchset! Just a minor below. On Tue, Mar 06, 2018 at 04:06:39PM +0900, Sergey Senozhatsky wrote: > This patch removes ZRAM's enforced "huge object" value and uses > zsmalloc huge-class watermark instead, which makes more sense. > > TEST > - I used a 1G zram device, LZO compression back-end, original > data set size was 444MB. Looking at zsmalloc classes stats the > test ended up to be pretty fair. > > BASE ZRAM/ZSMALLOC > ===================== > zram mm_stat > > 498978816 191482495 199831552 0 199831552 15634 0 > > zsmalloc classes > > class size almost_full almost_empty obj_allocated obj_used pages_used pages_per_zspage freeable > ... > 151 2448 0 0 1240 1240 744 3 0 > 168 2720 0 0 4200 4200 2800 2 0 > 190 3072 0 0 10100 10100 7575 3 0 > 202 3264 0 0 380 380 304 4 0 > 254 4096 0 0 10620 10620 10620 1 0 > > Total 7 46 106982 106187 48787 0 > > PATCHED ZRAM/ZSMALLOC > ===================== > > zram mm_stat > > 498978816 182579184 194248704 0 194248704 15628 0 > > zsmalloc classes > > class size almost_full almost_empty obj_allocated obj_used pages_used pages_per_zspage freeable > ... > 151 2448 0 0 1240 1240 744 3 0 > 168 2720 0 0 4200 4200 2800 2 0 > 190 3072 0 0 10100 10100 7575 3 0 > 202 3264 0 0 7180 7180 5744 4 0 > 254 4096 0 0 3820 3820 3820 1 0 > > Total 8 45 106959 106193 47424 0 > > As we can see, we reduced the number of objects stored in class-4096, > because a huge number of objects which we previously forcibly stored > in class-4096 now stored in non-huge class-3264. This results in lower > memory consumption: > - zsmalloc now uses 47424 physical pages, which is less than 48787 > pages zsmalloc used before. > > - objects that we store in class-3264 share zspages. That's why overall > the number of pages that both class-4096 and class-3264 consumed went > down from 10924 to 9564. > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > --- > drivers/block/zram/zram_drv.c | 9 ++++++++- > drivers/block/zram/zram_drv.h | 16 ---------------- > 2 files changed, 8 insertions(+), 17 deletions(-) > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c > index 85110e7931e5..1b8082e6d2f5 100644 > --- a/drivers/block/zram/zram_drv.c > +++ b/drivers/block/zram/zram_drv.c > @@ -44,6 +44,11 @@ static const char *default_compressor = "lzo"; > > /* Module params (documentation at end) */ > static unsigned int num_devices = 1; > +/* > + * Pages that compress to sizes equals or greater than this are stored > + * uncompressed in memory. > + */ > +static size_t huge_class_size; > > static void zram_free_page(struct zram *zram, size_t index); > > @@ -786,6 +791,8 @@ static bool zram_meta_alloc(struct zram *zram, u64 disksize) > return false; > } > > + if (!huge_class_size) > + huge_class_size = zs_huge_class_size(); If it is static, we can do this in zram_init? I believe it's more readable in that it's never changed betweens zram instances. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv2 2/2] zram: drop max_zpage_size and use zs_huge_class_size() 2018-03-13 9:02 ` Minchan Kim @ 2018-03-13 10:24 ` Sergey Senozhatsky 2018-03-13 13:58 ` Minchan Kim 0 siblings, 1 reply; 9+ messages in thread From: Sergey Senozhatsky @ 2018-03-13 10:24 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, linux-kernel, linux-mm, Sergey Senozhatsky, Mike Rapoport Hello Minchan, On (03/13/18 18:02), Minchan Kim wrote: > Sorry for being late. > I love this patchset! Just a minor below. :) [..] > > + if (!huge_class_size) > > + huge_class_size = zs_huge_class_size(); > > If it is static, we can do this in zram_init? I believe it's more readable in that > it's never changed betweens zram instances. We need to have at least one pool, because pool decides where the watermark is. At zram_init() stage we don't have a pool yet. We zs_create_pool() in zram_meta_alloc() so that's why I put zs_huge_class_size() there. I'm not in love with it, but that's the only place where we can have it. -ss ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv2 2/2] zram: drop max_zpage_size and use zs_huge_class_size() 2018-03-13 10:24 ` Sergey Senozhatsky @ 2018-03-13 13:58 ` Minchan Kim 2018-03-13 14:18 ` Sergey Senozhatsky 0 siblings, 1 reply; 9+ messages in thread From: Minchan Kim @ 2018-03-13 13:58 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Andrew Morton, linux-kernel, linux-mm, Sergey Senozhatsky, Mike Rapoport On Tue, Mar 13, 2018 at 07:24:37PM +0900, Sergey Senozhatsky wrote: > Hello Minchan, > > On (03/13/18 18:02), Minchan Kim wrote: > > Sorry for being late. > > I love this patchset! Just a minor below. > > :) > > [..] > > > + if (!huge_class_size) > > > + huge_class_size = zs_huge_class_size(); > > > > If it is static, we can do this in zram_init? I believe it's more readable in that > > it's never changed betweens zram instances. > > We need to have at least one pool, because pool decides where the > watermark is. At zram_init() stage we don't have a pool yet. We > zs_create_pool() in zram_meta_alloc() so that's why I put > zs_huge_class_size() there. I'm not in love with it, but that's > the only place where we can have it. Fair enough. Then what happens if client calls zs_huge_class_size without creating zs_create_pool? I think we should make zs_huge_class_size has a zs_pool as argument. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv2 2/2] zram: drop max_zpage_size and use zs_huge_class_size() 2018-03-13 13:58 ` Minchan Kim @ 2018-03-13 14:18 ` Sergey Senozhatsky 2018-03-13 14:29 ` Minchan Kim 0 siblings, 1 reply; 9+ messages in thread From: Sergey Senozhatsky @ 2018-03-13 14:18 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Andrew Morton, linux-kernel, linux-mm, Sergey Senozhatsky, Mike Rapoport On (03/13/18 22:58), Minchan Kim wrote: > > > If it is static, we can do this in zram_init? I believe it's more readable in that > > > it's never changed betweens zram instances. > > > > We need to have at least one pool, because pool decides where the > > watermark is. At zram_init() stage we don't have a pool yet. We > > zs_create_pool() in zram_meta_alloc() so that's why I put > > zs_huge_class_size() there. I'm not in love with it, but that's > > the only place where we can have it. > > Fair enough. Then what happens if client calls zs_huge_class_size > without creating zs_create_pool? Will receive 0. One of the version was returning SIZE_MAX in such case. size_t zs_huge_class_size(void) { + if (unlikely(!huge_class_size)) + return SIZE_MAX; return huge_class_size; } > I think we should make zs_huge_class_size has a zs_pool as argument. Can do, but the param will be unused. May be we can do something like below instead: size_t zs_huge_class_size(void) { + if (unlikely(!huge_class_size)) + return 3 * PAGE_SIZE / 4; return huge_class_size; } Should do no harm (unless I'm missing something). -ss ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv2 2/2] zram: drop max_zpage_size and use zs_huge_class_size() 2018-03-13 14:18 ` Sergey Senozhatsky @ 2018-03-13 14:29 ` Minchan Kim 2018-03-13 14:35 ` Sergey Senozhatsky 0 siblings, 1 reply; 9+ messages in thread From: Minchan Kim @ 2018-03-13 14:29 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Sergey Senozhatsky, Andrew Morton, linux-kernel, linux-mm, Mike Rapoport On Tue, Mar 13, 2018 at 11:18:13PM +0900, Sergey Senozhatsky wrote: > On (03/13/18 22:58), Minchan Kim wrote: > > > > If it is static, we can do this in zram_init? I believe it's more readable in that > > > > it's never changed betweens zram instances. > > > > > > We need to have at least one pool, because pool decides where the > > > watermark is. At zram_init() stage we don't have a pool yet. We > > > zs_create_pool() in zram_meta_alloc() so that's why I put > > > zs_huge_class_size() there. I'm not in love with it, but that's > > > the only place where we can have it. > > > > Fair enough. Then what happens if client calls zs_huge_class_size > > without creating zs_create_pool? > > Will receive 0. > One of the version was returning SIZE_MAX in such case. > > size_t zs_huge_class_size(void) > { > + if (unlikely(!huge_class_size)) > + return SIZE_MAX; > return huge_class_size; > } I really don't want to have such API which returns different size on different context. The thing is we need to create pool first to get right value. It means zs_huge_class_size should depend on zs_create_pool. So, I think passing zs_pool to zs_huge_class_size is right way to prevent such misuse and confusing. Yub, franky speaknig, zsmalloc is not popular like slab allocator or page allocator so this like discussion would be waste. However, we don't need big effort to do and this is review phase so I want to make more robust/readable. ;-) > > > I think we should make zs_huge_class_size has a zs_pool as argument. > > Can do, but the param will be unused. May be we can do something Yub, param wouldn't be unused but it's the way of creating dependency intentionally. It could make code more robust/readable. Please, let's pass zs_pool and returns always right huge size. > like below instead: > > size_t zs_huge_class_size(void) > { > + if (unlikely(!huge_class_size)) > + return 3 * PAGE_SIZE / 4; > return huge_class_size; > } > > Should do no harm (unless I'm missing something). > > -ss ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv2 2/2] zram: drop max_zpage_size and use zs_huge_class_size() 2018-03-13 14:29 ` Minchan Kim @ 2018-03-13 14:35 ` Sergey Senozhatsky 0 siblings, 0 replies; 9+ messages in thread From: Sergey Senozhatsky @ 2018-03-13 14:35 UTC (permalink / raw) To: Minchan Kim Cc: Sergey Senozhatsky, Sergey Senozhatsky, Andrew Morton, linux-kernel, linux-mm, Mike Rapoport On (03/13/18 23:29), Minchan Kim wrote: [..] > > Can do, but the param will be unused. May be we can do something > > Yub, param wouldn't be unused but it's the way of creating dependency > intentionally. It could make code more robust/readable. > > Please, let's pass zs_pool and returns always right huge size. OK, no prob. Will send an updated version tomorrow. -ss ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-03-13 14:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-06 7:06 [PATCHv2 0/2] zsmalloc/zram: drop zram's max_zpage_size Sergey Senozhatsky 2018-03-06 7:06 ` [PATCHv2 1/2] zsmalloc: introduce zs_huge_class_size() function Sergey Senozhatsky 2018-03-06 7:06 ` [PATCHv2 2/2] zram: drop max_zpage_size and use zs_huge_class_size() Sergey Senozhatsky 2018-03-13 9:02 ` Minchan Kim 2018-03-13 10:24 ` Sergey Senozhatsky 2018-03-13 13:58 ` Minchan Kim 2018-03-13 14:18 ` Sergey Senozhatsky 2018-03-13 14:29 ` Minchan Kim 2018-03-13 14:35 ` Sergey Senozhatsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).