[PATCH 0/2] zsmalloc/zram: drop zram's max_zpage_size

[PATCH 0/2] zsmalloc/zram: drop zram's max_zpage_size

[Sergey Senozhatsky]

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_object() function
  zram: drop max_zpage_size and use zs_huge_object()

 drivers/block/zram/zram_drv.c |  6 +++---
 drivers/block/zram/zram_drv.h | 16 ----------------
 include/linux/zsmalloc.h      |  2 ++
 mm/zsmalloc.c                 | 17 +++++++++++++++++
 4 files changed, 22 insertions(+), 19 deletions(-)

-- 
2.16.1

[PATCH 0/2] zsmalloc/zram: drop zram's max_zpage_size

[Sergey Senozhatsky]

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_object() function
  zram: drop max_zpage_size and use zs_huge_object()

 drivers/block/zram/zram_drv.c |  6 +++---
 drivers/block/zram/zram_drv.h | 16 ----------------
 include/linux/zsmalloc.h      |  2 ++
 mm/zsmalloc.c                 | 17 +++++++++++++++++
 4 files changed, 22 insertions(+), 19 deletions(-)

-- 
2.16.1

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

[PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

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_object() 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            | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 57a8e98f2708..9a1baf673cc1 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);
 
+bool zs_huge_object(size_t sz);
+
 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 c3013505c305..b3e295a806be 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
  * (see: fix_fullness_group())
  */
 static const int fullness_threshold_frac = 4;
+static size_t zs_huge_class_size;
 
 struct size_class {
 	spinlock_t lock;
@@ -1417,6 +1418,19 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
 }
 EXPORT_SYMBOL_GPL(zs_unmap_object);
 
+/*
+ * Check if the object's size falls into huge_class area. We must take
+ * ZS_HANDLE_SIZE into account and test the actual size we are going to
+ * use up. zs_malloc() unconditionally adds handle size before it performs
+ * size_class lookup, so we may endup in a huge class yet zs_huge_object()
+ * returned 'false'.
+ */
+bool zs_huge_object(size_t sz)
+{
+	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
+}
+EXPORT_SYMBOL_GPL(zs_huge_object);
+
 static unsigned long obj_malloc(struct size_class *class,
 				struct zspage *zspage, unsigned long handle)
 {
@@ -2404,6 +2418,9 @@ struct zs_pool *zs_create_pool(const char *name)
 			INIT_LIST_HEAD(&class->fullness_list[fullness]);
 
 		prev_class = class;
+		if (pages_per_zspage == 1 && objs_per_zspage == 1
+				&& !zs_huge_class_size)
+			zs_huge_class_size = size;
 	}
 
 	/* debug only, don't abort if it fails */
-- 
2.16.1

[PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

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_object() 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            | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 57a8e98f2708..9a1baf673cc1 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);
 
+bool zs_huge_object(size_t sz);
+
 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 c3013505c305..b3e295a806be 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
  * (see: fix_fullness_group())
  */
 static const int fullness_threshold_frac = 4;
+static size_t zs_huge_class_size;
 
 struct size_class {
 	spinlock_t lock;
@@ -1417,6 +1418,19 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
 }
 EXPORT_SYMBOL_GPL(zs_unmap_object);
 
+/*
+ * Check if the object's size falls into huge_class area. We must take
+ * ZS_HANDLE_SIZE into account and test the actual size we are going to
+ * use up. zs_malloc() unconditionally adds handle size before it performs
+ * size_class lookup, so we may endup in a huge class yet zs_huge_object()
+ * returned 'false'.
+ */
+bool zs_huge_object(size_t sz)
+{
+	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
+}
+EXPORT_SYMBOL_GPL(zs_huge_object);
+
 static unsigned long obj_malloc(struct size_class *class,
 				struct zspage *zspage, unsigned long handle)
 {
@@ -2404,6 +2418,9 @@ struct zs_pool *zs_create_pool(const char *name)
 			INIT_LIST_HEAD(&class->fullness_list[fullness]);
 
 		prev_class = class;
+		if (pages_per_zspage == 1 && objs_per_zspage == 1
+				&& !zs_huge_class_size)
+			zs_huge_class_size = size;
 	}
 
 	/* debug only, don't abort if it fails */
-- 
2.16.1

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

[PATCH 2/2] zram: drop max_zpage_size and use zs_huge_object()

[Sergey Senozhatsky]

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-endi, 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 |  6 +++---
 drivers/block/zram/zram_drv.h | 16 ----------------
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 0afa6c8c3857..3d2bc4b1423c 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -965,7 +965,7 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 		return ret;
 	}
 
-	if (unlikely(comp_len > max_zpage_size)) {
+	if (unlikely(zs_huge_object(comp_len))) {
 		if (zram_wb_enabled(zram) && allow_wb) {
 			zcomp_stream_put(zram->comp);
 			ret = write_to_bdev(zram, bvec, index, bio, &element);
@@ -1022,10 +1022,10 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 	dst = zs_map_object(zram->mem_pool, handle, ZS_MM_WO);
 
 	src = zstrm->buffer;
-	if (comp_len == PAGE_SIZE)
+	if (zs_huge_object(comp_len))
 		src = kmap_atomic(page);
 	memcpy(dst, src, comp_len);
-	if (comp_len == PAGE_SIZE)
+	if (zs_huge_object(comp_len))
 		kunmap_atomic(src);
 
 	zcomp_stream_put(zram->comp);
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.1

[PATCH 2/2] zram: drop max_zpage_size and use zs_huge_object()

[Sergey Senozhatsky]

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-endi, 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 |  6 +++---
 drivers/block/zram/zram_drv.h | 16 ----------------
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 0afa6c8c3857..3d2bc4b1423c 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -965,7 +965,7 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 		return ret;
 	}
 
-	if (unlikely(comp_len > max_zpage_size)) {
+	if (unlikely(zs_huge_object(comp_len))) {
 		if (zram_wb_enabled(zram) && allow_wb) {
 			zcomp_stream_put(zram->comp);
 			ret = write_to_bdev(zram, bvec, index, bio, &element);
@@ -1022,10 +1022,10 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 	dst = zs_map_object(zram->mem_pool, handle, ZS_MM_WO);
 
 	src = zstrm->buffer;
-	if (comp_len == PAGE_SIZE)
+	if (zs_huge_object(comp_len))
 		src = kmap_atomic(page);
 	memcpy(dst, src, comp_len);
-	if (comp_len == PAGE_SIZE)
+	if (zs_huge_object(comp_len))
 		kunmap_atomic(src);
 
 	zcomp_stream_put(zram->comp);
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.1

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

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Mike Rapoport]

On Wed, Feb 07, 2018 at 06:29:18PM +0900, Sergey Senozhatsky wrote:
> 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_object() 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            | 17 +++++++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
> index 57a8e98f2708..9a1baf673cc1 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);
> 
> +bool zs_huge_object(size_t sz);
> +
>  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 c3013505c305..b3e295a806be 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
>   * (see: fix_fullness_group())
>   */
>  static const int fullness_threshold_frac = 4;
> +static size_t zs_huge_class_size;
> 
>  struct size_class {
>  	spinlock_t lock;
> @@ -1417,6 +1418,19 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
>  }
>  EXPORT_SYMBOL_GPL(zs_unmap_object);
> 
> +/*
> + * Check if the object's size falls into huge_class area. We must take
> + * ZS_HANDLE_SIZE into account and test the actual size we are going to
> + * use up. zs_malloc() unconditionally adds handle size before it performs
> + * size_class lookup, so we may endup in a huge class yet zs_huge_object()
> + * returned 'false'.
> + */

Can you please reformat this comment as kernel-doc?

> +bool zs_huge_object(size_t sz)
> +{
> +	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
> +}
> +EXPORT_SYMBOL_GPL(zs_huge_object);
> +
>  static unsigned long obj_malloc(struct size_class *class,
>  				struct zspage *zspage, unsigned long handle)
>  {
> @@ -2404,6 +2418,9 @@ struct zs_pool *zs_create_pool(const char *name)
>  			INIT_LIST_HEAD(&class->fullness_list[fullness]);
> 
>  		prev_class = class;
> +		if (pages_per_zspage == 1 && objs_per_zspage == 1
> +				&& !zs_huge_class_size)
> +			zs_huge_class_size = size;
>  	}
> 
>  	/* debug only, don't abort if it fails */
> -- 
> 2.16.1
> 
> --
> 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>
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Mike Rapoport]

On Wed, Feb 07, 2018 at 06:29:18PM +0900, Sergey Senozhatsky wrote:
> 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_object() 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            | 17 +++++++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
> index 57a8e98f2708..9a1baf673cc1 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);
> 
> +bool zs_huge_object(size_t sz);
> +
>  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 c3013505c305..b3e295a806be 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
>   * (see: fix_fullness_group())
>   */
>  static const int fullness_threshold_frac = 4;
> +static size_t zs_huge_class_size;
> 
>  struct size_class {
>  	spinlock_t lock;
> @@ -1417,6 +1418,19 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
>  }
>  EXPORT_SYMBOL_GPL(zs_unmap_object);
> 
> +/*
> + * Check if the object's size falls into huge_class area. We must take
> + * ZS_HANDLE_SIZE into account and test the actual size we are going to
> + * use up. zs_malloc() unconditionally adds handle size before it performs
> + * size_class lookup, so we may endup in a huge class yet zs_huge_object()
> + * returned 'false'.
> + */

Can you please reformat this comment as kernel-doc?

> +bool zs_huge_object(size_t sz)
> +{
> +	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
> +}
> +EXPORT_SYMBOL_GPL(zs_huge_object);
> +
>  static unsigned long obj_malloc(struct size_class *class,
>  				struct zspage *zspage, unsigned long handle)
>  {
> @@ -2404,6 +2418,9 @@ struct zs_pool *zs_create_pool(const char *name)
>  			INIT_LIST_HEAD(&class->fullness_list[fullness]);
> 
>  		prev_class = class;
> +		if (pages_per_zspage == 1 && objs_per_zspage == 1
> +				&& !zs_huge_class_size)
> +			zs_huge_class_size = size;
>  	}
> 
>  	/* debug only, don't abort if it fails */
> -- 
> 2.16.1
> 
> --
> 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>
> 

-- 
Sincerely yours,
Mike.

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

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/08/18 18:30), Mike Rapoport wrote:
[..]
> > 
> > +/*
> > + * Check if the object's size falls into huge_class area. We must take
> > + * ZS_HANDLE_SIZE into account and test the actual size we are going to
> > + * use up. zs_malloc() unconditionally adds handle size before it performs
> > + * size_class lookup, so we may endup in a huge class yet zs_huge_object()
> > + * returned 'false'.
> > + */
> 
> Can you please reformat this comment as kernel-doc?

