Received-SPF: pass (google.com: domain of linux-kernel+bounces-168833-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240504003006.3303334-1-andrii@kernel.org> <20240504-rasch-gekrochen-3d577084beda@brauner>
 <2024050424-drift-evil-27de@gregkh>
In-Reply-To: <2024050424-drift-evil-27de@gregkh>
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date: Sat, 4 May 2024 14:50:36 -0700
Message-ID: <CAEf4BzbhdNcwJS8oofJQSOjSj7vyc2zR9n-mMZvrqtUw-YiaYQ@mail.gmail.com>
Subject: Re: [PATCH 0/5] ioctl()-based API to query VMAs from /proc/<pid>/maps
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Christian Brauner <brauner@kernel.org>, Andrii Nakryiko <andrii@kernel.org>, linux-fsdevel@vger.kernel.org, 
	viro@zeniv.linux.org.uk, akpm@linux-foundation.org, 
	linux-kernel@vger.kernel.org, bpf@vger.kernel.org, linux-mm@kvack.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, May 4, 2024 at 8:34=E2=80=AFAM Greg KH <gregkh@linuxfoundation.org>=
 wrote:
>
> On Sat, May 04, 2024 at 01:24:23PM +0200, Christian Brauner wrote:
> > On Fri, May 03, 2024 at 05:30:01PM -0700, Andrii Nakryiko wrote:
> > > Implement binary ioctl()-based interface to /proc/<pid>/maps file to =
allow
> > > applications to query VMA information more efficiently than through t=
extual
> > > processing of /proc/<pid>/maps contents. See patch #2 for the context=
,
> > > justification, and nuances of the API design.
> > >
> > > Patch #1 is a refactoring to keep VMA name logic determination in one=
 place.
> > > Patch #2 is the meat of kernel-side API.
> > > Patch #3 just syncs UAPI header (linux/fs.h) into tools/include.
> > > Patch #4 adjusts BPF selftests logic that currently parses /proc/<pid=
>/maps to
> > > optionally use this new ioctl()-based API, if supported.
> > > Patch #5 implements a simple C tool to demonstrate intended efficient=
 use (for
> > > both textual and binary interfaces) and allows benchmarking them. Pat=
ch itself
> > > also has performance numbers of a test based on one of the medium-siz=
ed
> > > internal applications taken from production.
> >
> > I don't have anything against adding a binary interface for this. But
> > it's somewhat odd to do ioctls based on /proc files. I wonder if there
> > isn't a more suitable place for this. prctl()? New vmstat() system call
> > using a pidfd/pid as reference? ioctl() on fs/pidfs.c?
>
> See my objection to the ioctl api in the patch review itself.

Will address them there.


>
> Also, as this is a new user/kernel api, it needs loads of documentation
> (there was none), and probably also cc: linux-api, right?

Will cc linux-api. And yes, I didn't want to invest too much time in
documentation upfront, as I knew that API itself will be tweaked and
tuned, moved to some other place (see Christian's pidfd suggestion).
But I'm happy to write it, I'd appreciate the pointers where exactly
this should live. Thanks!

>
> thanks,
>
> greg k-h