On Wed, Mar 03, 2021 at 02:44:49PM -0800, Andrew Morton wrote:
> On Wed, 3 Mar 2021 12:50:53 -0800 Minchan Kim <minchan@xxxxxxxxxx> 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 <linux/debugfs.h>
> > +#include <linux/kobject.h>
> > +
> > +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/YCIoHBGELFWAyfMi@xxxxxxxxx/
https://lore.kernel.org/linux-mm/YCLLKDEQ4NYqb5Y5@xxxxxxxxx/
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?