Is this - Documentation/doc-guide/kernel-doc.rst - the right thing
to use as a reference?

	-ss

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/08/18 18:30), Mike Rapoport wrote:
[..]
> > 
> > +/*
> > + * Check if the object's size falls into huge_class area. We must take
> > + * ZS_HANDLE_SIZE into account and test the actual size we are going to
> > + * use up. zs_malloc() unconditionally adds handle size before it performs
> > + * size_class lookup, so we may endup in a huge class yet zs_huge_object()
> > + * returned 'false'.
> > + */
> 
> Can you please reformat this comment as kernel-doc?

Is this - Documentation/doc-guide/kernel-doc.rst - the right thing
to use as a reference?

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

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Matthew Wilcox]

On Fri, Feb 09, 2018 at 11:55:20AM +0900, Sergey Senozhatsky wrote:
> On (02/08/18 18:30), Mike Rapoport wrote:
> [..]
> > > 
> > > +/*
> > > + * Check if the object's size falls into huge_class area. We must take
> > > + * ZS_HANDLE_SIZE into account and test the actual size we are going to
> > > + * use up. zs_malloc() unconditionally adds handle size before it performs
> > > + * size_class lookup, so we may endup in a huge class yet zs_huge_object()
> > > + * returned 'false'.
> > > + */
> > 
> > Can you please reformat this comment as kernel-doc?
> 
> Is this - Documentation/doc-guide/kernel-doc.rst - the right thing
> to use as a reference?

Yes.  I just sent a revision to it that makes it (I think) a little
easier to read.  Try this version:


Writing kernel-doc comments
===========================

The Linux kernel source files may contain structured documentation
comments in the kernel-doc format to describe the functions, types
and design of the code. It is easier to keep documentation up-to-date
when it is embedded in source files.

.. note:: The kernel-doc format is deceptively similar to javadoc,
   gtk-doc or Doxygen, yet distinctively different, for historical
   reasons. The kernel source contains tens of thousands of kernel-doc
   comments. Please stick to the style described here.

The kernel-doc structure is extracted from the comments, and proper
`Sphinx C Domain`_ function and type descriptions with anchors are
generated from them. The descriptions are filtered for special kernel-doc
highlights and cross-references. See below for details.

.. _Sphinx C Domain: http://www.sphinx-doc.org/en/stable/domains.html

Every function that is exported to loadable modules using
``EXPORT_SYMBOL`` or ``EXPORT_SYMBOL_GPL`` should have a kernel-doc
comment. Functions and data structures in header files which are intended
to be used by modules should also have kernel-doc comments.

It is good practice to also provide kernel-doc formatted documentation
for functions externally visible to other kernel files (not marked
``static``). We also recommend providing kernel-doc formatted
documentation for private (file ``static``) routines, for consistency of
kernel source code layout. This is lower priority and at the discretion
of the maintainer of that kernel source file.

How to format kernel-doc comments
---------------------------------

The opening comment mark ``/**`` is used for kernel-doc comments. The
``kernel-doc`` tool will extract comments marked this way. The rest of
the comment is formatted like a normal multi-line comment with a column
of asterisks on the left side, closing with ``*/`` on a line by itself.

The function and type kernel-doc comments should be placed just before
the function or type being described in order to maximise the chance
that somebody changing the code will also change the documentation. The
overview kernel-doc comments may be placed anywhere at the top indentation
level.

Function documentation
----------------------

The general format of a function and function-like macro kernel-doc comment is::

  /**
   * function_name() - Brief description of function.
   * @arg1: Describe the first argument.
   * @arg2: Describe the second argument.
   *        One can provide multiple line descriptions
   *        for arguments.
   *
   * A longer description, with more discussion of the function function_name()
   * that might be useful to those using or modifying it. Begins with an
   * empty comment line, and may include additional embedded empty
   * comment lines.
   *
   * The longer description may have multiple paragraphs.
   *
   * Context: Describes whether the function can sleep, what locks it takes,
   *          releases, or expects to be held. It can extend over multiple
   *          lines.
   * Return: Describe the return value of foobar.
   *
   * The return value description can also have multiple paragraphs, and should
   * be placed at the end of the comment block.
   */

The brief description following the function name may span multiple lines, and
ends with an argument description, a blank comment line, or the end of the
comment block.

Function parameters
~~~~~~~~~~~~~~~~~~~

Each function argument should be described in order, immediately following
the short function description.  Do not leave a blank line between the
function description and the arguments, nor between the arguments.

Each ``@argument:`` description may span multiple lines.

.. note::

   If the ``@argument`` description has multiple lines, the continuation
   of the description should start at the same column as the previous line::

      * @argument: some long description
      *            that continues on next lines

   or::

      * @argument:
      *		some long description
      *		that continues on next lines

If a function has a variable number of arguments, its description should
be written in kernel-doc notation as::

      * @...: description

Function context
~~~~~~~~~~~~~~~~

The context in which a function can be called should be described in a
section named ``Context``. This should include whether the function
sleeps or can be called from interrupt context, as well as what locks
it takes, releases and expects to be held by its caller.

Examples::

  * Context: Any context.
  * Context: Any context. Takes and releases the RCU lock.
  * Context: Any context. Expects <lock> to be held by caller.
  * Context: Process context. May sleep if @gfp flags permit.
  * Context: Process context. Takes and releases <mutex>.
  * Context: Softirq or process context. Takes and releases <lock>, BH-safe.
  * Context: Interrupt context.

Return values
~~~~~~~~~~~~~

The return value, if any, should be described in a dedicated section
named ``Return``.

.. note::

  #) The multi-line descriptive text you provide does *not* recognize
     line breaks, so if you try to format some text nicely, as in::

	* Return:
	* 0 - OK
	* -EINVAL - invalid argument
	* -ENOMEM - out of memory

     this will all run together and produce::

	Return: 0 - OK -EINVAL - invalid argument -ENOMEM - out of memory

     So, in order to produce the desired line breaks, you need to use a
     ReST list, e. g.::

      * Return:
      * * 0		- OK to runtime suspend the device
      * * -EBUSY	- Device should not be runtime suspended

  #) If the descriptive text you provide has lines that begin with
     some phrase followed by a colon, each of those phrases will be taken
     as a new section heading, which probably won't produce the desired
     effect.

Structure, union, and enumeration documentation
-----------------------------------------------

The general format of a struct, union, and enum kernel-doc comment is::

  /**
   * struct struct_name - Brief description.
   * @member1: Description of member1.
   * @member2: Description of member2.
   *           One can provide multiple line descriptions
   *           for members.
   *
   * Description of the structure.
   */

You can replace the ``struct`` in the above example with ``union`` or
``enum``  to describe unions or enums. ``member`` is used to mean struct
and union member names as well as enumerations in an enum.

The brief description following the structure name may span multiple
lines, and ends with a member description, a blank comment line, or the
end of the comment block.

Members
~~~~~~~

Members of structs, unions and enums should be documented the same way
as function parameters; they immediately succeed the short description
and may be multi-line.

Inside a struct or union description, you can use the ``private:`` and
``public:`` comment tags. Structure fields that are inside a ``private:``
area are not listed in the generated output documentation.

The ``private:`` and ``public:`` tags must begin immediately following a
``/*`` comment marker. They may optionally include comments between the
``:`` and the ending ``*/`` marker.

Example::

  /**
   * struct my_struct - short description
   * @a: first member
   * @b: second member
   * @d: fourth member
   *
   * Longer description
   */
  struct my_struct {
      int a;
      int b;
  /* private: internal use only */
      int c;
  /* public: the next one is public */
      int d;
  };

In-line member documentation comments
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The structure members may also be documented in-line within the definition.
There are two styles, single-line comments where both the opening ``/**`` and
closing ``*/`` are on the same line, and multi-line comments where they are each
on a line of their own, like all other kernel-doc comments::

  /**
   * struct foo - Brief description.
   * @foo: The Foo member.
   */
  struct foo {
        int foo;
        /**
         * @bar: The Bar member.
         */
        int bar;
        /**
         * @baz: The Baz member.
         *
         * Here, the member description may contain several paragraphs.
         */
        int baz;
        /** @foobar: Single line description. */
        int foobar;
  }

Nested structs/unions
~~~~~~~~~~~~~~~~~~~~~

It is possible to document nested structs and unions, like::

      /**
       * struct nested_foobar - a struct with nested unions and structs
       * @memb1: first member of anonymous union/anonymous struct
       * @memb2: second member of anonymous union/anonymous struct
       * @memb3: third member of anonymous union/anonymous struct
       * @memb4: fourth member of anonymous union/anonymous struct
       * @bar.st1.memb1: first member of struct st1 on union bar
       * @bar.st1.memb2: second member of struct st1 on union bar
       * @bar.st2.memb1: first member of struct st2 on union bar
       * @bar.st2.memb2: second member of struct st2 on union bar
      struct nested_foobar {
        /* Anonymous union/struct*/
        union {
          struct {
            int memb1;
            int memb2;
        }
          struct {
            void *memb3;
            int memb4;
          }
        }
        union {
          struct {
            int memb1;
            int memb2;
          } st1;
          struct {
            void *memb1;
            int memb2;
          } st2;
        } bar;
      };

.. note::

   #) When documenting nested structs or unions, if the struct/union ``foo``
      is named, the member ``bar`` inside it should be documented as
      ``@foo.bar:``
   #) When the nested struct/union is anonymous, the member ``bar`` in it
      should be documented as ``@bar:``

Typedef documentation
---------------------

The general format of a typedef kernel-doc comment is::

  /**
   * typedef type_name - Brief description.
   *
   * Description of the type.
   */

Typedefs with function prototypes can also be documented::

  /**
   * typedef type_name - Brief description.
   * @arg1: description of arg1
   * @arg2: description of arg2
   *
   * Description of the type.
   *
   * Context: Locking context.
   * Return: Meaning of the return value.
   */
   typedef void (*type_name)(struct v4l2_ctrl *arg1, void *arg2);

Highlights and cross-references
-------------------------------

The following special patterns are recognized in the kernel-doc comment
descriptive text and converted to proper reStructuredText markup and `Sphinx C
Domain`_ references.

.. attention:: The below are **only** recognized within kernel-doc comments,
	       **not** within normal reStructuredText documents.

``funcname()``
  Function reference.

