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

OK. `` 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.
> 
>^ _class

OK. `` name``

Thanks for reviewing it!

-ss

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  to be held by caller.
> >   * Context: Process context. May sleep if @gfp flags permit.
> >   * Context: Process context. Takes and releases .
> >   * Context: Softirq or process context. Takes and releases , BH-safe.
> >   * Context: Interrupt context.
> 
> I assume thatspelling 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 .
> 
> 
> 
> 
> 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.

   ^ _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  to be held by caller.
> >   * Context: Process context. May sleep if @gfp flags permit.
> >   * Context: Process context. Takes and releases .
> >   * Context: Softirq or process context. Takes and releases , BH-safe.
> >   * Context: Interrupt context.
> 
> I assume thatspelling 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 .
> 
> 
> 
> 
> 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.

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

[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

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  to be held by caller.
>   * Context: Process context. May sleep if @gfp flags permit.
>   * Context: Process context. Takes and releases .
>   * Context: Softirq or process context. Takes and releases , BH-safe.
>   * Context: Interrupt context.

I assume thatspelling 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 .




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  to be held by caller.
>   * Context: Process context. May sleep if @gfp flags permit.
>   * Context: Process context. Takes and releases .
>   * Context: Softirq or process context. Takes and releases , BH-safe.
>   * Context: Interrupt context.

I assume thatspelling 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 .




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:
> > > > +/*
> > > > + * 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

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

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

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

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 
> ---
>  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(>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 majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
> 

-- 
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 
> ---
>  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(>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 majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
> 

-- 
Sincerely yours,
Mike.