From: SeongJae Park <sjpark@xxxxxxxxx> On Tue, 22 Jun 2021 11:12:36 -0700 Shakeel Butt <shakeelb@xxxxxxxxxx> wrote: > On Mon, Jun 21, 2021 at 1:31 AM SeongJae Park <sj38.park@xxxxxxxxx> wrote: > > > > From: SeongJae Park <sjpark@xxxxxxxxx> > > > > DAMON is designed to be used by kernel space code such as the memory > > management subsystems, and therefore it provides only kernel space API. > > That said, letting the user space control DAMON could provide some > > benefits to them. For example, it will allow user space to analyze > > their specific workloads and make their own special optimizations. > > > > For such cases, this commit implements a simple DAMON application kernel > > module, namely 'damon-dbgfs', which merely wraps the DAMON api and > > exports those to the user space via the debugfs. > > > > 'damon-dbgfs' exports three files, ``attrs``, ``target_ids``, and > > ``monitor_on`` under its debugfs directory, ``<debugfs>/damon/``. > > > > Attributes > > ---------- > > > > Users can read and write the ``sampling interval``, ``aggregation > > interval``, ``regions update interval``, and min/max number of > > monitoring target regions by reading from and writing to the ``attrs`` > > file. For example, below commands set those values to 5 ms, 100 ms, > > 1,000 ms, 10, 1000 and check it again:: > > > > # cd <debugfs>/damon > > # echo 5000 100000 1000000 10 1000 > attrs > > # cat attrs > > 5000 100000 1000000 10 1000 > > > > Target IDs > > ---------- > > > > Some types of address spaces supports multiple monitoring target. For > > example, the virtual memory address spaces monitoring can have multiple > > processes as the monitoring targets. Users can set the targets by > > writing relevant id values of the targets to, and get the ids of the > > current targets by reading from the ``target_ids`` file. In case of the > > virtual address spaces monitoring, the values should be pids of the > > monitoring target processes. For example, below commands set processes > > having pids 42 and 4242 as the monitoring targets and check it again:: > > > > # cd <debugfs>/damon > > # echo 42 4242 > target_ids > > # cat target_ids > > 42 4242 > > > > Note that setting the target ids doesn't start the monitoring. > > > > Turning On/Off > > -------------- > > > > Setting the files as described above doesn't incur effect unless you > > explicitly start the monitoring. You can start, stop, and check the > > current status of the monitoring by writing to and reading from the > > ``monitor_on`` file. Writing ``on`` to the file starts the monitoring > > of the targets with the attributes. Writing ``off`` to the file stops > > those. DAMON also stops if every targets are invalidated (in case of > > the virtual memory monitoring, target processes are invalidated when > > terminated). Below example commands turn on, off, and check the status > > of DAMON:: > > > > # cd <debugfs>/damon > > # echo on > monitor_on > > # echo off > monitor_on > > # cat monitor_on > > off > > > > Please note that you cannot write to the above-mentioned debugfs files > > while the monitoring is turned on. If you write to the files while > > DAMON is running, an error code such as ``-EBUSY`` will be returned. > > > > Signed-off-by: SeongJae Park <sjpark@xxxxxxxxx> > > Reviewed-by: Leonard Foerster <foersleo@xxxxxxxxx> > > Reviewed-by: Fernand Sieber <sieberf@xxxxxxxxxx> > > > The high level comment I have for this patch is the layering of pid > reference counting. The dbgfs should treat the targets as abstract > objects and vaddr should handle the reference counting of pids. More > specifically move find_get_pid from dbgfs to vaddr and to add an > interface to the primitive for set_targets. > > At the moment, the pid reference is taken in dbgfs and put in vaddr. > This will be the source of bugs in future. Good point, and agreed on the problem. But, I'd like to move 'put_pid()' to dbgfs, because I think that would let extending the dbgfs user interface to pidfd a little bit simpler. Also, I think that would be easier to use for in-kernel programming interface usages. If you disagree, please feel free to let me know. Thanks, SeongJae Park