``@parameter``
  Name of a function parameter. (No cross-referencing, just formatting.)

``%CONST``
  Name of a constant. (No cross-referencing, just formatting.)

````literal````
  A literal block that should be handled as-is. The output will use a
  ``monospaced font``.

  Useful if you need to use special characters that would otherwise have some
  meaning either by kernel-doc script of by reStructuredText.

  This is particularly useful if you need to use things like ``%ph`` inside
  a function description.

``$ENVVAR``
  Name of an environment variable. (No cross-referencing, just formatting.)

``&struct name``
  Structure reference.

``&enum name``
  Enum reference.

``&typedef name``
  Typedef reference.

``&struct_name->member`` or ``&struct_name.member``
  Structure or union member reference. The cross-reference will be to the struct
  or union definition, not the member directly.

``&name``
  A generic type reference. Prefer using the full reference described above
  instead. This is mostly for legacy comments.

Cross-referencing from reStructuredText
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

To cross-reference the functions and types defined in the kernel-doc comments
from reStructuredText documents, please use the `Sphinx C Domain`_
references. For example::

  See function :c:func:`foo` and struct/union/enum/typedef :c:type:`bar`.

While the type reference works with just the type name, without the
struct/union/enum/typedef part in front, you may want to use::

  See :c:type:`struct foo <foo>`.
  See :c:type:`union bar <bar>`.
  See :c:type:`enum baz <baz>`.
  See :c:type:`typedef meh <meh>`.

This will produce prettier links, and is in line with how kernel-doc does the
cross-references.

For further details, please refer to the `Sphinx C Domain`_ documentation.

Overview documentation comments
-------------------------------

To facilitate having source code and comments close together, you can include
kernel-doc documentation blocks that are free-form comments instead of being
kernel-doc for functions, structures, unions, enums, or typedefs. This could be
used for something like a theory of operation for a driver or library code, for
example.

This is done by using a ``DOC:`` section keyword with a section title.

The general format of an overview or high-level documentation comment is::

  /**
   * DOC: Theory of Operation
   *
   * The whizbang foobar is a dilly of a gizmo. It can do whatever you
   * want it to do, at any time. It reads your mind. Here's how it works.
   *
   * foo bar splat
   *
   * The only drawback to this gizmo is that is can sometimes damage
   * hardware, software, or its subject(s).
   */

The title following ``DOC:`` acts as a heading within the source file, but also
as an identifier for extracting the documentation comment. Thus, the title must
be unique within the file.

Style guide
-----------

It is helpful to maintain a consistent style across the kernel.  It helps
the reader who can rely on conventions to understand a part of the kernel
which is new to them.  It also helps the writer who can simply copy and
paste function parameter descriptions instead of coming up with new and
excitingly different ways to say the same thing.

Context names
~~~~~~~~~~~~~

Refer to process context, softirq context and hardirq context.  Interrupt
context refers to both hard and soft interrupts.  Bottom half context
is deprecated; it is a synonym for softirq context.  Tasklet context
should not be used; tasklets run in softirq context.  Do not use user
context; this is a synonym for process context.

Common function parameters
~~~~~~~~~~~~~~~~~~~~~~~~~~

If you pass ``GFP_`` flags to a function, name the parameter ``gfp`` and
document it like this::

  * @gfp: Memory allocation flags.

If your function takes a ``struct file *`` argument, name the parameter ``file``
(unless you have a good reason to use another name) and document it like
this::

  * @file: File pointer.

If your function takes a ``struct dentry *`` argument, name the
parameter ``dentry`` and document it like this::

  * @dentry: Directory Entry pointer.

If your function takes a ``struct inode *`` argument, name the parameter
``inode`` and document it like this::

  * @inode: Inode pointer.

Including kernel-doc comments
=============================

The documentation comments may be included in any of the reStructuredText
documents using a dedicated kernel-doc Sphinx directive extension.

The kernel-doc directive is of the format::

  .. kernel-doc:: source
     :option:

The *source* is the path to a source file, relative to the kernel source
tree. The following directive options are supported:

export: *[source-pattern ...]*
  Include documentation for all functions in *source* that have been exported
  using ``EXPORT_SYMBOL`` or ``EXPORT_SYMBOL_GPL`` either in *source* or in any
  of the files specified by *source-pattern*.

  The *source-pattern* is useful when the kernel-doc comments have been placed
  in header files, while ``EXPORT_SYMBOL`` and ``EXPORT_SYMBOL_GPL`` are next to
  the function definitions.

  Examples::

    .. kernel-doc:: lib/bitmap.c
       :export:

    .. kernel-doc:: include/net/mac80211.h
       :export: net/mac80211/*.c

internal: *[source-pattern ...]*
  Include documentation for all functions and types in *source* that have
  **not** been exported using ``EXPORT_SYMBOL`` or ``EXPORT_SYMBOL_GPL`` either
  in *source* or in any of the files specified by *source-pattern*.

  Example::

    .. kernel-doc:: drivers/gpu/drm/i915/intel_audio.c
       :internal:

doc: *title*
  Include documentation for the ``DOC:`` paragraph identified by *title* in
  *source*. Spaces are allowed in *title*; do not quote the *title*. The *title*
  is only used as an identifier for the paragraph, and is not included in the
  output. Please make sure to have an appropriate heading in the enclosing
  reStructuredText document.

  Example::

    .. kernel-doc:: drivers/gpu/drm/i915/intel_audio.c
       :doc: High Definition Audio over HDMI and Display Port

functions: *function* *[...]*
  Include documentation for each *function* in *source*.

  Example::

    .. kernel-doc:: lib/bitmap.c
       :functions: bitmap_parselist bitmap_parselist_user

Without options, the kernel-doc directive includes all documentation comments
from the source file.

The kernel-doc extension is included in the kernel source tree, at
``Documentation/sphinx/kerneldoc.py``. Internally, it uses the
``scripts/kernel-doc`` script to extract the documentation comments from the
source.

.. _kernel_doc:

How to use kernel-doc to generate man pages
-------------------------------------------

If you just want to use kernel-doc to generate man pages you can do this
from the kernel git tree::

  $ scripts/kernel-doc -man $(git grep -l '/\*\*' -- :^Documentation :^tools) | scripts/split-man.pl /tmp/man

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Matthew Wilcox]

On Fri, Feb 09, 2018 at 11:55:20AM +0900, Sergey Senozhatsky wrote:
> On (02/08/18 18:30), Mike Rapoport wrote:
> [..]
> > > 
> > > +/*
> > > + * Check if the object's size falls into huge_class area. We must take
> > > + * ZS_HANDLE_SIZE into account and test the actual size we are going to
> > > + * use up. zs_malloc() unconditionally adds handle size before it performs
> > > + * size_class lookup, so we may endup in a huge class yet zs_huge_object()
> > > + * returned 'false'.
> > > + */
> > 
> > Can you please reformat this comment as kernel-doc?
> 
> Is this - Documentation/doc-guide/kernel-doc.rst - the right thing
> to use as a reference?

Yes.  I just sent a revision to it that makes it (I think) a little
easier to read.  Try this version:


Writing kernel-doc comments
===========================

The Linux kernel source files may contain structured documentation
comments in the kernel-doc format to describe the functions, types
and design of the code. It is easier to keep documentation up-to-date
when it is embedded in source files.

.. note:: The kernel-doc format is deceptively similar to javadoc,
   gtk-doc or Doxygen, yet distinctively different, for historical
   reasons. The kernel source contains tens of thousands of kernel-doc
   comments. Please stick to the style described here.

The kernel-doc structure is extracted from the comments, and proper
`Sphinx C Domain`_ function and type descriptions with anchors are
generated from them. The descriptions are filtered for special kernel-doc
highlights and cross-references. See below for details.

.. _Sphinx C Domain: http://www.sphinx-doc.org/en/stable/domains.html

Every function that is exported to loadable modules using
``EXPORT_SYMBOL`` or ``EXPORT_SYMBOL_GPL`` should have a kernel-doc
comment. Functions and data structures in header files which are intended
to be used by modules should also have kernel-doc comments.

It is good practice to also provide kernel-doc formatted documentation
for functions externally visible to other kernel files (not marked
``static``). We also recommend providing kernel-doc formatted
documentation for private (file ``static``) routines, for consistency of
kernel source code layout. This is lower priority and at the discretion
of the maintainer of that kernel source file.

How to format kernel-doc comments
---------------------------------

The opening comment mark ``/**`` is used for kernel-doc comments. The
``kernel-doc`` tool will extract comments marked this way. The rest of
the comment is formatted like a normal multi-line comment with a column
of asterisks on the left side, closing with ``*/`` on a line by itself.

The function and type kernel-doc comments should be placed just before
the function or type being described in order to maximise the chance
that somebody changing the code will also change the documentation. The
overview kernel-doc comments may be placed anywhere at the top indentation
level.

Function documentation
----------------------

The general format of a function and function-like macro kernel-doc comment is::

  /**
   * function_name() - Brief description of function.
   * @arg1: Describe the first argument.
   * @arg2: Describe the second argument.
   *        One can provide multiple line descriptions
   *        for arguments.
   *
   * A longer description, with more discussion of the function function_name()
   * that might be useful to those using or modifying it. Begins with an
   * empty comment line, and may include additional embedded empty
   * comment lines.
   *
   * The longer description may have multiple paragraphs.
   *
   * Context: Describes whether the function can sleep, what locks it takes,
   *          releases, or expects to be held. It can extend over multiple
   *          lines.
   * Return: Describe the return value of foobar.
   *
   * The return value description can also have multiple paragraphs, and should
   * be placed at the end of the comment block.
   */

The brief description following the function name may span multiple lines, and
ends with an argument description, a blank comment line, or the end of the
comment block.

Function parameters
~~~~~~~~~~~~~~~~~~~

Each function argument should be described in order, immediately following
the short function description.  Do not leave a blank line between the
function description and the arguments, nor between the arguments.

Each ``@argument:`` description may span multiple lines.

.. note::

   If the ``@argument`` description has multiple lines, the continuation
   of the description should start at the same column as the previous line::

      * @argument: some long description
      *            that continues on next lines

   or::

      * @argument:
      *		some long description
      *		that continues on next lines

If a function has a variable number of arguments, its description should
be written in kernel-doc notation as::

      * @...: description

Function context
~~~~~~~~~~~~~~~~

The context in which a function can be called should be described in a
section named ``Context``. This should include whether the function
sleeps or can be called from interrupt context, as well as what locks
it takes, releases and expects to be held by its caller.

