From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5472FC433DB for ; Fri, 19 Mar 2021 18:19:03 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A84686195E for ; Fri, 19 Mar 2021 18:19:02 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A84686195E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 485196B0074; Fri, 19 Mar 2021 14:19:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 435596B0075; Fri, 19 Mar 2021 14:19:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 263966B0078; Fri, 19 Mar 2021 14:19:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0060.hostedemail.com [216.40.44.60]) by kanga.kvack.org (Postfix) with ESMTP id 049C46B0074 for ; Fri, 19 Mar 2021 14:19:01 -0400 (EDT) Received: from smtpin40.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id BD4718145 for ; Fri, 19 Mar 2021 18:19:01 +0000 (UTC) X-FDA: 77937435282.40.C080F09 Received: from mail-pf1-f180.google.com (mail-pf1-f180.google.com [209.85.210.180]) by imf24.hostedemail.com (Postfix) with ESMTP id 60C06A0000FC for ; Fri, 19 Mar 2021 18:18:47 +0000 (UTC) Received: by mail-pf1-f180.google.com with SMTP id x26so6459792pfn.0 for ; Fri, 19 Mar 2021 11:18:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=+Y8V4e2Y/cYVNFx5v7FpeMFjnoVxeZfsqiFDE1cJkBk=; b=omXpfZjgBPzRaxnThzF9S5v1tuFuBcmJpvWvxAvWeZQ6BxYedBfwkPETfJx7Y2w+XS q3bOaFwozAySgDTa1AhINYXGyXBbR8gmBPFBL74Rfy2rwQvANTJLjDMYVVx9zqSgtJiy XZGdoqy8yu2ryDQENFhro2JkxCjBLt65/+tye4Q0B6u+vJ5bI2hsIn0YjUOMAdTlOA/p xHrpt1tW0HBNYblhaE4590a6A9r9lad2r/vVzwasXR4/5eYGLv0BpUnKc1rgl8V/6QRk 6nrs+LfBDmTJuyCqrRqQijA+jLGp936WfPuMKCRXZwSgwRS3jVhrEqvO0HwS86wBtD5D L3fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition :content-transfer-encoding:in-reply-to; bh=+Y8V4e2Y/cYVNFx5v7FpeMFjnoVxeZfsqiFDE1cJkBk=; b=WQs4zRQ3f6lXADlRumlneUOY2R1eLQbpFMIzvS3tFDdr78+IsEUkRvOdAnQWSk9KZq zstnpa2bHFiyrC1gonPVlgub1qHmKGPUcy19dODukeOn8mtLqKLFL7Cj/6Df1ltcwxZF tXdKVVdcAYosjJhZzFdcQjUfGL50FtJgAxim97F2nMnGSWfEpMs9hZGAEfnaK0B913Rx y1dP6TDTQROMBGTpLOiwgCVXvi6t4dLHXrQOx5AtpIf+yetNaGUOmyKIUwfqxCtpu9Mn N+r7OqHFMSKdzp8oIKk5yeyAEsvzdIvoJieeB0MrH978ceYME/tIeQOWRHLNJpce59Ed y1ig== X-Gm-Message-State: AOAM532xReeqWhaNWyzBpWjrA/PR5qSqPKwMCqaLobDR5pXPztlz5ITJ KyPp1/DOyXRBUwif5VLZ6D+hglTKmFU= X-Google-Smtp-Source: ABdhPJyQ4Sj9vhle02FpRqhf+PuS5VfGiY3pki6jzvPPaoYf0aLTiimedrdkhbtV3Uf9M4CFktQqLQ== X-Received: by 2002:a65:5c42:: with SMTP id v2mr12802624pgr.339.1616177921284; Fri, 19 Mar 2021 11:18:41 -0700 (PDT) Received: from google.com ([2620:15c:211:201:2033:9813:e1ed:7060]) by smtp.gmail.com with ESMTPSA id w189sm5988654pfw.86.2021.03.19.11.18.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 19 Mar 2021 11:18:40 -0700 (PDT) Date: Fri, 19 Mar 2021 11:18:38 -0700 From: Minchan Kim To: Dmitry Osipenko Cc: Greg Kroah-Hartman , Andrew Morton , linux-mm , LKML , joaodias@google.com, willy@infradead.org, david@redhat.com, surenb@google.com, John Hubbard , Nicolas Chauvet , "linux-tegra@vger.kernel.org" Subject: Re: [PATCH v4] mm: cma: support sysfs Message-ID: References: <33ec18ef-8652-643a-1a53-ff7c3caf4399@gmail.com> <78883205-e6da-5bc4-dcec-b6eb921567b1@gmail.com> <72db59eb-75dc-d1ed-7a83-17052e8f22a8@gmail.com> <82bde114-60c0-3fde-43f4-844522b80673@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <82bde114-60c0-3fde-43f4-844522b80673@gmail.com> X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 60C06A0000FC X-Stat-Signature: z6875yukiqni9y5b1taofe4oxsyzso4r Received-SPF: none (gmail.com>: No applicable sender policy available) receiver=imf24; identity=mailfrom; envelope-from=""; helo=mail-pf1-f180.google.com; client-ip=209.85.210.180 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1616177927-317161 Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Mar 19, 2021 at 08:29:29PM +0300, Dmitry Osipenko wrote: > 19.03.2021 19:30, Minchan Kim =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > > On Fri, Mar 19, 2021 at 07:24:05PM +0300, Dmitry Osipenko wrote: > >> 19.03.2021 18:50, Greg Kroah-Hartman =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > >>>> Then initialization order won't be a problem. > >>> I don't understand the problem here, what is wrong with the patch a= s-is? > >> > >> The cma->stat is NULL at the time when CMA is used on ARM because > >> cma->stat is initialized at the subsys level. This is the problem, > >> apparently. > >=20 > > That's true. > >=20 > >> > >>> Also, watch out, if you only make the kobject dynamic, how are you = going > >>> to get backwards from the kobject to the values you want to send to > >>> userspace in the show functions? > >> > >> Still there should be a wrapper around the kobj with a back referenc= e to > >> the cma entry. If you're suggesting that I should write a patch, the= n I > >> may take a look at it later on. Although, I assume that Minchan coul= d > >> just correct this patch and re-push it to -next. > >=20 > > This is ateempt to address it. Unless any objection, let me send it t= o > > akpm. > >=20 > > From 29a9fb4f300b754ebf55e6182ba84127658ef504 Mon Sep 17 00:00:00 200= 1 > > From: Minchan Kim > > Date: Fri, 22 Jan 2021 12:31:56 -0800 > > Subject: [PATCH] mm: cma: support sysfs > >=20 > > 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. > >=20 > > This patch introduces sysfs statistics for CMA, in order to provide > > some basic monitoring of the CMA allocator. > >=20 > > * the number of CMA page successful allocations > > * the number of CMA page allocation failures > >=20 > > These two values allow the user to calcuate the allocation > > failure rate for each CMA area. > >=20 > > e.g.) > > /sys/kernel/mm/cma/WIFI/alloc_pages_[success|fail] > > /sys/kernel/mm/cma/SENSOR/alloc_pages_[success|fail] > > /sys/kernel/mm/cma/BLUETOOTH/alloc_pages_[success|fail] > >=20 > > The cma_stat was intentionally allocated by dynamic allocation > > to harmonize with kobject lifetime management. > > https://lore.kernel.org/linux-mm/YCOAmXqt6dZkCQYs@kroah.com/ > >=20 > > Signed-off-by: Minchan Kim > > --- > > Documentation/ABI/testing/sysfs-kernel-mm-cma | 25 ++++ > > mm/Kconfig | 7 ++ > > mm/Makefile | 1 + > > mm/cma.c | 7 +- > > mm/cma.h | 20 ++++ > > mm/cma_sysfs.c | 107 ++++++++++++++++= ++ > > 6 files changed, 165 insertions(+), 2 deletions(-) > > create mode 100644 Documentation/ABI/testing/sysfs-kernel-mm-cma > > create mode 100644 mm/cma_sysfs.c > >=20 > > diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-cma b/Document= ation/ABI/testing/sysfs-kernel-mm-cma > > new file mode 100644 > > index 000000000000..02b2bb60c296 > > --- /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: > > + > > + alloc_pages_success > > + alloc_pages_fail > > + > > +What: /sys/kernel/mm/cma//alloc_pages_success > > +Date: Feb 2021 > > +Contact: Minchan Kim > > +Description: > > + the number of pages CMA API succeeded to allocate > > + > > +What: /sys/kernel/mm/cma//alloc_pages_fail > > +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 24c045b24b95..febb7e8e24de 100644 > > --- a/mm/Kconfig > > +++ b/mm/Kconfig > > @@ -513,6 +513,13 @@ config CMA_DEBUGFS > > help > > Turns on the DebugFS interface for CMA. > > =20 > > +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 72227b24a616..56968b23ed7a 100644 > > --- a/mm/Makefile > > +++ b/mm/Makefile > > @@ -109,6 +109,7 @@ obj-$(CONFIG_CMA) +=3D cma.o > > obj-$(CONFIG_MEMORY_BALLOON) +=3D balloon_compaction.o > > obj-$(CONFIG_PAGE_EXTENSION) +=3D page_ext.o > > obj-$(CONFIG_CMA_DEBUGFS) +=3D cma_debug.o > > +obj-$(CONFIG_CMA_SYSFS) +=3D cma_sysfs.o > > obj-$(CONFIG_USERFAULTFD) +=3D userfaultfd.o > > obj-$(CONFIG_IDLE_PAGE_TRACKING) +=3D page_idle.o > > obj-$(CONFIG_DEBUG_PAGE_REF) +=3D debug_page_ref.o > > diff --git a/mm/cma.c b/mm/cma.c > > index 908f04775686..ac050359faae 100644 > > --- a/mm/cma.c > > +++ b/mm/cma.c > > @@ -507,10 +507,13 @@ struct page *cma_alloc(struct cma *cma, size_t = count, unsigned int align, > > =20 > > pr_debug("%s(): returned %p\n", __func__, page); > > out: > > - if (page) > > + if (page) { > > count_vm_event(CMA_ALLOC_SUCCESS); > > - else > > + cma_sysfs_alloc_pages_count(cma, count); > > + } else { > > count_vm_event(CMA_ALLOC_FAIL); > > + cma_sysfs_fail_pages_count(cma, count); > > + } > > =20 > > return page; > > } > > diff --git a/mm/cma.h b/mm/cma.h > > index 42ae082cb067..70fd7633fe01 100644 > > --- a/mm/cma.h > > +++ b/mm/cma.h > > @@ -3,6 +3,12 @@ > > #define __MM_CMA_H__ > > =20 > > #include > > +#include > > + > > +struct cma_kobject { > > + struct cma *cma; > > + struct kobject kobj; > > +}; > > =20 > > struct cma { > > unsigned long base_pfn; > > @@ -16,6 +22,13 @@ struct cma { > > struct debugfs_u32_array dfs_bitmap; > > #endif > > char name[CMA_MAX_NAME]; > > +#ifdef CONFIG_CMA_SYSFS > > + /* the number of CMA page successful allocations */ > > + atomic64_t nr_pages_succeeded; > > + /* the number of CMA page allocation failures */ > > + atomic64_t nr_pages_failed; > > + struct cma_kobject *kobj; > > +#endif > > }; > > =20 > > extern struct cma cma_areas[MAX_CMA_AREAS]; > > @@ -26,4 +39,11 @@ static inline unsigned long cma_bitmap_maxno(struc= t cma *cma) > > return cma->count >> cma->order_per_bit; > > } > > =20 > > +#ifdef CONFIG_CMA_SYSFS > > +void cma_sysfs_alloc_pages_count(struct cma *cma, size_t count); > > +void cma_sysfs_fail_pages_count(struct cma *cma, size_t count); > > +#else > > +static inline void cma_sysfs_alloc_pages_count(struct cma *cma, size= _t count) {}; > > +static inline void cma_sysfs_fail_pages_count(struct cma *cma, size_= t count) {}; > > +#endif > > #endif > > diff --git a/mm/cma_sysfs.c b/mm/cma_sysfs.c > > new file mode 100644 > > index 000000000000..ca093e9e9f64 > > --- /dev/null > > +++ b/mm/cma_sysfs.c > > @@ -0,0 +1,107 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * CMA SysFS Interface > > + * > > + * Copyright (c) 2021 Minchan Kim > > + */ > > + > > +#include > > +#include > > +#include > > + > > +#include "cma.h" > > + > > +void cma_sysfs_alloc_pages_count(struct cma *cma, size_t count) > > +{ > > + atomic64_add(count, &cma->nr_pages_succeeded); > > +} > > + > > +void cma_sysfs_fail_pages_count(struct cma *cma, size_t count) > > +{ > > + atomic64_add(count, &cma->nr_pages_failed); > > +} > > + > > +#define CMA_ATTR_RO(_name) \ > > + static struct kobj_attribute _name##_attr =3D __ATTR_RO(_name) > > + > > +static ssize_t alloc_pages_success_show(struct kobject *kobj, > > + struct kobj_attribute *attr, char *buf) >=20 > The indentations are still wrong. >=20 > CHECK: Alignment should match open parenthesis Hmm, I didn't know that we have that kinds of rule. Maybe, my broken checkpatch couldn't catch it up. Thanks. $ ./scripts/checkpatch.pl 0001-mm-cma-support-sysfs.patch Traceback (most recent call last): File "scripts/spdxcheck.py", line 6, in from ply import lex, yacc < snip > >=20 > > + if (ZERO_OR_NULL_PTR(cma_kobjs)) > > + goto out; > > + > > + do { > > + cma =3D &cma_areas[i]; > > + cma->kobj =3D &cma_kobjs[i]; >=20 > Does cma really need are pointer to cma_kobj? Do you mean something like this? struct cma { .. .. struct kobject *kobj; }; Then, how could we we make kobject dynamic model? We need to get struct cma from kboj like below. static ssize_t alloc_pages_success_show(struct kobject *kobj, struct kobj_attribute *attr, char= *buf) { struct cma_kobject *cma_kobj =3D container_of(kobj, struct cma_kobject, k= obj); struct cma *cma =3D cma_kobj->cma; return sysfs_emit(buf, "%llu\n", atomic64_read(&cma->nr_pages_succeeded)); } So kobj should be not a pointer in the container struct. Am I missing your point? Otherwise, it would be great if you suggest better idea. < snip> > > + kobject_put(&cma->kobj->kobj); > > + goto out; > > + } > > + } while (++i < cma_area_count); > > + > > + return 0; > > +out: > > + while (--i >=3D 0) { > > + cma =3D &cma_areas[i]; > > + kobject_put(&cma->kobj->kobj); >=20 > Should the cma->kobj be set to NULL by cma_kobj_release() if cma->kobj = is really needed? True. Then, let's fix cma_kobj_release, too. >=20 > > + } > > + > > + kfree(cma_kobjs); > > + kobject_put(cma_kobj_root); > > + > > + return -ENOMEM; > > +} > > +subsys_initcall(cma_sysfs_init); > >=20 >=20 > Tested-by: Dmitry Osipenko Thanks!