linux-kernel - Re: [PATCH v4] mm: cma: support sysfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YEKVvZOAVvyu6eSo@google.com>
Date:   Fri, 5 Mar 2021 12:34:05 -0800
From:   Minchan Kim <minchan@...nel.org>
To:     David Hildenbrand <david@...hat.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-mm <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>, gregkh@...uxfoundation.org,
        surenb@...gle.com, joaodias@...gle.com, jhubbard@...dia.com,
        willy@...radead.org
Subject: Re: [PATCH v4] mm: cma: support sysfs

On Fri, Mar 05, 2021 at 06:34:22PM +0100, David Hildenbrand wrote:
> On 04.03.21 17:17, Minchan Kim wrote:
> > Since CMA is getting used more widely, it's more important to
> > keep monitoring CMA statistics for system health since it's
> > directly related to user experience.
> > 
> > This patch introduces sysfs statistics for CMA, in order to provide
> > some basic monitoring of the CMA allocator.
> > 
> >   * the number of CMA page allocation attempts
> >   * the number of CMA page allocation failures
> > 
> > These two values allow the user to calcuate the allocation
> > failure rate for each CMA area.
> > 
> > e.g.)
> >    /sys/kernel/mm/cma/WIFI/cma_alloc_pages_[attempts|fails]
> >    /sys/kernel/mm/cma/SENSOR/cma_alloc_pages_[attempts|fails]
> >    /sys/kernel/mm/cma/BLUETOOTH/cma_alloc_pages_[attempts|fails]
> > 
> > The cma_stat was intentionally allocated by dynamic allocation
> > to harmonize with kobject lifetime management.
> > https://lore.kernel.org/linux-mm/YCOAmXqt6dZkCQYs@kroah.com/
> > 
> > Reviewed-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > Reviewed-by: John Hubbard <jhubbard@...dia.com>
> > Signed-off-by: Minchan Kim <minchan@...nel.org>
> > ---
> >  From v3 - https://lore.kernel.org/linux-mm/20210303205053.2906924-1-minchan@kernel.org/
> >   * kmalloc_array - akpm
> >   * add why cma_stat was implemented by dynamic allocation - akpm
> >   * use !__GFP_NOWARN facility to print error - akpm
> > 
> >  From v2 - https://lore.kernel.org/linux-mm/20210208180142.2765456-1-minchan@kernel.org/
> >   * sysfs doc and description modification - jhubbard
> > 
> >  From v1 - https://lore.kernel.org/linux-mm/20210203155001.4121868-1-minchan@kernel.org/
> >   * fix sysfs build and refactoring - willy
> >   * rename and drop some attributes - jhubbard
> > 
> >   Documentation/ABI/testing/sysfs-kernel-mm-cma |  25 ++++
> >   mm/Kconfig                                    |   7 ++
> >   mm/Makefile                                   |   1 +
> >   mm/cma.c                                      |   6 +-
> >   mm/cma.h                                      |  18 +++
> >   mm/cma_sysfs.c                                | 110 ++++++++++++++++++
> >   6 files changed, 166 insertions(+), 1 deletion(-)
> >   create mode 100644 Documentation/ABI/testing/sysfs-kernel-mm-cma
> >   create mode 100644 mm/cma_sysfs.c
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-cma b/Documentation/ABI/testing/sysfs-kernel-mm-cma
> > new file mode 100644
> > index 000000000000..f518af819cee
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-kernel-mm-cma
> > @@ -0,0 +1,25 @@
> > +What:		/sys/kernel/mm/cma/
> > +Date:		Feb 2021
> > +Contact:	Minchan Kim <minchan@...nel.org>
> > +Description:
> > +		/sys/kernel/mm/cma/ contains a subdirectory for each CMA
> > +		heap name (also sometimes called CMA areas).
> > +
> > +		Each CMA heap subdirectory (that is, each
> > +		/sys/kernel/mm/cma/<cma-heap-name> directory) contains the
> > +		following items:
> > +
> > +			cma_alloc_pages_attempts
> > +			cma_alloc_pages_fails
> 
> Nit: why "cma_" again when we are already under "/cma/" ?

Originally, there was desire to add cma_alloc_attempts as well as
cma_alloc_pages_attempts. 

> 
> I'd simply go with something like
> 
> "total_alloc_attempts"
> "failed_alloc_attempts"

If we really want to remove the cma prefix, maybe,

alloc_pages_attempts
alloc_pages_fails

If someone want to count cma_alloc itself, Then

alloc_success
alloc_fail

Does that make sense?