Examples::

  * Context: Any context.
  * Context: Any context. Takes and releases the RCU lock.
  * Context: Any context. Expects <lock> to be held by caller.
  * Context: Process context. May sleep if @gfp flags permit.
  * Context: Process context. Takes and releases <mutex>.
  * Context: Softirq or process context. Takes and releases <lock>, BH-safe.
  * Context: Interrupt context.

Return values
~~~~~~~~~~~~~

The return value, if any, should be described in a dedicated section
named ``Return``.

.. note::

  #) The multi-line descriptive text you provide does *not* recognize
     line breaks, so if you try to format some text nicely, as in::

	* Return:
	* 0 - OK
	* -EINVAL - invalid argument
	* -ENOMEM - out of memory

     this will all run together and produce::

	Return: 0 - OK -EINVAL - invalid argument -ENOMEM - out of memory

     So, in order to produce the desired line breaks, you need to use a
     ReST list, e. g.::

      * Return:
      * * 0		- OK to runtime suspend the device
      * * -EBUSY	- Device should not be runtime suspended

  #) If the descriptive text you provide has lines that begin with
     some phrase followed by a colon, each of those phrases will be taken
     as a new section heading, which probably won't produce the desired
     effect.

Structure, union, and enumeration documentation
-----------------------------------------------

The general format of a struct, union, and enum kernel-doc comment is::

  /**
   * struct struct_name - Brief description.
   * @member1: Description of member1.
   * @member2: Description of member2.
   *           One can provide multiple line descriptions
   *           for members.
   *
   * Description of the structure.
   */

You can replace the ``struct`` in the above example with ``union`` or
``enum``  to describe unions or enums. ``member`` is used to mean struct
and union member names as well as enumerations in an enum.

The brief description following the structure name may span multiple
lines, and ends with a member description, a blank comment line, or the
end of the comment block.

Members
~~~~~~~

Members of structs, unions and enums should be documented the same way
as function parameters; they immediately succeed the short description
and may be multi-line.

Inside a struct or union description, you can use the ``private:`` and
``public:`` comment tags. Structure fields that are inside a ``private:``
area are not listed in the generated output documentation.

The ``private:`` and ``public:`` tags must begin immediately following a
``/*`` comment marker. They may optionally include comments between the
``:`` and the ending ``*/`` marker.

Example::

  /**
   * struct my_struct - short description
   * @a: first member
   * @b: second member
   * @d: fourth member
   *
   * Longer description
   */
  struct my_struct {
      int a;
      int b;
  /* private: internal use only */
      int c;
  /* public: the next one is public */
      int d;
  };

In-line member documentation comments
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The structure members may also be documented in-line within the definition.
There are two styles, single-line comments where both the opening ``/**`` and
closing ``*/`` are on the same line, and multi-line comments where they are each
on a line of their own, like all other kernel-doc comments::

  /**
   * struct foo - Brief description.
   * @foo: The Foo member.
   */
  struct foo {
        int foo;
        /**
         * @bar: The Bar member.
         */
        int bar;
        /**
         * @baz: The Baz member.
         *
         * Here, the member description may contain several paragraphs.
         */
        int baz;
        /** @foobar: Single line description. */
        int foobar;
  }

Nested structs/unions
~~~~~~~~~~~~~~~~~~~~~

It is possible to document nested structs and unions, like::

      /**
       * struct nested_foobar - a struct with nested unions and structs
       * @memb1: first member of anonymous union/anonymous struct
       * @memb2: second member of anonymous union/anonymous struct
       * @memb3: third member of anonymous union/anonymous struct
       * @memb4: fourth member of anonymous union/anonymous struct
       * @bar.st1.memb1: first member of struct st1 on union bar
       * @bar.st1.memb2: second member of struct st1 on union bar
       * @bar.st2.memb1: first member of struct st2 on union bar
       * @bar.st2.memb2: second member of struct st2 on union bar
      struct nested_foobar {
        /* Anonymous union/struct*/
        union {
          struct {
            int memb1;
            int memb2;
        }
          struct {
            void *memb3;
            int memb4;
          }
        }
        union {
          struct {
            int memb1;
            int memb2;
          } st1;
          struct {
            void *memb1;
            int memb2;
          } st2;
        } bar;
      };

.. note::

   #) When documenting nested structs or unions, if the struct/union ``foo``
      is named, the member ``bar`` inside it should be documented as
      ``@foo.bar:``
   #) When the nested struct/union is anonymous, the member ``bar`` in it
      should be documented as ``@bar:``

Typedef documentation
---------------------

The general format of a typedef kernel-doc comment is::

  /**
   * typedef type_name - Brief description.
   *
   * Description of the type.
   */

Typedefs with function prototypes can also be documented::

  /**
   * typedef type_name - Brief description.
   * @arg1: description of arg1
   * @arg2: description of arg2
   *
   * Description of the type.
   *
   * Context: Locking context.
   * Return: Meaning of the return value.
   */
   typedef void (*type_name)(struct v4l2_ctrl *arg1, void *arg2);

Highlights and cross-references
-------------------------------

The following special patterns are recognized in the kernel-doc comment
descriptive text and converted to proper reStructuredText markup and `Sphinx C
Domain`_ references.

.. attention:: The below are **only** recognized within kernel-doc comments,
	       **not** within normal reStructuredText documents.

``funcname()``
  Function reference.

``@parameter``
  Name of a function parameter. (No cross-referencing, just formatting.)

``%CONST``
  Name of a constant. (No cross-referencing, just formatting.)

````literal````
  A literal block that should be handled as-is. The output will use a
  ``monospaced font``.

  Useful if you need to use special characters that would otherwise have some
  meaning either by kernel-doc script of by reStructuredText.

  This is particularly useful if you need to use things like ``%ph`` inside
  a function description.

``$ENVVAR``
  Name of an environment variable. (No cross-referencing, just formatting.)

``&struct name``
  Structure reference.

``&enum name``
  Enum reference.

``&typedef name``
  Typedef reference.

``&struct_name->member`` or ``&struct_name.member``
  Structure or union member reference. The cross-reference will be to the struct
  or union definition, not the member directly.

``&name``
  A generic type reference. Prefer using the full reference described above
  instead. This is mostly for legacy comments.

Cross-referencing from reStructuredText
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

To cross-reference the functions and types defined in the kernel-doc comments
from reStructuredText documents, please use the `Sphinx C Domain`_
references. For example::

  See function :c:func:`foo` and struct/union/enum/typedef :c:type:`bar`.

While the type reference works with just the type name, without the
struct/union/enum/typedef part in front, you may want to use::

  See :c:type:`struct foo <foo>`.
  See :c:type:`union bar <bar>`.
  See :c:type:`enum baz <baz>`.
  See :c:type:`typedef meh <meh>`.

This will produce prettier links, and is in line with how kernel-doc does the
cross-references.

For further details, please refer to the `Sphinx C Domain`_ documentation.

Overview documentation comments
-------------------------------

To facilitate having source code and comments close together, you can include
kernel-doc documentation blocks that are free-form comments instead of being
kernel-doc for functions, structures, unions, enums, or typedefs. This could be
used for something like a theory of operation for a driver or library code, for
example.

This is done by using a ``DOC:`` section keyword with a section title.

The general format of an overview or high-level documentation comment is::

  /**
   * DOC: Theory of Operation
   *
   * The whizbang foobar is a dilly of a gizmo. It can do whatever you
   * want it to do, at any time. It reads your mind. Here's how it works.
   *
   * foo bar splat
   *
   * The only drawback to this gizmo is that is can sometimes damage
   * hardware, software, or its subject(s).
   */

The title following ``DOC:`` acts as a heading within the source file, but also
as an identifier for extracting the documentation comment. Thus, the title must
be unique within the file.

Style guide
-----------

It is helpful to maintain a consistent style across the kernel.  It helps
the reader who can rely on conventions to understand a part of the kernel
which is new to them.  It also helps the writer who can simply copy and
paste function parameter descriptions instead of coming up with new and
excitingly different ways to say the same thing.

Context names
~~~~~~~~~~~~~

Refer to process context, softirq context and hardirq context.  Interrupt
context refers to both hard and soft interrupts.  Bottom half context
is deprecated; it is a synonym for softirq context.  Tasklet context
should not be used; tasklets run in softirq context.  Do not use user
context; this is a synonym for process context.

Common function parameters
~~~~~~~~~~~~~~~~~~~~~~~~~~

If you pass ``GFP_`` flags to a function, name the parameter ``gfp`` and
document it like this::

  * @gfp: Memory allocation flags.

If your function takes a ``struct file *`` argument, name the parameter ``file``
(unless you have a good reason to use another name) and document it like
this::

  * @file: File pointer.

If your function takes a ``struct dentry *`` argument, name the
parameter ``dentry`` and document it like this::

  * @dentry: Directory Entry pointer.

If your function takes a ``struct inode *`` argument, name the parameter
``inode`` and document it like this::

  * @inode: Inode pointer.

Including kernel-doc comments
=============================

The documentation comments may be included in any of the reStructuredText
documents using a dedicated kernel-doc Sphinx directive extension.

The kernel-doc directive is of the format::

  .. kernel-doc:: source
     :option:

The *source* is the path to a source file, relative to the kernel source
tree. The following directive options are supported:

