Received-SPF: pass (google.com: domain of linux-kernel+bounces-211020-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240611110058.3444968-1-andrii@kernel.org> <20240611115950.35197b36eafe0a804ecaa0de@linux-foundation.org>
In-Reply-To: <20240611115950.35197b36eafe0a804ecaa0de@linux-foundation.org>
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date: Wed, 12 Jun 2024 07:57:59 +0100
Message-ID: <CAEf4BzZKYTnj-=PBSQ_74-o4rzKx2_VO4PRLwyq4szCcZvoGbQ@mail.gmail.com>
Subject: Re: [PATCH v4 0/7] ioctl()-based API to query VMAs from /proc/<pid>/maps
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrii Nakryiko <andrii@kernel.org>, linux-fsdevel@vger.kernel.org, brauner@kernel.org, 
	viro@zeniv.linux.org.uk, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, 
	gregkh@linuxfoundation.org, linux-mm@kvack.org, liam.howlett@oracle.com, 
	surenb@google.com, rppt@kernel.org, Alexey Dobriyan <adobriyan@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Jun 11, 2024 at 7:59=E2=80=AFPM Andrew Morton <akpm@linux-foundatio=
n.org> wrote:
>
>
> (Please cc Alexey on procfs changes)

ack, will do

>
> On Tue, 11 Jun 2024 04:00:48 -0700 Andrii Nakryiko <andrii@kernel.org> wr=
ote:
>
> > Implement binary ioctl()-based interface to /proc/<pid>/maps file to al=
low
> > applications to query VMA information more efficiently than reading *al=
l* VMAs
> > nonselectively through text-based interface of /proc/<pid>/maps file.
>
> Looks nice but I'll await further reviewer input.
>

Thanks! I'll work on adding more tests meanwhile.

> >
> > ...
> >
> >  Documentation/filesystems/proc.rst          |   9 +
> >  fs/proc/task_mmu.c                          | 366 +++++++++++--
> >  include/uapi/linux/fs.h                     | 156 +++++-
> >  tools/include/uapi/linux/fs.h               | 550 ++++++++++++++++++++
> >  tools/testing/selftests/bpf/.gitignore      |   1 +
> >  tools/testing/selftests/bpf/Makefile        |   2 +-
> >  tools/testing/selftests/bpf/procfs_query.c  | 386 ++++++++++++++
> >  tools/testing/selftests/bpf/test_progs.c    |   3 +
> >  tools/testing/selftests/bpf/test_progs.h    |   2 +
> >  tools/testing/selftests/bpf/trace_helpers.c | 104 +++-
> >  10 files changed, 1508 insertions(+), 71 deletions(-)
> >  create mode 100644 tools/include/uapi/linux/fs.h
> >  create mode 100644 tools/testing/selftests/bpf/procfs_query.c
>
> Should the selftests be under bpf/?  This is a procfs feature which
> could be used by many things apart from bpf and it really isn't a bpf
> thing at all.  Wouldn't tools/testing/selftests/proc/ be a more
> appropriate place?
>

Yep, agreed. I used BPF selftests as a quick and simple way to
validate it's working end-to-end (because we use /proc/<pid>/maps
across a bunch of pre-existing BPF selftests, so that gave me good
coverage and signal). I'll look into adding more tests under
selftests/proc for the next revision.

As for the procfs_query.c, it's not really a test, rather a custom
testing/benchmarking tool convenient for development, so I think I'll
drop it from the patch set for the next revision (and maybe will put
it up in a separate repo on Github or something).