On Thu, Jun 10, 2021 at 3:19 AM Matthew Bobrowski <repnop@xxxxxxxxxx> wrote: > > Hey Jan/Amir/Christian, > > Sending through v2 of the fanotify pidfd patch series. This series > contains the necessary fixes/suggestions that had come out of the > previous discussions, which can be found here [0], here [1], and here > [3]. > > The main difference in this series is that we perform pidfd creation a > little earlier on i.e. in copy_event_to_user() so that clean up of the > pidfd can be performed nicely in the event of an info > generation/copying error. Additionally, we introduce two errors. One > being FAN_NOPIDFD, which is supplied to the listener in the event that > a pidfd cannot be created due to early process termination. The other > being FAN_EPIDFD, which will be supplied in the event that an error > was encountered during pidfd creation. > > Please let me know what you think. > > [0] > https://lore.kernel.org/linux-fsdevel/48d18055deb4617d97c695a08dca77eb57309\ > 7e9.1621473846.git.repnop@xxxxxxxxxx/ > > [1] > https://lore.kernel.org/linux-fsdevel/24c761bd0bd1618c911a392d0c310c24da7d8\ > 941.1621473846.git.repnop@xxxxxxxxxx/ > > [2] > https://lore.kernel.org/linux-fsdevel/48d18055deb4617d97c695a08dca77eb57309\ > 7e9.1621473846.git.repnop@xxxxxxxxxx/ > > > Matthew Bobrowski (5): > kernel/pid.c: remove static qualifier from pidfd_create() > kernel/pid.c: implement additional checks upon pidfd_create() > parameters > fanotify/fanotify_user.c: minor cosmetic adjustments to fid labels > fanotify/fanotify_user.c: introduce a generic info record copying > helper Above fanotify commits look good to me. Please remove /fanotify_user.c from commit titles and use 'pidfd:' for the pidfd commit titles. > 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. 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. 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. Thanks, Amir.
