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?
> ...
>
> +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);
