From: Jan Kara <jack@suse.cz>
To: Matthew Bobrowski <repnop@google.com>
Cc: Amir Goldstein <amir73il@gmail.com>, Jan Kara <jack@suse.cz>,
Christian Brauner <christian.brauner@ubuntu.com>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Linux API <linux-api@vger.kernel.org>
Subject: Re: [PATCH v2 0/5] Add pidfd support to the fanotify API
Date: Thu, 10 Jun 2021 13:32:40 +0200 [thread overview]
Message-ID: <20210610113240.GC23539@quack2.suse.cz> (raw)
In-Reply-To: <YMG3crGB2RYZtVmf@google.com>
On Thu 10-06-21 16:55:46, Matthew Bobrowski wrote:
> > > fanotify: add pidfd support to the fanotify API
> > >
> >
> > This one looks mostly fine. Gave some minor comments.
> >
> > The biggest thing I am missing is a link to an LTP test draft and
> > man page update draft.
>
> Fair point, the way I approached it was that I'd get the ACK from all of
> you on the overall implementation and then go ahead with providing
> additional things like LTP and man-pages drafts, before the merge is
> performed.
>
> > In general, I think it is good practice to provide a test along with any
> > fix, but for UAPI changes we need to hold higher standards - both the
> > test and man page draft should be a must before merge IMO.
>
> Agree, moving forward I will take this approach.
>
> > We already know there is going to be a clause about FAN_NOPIDFD
> > and so on... I think it is especially hard for people on linux-api list to
> > review a UAPI change without seeing the contract in a user manual
> > format. Yes, much of the information is in the commit message, but it
> > is not the same thing as reading a user manual and verifying that the
> > contract makes sense to a programmer.
>
> Makes sense.
I agree with Amir that before your patches can get merged we need a manpage
update & LTP coverage. But I fully understand your approach of trying to
figure out how things will look like before writing the tests and manpage
to save some adaptation of tests & doc as the code changes. For relatively
simple changes like this one that approach is fine by me as well (for more
complex API changes it's often easier to actually *start* with a manpage to
get an idea where we are actually heading). I just want the tests & doc to
be part of at least one submission so that e.g. people on linux-api have a
good chance to review stuff without having to dive into code details.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
next prev parent reply other threads:[~2021-06-10 11:32 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 0:19 [PATCH v2 0/5] Add pidfd support to the fanotify API Matthew Bobrowski
2021-06-10 0:20 ` [PATCH v2 1/5] kernel/pid.c: remove static qualifier from pidfd_create() Matthew Bobrowski
2021-06-15 11:34 ` Christian Brauner
2021-06-10 0:20 ` [PATCH v2 2/5] kernel/pid.c: implement additional checks upon pidfd_create() parameters Matthew Bobrowski
2021-06-15 11:35 ` Christian Brauner
2021-06-10 0:21 ` [PATCH v2 3/5] fanotify/fanotify_user.c: minor cosmetic adjustments to fid labels Matthew Bobrowski
2021-06-10 0:21 ` [PATCH v2 4/5] fanotify/fanotify_user.c: introduce a generic info record copying helper Matthew Bobrowski
2021-06-10 0:21 ` [PATCH v2 5/5] fanotify: add pidfd support to the fanotify API Matthew Bobrowski
2021-06-10 5:18 ` Amir Goldstein
2021-06-10 6:35 ` Matthew Bobrowski
2021-06-10 7:11 ` Amir Goldstein
2021-06-10 7:24 ` Matthew Bobrowski
2021-06-10 11:23 ` Jan Kara
2021-06-11 0:32 ` Matthew Bobrowski
2021-07-10 14:49 ` Amir Goldstein
2021-07-14 0:18 ` Matthew Bobrowski
2021-06-10 5:37 ` [PATCH v2 0/5] Add " Amir Goldstein
2021-06-10 6:55 ` Matthew Bobrowski
2021-06-10 11:32 ` Jan Kara [this message]
2021-06-11 0:35 ` Matthew Bobrowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210610113240.GC23539@quack2.suse.cz \
--to=jack@suse.cz \
--cc=amir73il@gmail.com \
--cc=christian.brauner@ubuntu.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=repnop@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.