export: *[source-pattern ...]*
  Include documentation for all functions in *source* that have been exported
  using ``EXPORT_SYMBOL`` or ``EXPORT_SYMBOL_GPL`` either in *source* or in any
  of the files specified by *source-pattern*.

  The *source-pattern* is useful when the kernel-doc comments have been placed
  in header files, while ``EXPORT_SYMBOL`` and ``EXPORT_SYMBOL_GPL`` are next to
  the function definitions.

  Examples::

    .. kernel-doc:: lib/bitmap.c
       :export:

    .. kernel-doc:: include/net/mac80211.h
       :export: net/mac80211/*.c

internal: *[source-pattern ...]*
  Include documentation for all functions and types in *source* that have
  **not** been exported using ``EXPORT_SYMBOL`` or ``EXPORT_SYMBOL_GPL`` either
  in *source* or in any of the files specified by *source-pattern*.

  Example::

    .. kernel-doc:: drivers/gpu/drm/i915/intel_audio.c
       :internal:

doc: *title*
  Include documentation for the ``DOC:`` paragraph identified by *title* in
  *source*. Spaces are allowed in *title*; do not quote the *title*. The *title*
  is only used as an identifier for the paragraph, and is not included in the
  output. Please make sure to have an appropriate heading in the enclosing
  reStructuredText document.

  Example::

    .. kernel-doc:: drivers/gpu/drm/i915/intel_audio.c
       :doc: High Definition Audio over HDMI and Display Port

functions: *function* *[...]*
  Include documentation for each *function* in *source*.

  Example::

    .. kernel-doc:: lib/bitmap.c
       :functions: bitmap_parselist bitmap_parselist_user

Without options, the kernel-doc directive includes all documentation comments
from the source file.

The kernel-doc extension is included in the kernel source tree, at
``Documentation/sphinx/kerneldoc.py``. Internally, it uses the
``scripts/kernel-doc`` script to extract the documentation comments from the
source.

.. _kernel_doc:

How to use kernel-doc to generate man pages
-------------------------------------------

If you just want to use kernel-doc to generate man pages you can do this
from the kernel git tree::

  $ scripts/kernel-doc -man $(git grep -l '/\*\*' -- :^Documentation :^tools) | scripts/split-man.pl /tmp/man

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

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/08/18 20:10), Matthew Wilcox wrote:
> > > > +/*
> > > > + * Check if the object's size falls into huge_class area. We must take
> > > > + * ZS_HANDLE_SIZE into account and test the actual size we are going to
> > > > + * use up. zs_malloc() unconditionally adds handle size before it performs
> > > > + * size_class lookup, so we may endup in a huge class yet zs_huge_object()
> > > > + * returned 'false'.
> > > > + */
> > > 
> > > Can you please reformat this comment as kernel-doc?
> > 
> > Is this - Documentation/doc-guide/kernel-doc.rst - the right thing
> > to use as a reference?
> 
> Yes.  I just sent a revision to it that makes it (I think) a little
> easier to read.  Try this version:

That's helpful, thanks! Will take a look and re-spin the patch.

	-ss

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/08/18 20:10), Matthew Wilcox wrote:
> > > > +/*
> > > > + * Check if the object's size falls into huge_class area. We must take
> > > > + * ZS_HANDLE_SIZE into account and test the actual size we are going to
> > > > + * use up. zs_malloc() unconditionally adds handle size before it performs
> > > > + * size_class lookup, so we may endup in a huge class yet zs_huge_object()
> > > > + * returned 'false'.
> > > > + */
> > > 
> > > Can you please reformat this comment as kernel-doc?
> > 
> > Is this - Documentation/doc-guide/kernel-doc.rst - the right thing
> > to use as a reference?
> 
> Yes.  I just sent a revision to it that makes it (I think) a little
> easier to read.  Try this version:

That's helpful, thanks! Will take a look and re-spin the patch.

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

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/08/18 20:10), Matthew Wilcox wrote:
[..]
> Examples::
> 
>   * Context: Any context.
>   * Context: Any context. Takes and releases the RCU lock.
>   * Context: Any context. Expects <lock> to be held by caller.
>   * Context: Process context. May sleep if @gfp flags permit.
>   * Context: Process context. Takes and releases <mutex>.
>   * Context: Softirq or process context. Takes and releases <lock>, BH-safe.
>   * Context: Interrupt context.

I assume that  <mutex>  spelling serves as a placeholder and should be
replaced with a lock name in a real comment. E.g.

	Takes and releases audit_cmd_mutex.

or should it actually be

	Takes and releases <audit_cmd_mutex>.




So below is zs_huge_object() documentation I came up with:

---

+/**
+ * zs_huge_object() - Test if a compressed object's size is too big for normal
+ *                    zspool classes and it will be stored in a huge class.
+ * @sz: Size in bytes of the compressed object.
+ *
+ * The functions checks if the object's size falls into huge_class area.
+ * We must take ZS_HANDLE_SIZE into account and test the actual size we
+ * are going to use up, because zs_malloc() unconditionally adds the
+ * handle size before it performs size_class lookup.
+ *
+ * Context: Any context.
+ *
+ * Return:
+ * * true  - The object's size is too big, it will be stored in a huge class.
+ * * false - The object will be store in normal zspool classes.
+ */
---

looks OK?

	-ss

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/08/18 20:10), Matthew Wilcox wrote:
[..]
> Examples::
> 
>   * Context: Any context.
>   * Context: Any context. Takes and releases the RCU lock.
>   * Context: Any context. Expects <lock> to be held by caller.
>   * Context: Process context. May sleep if @gfp flags permit.
>   * Context: Process context. Takes and releases <mutex>.
>   * Context: Softirq or process context. Takes and releases <lock>, BH-safe.
>   * Context: Interrupt context.

I assume that  <mutex>  spelling serves as a placeholder and should be
replaced with a lock name in a real comment. E.g.

	Takes and releases audit_cmd_mutex.

or should it actually be

	Takes and releases <audit_cmd_mutex>.




So below is zs_huge_object() documentation I came up with:

---

+/**
+ * zs_huge_object() - Test if a compressed object's size is too big for normal
+ *                    zspool classes and it will be stored in a huge class.
+ * @sz: Size in bytes of the compressed object.
+ *
+ * The functions checks if the object's size falls into huge_class area.
+ * We must take ZS_HANDLE_SIZE into account and test the actual size we
+ * are going to use up, because zs_malloc() unconditionally adds the
+ * handle size before it performs size_class lookup.
+ *
+ * Context: Any context.
+ *
+ * Return:
+ * * true  - The object's size is too big, it will be stored in a huge class.
+ * * false - The object will be store in normal zspool classes.
+ */
---

looks OK?

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

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/09/18 14:36), Sergey Senozhatsky wrote:
> +/**
> + * zs_huge_object() - Test if a compressed object's size is too big for normal
> + *                    zspool classes and it will be stored in a huge class.
> + * @sz: Size in bytes of the compressed object.
> + *
> + * The functions checks if the object's size falls into huge_class area.
> + * We must take ZS_HANDLE_SIZE into account and test the actual size we
> + * are going to use up, because zs_malloc() unconditionally adds the
> + * handle size before it performs size_class lookup.
> + *
> + * Context: Any context.
> + *
> + * Return:
> + * * true  - The object's size is too big, it will be stored in a huge class.
> + * * false - The object will be store in normal zspool classes.
> + */
> ---
> 
> looks OK?

Modulo silly typos... and broken English.

	-ss

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/09/18 14:36), Sergey Senozhatsky wrote:
> +/**
> + * zs_huge_object() - Test if a compressed object's size is too big for normal
> + *                    zspool classes and it will be stored in a huge class.
> + * @sz: Size in bytes of the compressed object.
> + *
> + * The functions checks if the object's size falls into huge_class area.
> + * We must take ZS_HANDLE_SIZE into account and test the actual size we
> + * are going to use up, because zs_malloc() unconditionally adds the
> + * handle size before it performs size_class lookup.
> + *
> + * Context: Any context.
> + *
> + * Return:
> + * * true  - The object's size is too big, it will be stored in a huge class.
> + * * false - The object will be store in normal zspool classes.
> + */
> ---
> 
> looks OK?

Modulo silly typos... and broken English.

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

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Mike Rapoport]

On Fri, Feb 09, 2018 at 02:36:30PM +0900, Sergey Senozhatsky wrote:
> On (02/08/18 20:10), Matthew Wilcox wrote:
> [..]
> > Examples::
> > 
> >   * Context: Any context.
> >   * Context: Any context. Takes and releases the RCU lock.
> >   * Context: Any context. Expects <lock> to be held by caller.
> >   * Context: Process context. May sleep if @gfp flags permit.
> >   * Context: Process context. Takes and releases <mutex>.
> >   * Context: Softirq or process context. Takes and releases <lock>, BH-safe.
> >   * Context: Interrupt context.
> 
> I assume that  <mutex>  spelling serves as a placeholder and should be
> replaced with a lock name in a real comment. E.g.
> 
> 	Takes and releases audit_cmd_mutex.
> 
> or should it actually be
> 
> 	Takes and releases <audit_cmd_mutex>.
> 
> 
> 
> 
> So below is zs_huge_object() documentation I came up with:
>
> ---
> 
> +/**
> + * zs_huge_object() - Test if a compressed object's size is too big for normal
> + *                    zspool classes and it will be stored in a huge class.

Maybe "it should be stored ..."?

> + * @sz: Size in bytes of the compressed object.
> + *
> + * The functions checks if the object's size falls into huge_class area.
> + * We must take ZS_HANDLE_SIZE into account and test the actual size we

                ^ %ZS_HANDLE_SIZE

> + * are going to use up, because zs_malloc() unconditionally adds the

I think 's/use up/use/' here

> + * handle size before it performs size_class lookup.

                                   ^ &size_class
> + *
> + * Context: Any context.
> + *
> + * Return:
> + * * true  - The object's size is too big, it will be stored in a huge class.
> + * * false - The object will be store in normal zspool classes.
> + */
> ---
> 
> looks OK?
> 
> 	-ss
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Mike Rapoport]

On Fri, Feb 09, 2018 at 02:36:30PM +0900, Sergey Senozhatsky wrote:
> On (02/08/18 20:10), Matthew Wilcox wrote:
> [..]
> > Examples::
> > 
> >   * Context: Any context.
> >   * Context: Any context. Takes and releases the RCU lock.
> >   * Context: Any context. Expects <lock> to be held by caller.
> >   * Context: Process context. May sleep if @gfp flags permit.
> >   * Context: Process context. Takes and releases <mutex>.
> >   * Context: Softirq or process context. Takes and releases <lock>, BH-safe.
> >   * Context: Interrupt context.
> 
> I assume that  <mutex>  spelling serves as a placeholder and should be
> replaced with a lock name in a real comment. E.g.
> 
> 	Takes and releases audit_cmd_mutex.
> 
> or should it actually be
> 
> 	Takes and releases <audit_cmd_mutex>.
> 
> 
> 
> 
> So below is zs_huge_object() documentation I came up with:
>
> ---
> 
> +/**
> + * zs_huge_object() - Test if a compressed object's size is too big for normal
> + *                    zspool classes and it will be stored in a huge class.

Maybe "it should be stored ..."?

> + * @sz: Size in bytes of the compressed object.
> + *
> + * The functions checks if the object's size falls into huge_class area.
> + * We must take ZS_HANDLE_SIZE into account and test the actual size we

                ^ %ZS_HANDLE_SIZE

> + * are going to use up, because zs_malloc() unconditionally adds the

I think 's/use up/use/' here

> + * handle size before it performs size_class lookup.

                                   ^ &size_class
> + *
> + * Context: Any context.
> + *
> + * Return:
> + * * true  - The object's size is too big, it will be stored in a huge class.
> + * * false - The object will be store in normal zspool classes.
> + */
> ---
> 
> looks OK?
> 
> 	-ss
> 

