Message-ID: <aACaf4_molKromnT@telecaster>
Date: Wed, 16 Apr 2025 23:06:55 -0700
From: Omar Sandoval <osandov@...ndov.com>
To: Ye Liu <ye.liu@...ux.dev>
Cc: paulmck@...nel.org, Sweet Tea Dorminy <sweettea-kernel@...miny.me>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-toolchains@...r.kernel.org,
	linux-mm@...ck.org, Ye Liu <liuye@...inos.cn>,
	linux-debuggers@...r.kernel.org
Subject: Re: [PATCH] tools/drgn: Add script to display page state for a given
 PID and VADDR

On Thu, Apr 17, 2025 at 09:29:23AM +0800, Ye Liu wrote:
> 
> 在 2025/4/16 12:02, Paul E. McKenney 写道:
> > On Tue, Apr 15, 2025 at 11:28:41PM -0400, Sweet Tea Dorminy wrote:
> >>
> >> On 4/15/25 10:46 PM, Ye Liu wrote:
> >>> 在 2025/4/16 10:14, Andrew Morton 写道:
> >>>> On Tue, 15 Apr 2025 15:50:24 +0800 Ye Liu <ye.liu@...ux.dev> wrote:
> >>>>
> >>>>> From: Ye Liu <liuye@...inos.cn>
> >>>>>
> >>>>> Introduces a new drgn script, `show_page_info.py`, which allows users
> >>>>> to analyze the state of a page given a process ID (PID) and a virtual
> >>>>> address (VADDR). This can help kernel developers or debuggers easily
> >>>>> inspect page-related information in a live kernel or vmcore.
> >>>>>
> >>>>> The script extracts information such as the page flags, mapping, and
> >>>>> other metadata relevant to diagnosing memory issues.
> >>>>>
> >>>>> Currently, there is no specific maintainer entry for `tools/drgn/` in the
> >>>>> MAINTAINERS file. Therefore, this patch is sent to the general kernel and
> >>>>> tools mailing lists for review.
> >>>> Help.  My copy of linux has no tools/drgn/
> >>> I noticed that the current upstream Linux tree doesn't contain a
> >>> `tools/drgn/` directory.
> >>>
> >>> I'm interested in contributing a drgn script tool as well.
> >>> Given that this directory does not yet exist in mainline, where would
> >>> be the appropriate place to add new drgn scripts? Would it make sense
> >>> to create a new `tools/drgn/` directory, or is there a preferred
> >>> location for such debugging scripts?
> >>>
> >>> Thanks,
> >>> Ye
> >> I believe the traditional thing to do with new drgn scripts is to add them
> >> to the contrib directory in drgn via pull request:
> >> https://github.com/osandov/drgn/blob/main/contrib/README.rst
> > I have an RCU-related drgn script in tools/rcu, so maybe this one should
> > go in tools/mm.
> 
> 
> To determine the most appropriate place to submit this script, I looked
> 
> into existing drgn-based tooling in the kernel tree. Several drgn scripts
> have already been added under `tools/`, such as:
> 
> - `tools/sched_ext/scx_show_state.py`
> - `tools/cgroup/iocost_monitor.py`
> - `tools/cgroup/memcg_slabinfo.py`
> - `tools/writeback/wb_monitor.py`
> - `tools/rcu/rcu-cbs.py`
> - `tools/workqueue/wq_dump.py`
> - `tools/workqueue/wq_monitor.py`
> 
> Given this precedent, I believe it would be reasonable to place
> `show_page_info.py` under `tools/mm/`, since it's focused on memory
> subsystem internals and would follow a similar organizational pattern
> to the above.
> 
> I'd appreciate any input on whether this is a suitable direction.
> I'm happy to send the script for review once the location is agreed upon.
> 
> Thanks,  
> 
> Ye Liu

Hi,

The drgn repository and the kernel tools directory are both valid places
to put drgn scripts, and it's ultimately up to you where you'd prefer to
put it. Here are some factors to consider, though:

1. Reusability: if your script is very generic and would be widely
   useful, the ideal is to add it as a helper to drgn upstream. For
   scripts that are less generic but could still be useful to many
   people, I'd personally prefer for them to go into the drgn
   repository's tools or contrib directories. At the other extreme, if
   your script is only useful to a handful of developers of a specific
   subsystem, the kernel tools directory makes more sense.
2. Kernel version coupling: there are a couple of options for dealing
   with kernel changes that require drgn scripts to be updated (e.g.,
   struct member renames, data structure changes).  Scripts in the
   kernel tools directory tend to only handle the current version. This
   is simpler, but it also means that sometimes you can't use features
   from a new version of the script on old kernels. On the other hand,
   the drgn repository supports every kernel version that's still
   meaningfully deployed. This can complicate scripts with
   version-dependent logic, but it means you can always use the latest
   and greatest features on any kernel version. I prefer the latter
   approach, but the choice is yours (except for drgn helpers upstream,
   which are required to support all kernel versions).
3. Maintainership: who wants to own the script? A lot of the current
   drgn scripts in the kernel tools directory are written and maintained
   by the relevant subsystem maintainers, so it's a no-brainer for them
   to own it without any involvement from the drgn project. It's not as
   obvious for other contributors. If the subsystem maintainer is
   willing to own a drgn script in the kernel repo, then I won't
   complain. I'm willing to take just about anything for the drgn
   repository's contrib directory, and I'm slightly more selective for
   helpers and tools.

All that being said, your script looks pretty widely useful, so I think
it'd make sense in the drgn repository's contrib directory (or even the
tools directory if you want to make it a full-fledged tool with test
cases, support for all kernel versions, documentation, etc., but contrib
is fine).

Thanks,
Omar

Powered by blists - more mailing lists