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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9E74DC433EF for ; Mon, 28 Feb 2022 16:23:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0B1908D0002; Mon, 28 Feb 2022 11:23:25 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 089EA8D0001; Mon, 28 Feb 2022 11:23:25 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EB8CD8D0002; Mon, 28 Feb 2022 11:23:24 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0232.hostedemail.com [216.40.44.232]) by kanga.kvack.org (Postfix) with ESMTP id DE0498D0001 for ; Mon, 28 Feb 2022 11:23:24 -0500 (EST) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id A514E181C2212 for ; Mon, 28 Feb 2022 16:23:24 +0000 (UTC) X-FDA: 79192708728.24.F9C4A67 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf17.hostedemail.com (Postfix) with ESMTP id 20EC04000C for ; Mon, 28 Feb 2022 16:23:23 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 1E61661218; Mon, 28 Feb 2022 16:23:23 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 86FD8C340E7; Mon, 28 Feb 2022 16:23:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1646065402; bh=4uwUbtFzUaPQDZkQalpW48JK5+hWGBKg6MdMcnnUZM8=; h=From:To:Cc:Subject:Date:In-Reply-To:From; b=bHFmNnssY5Wmor9erI1rBF/Nrqyn6U0q4md+d76eNWWpYLtwvmt1Un+e0ubpKGaUS NhxReyE0A1hdAAofrECxThFI+RvJdGK4kUTF82sdYmA8tf3qUi1hMyfE8bGw5y0HaL LLw1zDkoyX1tflw6SRVHuI/l3HnWhYWiTamRlZpuhNJgFcVrKt/rgvDiqSET/Cfgnx HAn50WvP45NdTTun1u+AI4tst8J9zLDAPE8aiCsyUSBW32Ye/uC71Z1ANHkOINCkKN QcgKmyuq2Zt4qLgkh98AH1qeyC26kBsz/RqBqyJ8tuYefstecWvBhplJG9ZEZZwbbG ZkJhlTK4dUVHg== From: SeongJae Park To: xhao@linux.alibaba.com Cc: SeongJae Park , akpm@linux-foundation.org, corbet@lwn.net, skhan@linuxfoundation.org, rientjes@google.com, gregkh@linuxfoundation.org, linux-damon@amazon.com, linux-mm@kvack.org, linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 03/13] mm/damon: Implement a minimal stub for sysfs-based DAMON interface Date: Mon, 28 Feb 2022 16:23:18 +0000 Message-Id: <20220228162318.4046-1-sj@kernel.org> X-Mailer: git-send-email 2.17.1 MIME-Version: 1.0 In-Reply-To: <0f1051ce-2ce7-8659-f881-b5889018b8a2@linux.alibaba.com> Content-Type: text/plain; charset=UTF-8 X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 20EC04000C X-Stat-Signature: 779a3gi3wpi93kp4m5tob5sitr5o3a9h Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=bHFmNnss; spf=pass (imf17.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=none) header.from=kernel.org X-HE-Tag: 1646065403-385934 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: Hi Xin, On Tue, 1 Mar 2022 00:09:35 +0800 xhao@linux.alibaba.com wrote: > Hi SeongJae: >=20 > On 2/28/22 4:13 PM, SeongJae Park wrote: > > DAMON's debugfs-based user interface served very well, so far. Howev= er, > > it unnecessarily depends on debugfs, while DAMON is not aimed to be u= sed > > for only debugging. Also, the interface receives multiple values via > > one file. For example, schemes file receives 18 values separated by > > white spaces. As a result, it is ineffient, hard to be used, and > > difficult to be extended. Especially, keeping backward compatibility= of > > user space tools is getting only challenging. It would be better to > > implement another reliable and flexible interface and deprecate the > > debugfs interface in long term. > > > > To this end, this commit implements a stub of a part of the new user > > interface of DAMON using sysfs. Specifically, this commit implements > > the sysfs control parts for virtual address space monitoring. > > > > More specifically, the idea of the new interface is, using directory > > hierarchies and making one file for one value. The hierarchy that th= is > > commit is introducing is as below. In the below figure, > > parents-children relations are represented with indentations, each > > directory is having ``/`` suffix, and files in each directory are > > separated by comma (","). > > > > /sys/kernel/mm/damon/admin > > =E2=94=82 kdamonds/nr_kdamonds > > =E2=94=82 =E2=94=82 0/state,pid > > =E2=94=82 =E2=94=82 =E2=94=82 contexts/nr_contexts > > =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 0/operations > > =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 monitoring_att= rs/ > > =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 inte= rvals/sample_us,aggr_us,update_us > > =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 nr_r= egions/min,max > > =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 targets/nr_tar= gets > > =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 0/pi= d_target > > =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 ... > > =E2=94=82 =E2=94=82 =E2=94=82 =E2=94=82 ... > > =E2=94=82 =E2=94=82 ... > > > > Writing a number to each 'nr' file makes directories of name <0> = to > > in the directory of the 'nr' file. That's all this commit does= . > > Writing proper values to relevant files will construct the DAMON > > contexts, and writing a special keyword, 'on', to 'state' files for e= ach > > kdamond will ask DAMON to start the constructed contexts. > > > > For a short example, using below commands for > > monitoring virtual address spaces of a given workload is imaginable: > > > > # cd /sys/kernel/mm/damon/admin/ > > # echo 1 > kdamonds/nr_kdamonds > > # echo 1 > kdamonds/0/contexts/nr_contexts > > # echo vaddr > kdamonds/0/contexts/0/operations > > # echo 1 > kdamonds/0/contexts/0/targets/nr_targets > > # echo $(pidof ) > kdamonds/0/contexts/0/targets/0/pid= _target > > # echo on > kdamonds/0/state > > > > Please note that this commit is implementing only the sysfs part stub= as > > abovely mentioned. This commit doesn't implement the special keyword= s > > for 'state' files. Following commits will do that. > > > > Signed-off-by: SeongJae Park > > --- > > mm/damon/Kconfig | 7 + > > mm/damon/Makefile | 1 + > > mm/damon/sysfs.c | 1082 ++++++++++++++++++++++++++++++++++++++++++= +++ > > 3 files changed, 1090 insertions(+) > > create mode 100644 mm/damon/sysfs.c > > > > diff --git a/mm/damon/Kconfig b/mm/damon/Kconfig > > index 01bad77ad7ae..9b559c76d6dd 100644 > > --- a/mm/damon/Kconfig > > +++ b/mm/damon/Kconfig > > @@ -52,6 +52,13 @@ config DAMON_VADDR_KUNIT_TEST > > =20 > > If unsure, say N. > > =20 > > +config DAMON_SYSFS > > + bool "DAMON sysfs interface" > > + depends on DAMON && SYSFS > > + help > > + This builds the sysfs interface for DAMON. The user space can us= e > > + the interface for arbitrary data access monitoring. > > + > > config DAMON_DBGFS > > bool "DAMON debugfs interface" > > depends on DAMON_VADDR && DAMON_PADDR && DEBUG_FS > > diff --git a/mm/damon/Makefile b/mm/damon/Makefile > > index aebbf6c14c51..dbf7190b4144 100644 > > --- a/mm/damon/Makefile > > +++ b/mm/damon/Makefile > > @@ -3,5 +3,6 @@ > > obj-y :=3D core.o > > obj-$(CONFIG_DAMON_VADDR) +=3D ops-common.o vaddr.o > > obj-$(CONFIG_DAMON_PADDR) +=3D ops-common.o paddr.o > > +obj-$(CONFIG_DAMON_SYSFS) +=3D sysfs.o > > obj-$(CONFIG_DAMON_DBGFS) +=3D dbgfs.o > > obj-$(CONFIG_DAMON_RECLAIM) +=3D reclaim.o > > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c > > new file mode 100644 > > index 000000000000..87cf28ae6a6f > > --- /dev/null > > +++ b/mm/damon/sysfs.c > > @@ -0,0 +1,1082 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * DAMON sysfs Interface > > + * > > + * Copyright (c) 2022 SeongJae Park > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +static DEFINE_MUTEX(damon_sysfs_lock); > > + > > +/* > > + * unsigned long range directory > > + */ > > + > > +struct damon_sysfs_ul_range { > > + struct kobject kobj; > > + unsigned long min; > > + unsigned long max; > > +}; > > + > > +static struct damon_sysfs_ul_range *damon_sysfs_ul_range_alloc( > > + unsigned long min, > > + unsigned long max) > > +{ > > + struct damon_sysfs_ul_range *range =3D kmalloc(sizeof(*range), > > + GFP_KERNEL); > > + > > + if (!range) > > + return NULL; > > + range->kobj =3D (struct kobject){}; > > + range->min =3D min; > > + range->max =3D max; > > + > > + return range; > > +} > > + > > +static ssize_t min_show(struct kobject *kobj, struct kobj_attribute = *attr, > > + char *buf) > > +{ > > + struct damon_sysfs_ul_range *range =3D container_of(kobj, > > + struct damon_sysfs_ul_range, kobj); > > + > > + return sysfs_emit(buf, "%lu\n", range->min); > > +} > > + >=20 > I have do some test about interface "min" and "max", it looks have som= e=20 > bugs. >=20 > [root@rt2k03395 nr_regions]# echo 10 > max > [root@rt2k03395 nr_regions]# echo 20 > min > [root@rt2k03395 nr_regions]# ls > [root@rt2k03395 nr_regions]# cat max > 10 > [root@rt2k03395 nr_regions]# cat min > 20 >=20 > how about do some fix like this: It's an intended behavior. The intention is to let the users put input i= n any order (e.g., writing min first and then max later), and check the validit= y just before applying the input to DAMON, which is, when writing 'on' to the 's= tatus' file. One additional advantage of this is confining the validity check i= n one place and therefore management of the code could be a little bit simpler. So, I think the fix is not really needed. Thanks, SJ [...]