-- 
Sincerely yours,
Mike.

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

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/09/18 13:11), Mike Rapoport wrote:
[..]
> > +/**
> > + * zs_huge_object() - Test if a compressed object's size is too big for normal
> > + *                    zspool classes and it will be stored in a huge class.
> 
> Maybe "it should be stored ..."?

Agreed.

> > + * @sz: Size in bytes of the compressed object.
> > + *
> > + * The functions checks if the object's size falls into huge_class area.
> > + * We must take ZS_HANDLE_SIZE into account and test the actual size we
> 
>                 ^ %ZS_HANDLE_SIZE

Indeed. ``%CONST``

> > + * are going to use up, because zs_malloc() unconditionally adds the
> 
> I think 's/use up/use/' here

Agreed.

> > + * handle size before it performs size_class lookup.
> 
>                                    ^ &size_class

OK. ``&struct name``

Thanks for reviewing it!

	-ss

Re: [PATCH 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/09/18 13:11), Mike Rapoport wrote:
[..]
> > +/**
> > + * zs_huge_object() - Test if a compressed object's size is too big for normal
> > + *                    zspool classes and it will be stored in a huge class.
> 
> Maybe "it should be stored ..."?

Agreed.

> > + * @sz: Size in bytes of the compressed object.
> > + *
> > + * The functions checks if the object's size falls into huge_class area.
> > + * We must take ZS_HANDLE_SIZE into account and test the actual size we
> 
>                 ^ %ZS_HANDLE_SIZE

Indeed. ``%CONST``

> > + * are going to use up, because zs_malloc() unconditionally adds the
> 
> I think 's/use up/use/' here

Agreed.

> > + * handle size before it performs size_class lookup.
> 
>                                    ^ &size_class

OK. ``&struct name``

Thanks for reviewing it!

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

[PATCHv2 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

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_object() 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            | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 57a8e98f2708..9a1baf673cc1 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);
 
+bool zs_huge_object(size_t sz);
+
 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 c3013505c305..922180183ca3 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
  * (see: fix_fullness_group())
  */
 static const int fullness_threshold_frac = 4;
+static size_t zs_huge_class_size;
 
 struct size_class {
 	spinlock_t lock;
@@ -1417,6 +1418,28 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
 }
 EXPORT_SYMBOL_GPL(zs_unmap_object);
 
+/**
+ * zs_huge_object() - Test if a compressed object's size is too big for normal
+ *                    zspool classes and it shall be stored in a huge class.
+ * @sz: Size of the compressed object (in bytes).
+ *
+ * The function checks if the object's size falls into huge_class
+ * area. We must take handle size into account and test the actual
+ * size we are going to use, because zs_malloc() unconditionally
+ * adds %ZS_HANDLE_SIZE before it performs %size_class lookup.
+ *
+ * Context: Any context.
+ *
+ * Return:
+ * * true  - The object's size is too big, it will be stored in a huge class.
+ * * false - The object will be store in normal zspool classes.
+ */
+bool zs_huge_object(size_t sz)
+{
+	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
+}
+EXPORT_SYMBOL_GPL(zs_huge_object);
+
 static unsigned long obj_malloc(struct size_class *class,
 				struct zspage *zspage, unsigned long handle)
 {
@@ -2404,6 +2427,9 @@ struct zs_pool *zs_create_pool(const char *name)
 			INIT_LIST_HEAD(&class->fullness_list[fullness]);
 
 		prev_class = class;
+		if (pages_per_zspage == 1 && objs_per_zspage == 1
+				&& !zs_huge_class_size)
+			zs_huge_class_size = size;
 	}
 
 	/* debug only, don't abort if it fails */
-- 
2.16.1

[PATCHv2 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

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_object() 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            | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 57a8e98f2708..9a1baf673cc1 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);
 
+bool zs_huge_object(size_t sz);
+
 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 c3013505c305..922180183ca3 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
  * (see: fix_fullness_group())
  */
 static const int fullness_threshold_frac = 4;
+static size_t zs_huge_class_size;
 
 struct size_class {
 	spinlock_t lock;
@@ -1417,6 +1418,28 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
 }
 EXPORT_SYMBOL_GPL(zs_unmap_object);
 
+/**
+ * zs_huge_object() - Test if a compressed object's size is too big for normal
+ *                    zspool classes and it shall be stored in a huge class.
+ * @sz: Size of the compressed object (in bytes).
+ *
+ * The function checks if the object's size falls into huge_class
+ * area. We must take handle size into account and test the actual
+ * size we are going to use, because zs_malloc() unconditionally
+ * adds %ZS_HANDLE_SIZE before it performs %size_class lookup.
+ *
+ * Context: Any context.
+ *
+ * Return:
+ * * true  - The object's size is too big, it will be stored in a huge class.
+ * * false - The object will be store in normal zspool classes.
+ */
+bool zs_huge_object(size_t sz)
+{
+	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
+}
+EXPORT_SYMBOL_GPL(zs_huge_object);
+
 static unsigned long obj_malloc(struct size_class *class,
 				struct zspage *zspage, unsigned long handle)
 {
@@ -2404,6 +2427,9 @@ struct zs_pool *zs_create_pool(const char *name)
 			INIT_LIST_HEAD(&class->fullness_list[fullness]);
 
 		prev_class = class;
+		if (pages_per_zspage == 1 && objs_per_zspage == 1
+				&& !zs_huge_class_size)
+			zs_huge_class_size = size;
 	}
 
 	/* debug only, don't abort if it fails */
-- 
2.16.1

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

Re: [PATCHv2 1/2] zsmalloc: introduce zs_huge_object() function

[Mike Rapoport]

Some more nitpicks :)

On Sat, Feb 10, 2018 at 05:23:21PM +0900, Sergey Senozhatsky wrote:
> 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_object() 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            | 26 ++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
> index 57a8e98f2708..9a1baf673cc1 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);
> 
> +bool zs_huge_object(size_t sz);
> +
>  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 c3013505c305..922180183ca3 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
>   * (see: fix_fullness_group())
>   */
>  static const int fullness_threshold_frac = 4;
> +static size_t zs_huge_class_size;
> 
>  struct size_class {
>  	spinlock_t lock;
> @@ -1417,6 +1418,28 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
>  }
>  EXPORT_SYMBOL_GPL(zs_unmap_object);
> 
> +/**
> + * zs_huge_object() - Test if a compressed object's size is too big for normal
> + *                    zspool classes and it shall be stored in a huge class.

I think "is should be stored" is more appropriate

> + * @sz: Size of the compressed object (in bytes).
> + *
> + * The function checks if the object's size falls into huge_class
> + * area. We must take handle size into account and test the actual
> + * size we are going to use, because zs_malloc() unconditionally
> + * adds %ZS_HANDLE_SIZE before it performs %size_class lookup.

                                            ^ &size_class ;-)

> + *
> + * Context: Any context.
> + *
> + * Return:
> + * * true  - The object's size is too big, it will be stored in a huge class.
> + * * false - The object will be store in normal zspool classes.
> + */
> +bool zs_huge_object(size_t sz)
> +{
> +	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
> +}
> +EXPORT_SYMBOL_GPL(zs_huge_object);
> +
>  static unsigned long obj_malloc(struct size_class *class,
>  				struct zspage *zspage, unsigned long handle)
>  {
> @@ -2404,6 +2427,9 @@ struct zs_pool *zs_create_pool(const char *name)
>  			INIT_LIST_HEAD(&class->fullness_list[fullness]);
> 
>  		prev_class = class;
> +		if (pages_per_zspage == 1 && objs_per_zspage == 1
> +				&& !zs_huge_class_size)
> +			zs_huge_class_size = size;
>  	}
> 
>  	/* debug only, don't abort if it fails */
> -- 
> 2.16.1
> 

-- 
Sincerely yours,
Mike.

Re: [PATCHv2 1/2] zsmalloc: introduce zs_huge_object() function

[Mike Rapoport]

Some more nitpicks :)

On Sat, Feb 10, 2018 at 05:23:21PM +0900, Sergey Senozhatsky wrote:
> 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_object() 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            | 26 ++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
> index 57a8e98f2708..9a1baf673cc1 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);
> 
> +bool zs_huge_object(size_t sz);
> +
>  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 c3013505c305..922180183ca3 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
>   * (see: fix_fullness_group())
>   */
>  static const int fullness_threshold_frac = 4;
> +static size_t zs_huge_class_size;
> 
>  struct size_class {
>  	spinlock_t lock;
> @@ -1417,6 +1418,28 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
>  }
>  EXPORT_SYMBOL_GPL(zs_unmap_object);
> 
> +/**
> + * zs_huge_object() - Test if a compressed object's size is too big for normal
> + *                    zspool classes and it shall be stored in a huge class.

I think "is should be stored" is more appropriate

> + * @sz: Size of the compressed object (in bytes).
> + *
> + * The function checks if the object's size falls into huge_class
> + * area. We must take handle size into account and test the actual
> + * size we are going to use, because zs_malloc() unconditionally
> + * adds %ZS_HANDLE_SIZE before it performs %size_class lookup.

                                            ^ &size_class ;-)

> + *
> + * Context: Any context.
> + *
> + * Return:
> + * * true  - The object's size is too big, it will be stored in a huge class.
> + * * false - The object will be store in normal zspool classes.
> + */
> +bool zs_huge_object(size_t sz)
> +{
> +	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
> +}
> +EXPORT_SYMBOL_GPL(zs_huge_object);
> +
>  static unsigned long obj_malloc(struct size_class *class,
>  				struct zspage *zspage, unsigned long handle)
>  {
> @@ -2404,6 +2427,9 @@ struct zs_pool *zs_create_pool(const char *name)
>  			INIT_LIST_HEAD(&class->fullness_list[fullness]);
> 
>  		prev_class = class;
> +		if (pages_per_zspage == 1 && objs_per_zspage == 1
> +				&& !zs_huge_class_size)
> +			zs_huge_class_size = size;
>  	}
> 
>  	/* debug only, don't abort if it fails */
> -- 
> 2.16.1
> 

-- 
Sincerely yours,
Mike.

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

