Re: [PATCH v4] mm: cma: support sysfs

Minchan Kim
2021-03-05T20:40:34Z

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@linuxfoundation.org>
> > Reviewed-by: John Hubbard <jhubbard@nvidia.com>
> > Signed-off-by: Minchan Kim <minchan@kernel.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@kernel.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?