Re: [PATCH v3] mm: cma: support sysfs
On Wed, Mar 03, 2021 at 10:09:57PM -0800, Andrew Morton wrote:
> On Wed, 3 Mar 2021 17:38:12 -0800 Minchan Kim wrote:
>
> > > > #endif
> > > > char name[CMA_MAX_NAME];
> > > > +#ifdef CONFIG_CMA_SYSFS
> > > > + struct cma_stat *stat;
> > > > +#endif
> > > > };
> > >
> > > Why aren't the stat fields simply placed directly into struct cma_stat?
> >
> > It have a related long discussion.
> > https://lore.kernel.org/linux-mm/yciohbgelfway...@kroah.com/
> > https://lore.kernel.org/linux-mm/ycllkdeq4nyqb...@kroah.com/
> >
> > TLDR - Greg really want to see kobject stuff working as dynamic
> > property.
>
> Please add to changelog?
Sure.
>
> > >
> > > ?
> > >
> > > > + if (!cma_stats) {
> > > > + pr_err("failed to create cma_stats\n");
> > >
> > > Probably unneeded - the ENOMEM stack backtrace will point straight here.
> >
> > I failed to find the point you mentioned to print backtrace.
> > Where code do you mean to dump the backtrace?
>
> The thing which __GFP_NOWARN disables.
Got the point.
Re: [PATCH v3] mm: cma: support sysfs
On Wed, 3 Mar 2021 17:38:12 -0800 Minchan Kim wrote:
> > > #endif
> > > char name[CMA_MAX_NAME];
> > > +#ifdef CONFIG_CMA_SYSFS
> > > + struct cma_stat *stat;
> > > +#endif
> > > };
> >
> > Why aren't the stat fields simply placed directly into struct cma_stat?
>
> It have a related long discussion.
> https://lore.kernel.org/linux-mm/yciohbgelfway...@kroah.com/
> https://lore.kernel.org/linux-mm/ycllkdeq4nyqb...@kroah.com/
>
> TLDR - Greg really want to see kobject stuff working as dynamic
> property.
Please add to changelog?
> >
> > ?
> >
> > > + if (!cma_stats) {
> > > + pr_err("failed to create cma_stats\n");
> >
> > Probably unneeded - the ENOMEM stack backtrace will point straight here.
>
> I failed to find the point you mentioned to print backtrace.
> Where code do you mean to dump the backtrace?
The thing which __GFP_NOWARN disables.
Re: [PATCH v3] mm: cma: support sysfs
On Wed, Mar 03, 2021 at 02:44:49PM -0800, Andrew Morton wrote:
> On Wed, 3 Mar 2021 12:50:53 -0800 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]
> >
> > ...
> >
> > --- a/mm/cma.h
> > +++ b/mm/cma.h
> > @@ -3,6 +3,14 @@
> > #define __MM_CMA_H__
> >
> > #include
> > +#include
> > +
> > +struct cma_stat {
> > + spinlock_t lock;
> > + unsigned long pages_attempts; /* the number of CMA page allocation
> > attempts */
> > + unsigned long pages_fails; /* the number of CMA page allocation
> > failures */
> > + struct kobject kobj;
> > +};
> >
> > struct cma {
> > unsigned long base_pfn;
> > @@ -16,6 +24,9 @@ struct cma {
> > struct debugfs_u32_array dfs_bitmap;
> > #endif
> > char name[CMA_MAX_NAME];
> > +#ifdef CONFIG_CMA_SYSFS
> > + struct cma_stat *stat;
> > +#endif
> > };
>
> Why aren't the stat fields simply placed directly into struct cma_stat?
It have a related long discussion.
https://lore.kernel.org/linux-mm/yciohbgelfway...@kroah.com/
https://lore.kernel.org/linux-mm/ycllkdeq4nyqb...@kroah.com/
TLDR - Greg really want to see kobject stuff working as dynamic
property.
>
> > ...
> >
> > +static int __init cma_sysfs_init(void)
> > +{
> > + int i = 0;
> > + struct cma *cma;
> > +
> > + cma_kobj = kobject_create_and_add("cma", mm_kobj);
> > + if (!cma_kobj) {
> > + pr_err("failed to create cma kobject\n");
> > + return -ENOMEM;
> > + }
> > +
> > + cma_stats = kzalloc(array_size(sizeof(struct cma_stat),
> > + cma_area_count), GFP_KERNEL);
>
> kmalloc_array(..., GFP_KERNEL|__GFP_ZERO);
Yub.
>
> ?
>
> > + if (!cma_stats) {
> > + pr_err("failed to create cma_stats\n");
>
> Probably unneeded - the ENOMEM stack backtrace will point straight here.
I failed to find the point you mentioned to print backtrace.
Where code do you mean to dump the backtrace?
Re: [PATCH v3] mm: cma: support sysfs
On Wed, 3 Mar 2021 12:50:53 -0800 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]
>
> ...
>
> --- a/mm/cma.h
> +++ b/mm/cma.h
> @@ -3,6 +3,14 @@
> #define __MM_CMA_H__
>
> #include
> +#include
> +
> +struct cma_stat {
> + spinlock_t lock;
> + unsigned long pages_attempts; /* the number of CMA page allocation
> attempts */
> + unsigned long pages_fails; /* the number of CMA page allocation
> failures */
> + struct kobject kobj;
> +};
>
> struct cma {
> unsigned long base_pfn;
> @@ -16,6 +24,9 @@ struct cma {
> struct debugfs_u32_array dfs_bitmap;
> #endif
> char name[CMA_MAX_NAME];
> +#ifdef CONFIG_CMA_SYSFS
> + struct cma_stat *stat;
> +#endif
> };
Why aren't the stat fields simply placed directly into struct cma_stat?
> ...
>
> +static int __init cma_sysfs_init(void)
> +{
> + int i = 0;
> + struct cma *cma;
> +
> + cma_kobj = kobject_create_and_add("cma", mm_kobj);
> + if (!cma_kobj) {
> + pr_err("failed to create cma kobject\n");
> + return -ENOMEM;
> + }
> +
> + cma_stats = kzalloc(array_size(sizeof(struct cma_stat),
> + cma_area_count), GFP_KERNEL);
kmalloc_array(..., GFP_KERNEL|__GFP_ZERO);
?
> + if (!cma_stats) {
> + pr_err("failed to create cma_stats\n");
Probably unneeded - the ENOMEM stack backtrace will point straight here.
> + goto out;
> + }
> +
> + do {
> + cma = &cma_areas[i];
> + cma->stat = &cma_stats[i];
> + spin_lock_init(&cma->stat->lock);
> + if (kobject_init_and_add(&cma->stat->kobj, &cma_ktype,
> + cma_kobj, "%s", cma->name)) {
> + kobject_put(&cma->stat->kobj);
> + goto out;
> + }
> + } while (++i < cma_area_count);
> +
> + return 0;
> +out:
> + while (--i >= 0) {
> + cma = &cma_areas[i];
> + kobject_put(&cma->stat->kobj);
> + }
> +
> + kfree(cma_stats);
> + kobject_put(cma_kobj);
> +
> + return -ENOMEM;
> +}
> +subsys_initcall(cma_sysfs_init);
Re: [PATCH v3] mm: cma: support sysfs
On Wed, Feb 10, 2021 at 12:05:48AM -0800, John Hubbard wrote: > On 2/9/21 11:55 PM, 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] > > > > Signed-off-by: Minchan Kim > > --- > > Looks good. > > Reviewed-by: John Hubbard Thanks for reviewing, John and Greg!
Re: [PATCH v3] mm: cma: support sysfs
On 2/9/21 11:55 PM, 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]
Signed-off-by: Minchan Kim
---
Looks good.
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
From v2 -
https://lore.kernel.org/linux-mm/20210208180142.2765456-1-minc...@kernel.org/
* sysfs doc and description modification - jhubbard
From v1 -
https://lore.kernel.org/linux-mm/20210203155001.4121868-1-minc...@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| 114 ++
6 files changed, 170 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 ..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
+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/ directory) contains the
+ following items:
+
+ cma_alloc_pages_attempts
+ cma_alloc_pages_fails
+
+What: /sys/kernel/mm/cma//cma_alloc_pages_attempts
+Date: Feb 2021
+Contact: Minchan Kim
+Description:
+ the number of pages CMA API tried to allocate
+
+What: /sys/kernel/mm/cma//cma_alloc_pages_fails
+Date: Feb 2021
+Contact: Minchan Kim
+Description:
+ the number of pages CMA API failed to allocate
diff --git a/mm/Kconfig b/mm/Kconfig
index ec35bf406439..ad7e9c065657 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -513,6 +513,13 @@ config CMA_DEBUGFS
help
Turns on the DebugFS interface for CMA.
+config CMA_SYSFS
+ bool "CMA information through sysfs interface"
+ depends on CMA && SYSFS
+ help
+ This option exposes some sysfs attributes to get information
+ from CMA.
+
config CMA_AREAS
int "Maximum count of the CMA areas"
depends on CMA
diff --git a/mm/Makefile b/mm/Makefile
index b2a564eec27f..e10c14300191 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -109,6 +109,7 @@ obj-$(CONFIG_CMA) += cma.o
obj-$(CONFIG_MEMORY_BALLOON) += balloon_compaction.o
obj-$(CONFIG_PAGE_EXTENSION) += page_ext.o
obj-$(CONFIG_CMA_DEBUGFS) += cma_debug.o
+obj-$(CONFIG_CMA_SYSFS) += cma_sysfs.o
obj-$(CONFIG_USERFAULTFD) += userfaultfd.o
obj-$(CONFIG_IDLE_PAGE_TRACKING) += page_idle.o
obj-$(CONFIG_DEBUG_PAGE_REF) += debug_page_ref.o
diff --git a/mm/cma.c b/mm/cma.c
index 23d4a97c834a..b42ccafc71e5 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -447,9 +447,10 @@ struct page *cma_alloc(struct cma *cma, size_t count,
unsigned int align,
offset = cma_bitmap_aligned_offset(cma, align);
bitmap_maxno = cma_bitmap_maxno(cma);
bitmap_count = cma_bitmap_pages_to_bits(cma, count);
+ cma_sysfs_alloc_count(cma, count);
if (bitmap_count > bitmap_maxno)
- return NULL;
+ goto out;
for (;;) {
mutex_lock(&cma->lock);
@@ -504,6 +505,9 @@ struct page *cma_alloc(struct cma *cma, size_t count,
unsigned int align,
__func__, count, ret);
cma_debug_show_areas(cma);
}
+out:
+ if (!page)
+ cma_sysfs_fail_count(cma, count);
pr_debug("%s(): returned %p\n", __func__, page);
return page;
diff --git a/mm/cma.h b/mm/cma.h
index 42ae082cb067..24a1d61eabc7 100644
--- a/mm/cma.h
+++ b/mm/cma.h
@@ -3,6 +3,14 @@
#define __MM_CMA_H__
#include
+#include
+
+struct cma_stat {
+ spinlock_t lock;
+ unsigned long pages_attempts; /* the number of CMA page allocation
attempts */
+ unsigned
Re: [PATCH v3] mm: cma: support sysfs
On Tue, Feb 09, 2021 at 11:55:09PM -0800, 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] > > Signed-off-by: Minchan Kim For the sysfs stuff: Reviewed-by: Greg Kroah-Hartman