Re: [PATCHv2 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/11/18 09:05), Mike Rapoport wrote:
[..]
> > +/**
> > + * zs_huge_object() - Test if a compressed object's size is too big for normal
> > + *                    zspool classes and it shall be stored in a huge class.
> 
> I think "is should be stored" is more appropriate
> 
> > + * @sz: Size of the compressed object (in bytes).
> > + *
> > + * The function checks if the object's size falls into huge_class
> > + * area. We must take handle size into account and test the actual
> > + * size we are going to use, because zs_malloc() unconditionally
> > + * adds %ZS_HANDLE_SIZE before it performs %size_class lookup.
> 
>                                             ^ &size_class ;-)

I'm sorry, Mike. Lost in branches/versions and sent out a half baked
version.

	-ss

Re: [PATCHv2 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/11/18 09:05), Mike Rapoport wrote:
[..]
> > +/**
> > + * zs_huge_object() - Test if a compressed object's size is too big for normal
> > + *                    zspool classes and it shall be stored in a huge class.
> 
> I think "is should be stored" is more appropriate
> 
> > + * @sz: Size of the compressed object (in bytes).
> > + *
> > + * The function checks if the object's size falls into huge_class
> > + * area. We must take handle size into account and test the actual
> > + * size we are going to use, because zs_malloc() unconditionally
> > + * adds %ZS_HANDLE_SIZE before it performs %size_class lookup.
> 
>                                             ^ &size_class ;-)

I'm sorry, Mike. Lost in branches/versions and sent out a half baked
version.

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

[PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

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_object() 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            | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 57a8e98f2708..9a1baf673cc1 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);
 
+bool zs_huge_object(size_t sz);
+
 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 c3013505c305..e43fc6ebb8e1 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
  * (see: fix_fullness_group())
  */
 static const int fullness_threshold_frac = 4;
+static size_t zs_huge_class_size;
 
 struct size_class {
 	spinlock_t lock;
@@ -1417,6 +1418,28 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
 }
 EXPORT_SYMBOL_GPL(zs_unmap_object);
 
+/**
+ * zs_huge_object() - Test if a compressed object's size is too big for normal
+ *                    zspool classes and it should be stored in a huge class.
+ * @sz: Size of the compressed object (in bytes).
+ *
+ * The function checks if the object's size falls into huge_class
+ * area. We must take handle size into account and test the actual
+ * size we are going to use, because zs_malloc() unconditionally
+ * adds %ZS_HANDLE_SIZE before it performs &size_class lookup.
+ *
+ * Context: Any context.
+ *
+ * Return:
+ * * true  - The object is too big, it will be stored in the huge class.
+ * * false - The object will be stored in a normal zspool class.
+ */
+bool zs_huge_object(size_t sz)
+{
+	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
+}
+EXPORT_SYMBOL_GPL(zs_huge_object);
+
 static unsigned long obj_malloc(struct size_class *class,
 				struct zspage *zspage, unsigned long handle)
 {
@@ -2404,6 +2427,9 @@ struct zs_pool *zs_create_pool(const char *name)
 			INIT_LIST_HEAD(&class->fullness_list[fullness]);
 
 		prev_class = class;
+		if (pages_per_zspage == 1 && objs_per_zspage == 1
+				&& !zs_huge_class_size)
+			zs_huge_class_size = size;
 	}
 
 	/* debug only, don't abort if it fails */
-- 
2.16.1

[PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

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_object() 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            | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 57a8e98f2708..9a1baf673cc1 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);
 
+bool zs_huge_object(size_t sz);
+
 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 c3013505c305..e43fc6ebb8e1 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
  * (see: fix_fullness_group())
  */
 static const int fullness_threshold_frac = 4;
+static size_t zs_huge_class_size;
 
 struct size_class {
 	spinlock_t lock;
@@ -1417,6 +1418,28 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
 }
 EXPORT_SYMBOL_GPL(zs_unmap_object);
 
+/**
+ * zs_huge_object() - Test if a compressed object's size is too big for normal
+ *                    zspool classes and it should be stored in a huge class.
+ * @sz: Size of the compressed object (in bytes).
+ *
+ * The function checks if the object's size falls into huge_class
+ * area. We must take handle size into account and test the actual
+ * size we are going to use, because zs_malloc() unconditionally
+ * adds %ZS_HANDLE_SIZE before it performs &size_class lookup.
+ *
+ * Context: Any context.
+ *
+ * Return:
+ * * true  - The object is too big, it will be stored in the huge class.
+ * * false - The object will be stored in a normal zspool class.
+ */
+bool zs_huge_object(size_t sz)
+{
+	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
+}
+EXPORT_SYMBOL_GPL(zs_huge_object);
+
 static unsigned long obj_malloc(struct size_class *class,
 				struct zspage *zspage, unsigned long handle)
 {
@@ -2404,6 +2427,9 @@ struct zs_pool *zs_create_pool(const char *name)
 			INIT_LIST_HEAD(&class->fullness_list[fullness]);
 
 		prev_class = class;
+		if (pages_per_zspage == 1 && objs_per_zspage == 1
+				&& !zs_huge_class_size)
+			zs_huge_class_size = size;
 	}
 
 	/* debug only, don't abort if it fails */
-- 
2.16.1

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

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Minchan Kim]

Hi Sergey,

On Wed, Feb 14, 2018 at 02:57:47PM +0900, Sergey Senozhatsky wrote:
> 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_object() 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            | 26 ++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
> index 57a8e98f2708..9a1baf673cc1 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);
>  
> +bool zs_huge_object(size_t sz);
> +
>  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 c3013505c305..e43fc6ebb8e1 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
>   * (see: fix_fullness_group())
>   */
>  static const int fullness_threshold_frac = 4;
> +static size_t zs_huge_class_size;
>  
>  struct size_class {
>  	spinlock_t lock;
> @@ -1417,6 +1418,28 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
>  }
>  EXPORT_SYMBOL_GPL(zs_unmap_object);
>  
> +/**
> + * zs_huge_object() - Test if a compressed object's size is too big for normal
> + *                    zspool classes and it should be stored in a huge class.
> + * @sz: Size of the compressed object (in bytes).
> + *
> + * The function checks if the object's size falls into huge_class
> + * area. We must take handle size into account and test the actual
> + * size we are going to use, because zs_malloc() unconditionally
> + * adds %ZS_HANDLE_SIZE before it performs &size_class lookup.
> + *
> + * Context: Any context.
> + *
> + * Return:
> + * * true  - The object is too big, it will be stored in the huge class.
> + * * false - The object will be stored in a normal zspool class.
> + */
> +bool zs_huge_object(size_t sz)
> +{
> +	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
> +}
> +EXPORT_SYMBOL_GPL(zs_huge_object);
> +
>  static unsigned long obj_malloc(struct size_class *class,
>  				struct zspage *zspage, unsigned long handle)
>  {
> @@ -2404,6 +2427,9 @@ struct zs_pool *zs_create_pool(const char *name)
>  			INIT_LIST_HEAD(&class->fullness_list[fullness]);
>  
>  		prev_class = class;
> +		if (pages_per_zspage == 1 && objs_per_zspage == 1
> +				&& !zs_huge_class_size)
> +			zs_huge_class_size = size;
>  	}

Sorry for the long delay. I was horribly busy for a few weeks. ;-(

1.
I didn't see your next patch but I bet it is thing to apply this new API to zram.
Normally, it's hard to review without callsite in same patch because sometime
we need to know context where the API is used. In my understanding, if we judge
new API is really complex to get review from others, then, introducing new API
function without callsite is justified enough. In this case, I think it's simple
enough. :)

2. 

Can't zram ask to zsmalloc about what size is for hugeobject from?
With that, zram can save the wartermark in itself and use it.
What I mean is as follows,

zram:
	size_t huge_size = _zs_huge_object(pool);
	..
	..
	if (comp_size >= huge_size)
		memcpy(dst, src, 4K);
	

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Minchan Kim]

Hi Sergey,

On Wed, Feb 14, 2018 at 02:57:47PM +0900, Sergey Senozhatsky wrote:
> 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_object() 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            | 26 ++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
> index 57a8e98f2708..9a1baf673cc1 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);
>  
> +bool zs_huge_object(size_t sz);
> +
>  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 c3013505c305..e43fc6ebb8e1 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -192,6 +192,7 @@ static struct vfsmount *zsmalloc_mnt;
>   * (see: fix_fullness_group())
>   */
>  static const int fullness_threshold_frac = 4;
> +static size_t zs_huge_class_size;
>  
>  struct size_class {
>  	spinlock_t lock;
> @@ -1417,6 +1418,28 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
>  }
>  EXPORT_SYMBOL_GPL(zs_unmap_object);
>  
> +/**
> + * zs_huge_object() - Test if a compressed object's size is too big for normal
> + *                    zspool classes and it should be stored in a huge class.
> + * @sz: Size of the compressed object (in bytes).
> + *
> + * The function checks if the object's size falls into huge_class
> + * area. We must take handle size into account and test the actual
> + * size we are going to use, because zs_malloc() unconditionally
> + * adds %ZS_HANDLE_SIZE before it performs &size_class lookup.
> + *
> + * Context: Any context.
> + *
> + * Return:
> + * * true  - The object is too big, it will be stored in the huge class.
> + * * false - The object will be stored in a normal zspool class.
> + */
> +bool zs_huge_object(size_t sz)
> +{
> +	return sz + ZS_HANDLE_SIZE >= zs_huge_class_size;
> +}
> +EXPORT_SYMBOL_GPL(zs_huge_object);
> +
>  static unsigned long obj_malloc(struct size_class *class,
>  				struct zspage *zspage, unsigned long handle)
>  {
> @@ -2404,6 +2427,9 @@ struct zs_pool *zs_create_pool(const char *name)
>  			INIT_LIST_HEAD(&class->fullness_list[fullness]);
>  
>  		prev_class = class;
> +		if (pages_per_zspage == 1 && objs_per_zspage == 1
> +				&& !zs_huge_class_size)
> +			zs_huge_class_size = size;
>  	}

Sorry for the long delay. I was horribly busy for a few weeks. ;-(

1.
I didn't see your next patch but I bet it is thing to apply this new API to zram.
Normally, it's hard to review without callsite in same patch because sometime
we need to know context where the API is used. In my understanding, if we judge
new API is really complex to get review from others, then, introducing new API
function without callsite is justified enough. In this case, I think it's simple
enough. :)

2. 

Can't zram ask to zsmalloc about what size is for hugeobject from?
With that, zram can save the wartermark in itself and use it.
What I mean is as follows,

zram:
	size_t huge_size = _zs_huge_object(pool);
	..
	..
	if (comp_size >= huge_size)
		memcpy(dst, src, 4K);
	

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

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/20/18 10:24), Minchan Kim wrote:
> Hi Sergey,

[..]

> Sorry for the long delay. I was horribly busy for a few weeks. ;-(

My turn to say "Sorry for the delay" :)

[..]
> I think it's simple enough. :)

Right. The changes are pretty trivial, that's why I kept then in
2 simple patches. Besides, I didn't want to mix zsmalloc and zram
changes.

> Can't zram ask to zsmalloc about what size is for hugeobject from?
> With that, zram can save the wartermark in itself and use it.
> What I mean is as follows,
> 
> zram:
> 	size_t huge_size = _zs_huge_object(pool);
> 	..
> 	..
> 	if (comp_size >= huge_size)
> 		memcpy(dst, src, 4K);

Yes, can do. My plan was to keep it completely internally to zsmalloc.
Who knows, it might become smart enough one day to do something more
than just size comparison. Any reason you used that leading underscore
in _zs_huge_object()?

	-ss

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/20/18 10:24), Minchan Kim wrote:
> Hi Sergey,

[..]

> Sorry for the long delay. I was horribly busy for a few weeks. ;-(

My turn to say "Sorry for the delay" :)

[..]
> I think it's simple enough. :)

Right. The changes are pretty trivial, that's why I kept then in
2 simple patches. Besides, I didn't want to mix zsmalloc and zram
changes.

> Can't zram ask to zsmalloc about what size is for hugeobject from?
> With that, zram can save the wartermark in itself and use it.
> What I mean is as follows,
> 
> zram:
> 	size_t huge_size = _zs_huge_object(pool);
> 	..
> 	..
> 	if (comp_size >= huge_size)
> 		memcpy(dst, src, 4K);

Yes, can do. My plan was to keep it completely internally to zsmalloc.
Who knows, it might become smart enough one day to do something more
than just size comparison. Any reason you used that leading underscore
in _zs_huge_object()?

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

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Minchan Kim]

Hi Sergey,

On Mon, Feb 26, 2018 at 02:49:27PM +0900, Sergey Senozhatsky wrote:
> > I think it's simple enough. :)
> 
> Right. The changes are pretty trivial, that's why I kept then in
> 2 simple patches. Besides, I didn't want to mix zsmalloc and zram
> changes.

As I said earlier, it's not thing we usually do, at least, MM.
Anyway, I don't want to insist on it because it depends each
person's point of view what's the better for review, git-bisect.

> 
> > Can't zram ask to zsmalloc about what size is for hugeobject from?
> > With that, zram can save the wartermark in itself and use it.
> > What I mean is as follows,
> > 
> > zram:
> > 	size_t huge_size = _zs_huge_object(pool);
> > 	..
> > 	..
> > 	if (comp_size >= huge_size)
> > 		memcpy(dst, src, 4K);
> 
> Yes, can do. My plan was to keep it completely internally to zsmalloc.
> Who knows, it might become smart enough one day to do something more
> than just size comparison. Any reason you used that leading underscore

Let's do that in future if someone want it. :)

> in _zs_huge_object()?


Nope. It's just typo. Let's think better name.
How about using zs_huge_size()?

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Minchan Kim]

Hi Sergey,

On Mon, Feb 26, 2018 at 02:49:27PM +0900, Sergey Senozhatsky wrote:
> > I think it's simple enough. :)
> 
> Right. The changes are pretty trivial, that's why I kept then in
> 2 simple patches. Besides, I didn't want to mix zsmalloc and zram
> changes.

As I said earlier, it's not thing we usually do, at least, MM.
Anyway, I don't want to insist on it because it depends each
person's point of view what's the better for review, git-bisect.

> 
> > Can't zram ask to zsmalloc about what size is for hugeobject from?
> > With that, zram can save the wartermark in itself and use it.
> > What I mean is as follows,
> > 
> > zram:
> > 	size_t huge_size = _zs_huge_object(pool);
> > 	..
> > 	..
> > 	if (comp_size >= huge_size)
> > 		memcpy(dst, src, 4K);
> 
> Yes, can do. My plan was to keep it completely internally to zsmalloc.
> Who knows, it might become smart enough one day to do something more
> than just size comparison. Any reason you used that leading underscore

Let's do that in future if someone want it. :)

> in _zs_huge_object()?


Nope. It's just typo. Let's think better name.
How about using zs_huge_size()?



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

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/26/18 14:58), Minchan Kim wrote:
[..]
> > Right. The changes are pretty trivial, that's why I kept then in
> > 2 simple patches. Besides, I didn't want to mix zsmalloc and zram
> > changes.
> 
> As I said earlier, it's not thing we usually do, at least, MM.
> Anyway, I don't want to insist on it because it depends each
> person's point of view what's the better for review, git-bisect.

Thanks :)

> > > 	size_t huge_size = _zs_huge_object(pool);
> > > 	..
> > > 	..
> > > 	if (comp_size >= huge_size)
> > > 		memcpy(dst, src, 4K);
> > 
> > Yes, can do. My plan was to keep it completely internally to zsmalloc.
> > Who knows, it might become smart enough one day to do something more
> > than just size comparison. Any reason you used that leading underscore
> 
> Let's do that in future if someone want it. :)

OK.

> > in _zs_huge_object()?
> 
> 
> Nope. It's just typo. Let's think better name.
> How about using zs_huge_size()?

hm, I think `huge_size' on it's own is a bit general and cryptic.
zs_huge_object_size() or zs_huge_class_size()?

	-ss

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/26/18 14:58), Minchan Kim wrote:
[..]
> > Right. The changes are pretty trivial, that's why I kept then in
> > 2 simple patches. Besides, I didn't want to mix zsmalloc and zram
> > changes.
> 
> As I said earlier, it's not thing we usually do, at least, MM.
> Anyway, I don't want to insist on it because it depends each
> person's point of view what's the better for review, git-bisect.

Thanks :)

> > > 	size_t huge_size = _zs_huge_object(pool);
> > > 	..
> > > 	..
> > > 	if (comp_size >= huge_size)
> > > 		memcpy(dst, src, 4K);
> > 
> > Yes, can do. My plan was to keep it completely internally to zsmalloc.
> > Who knows, it might become smart enough one day to do something more
> > than just size comparison. Any reason you used that leading underscore
> 
> Let's do that in future if someone want it. :)

OK.

> > in _zs_huge_object()?
> 
> 
> Nope. It's just typo. Let's think better name.
> How about using zs_huge_size()?

hm, I think `huge_size' on it's own is a bit general and cryptic.
zs_huge_object_size() or zs_huge_class_size()?

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

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Minchan Kim]

On Mon, Feb 26, 2018 at 03:50:35PM +0900, Sergey Senozhatsky wrote:
> On (02/26/18 14:58), Minchan Kim wrote:
> [..]
> > > Right. The changes are pretty trivial, that's why I kept then in
> > > 2 simple patches. Besides, I didn't want to mix zsmalloc and zram
> > > changes.
> > 
> > As I said earlier, it's not thing we usually do, at least, MM.
> > Anyway, I don't want to insist on it because it depends each
> > person's point of view what's the better for review, git-bisect.
> 
> Thanks :)
> 
> > > > 	size_t huge_size = _zs_huge_object(pool);
> > > > 	..
> > > > 	..
> > > > 	if (comp_size >= huge_size)
> > > > 		memcpy(dst, src, 4K);
> > > 
> > > Yes, can do. My plan was to keep it completely internally to zsmalloc.
> > > Who knows, it might become smart enough one day to do something more
> > > than just size comparison. Any reason you used that leading underscore
> > 
> > Let's do that in future if someone want it. :)
> 
> OK.
> 
> > > in _zs_huge_object()?
> > 
> > 
> > Nope. It's just typo. Let's think better name.
> > How about using zs_huge_size()?
> 
> hm, I think `huge_size' on it's own is a bit general and cryptic.
> zs_huge_object_size() or zs_huge_class_size()?

I wanted to use more general word to hide zsmalloc internal but
I realized it's really impossible to hide them all.
If so, let's use zs_huge_class_size and then let's add big fat
comment what the API represents in there.

Thanks, Sergey!

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Minchan Kim]

On Mon, Feb 26, 2018 at 03:50:35PM +0900, Sergey Senozhatsky wrote:
> On (02/26/18 14:58), Minchan Kim wrote:
> [..]
> > > Right. The changes are pretty trivial, that's why I kept then in
> > > 2 simple patches. Besides, I didn't want to mix zsmalloc and zram
> > > changes.
> > 
> > As I said earlier, it's not thing we usually do, at least, MM.
> > Anyway, I don't want to insist on it because it depends each
> > person's point of view what's the better for review, git-bisect.
> 
> Thanks :)
> 
> > > > 	size_t huge_size = _zs_huge_object(pool);
> > > > 	..
> > > > 	..
> > > > 	if (comp_size >= huge_size)
> > > > 		memcpy(dst, src, 4K);
> > > 
> > > Yes, can do. My plan was to keep it completely internally to zsmalloc.
> > > Who knows, it might become smart enough one day to do something more
> > > than just size comparison. Any reason you used that leading underscore
> > 
> > Let's do that in future if someone want it. :)
> 
> OK.
> 
> > > in _zs_huge_object()?
> > 
> > 
> > Nope. It's just typo. Let's think better name.
> > How about using zs_huge_size()?
> 
> hm, I think `huge_size' on it's own is a bit general and cryptic.
> zs_huge_object_size() or zs_huge_class_size()?

I wanted to use more general word to hide zsmalloc internal but
I realized it's really impossible to hide them all.
If so, let's use zs_huge_class_size and then let's add big fat
comment what the API represents in there.

Thanks, Sergey!

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

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/26/18 16:46), Minchan Kim wrote:
[..]
> > hm, I think `huge_size' on it's own is a bit general and cryptic.
> > zs_huge_object_size() or zs_huge_class_size()?
> 
> I wanted to use more general word to hide zsmalloc internal but
> I realized it's really impossible to hide them all.
> If so, let's use zs_huge_class_size and then let's add big fat
> comment what the API represents in there.

Will do. Thanks!

	-ss

Re: [PATCHv3 1/2] zsmalloc: introduce zs_huge_object() function

[Sergey Senozhatsky]

On (02/26/18 16:46), Minchan Kim wrote:
[..]
> > hm, I think `huge_size' on it's own is a bit general and cryptic.
> > zs_huge_object_size() or zs_huge_class_size()?
> 
> I wanted to use more general word to hide zsmalloc internal but
> I realized it's really impossible to hide them all.
> If so, let's use zs_huge_class_size and then let's add big fat
> comment what the API represents in there.

Will do. Thanks!

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