All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] fanotify: Document FAN_REPORT_PIDFD Feature
@ 2022-04-11 23:17 Amir Goldstein
  2022-04-13 18:24 ` Alejandro Colomar
  0 siblings, 1 reply; 5+ messages in thread
From: Amir Goldstein @ 2022-04-11 23:17 UTC (permalink / raw)
  To: mtk.manpages, alx.manpages; +Cc: linux-man, jack, amir73il

Update the fanotify API documentation to include details on the new
FAN_REPORT_PIDFD feature. This patch also includes a generic section
describing the concept of information records which are supported by
the fanotify API.

Signed-off-by: Matthew Bobrowski <repnop@google.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 man2/fanotify_init.2 |  34 +++++++
 man7/fanotify.7      | 208 +++++++++++++++++++++++++++++++++++--------
 2 files changed, 204 insertions(+), 38 deletions(-)

diff --git a/man2/fanotify_init.2 b/man2/fanotify_init.2
index 7f9a21a52..1e4691c88 100644
--- a/man2/fanotify_init.2
+++ b/man2/fanotify_init.2
@@ -283,6 +283,40 @@ for additional details.
 This is a synonym for
 .RB ( FAN_REPORT_DIR_FID | FAN_REPORT_NAME ).
 .PP
+.TP
+.BR FAN_REPORT_PIDFD " (since Linux 5.15)"
+.\" commit af579beb666aefb17e9a335c12c788c92932baf1
+Events for fanotify groups initialized with this flag will contain an
+additional information record alongside the generic
+.I fanotify_event_metadata
+structure.
+This information record will be of type
+.B FAN_EVENT_INFO_TYPE_PIDFD
+and will contain a pidfd for the process that was responsible for
+generating an event.
+A pidfd returned in this information record object is no different
+to the pidfd that is returned when calling
+.BR pidfd_open(2) .
+Usage of this information record are for applications that may be
+interested in reliably determining whether the process responsible for
+generating an event has been recycled or terminated.
+The use of the
+.B FAN_REPORT_TID
+flag along with
+.B FAN_REPORT_PIDFD
+is currently not supported and attempting to do so will result in the
+error
+.B EINVAL
+being returned.
+This limitation is currently imposed by the pidfd API as it currently
+only supports the creation of pidfds for thread-group
+leaders.
+Creating pidfds for non-thread-group leaders may be supported at some
+point in the future, so this restriction may eventually be
+lifted.
+For more details on information records, see
+.BR fanotify(7) .
+.PP
 The
 .I event_f_flags
 argument
diff --git a/man7/fanotify.7 b/man7/fanotify.7
index f8345b3f5..57dd2b040 100644
--- a/man7/fanotify.7
+++ b/man7/fanotify.7
@@ -118,16 +118,6 @@ until either a file event occurs or the call is interrupted by a signal
 (see
 .BR signal (7)).
 .PP
-The use of one of the flags
-.BR FAN_REPORT_FID ,
-.B FAN_REPORT_DIR_FID
-in
-.BR fanotify_init (2)
-influences what data structures are returned to the event listener for each
-event.
-Events reported to a group initialized with one of these flags will
-use file handles to identify filesystem objects instead of file descriptors.
-.PP
 After a successful
 .BR read (2),
 the read buffer contains one or more of the following structures:
@@ -146,20 +136,63 @@ struct fanotify_event_metadata {
 .EE
 .in
 .PP
-In case of an fanotify group that identifies filesystem objects by file
-handles, you should also expect to receive one or more additional information
-records of the structure detailed below following the generic
+Information records are supplemental pieces of information that may be
+provided alongside the generic
+.I fanotify_event_metadata
+structure.
+The
+.I flags
+passed to
+.BR fanotify_init (2)
+have influence over the type of information records that may be returned
+for an event.
+For example, if a notification group is initialized with
+.B FAN_REPORT_FID
+or
+.BR FAN_REPORT_DIR_FID ,
+then event listeners should also expect to receive a
+.I fanotify_event_info_fid
+structure alongside the
+.I fanotify_event_metadata
+structure, whereby file handles are used to identify filesystem
+objects rather than file descriptors.
+Information records may also be stacked, meaning that using the
+various
+.B FAN_REPORT_*
+flags in conjunction with one another is supported.
+In such cases, multiple information records can be returned for an
+event alongside the generic
+.I fanotify_event_metadata
+structure.
+For example, if a notification group is initialized with
+.B FAN_REPORT_FID
+and
+.BR FAN_REPORT_PIDFD ,
+then an event listener should also expect to receive both
+.I fanotify_event_info_fid
+and
+.I fanotify_event_info_pidfd
+structures alongside the generic
+.I fanotify_event_metadata
+structure.
+Importantly, fanotify provides no guarantee around the ordering of
+information records when a notification group is intialized with a
+stacked based configuration.
+Each information record has a nested structure of type
+.IR fanotify_event_info_header .
+It is imperative for event listeners to inspect the
+.I info_type
+field of this structure in order to determine the type of information
+record that had been received for a given event.
+.PP
+In cases where an fanotify group identifies filesystem objects by file
+handles, event listeners should also expect to receive one or more of
+the below information record objects alongside the generic
 .I fanotify_event_metadata
 structure within the read buffer:
 .PP
 .in +4n
 .EX
-struct fanotify_event_info_header {
-    __u8 info_type;
-    __u8 pad;
-    __u16 len;
-};
-
 struct fanotify_event_info_fid {
     struct fanotify_event_info_header hdr;
     __kernel_fsid_t fsid;
@@ -168,6 +201,40 @@ struct fanotify_event_info_fid {
 .EE
 .in
 .PP
+In cases where an fanotify group is initialized with
+.BR FAN_REPORT_PIDFD ,
+event listeners should expect to receive the below information record
+object alongside the generic
+.I fanotify_event_metadata
+structure within the read buffer:
+.PP
+.in +4n
+.EX
+struct fanotify_event_info_pidfd {
+        struct fanotify_event_info_header hdr;
+        __s32 pidfd;
+};
+.EE
+.in
+.PP
+All information records contain a nested structure of type
+.IR fanotify_event_info_header .
+This structure holds meta-information about the information record
+that may have been returned alongside the generic
+.I fanotify_event_metadata
+structure.
+This structure is defined as follows:
+.PP
+.in +4n
+.EX
+struct fanotify_event_info_header {
+	__u8 info_type;
+	__u8 pad;
+	__u16 len;
+};
+.EE
+.in
+.PP
 For performance reasons, it is recommended to use a large
 buffer size (for example, 4096 bytes),
 so that multiple events can be retrieved by a single
@@ -385,6 +452,48 @@ The
 flag is reported in an event mask only if the fanotify group identifies
 filesystem objects by file handles.
 .PP
+Information records that are supplied alongside the generic
+.I fanotify_event_metadata
+structure will always contain a nested structure of type
+.IR fanotify_event_info_header .
+The fields of the
+.I fanotify_event_info_header
+are as follows:
+.TP
+.I info_type
+A unique integer value representing the type of information record
+object received for an event.
+The value of this field can be set to one of the following:
+.BR FAN_EVENT_INFO_TYPE_FID ,
+.BR FAN_EVENT_INFO_TYPE_DFID ,
+.B FAN_EVENT_INFO_TYPE_DFID_NAME
+or
+.BR FAN_EVENT_INFO_TYPE_PIDFD .
+The value set for this field is dependent on the flags that have been
+supplied to
+.BR fanotify_init (2) .
+Refer to the field details of each information record object type
+below to understand the different cases in which the
+.I info_type
+values can be set.
+.TP
+.I pad
+This field is currently not used by any information record object type
+and therefore is set to zero.
+.TP
+.I len
+The value of
+.I len
+is set to the size of the information record object, including the
+.IR fanotify_event_info_header .
+The total size of all additional information records is not expected
+to be larger than
+(
+.I event_len
+\-
+.I metadata_len
+).
+.PP
 The fields of the
 .I fanotify_event_info_fid
 structure are as follows:
@@ -392,8 +501,6 @@ structure are as follows:
 .I hdr
 This is a structure of type
 .IR fanotify_event_info_header .
-It is a generic header that contains information used to describe an
-additional information record attached to the event.
 For example, when an fanotify file descriptor is created using
 .BR FAN_REPORT_FID ,
 a single information record is expected to be attached to the event with
@@ -414,23 +521,6 @@ identifying a parent directory object, and one with
 field value of
 .BR FAN_EVENT_INFO_TYPE_FID ,
 identifying a non-directory object.
-The
-.I fanotify_event_info_header
-contains a
-.I len
-field.
-The value of
-.I len
-is the size of the additional information record including the
-.I fanotify_event_info_header
-itself.
-The total size of all additional information records is not expected
-to be bigger than
-(
-.I event_len
-\-
-.I metadata_len
-).
 .TP
 .I fsid
 This is a unique identifier of the filesystem containing the object
@@ -498,6 +588,48 @@ and the file handle is followed by a null terminated string that identifies the
 name of a directory entry in that directory, or '.' to identify the directory
 object itself.
 .PP
+The fields of the
+.I fanotify_event_info_pidfd
+structure are as follows:
+.TP
+.I hdr
+This is a structure of type
+.IR fanotify_event_info_header .
+When an fanotify group is initialized using
+.BR FAN_REPORT_PIDFD ,
+the
+.I info_type
+field value of the
+.I fanotify_event_info_header
+is set to
+.BR FAN_EVENT_INFO_TYPE_PIDFD .
+.TP
+.I pidfd
+This is a process file descriptor that refers to the process
+responsible for generating the event.
+The returned process file descriptor is no different from one which
+could be obtained manually if
+.BR pidfd_open(2)
+were to be called on
+.IR fanotify_event_metadata.pid .
+In the instance that an error is encountered during pidfd creation for
+one of two possible error types represented by a negative integer
+value may be returned in this
+.I pidfd
+field.
+In cases where the process responsible for generating the event has
+terminated prior to the event listener being able to read events from the
+notification queue,
+.B FAN_NOPIDFD
+is returned.
+The pidfd creation for an event is only performed at the time the
+events are read from the notification queue.
+All other possible pidfd creation failures are represented by
+.BR FAN_EPIDFD .
+Once the event listener has dealt with an event and the pidfd is no
+longer required, the pidfd should be closed via
+.BR close(2) .
+.PP
 The following macros are provided to iterate over a buffer containing
 fanotify event metadata returned by a
 .BR read (2)
-- 
2.35.1.1178.g4f1659d476-goog

/M

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] fanotify: Document FAN_REPORT_PIDFD Feature
  2022-04-11 23:17 [PATCH v3] fanotify: Document FAN_REPORT_PIDFD Feature Amir Goldstein
@ 2022-04-13 18:24 ` Alejandro Colomar
  2022-04-13 23:40   ` Matthew Bobrowski
  0 siblings, 1 reply; 5+ messages in thread
From: Alejandro Colomar @ 2022-04-13 18:24 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: linux-man, jack, amir73il, mtk.manpages


[-- Attachment #1.1: Type: text/plain, Size: 13286 bytes --]

Hi Amir!

On 4/12/22 01:17, Amir Goldstein wrote:
> Update the fanotify API documentation to include details on the new
> FAN_REPORT_PIDFD feature. This patch also includes a generic section
> describing the concept of information records which are supported by
> the fanotify API.
> 
> Signed-off-by: Matthew Bobrowski <repnop@google.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks for the patch.  Please see some comments below.

Cheers,

Alex

> ---
>   man2/fanotify_init.2 |  34 +++++++
>   man7/fanotify.7      | 208 +++++++++++++++++++++++++++++++++++--------
>   2 files changed, 204 insertions(+), 38 deletions(-)
> 
> diff --git a/man2/fanotify_init.2 b/man2/fanotify_init.2
> index 7f9a21a52..1e4691c88 100644
> --- a/man2/fanotify_init.2
> +++ b/man2/fanotify_init.2
> @@ -283,6 +283,40 @@ for additional details.
>   This is a synonym for
>   .RB ( FAN_REPORT_DIR_FID | FAN_REPORT_NAME ).
>   .PP
> +.TP
> +.BR FAN_REPORT_PIDFD " (since Linux 5.15)"
> +.\" commit af579beb666aefb17e9a335c12c788c92932baf1
> +Events for fanotify groups initialized with this flag will contain an
> +additional information record alongside the generic
> +.I fanotify_event_metadata
> +structure.
> +This information record will be of type
> +.B FAN_EVENT_INFO_TYPE_PIDFD
> +and will contain a pidfd for the process that was responsible for
> +generating an event.
> +A pidfd returned in this information record object is no different
> +to the pidfd that is returned when calling
> +.BR pidfd_open(2) .

Misplaced space.  Should be:

.BR pidfd_open (2).

> +Usage of this information record are for applications that may be
> +interested in reliably determining whether the process responsible for
> +generating an event has been recycled or terminated.
> +The use of the
> +.B FAN_REPORT_TID
> +flag along with
> +.B FAN_REPORT_PIDFD
> +is currently not supported and attempting to do so will result in the
> +error
> +.B EINVAL
> +being returned.
> +This limitation is currently imposed by the pidfd API as it currently
> +only supports the creation of pidfds for thread-group
> +leaders.

Please use semantic newlines.
See man-pages(7):

    Use semantic newlines
        In the source of a manual page, new sentences  should  be
        started on new lines, long sentences should be split into
        lines at clause breaks (commas, semicolons,  colons,  and
        so on), and long clauses should be split at phrase bound-
        aries.  This convention,  sometimes  known  as  "semantic
        newlines",  makes it easier to see the effect of patches,
        which often operate at the level of individual sentences,
        clauses, or phrases.


> +Creating pidfds for non-thread-group leaders may be supported at some
> +point in the future, so this restriction may eventually be
> +lifted.
> +For more details on information records, see
> +.BR fanotify(7) .

Misplaced space.  Should be:

.BR fanotify (7).

> +.PP
>   The
>   .I event_f_flags
>   argument
> diff --git a/man7/fanotify.7 b/man7/fanotify.7
> index f8345b3f5..57dd2b040 100644
> --- a/man7/fanotify.7
> +++ b/man7/fanotify.7
> @@ -118,16 +118,6 @@ until either a file event occurs or the call is interrupted by a signal
>   (see
>   .BR signal (7)).
>   .PP
> -The use of one of the flags
> -.BR FAN_REPORT_FID ,
> -.B FAN_REPORT_DIR_FID
> -in
> -.BR fanotify_init (2)
> -influences what data structures are returned to the event listener for each
> -event.
> -Events reported to a group initialized with one of these flags will
> -use file handles to identify filesystem objects instead of file descriptors.
> -.PP
>   After a successful
>   .BR read (2),
>   the read buffer contains one or more of the following structures:
> @@ -146,20 +136,63 @@ struct fanotify_event_metadata {
>   .EE
>   .in
>   .PP
> -In case of an fanotify group that identifies filesystem objects by file
> -handles, you should also expect to receive one or more additional information
> -records of the structure detailed below following the generic
> +Information records are supplemental pieces of information that may be
> +provided alongside the generic
> +.I fanotify_event_metadata
> +structure.
> +The
> +.I flags
> +passed to
> +.BR fanotify_init (2) > +have influence over the type of information records that may be returned
> +for an event.
> +For example, if a notification group is initialized with
> +.B FAN_REPORT_FID
> +or
> +.BR FAN_REPORT_DIR_FID ,
> +then event listeners should also expect to receive a
> +.I fanotify_event_info_fid
> +structure alongside the
> +.I fanotify_event_metadata
> +structure, whereby file handles are used to identify filesystem
> +objects rather than file descriptors.
> +Information records may also be stacked, meaning that using the
> +various
> +.B FAN_REPORT_*
> +flags in conjunction with one another is supported.
> +In such cases, multiple information records can be returned for an
> +event alongside the generic
> +.I fanotify_event_metadata
> +structure.
> +For example, if a notification group is initialized with
> +.B FAN_REPORT_FID
> +and
> +.BR FAN_REPORT_PIDFD ,
> +then an event listener should also expect to receive both
> +.I fanotify_event_info_fid
> +and
> +.I fanotify_event_info_pidfd
> +structures alongside the generic
> +.I fanotify_event_metadata
> +structure.
> +Importantly, fanotify provides no guarantee around the ordering of
> +information records when a notification group is intialized with a
> +stacked based configuration.
> +Each information record has a nested structure of type
> +.IR fanotify_event_info_header .
> +It is imperative for event listeners to inspect the
> +.I info_type
> +field of this structure in order to determine the type of information
> +record that had been received for a given event.
> +.PP
> +In cases where an fanotify group identifies filesystem objects by file
> +handles, event listeners should also expect to receive one or more of
> +the below information record objects alongside the generic
>   .I fanotify_event_metadata
>   structure within the read buffer:
>   .PP
>   .in +4n
>   .EX
> -struct fanotify_event_info_header {
> -    __u8 info_type;
> -    __u8 pad;
> -    __u16 len;
> -};
> -
>   struct fanotify_event_info_fid {
>       struct fanotify_event_info_header hdr;
>       __kernel_fsid_t fsid;
> @@ -168,6 +201,40 @@ struct fanotify_event_info_fid {
>   .EE
>   .in
>   .PP
> +In cases where an fanotify group is initialized with
> +.BR FAN_REPORT_PIDFD ,
> +event listeners should expect to receive the below information record
> +object alongside the generic
> +.I fanotify_event_metadata
> +structure within the read buffer:
> +.PP
> +.in +4n
> +.EX
> +struct fanotify_event_info_pidfd {
> +        struct fanotify_event_info_header hdr;
> +        __s32 pidfd;
> +};
> +.EE
> +.in
> +.PP
> +All information records contain a nested structure of type
> +.IR fanotify_event_info_header .
> +This structure holds meta-information about the information record
> +that may have been returned alongside the generic
> +.I fanotify_event_metadata
> +structure.
> +This structure is defined as follows:
> +.PP
> +.in +4n
> +.EX
> +struct fanotify_event_info_header {
> +	__u8 info_type;
> +	__u8 pad;
> +	__u16 len;
> +};
> +.EE
> +.in
> +.PP
>   For performance reasons, it is recommended to use a large
>   buffer size (for example, 4096 bytes),
>   so that multiple events can be retrieved by a single
> @@ -385,6 +452,48 @@ The
>   flag is reported in an event mask only if the fanotify group identifies
>   filesystem objects by file handles.
>   .PP
> +Information records that are supplied alongside the generic
> +.I fanotify_event_metadata
> +structure will always contain a nested structure of type
> +.IR fanotify_event_info_header .
> +The fields of the
> +.I fanotify_event_info_header
> +are as follows:
> +.TP
> +.I info_type
> +A unique integer value representing the type of information record
> +object received for an event.
> +The value of this field can be set to one of the following:
> +.BR FAN_EVENT_INFO_TYPE_FID ,
> +.BR FAN_EVENT_INFO_TYPE_DFID ,
> +.B FAN_EVENT_INFO_TYPE_DFID_NAME
> +or
> +.BR FAN_EVENT_INFO_TYPE_PIDFD .

Use Oxford-style commas.  See:

$ git show 3ded684c1a4b6104e1d4b7400015f8bf76dc75b9 | head -n25
commit 3ded684c1a4b6104e1d4b7400015f8bf76dc75b9
Author: Michael Kerrisk <mtk.manpages@gmail.com>
Date:   Sat Jan 9 11:14:08 2021 +0100

     Various pages: tfix (Oxford comma)

     Found using:

         git grep -lE '^[^.].*,.*,.*[^,] (and|or)\>'

     Signed-off-by: Michael Kerrisk <mtk.manpages@gmail.com>

diff --git a/man1/intro.1 b/man1/intro.1
index 57023b652..9bf7190df 100644
--- a/man1/intro.1
+++ b/man1/intro.1
@@ -220,7 +220,7 @@ and
  .I pwd
  commands and explore
  .I cd
-usage: "cd", "cd .", "cd ..", "cd /" and "cd \(ti".
+usage: "cd", "cd .", "cd ..", "cd /", and "cd \(ti".
  .SS Directories
  The command
  .I mkdir


> +The value set for this field is dependent on the flags that have been
> +supplied to
> +.BR fanotify_init (2) .

Spurious space.  Should be:

.BR fanotify_init (2).

> +Refer to the field details of each information record object type
> +below to understand the different cases in which the
> +.I info_type
> +values can be set.
> +.TP
> +.I pad
> +This field is currently not used by any information record object type
> +and therefore is set to zero.
> +.TP
> +.I len
> +The value of
> +.I len
> +is set to the size of the information record object, including the
> +.IR fanotify_event_info_header .
> +The total size of all additional information records is not expected
> +to be larger than
> +(
> +.I event_len
> +\-
> +.I metadata_len
> +).

The above can be simplified to:

.RI ( event_len
\-
.IR metadata_len ).

> +.PP
>   The fields of the
>   .I fanotify_event_info_fid
>   structure are as follows:
> @@ -392,8 +501,6 @@ structure are as follows:
>   .I hdr
>   This is a structure of type
>   .IR fanotify_event_info_header .
> -It is a generic header that contains information used to describe an
> -additional information record attached to the event.
>   For example, when an fanotify file descriptor is created using
>   .BR FAN_REPORT_FID ,
>   a single information record is expected to be attached to the event with
> @@ -414,23 +521,6 @@ identifying a parent directory object, and one with
>   field value of
>   .BR FAN_EVENT_INFO_TYPE_FID ,
>   identifying a non-directory object.
> -The
> -.I fanotify_event_info_header
> -contains a
> -.I len
> -field.
> -The value of
> -.I len
> -is the size of the additional information record including the
> -.I fanotify_event_info_header
> -itself.
> -The total size of all additional information records is not expected
> -to be bigger than
> -(
> -.I event_len
> -\-
> -.I metadata_len
> -).
>   .TP
>   .I fsid
>   This is a unique identifier of the filesystem containing the object
> @@ -498,6 +588,48 @@ and the file handle is followed by a null terminated string that identifies the
>   name of a directory entry in that directory, or '.' to identify the directory
>   object itself.
>   .PP
> +The fields of the
> +.I fanotify_event_info_pidfd
> +structure are as follows:
> +.TP
> +.I hdr
> +This is a structure of type
> +.IR fanotify_event_info_header .
> +When an fanotify group is initialized using
> +.BR FAN_REPORT_PIDFD ,
> +the
> +.I info_type
> +field value of the
> +.I fanotify_event_info_header
> +is set to
> +.BR FAN_EVENT_INFO_TYPE_PIDFD .
> +.TP
> +.I pidfd
> +This is a process file descriptor that refers to the process
> +responsible for generating the event.
> +The returned process file descriptor is no different from one which
> +could be obtained manually if
> +.BR pidfd_open(2)

Missing a space before "(2)".

> +were to be called on
> +.IR fanotify_event_metadata.pid .
> +In the instance that an error is encountered during pidfd creation for
> +one of two possible error types represented by a negative integer
> +value may be returned in this
> +.I pidfd
> +field.
> +In cases where the process responsible for generating the event has
> +terminated prior to the event listener being able to read events from the
> +notification queue,
> +.B FAN_NOPIDFD
> +is returned.
> +The pidfd creation for an event is only performed at the time the
> +events are read from the notification queue.
> +All other possible pidfd creation failures are represented by
> +.BR FAN_EPIDFD .
> +Once the event listener has dealt with an event and the pidfd is no
> +longer required, the pidfd should be closed via
> +.BR close(2) .

Space is misplaced.  Should be:

.BR close (2).

> +.PP
>   The following macros are provided to iterate over a buffer containing
>   fanotify event metadata returned by a
>   .BR read (2)


-- 
--
Alejandro Colomar
Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] fanotify: Document FAN_REPORT_PIDFD Feature
  2022-04-13 18:24 ` Alejandro Colomar
@ 2022-04-13 23:40   ` Matthew Bobrowski
  2022-04-25 20:18     ` Alejandro Colomar
  0 siblings, 1 reply; 5+ messages in thread
From: Matthew Bobrowski @ 2022-04-13 23:40 UTC (permalink / raw)
  To: Alejandro Colomar; +Cc: linux-man, jack, amir73il, mtk.manpages

Haha, I created this patch using one of Amir's branches, as he
performed a rebase and handled some conflicts. It must've preserved
the display name "Amir Goldstein" in the "From:" header...

On Wed, Apr 13, 2022 at 08:24:21PM +0200, Alejandro Colomar wrote:
> Hi Amir!
> 
> On 4/12/22 01:17, Amir Goldstein wrote:
> > Update the fanotify API documentation to include details on the new
> > FAN_REPORT_PIDFD feature. This patch also includes a generic section
> > describing the concept of information records which are supported by
> > the fanotify API.
> > 
> > Signed-off-by: Matthew Bobrowski <repnop@google.com>
> > Reviewed-by: Jan Kara <jack@suse.cz>
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> Thanks for the patch.  Please see some comments below.

Thanks for the review, I'll update and send through a follow up patch
shortly. I left a question on your comment about the use of semantic
newlines. I wasn't sure whether that comment was a general rule that
is to be applied across this entire patch (in which it definitely can,
I just wasn't aware of the rule prior to you explicitly pointing it
out), or whether there was a specific example you were referring to in
the code block directly above your comment.

> > ---
> >   man2/fanotify_init.2 |  34 +++++++
> >   man7/fanotify.7      | 208 +++++++++++++++++++++++++++++++++++--------
> >   2 files changed, 204 insertions(+), 38 deletions(-)
> > 
> > diff --git a/man2/fanotify_init.2 b/man2/fanotify_init.2
> > index 7f9a21a52..1e4691c88 100644
> > --- a/man2/fanotify_init.2
> > +++ b/man2/fanotify_init.2
> > @@ -283,6 +283,40 @@ for additional details.
> >   This is a synonym for
> >   .RB ( FAN_REPORT_DIR_FID | FAN_REPORT_NAME ).
> >   .PP
> > +.TP
> > +.BR FAN_REPORT_PIDFD " (since Linux 5.15)"
> > +.\" commit af579beb666aefb17e9a335c12c788c92932baf1
> > +Events for fanotify groups initialized with this flag will contain an
> > +additional information record alongside the generic
> > +.I fanotify_event_metadata
> > +structure.
> > +This information record will be of type
> > +.B FAN_EVENT_INFO_TYPE_PIDFD
> > +and will contain a pidfd for the process that was responsible for
> > +generating an event.
> > +A pidfd returned in this information record object is no different
> > +to the pidfd that is returned when calling
> > +.BR pidfd_open(2) .
> 
> Misplaced space.  Should be:
> 
> .BR pidfd_open (2).
> 
> > +Usage of this information record are for applications that may be
> > +interested in reliably determining whether the process responsible for
> > +generating an event has been recycled or terminated.
> > +The use of the
> > +.B FAN_REPORT_TID
> > +flag along with
> > +.B FAN_REPORT_PIDFD
> > +is currently not supported and attempting to do so will result in the
> > +error
> > +.B EINVAL
> > +being returned.
> > +This limitation is currently imposed by the pidfd API as it currently
> > +only supports the creation of pidfds for thread-group
> > +leaders.
> 
> Please use semantic newlines.
> See man-pages(7):
> 
>    Use semantic newlines
>        In the source of a manual page, new sentences  should  be
>        started on new lines, long sentences should be split into
>        lines at clause breaks (commas, semicolons,  colons,  and
>        so on), and long clauses should be split at phrase bound-
>        aries.  This convention,  sometimes  known  as  "semantic
>        newlines",  makes it easier to see the effect of patches,
>        which often operate at the level of individual sentences,
>        clauses, or phrases.

Fair point. Based on the above change block, I assume you're referring
to the the word "leaders" starting on a new line? The rest appears to
be inline with what's expected of semantic newlines?

> > +Creating pidfds for non-thread-group leaders may be supported at some
> > +point in the future, so this restriction may eventually be
> > +lifted.
> > +For more details on information records, see
> > +.BR fanotify(7) .
> 
> Misplaced space.  Should be:
> 
> .BR fanotify (7).
> 
> > +.PP
> >   The
> >   .I event_f_flags
> >   argument
> > diff --git a/man7/fanotify.7 b/man7/fanotify.7
> > index f8345b3f5..57dd2b040 100644
> > --- a/man7/fanotify.7
> > +++ b/man7/fanotify.7
> > @@ -118,16 +118,6 @@ until either a file event occurs or the call is interrupted by a signal
> >   (see
> >   .BR signal (7)).
> >   .PP
> > -The use of one of the flags
> > -.BR FAN_REPORT_FID ,
> > -.B FAN_REPORT_DIR_FID
> > -in
> > -.BR fanotify_init (2)
> > -influences what data structures are returned to the event listener for each
> > -event.
> > -Events reported to a group initialized with one of these flags will
> > -use file handles to identify filesystem objects instead of file descriptors.
> > -.PP
> >   After a successful
> >   .BR read (2),
> >   the read buffer contains one or more of the following structures:
> > @@ -146,20 +136,63 @@ struct fanotify_event_metadata {
> >   .EE
> >   .in
> >   .PP
> > -In case of an fanotify group that identifies filesystem objects by file
> > -handles, you should also expect to receive one or more additional information
> > -records of the structure detailed below following the generic
> > +Information records are supplemental pieces of information that may be
> > +provided alongside the generic
> > +.I fanotify_event_metadata
> > +structure.
> > +The
> > +.I flags
> > +passed to
> > +.BR fanotify_init (2) > +have influence over the type of information records that may be returned
> > +for an event.
> > +For example, if a notification group is initialized with
> > +.B FAN_REPORT_FID
> > +or
> > +.BR FAN_REPORT_DIR_FID ,
> > +then event listeners should also expect to receive a
> > +.I fanotify_event_info_fid
> > +structure alongside the
> > +.I fanotify_event_metadata
> > +structure, whereby file handles are used to identify filesystem
> > +objects rather than file descriptors.
> > +Information records may also be stacked, meaning that using the
> > +various
> > +.B FAN_REPORT_*
> > +flags in conjunction with one another is supported.
> > +In such cases, multiple information records can be returned for an
> > +event alongside the generic
> > +.I fanotify_event_metadata
> > +structure.
> > +For example, if a notification group is initialized with
> > +.B FAN_REPORT_FID
> > +and
> > +.BR FAN_REPORT_PIDFD ,
> > +then an event listener should also expect to receive both
> > +.I fanotify_event_info_fid
> > +and
> > +.I fanotify_event_info_pidfd
> > +structures alongside the generic
> > +.I fanotify_event_metadata
> > +structure.
> > +Importantly, fanotify provides no guarantee around the ordering of
> > +information records when a notification group is intialized with a
> > +stacked based configuration.
> > +Each information record has a nested structure of type
> > +.IR fanotify_event_info_header .
> > +It is imperative for event listeners to inspect the
> > +.I info_type
> > +field of this structure in order to determine the type of information
> > +record that had been received for a given event.
> > +.PP
> > +In cases where an fanotify group identifies filesystem objects by file
> > +handles, event listeners should also expect to receive one or more of
> > +the below information record objects alongside the generic
> >   .I fanotify_event_metadata
> >   structure within the read buffer:
> >   .PP
> >   .in +4n
> >   .EX
> > -struct fanotify_event_info_header {
> > -    __u8 info_type;
> > -    __u8 pad;
> > -    __u16 len;
> > -};
> > -
> >   struct fanotify_event_info_fid {
> >       struct fanotify_event_info_header hdr;
> >       __kernel_fsid_t fsid;
> > @@ -168,6 +201,40 @@ struct fanotify_event_info_fid {
> >   .EE
> >   .in
> >   .PP
> > +In cases where an fanotify group is initialized with
> > +.BR FAN_REPORT_PIDFD ,
> > +event listeners should expect to receive the below information record
> > +object alongside the generic
> > +.I fanotify_event_metadata
> > +structure within the read buffer:
> > +.PP
> > +.in +4n
> > +.EX
> > +struct fanotify_event_info_pidfd {
> > +        struct fanotify_event_info_header hdr;
> > +        __s32 pidfd;
> > +};
> > +.EE
> > +.in
> > +.PP
> > +All information records contain a nested structure of type
> > +.IR fanotify_event_info_header .
> > +This structure holds meta-information about the information record
> > +that may have been returned alongside the generic
> > +.I fanotify_event_metadata
> > +structure.
> > +This structure is defined as follows:
> > +.PP
> > +.in +4n
> > +.EX
> > +struct fanotify_event_info_header {
> > +	__u8 info_type;
> > +	__u8 pad;
> > +	__u16 len;
> > +};
> > +.EE
> > +.in
> > +.PP
> >   For performance reasons, it is recommended to use a large
> >   buffer size (for example, 4096 bytes),
> >   so that multiple events can be retrieved by a single
> > @@ -385,6 +452,48 @@ The
> >   flag is reported in an event mask only if the fanotify group identifies
> >   filesystem objects by file handles.
> >   .PP
> > +Information records that are supplied alongside the generic
> > +.I fanotify_event_metadata
> > +structure will always contain a nested structure of type
> > +.IR fanotify_event_info_header .
> > +The fields of the
> > +.I fanotify_event_info_header
> > +are as follows:
> > +.TP
> > +.I info_type
> > +A unique integer value representing the type of information record
> > +object received for an event.
> > +The value of this field can be set to one of the following:
> > +.BR FAN_EVENT_INFO_TYPE_FID ,
> > +.BR FAN_EVENT_INFO_TYPE_DFID ,
> > +.B FAN_EVENT_INFO_TYPE_DFID_NAME
> > +or
> > +.BR FAN_EVENT_INFO_TYPE_PIDFD .
> 
> Use Oxford-style commas.  See:
> 
> $ git show 3ded684c1a4b6104e1d4b7400015f8bf76dc75b9 | head -n25
> commit 3ded684c1a4b6104e1d4b7400015f8bf76dc75b9
> Author: Michael Kerrisk <mtk.manpages@gmail.com>
> Date:   Sat Jan 9 11:14:08 2021 +0100
> 
>     Various pages: tfix (Oxford comma)
> 
>     Found using:
> 
>         git grep -lE '^[^.].*,.*,.*[^,] (and|or)\>'
> 
>     Signed-off-by: Michael Kerrisk <mtk.manpages@gmail.com>
> 
> diff --git a/man1/intro.1 b/man1/intro.1
> index 57023b652..9bf7190df 100644
> --- a/man1/intro.1
> +++ b/man1/intro.1
> @@ -220,7 +220,7 @@ and
>  .I pwd
>  commands and explore
>  .I cd
> -usage: "cd", "cd .", "cd ..", "cd /" and "cd \(ti".
> +usage: "cd", "cd .", "cd ..", "cd /", and "cd \(ti".
>  .SS Directories
>  The command
>  .I mkdir
> 
> 
> > +The value set for this field is dependent on the flags that have been
> > +supplied to
> > +.BR fanotify_init (2) .
> 
> Spurious space.  Should be:
> 
> .BR fanotify_init (2).
> 
> > +Refer to the field details of each information record object type
> > +below to understand the different cases in which the
> > +.I info_type
> > +values can be set.
> > +.TP
> > +.I pad
> > +This field is currently not used by any information record object type
> > +and therefore is set to zero.
> > +.TP
> > +.I len
> > +The value of
> > +.I len
> > +is set to the size of the information record object, including the
> > +.IR fanotify_event_info_header .
> > +The total size of all additional information records is not expected
> > +to be larger than
> > +(
> > +.I event_len
> > +\-
> > +.I metadata_len
> > +).
> 
> The above can be simplified to:
> 
> .RI ( event_len
> \-
> .IR metadata_len ).
> 
> > +.PP
> >   The fields of the
> >   .I fanotify_event_info_fid
> >   structure are as follows:
> > @@ -392,8 +501,6 @@ structure are as follows:
> >   .I hdr
> >   This is a structure of type
> >   .IR fanotify_event_info_header .
> > -It is a generic header that contains information used to describe an
> > -additional information record attached to the event.
> >   For example, when an fanotify file descriptor is created using
> >   .BR FAN_REPORT_FID ,
> >   a single information record is expected to be attached to the event with
> > @@ -414,23 +521,6 @@ identifying a parent directory object, and one with
> >   field value of
> >   .BR FAN_EVENT_INFO_TYPE_FID ,
> >   identifying a non-directory object.
> > -The
> > -.I fanotify_event_info_header
> > -contains a
> > -.I len
> > -field.
> > -The value of
> > -.I len
> > -is the size of the additional information record including the
> > -.I fanotify_event_info_header
> > -itself.
> > -The total size of all additional information records is not expected
> > -to be bigger than
> > -(
> > -.I event_len
> > -\-
> > -.I metadata_len
> > -).
> >   .TP
> >   .I fsid
> >   This is a unique identifier of the filesystem containing the object
> > @@ -498,6 +588,48 @@ and the file handle is followed by a null terminated string that identifies the
> >   name of a directory entry in that directory, or '.' to identify the directory
> >   object itself.
> >   .PP
> > +The fields of the
> > +.I fanotify_event_info_pidfd
> > +structure are as follows:
> > +.TP
> > +.I hdr
> > +This is a structure of type
> > +.IR fanotify_event_info_header .
> > +When an fanotify group is initialized using
> > +.BR FAN_REPORT_PIDFD ,
> > +the
> > +.I info_type
> > +field value of the
> > +.I fanotify_event_info_header
> > +is set to
> > +.BR FAN_EVENT_INFO_TYPE_PIDFD .
> > +.TP
> > +.I pidfd
> > +This is a process file descriptor that refers to the process
> > +responsible for generating the event.
> > +The returned process file descriptor is no different from one which
> > +could be obtained manually if
> > +.BR pidfd_open(2)
> 
> Missing a space before "(2)".
> 
> > +were to be called on
> > +.IR fanotify_event_metadata.pid .
> > +In the instance that an error is encountered during pidfd creation for
> > +one of two possible error types represented by a negative integer
> > +value may be returned in this
> > +.I pidfd
> > +field.
> > +In cases where the process responsible for generating the event has
> > +terminated prior to the event listener being able to read events from the
> > +notification queue,
> > +.B FAN_NOPIDFD
> > +is returned.
> > +The pidfd creation for an event is only performed at the time the
> > +events are read from the notification queue.
> > +All other possible pidfd creation failures are represented by
> > +.BR FAN_EPIDFD .
> > +Once the event listener has dealt with an event and the pidfd is no
> > +longer required, the pidfd should be closed via
> > +.BR close(2) .
> 
> Space is misplaced.  Should be:
> 
> .BR close (2).
> 
> > +.PP
> >   The following macros are provided to iterate over a buffer containing
> >   fanotify event metadata returned by a
> >   .BR read (2)

/M

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] fanotify: Document FAN_REPORT_PIDFD Feature
  2022-04-13 23:40   ` Matthew Bobrowski
@ 2022-04-25 20:18     ` Alejandro Colomar
  2022-04-26 21:23       ` Matthew Bobrowski
  0 siblings, 1 reply; 5+ messages in thread
From: Alejandro Colomar @ 2022-04-25 20:18 UTC (permalink / raw)
  To: Matthew Bobrowski; +Cc: linux-man, jack, amir73il, mtk.manpages

Hi Matthew,

On 4/14/22 01:40, Matthew Bobrowski wrote:
> Haha, I created this patch using one of Amir's branches, as he
> performed a rebase and handled some conflicts. It must've preserved
> the display name "Amir Goldstein" in the "From:" header...

:)

> 
> On Wed, Apr 13, 2022 at 08:24:21PM +0200, Alejandro Colomar wrote:
>> Hi Amir!
>>
>> On 4/12/22 01:17, Amir Goldstein wrote:
>>> Update the fanotify API documentation to include details on the new
>>> FAN_REPORT_PIDFD feature. This patch also includes a generic section
>>> describing the concept of information records which are supported by
>>> the fanotify API.
>>>
>>> Signed-off-by: Matthew Bobrowski <repnop@google.com>
>>> Reviewed-by: Jan Kara <jack@suse.cz>
>>> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>>
>> Thanks for the patch.  Please see some comments below.
> 
> Thanks for the review, I'll update and send through a follow up patch
> shortly. I left a question on your comment about the use of semantic
> newlines. I wasn't sure whether that comment was a general rule that
> is to be applied across this entire patch (in which it definitely can,
> I just wasn't aware of the rule prior to you explicitly pointing it
> out), or whether there was a specific example you were referring to in
> the code block directly above your comment.

General rule to be applied across the entire patch, if you do the 
favour.  I just mentioned it at a point where it is clear that it could 
be applied, to give some context.

Thank you,

Alex

> 
>>> ---
>>>    man2/fanotify_init.2 |  34 +++++++
>>>    man7/fanotify.7      | 208 +++++++++++++++++++++++++++++++++++--------
>>>    2 files changed, 204 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/man2/fanotify_init.2 b/man2/fanotify_init.2
>>> index 7f9a21a52..1e4691c88 100644
>>> --- a/man2/fanotify_init.2
>>> +++ b/man2/fanotify_init.2
>>> @@ -283,6 +283,40 @@ for additional details.
>>>    This is a synonym for
>>>    .RB ( FAN_REPORT_DIR_FID | FAN_REPORT_NAME ).
>>>    .PP
>>> +.TP
>>> +.BR FAN_REPORT_PIDFD " (since Linux 5.15)"
>>> +.\" commit af579beb666aefb17e9a335c12c788c92932baf1
>>> +Events for fanotify groups initialized with this flag will contain an
>>> +additional information record alongside the generic
>>> +.I fanotify_event_metadata
>>> +structure.
>>> +This information record will be of type
>>> +.B FAN_EVENT_INFO_TYPE_PIDFD
>>> +and will contain a pidfd for the process that was responsible for
>>> +generating an event.
>>> +A pidfd returned in this information record object is no different
>>> +to the pidfd that is returned when calling
>>> +.BR pidfd_open(2) .
>>
>> Misplaced space.  Should be:
>>
>> .BR pidfd_open (2).
>>
>>> +Usage of this information record are for applications that may be
>>> +interested in reliably determining whether the process responsible for
>>> +generating an event has been recycled or terminated.
>>> +The use of the
>>> +.B FAN_REPORT_TID
>>> +flag along with
>>> +.B FAN_REPORT_PIDFD
>>> +is currently not supported and attempting to do so will result in the
>>> +error

s.n.

>>> +.B EINVAL
>>> +being returned.
>>> +This limitation is currently imposed by the pidfd API as it currently
>>> +only supports the creation of pidfds for thread-group
>>> +leaders.
>>
>> Please use semantic newlines.
>> See man-pages(7):
>>
>>     Use semantic newlines
>>         In the source of a manual page, new sentences  should  be
>>         started on new lines, long sentences should be split into
>>         lines at clause breaks (commas, semicolons,  colons,  and
>>         so on), and long clauses should be split at phrase bound-
>>         aries.  This convention,  sometimes  known  as  "semantic
>>         newlines",  makes it easier to see the effect of patches,
>>         which often operate at the level of individual sentences,
>>         clauses, or phrases.
> 
> Fair point. Based on the above change block, I assume you're referring
> to the the word "leaders" starting on a new line? The rest appears to
> be inline with what's expected of semantic newlines?

I wrote "s.n." in the more grievous cases where semantic newlines could 
be improved.

Don't worry too much about it if you can't see some of the lines.  Just 
do some cleanup, and I'll continue from there.  Thanks.

> 
>>> +Creating pidfds for non-thread-group leaders may be supported at some
>>> +point in the future, so this restriction may eventually be

s.n.

>>> +lifted.
>>> +For more details on information records, see
>>> +.BR fanotify(7) .
>>
>> Misplaced space.  Should be:
>>
>> .BR fanotify (7).
>>
>>> +.PP
>>>    The
>>>    .I event_f_flags
>>>    argument
>>> diff --git a/man7/fanotify.7 b/man7/fanotify.7
>>> index f8345b3f5..57dd2b040 100644
>>> --- a/man7/fanotify.7
>>> +++ b/man7/fanotify.7
>>> @@ -118,16 +118,6 @@ until either a file event occurs or the call is interrupted by a signal
>>>    (see
>>>    .BR signal (7)).
>>>    .PP
>>> -The use of one of the flags
>>> -.BR FAN_REPORT_FID ,
>>> -.B FAN_REPORT_DIR_FID
>>> -in
>>> -.BR fanotify_init (2)
>>> -influences what data structures are returned to the event listener for each
>>> -event.
>>> -Events reported to a group initialized with one of these flags will
>>> -use file handles to identify filesystem objects instead of file descriptors.
>>> -.PP
>>>    After a successful
>>>    .BR read (2),
>>>    the read buffer contains one or more of the following structures:
>>> @@ -146,20 +136,63 @@ struct fanotify_event_metadata {
>>>    .EE
>>>    .in
>>>    .PP
>>> -In case of an fanotify group that identifies filesystem objects by file
>>> -handles, you should also expect to receive one or more additional information
>>> -records of the structure detailed below following the generic
>>> +Information records are supplemental pieces of information that may be
>>> +provided alongside the generic
>>> +.I fanotify_event_metadata
>>> +structure.
>>> +The
>>> +.I flags
>>> +passed to
>>> +.BR fanotify_init (2) > +have influence over the type of information records that may be returned
>>> +for an event.
>>> +For example, if a notification group is initialized with
>>> +.B FAN_REPORT_FID
>>> +or
>>> +.BR FAN_REPORT_DIR_FID ,
>>> +then event listeners should also expect to receive a
>>> +.I fanotify_event_info_fid
>>> +structure alongside the
>>> +.I fanotify_event_metadata
>>> +structure, whereby file handles are used to identify filesystem
>>> +objects rather than file descriptors.

s.n.

>>> +Information records may also be stacked, meaning that using the
>>> +various

s.n.

>>> +.B FAN_REPORT_*
>>> +flags in conjunction with one another is supported.
>>> +In such cases, multiple information records can be returned for an
>>> +event alongside the generic
>>> +.I fanotify_event_metadata
>>> +structure.
>>> +For example, if a notification group is initialized with
>>> +.B FAN_REPORT_FID
>>> +and
>>> +.BR FAN_REPORT_PIDFD ,
>>> +then an event listener should also expect to receive both
>>> +.I fanotify_event_info_fid
>>> +and
>>> +.I fanotify_event_info_pidfd
>>> +structures alongside the generic
>>> +.I fanotify_event_metadata
>>> +structure.
>>> +Importantly, fanotify provides no guarantee around the ordering of
>>> +information records when a notification group is intialized with a
>>> +stacked based configuration.
>>> +Each information record has a nested structure of type
>>> +.IR fanotify_event_info_header .
>>> +It is imperative for event listeners to inspect the
>>> +.I info_type
>>> +field of this structure in order to determine the type of information
>>> +record that had been received for a given event.

s.n.

>>> +.PP
>>> +In cases where an fanotify group identifies filesystem objects by file
>>> +handles, event listeners should also expect to receive one or more of

s.n.

>>> +the below information record objects alongside the generic
>>>    .I fanotify_event_metadata
>>>    structure within the read buffer:
>>>    .PP
>>>    .in +4n
>>>    .EX
>>> -struct fanotify_event_info_header {
>>> -    __u8 info_type;
>>> -    __u8 pad;
>>> -    __u16 len;
>>> -};
>>> -
>>>    struct fanotify_event_info_fid {
>>>        struct fanotify_event_info_header hdr;
>>>        __kernel_fsid_t fsid;
>>> @@ -168,6 +201,40 @@ struct fanotify_event_info_fid {
>>>    .EE
>>>    .in
>>>    .PP
>>> +In cases where an fanotify group is initialized with
>>> +.BR FAN_REPORT_PIDFD ,
>>> +event listeners should expect to receive the below information record
>>> +object alongside the generic

s.n.

>>> +.I fanotify_event_metadata
>>> +structure within the read buffer:
>>> +.PP
>>> +.in +4n
>>> +.EX
>>> +struct fanotify_event_info_pidfd {
>>> +        struct fanotify_event_info_header hdr;
>>> +        __s32 pidfd;
>>> +};
>>> +.EE
>>> +.in
>>> +.PP
>>> +All information records contain a nested structure of type
>>> +.IR fanotify_event_info_header .
>>> +This structure holds meta-information about the information record
>>> +that may have been returned alongside the generic
>>> +.I fanotify_event_metadata
>>> +structure.
>>> +This structure is defined as follows:
>>> +.PP
>>> +.in +4n
>>> +.EX
>>> +struct fanotify_event_info_header {
>>> +	__u8 info_type;
>>> +	__u8 pad;
>>> +	__u16 len;
>>> +};
>>> +.EE
>>> +.in
>>> +.PP
>>>    For performance reasons, it is recommended to use a large
>>>    buffer size (for example, 4096 bytes),
>>>    so that multiple events can be retrieved by a single
>>> @@ -385,6 +452,48 @@ The
>>>    flag is reported in an event mask only if the fanotify group identifies
>>>    filesystem objects by file handles.
>>>    .PP
>>> +Information records that are supplied alongside the generic
>>> +.I fanotify_event_metadata
>>> +structure will always contain a nested structure of type
>>> +.IR fanotify_event_info_header .
>>> +The fields of the
>>> +.I fanotify_event_info_header
>>> +are as follows:
>>> +.TP
>>> +.I info_type
>>> +A unique integer value representing the type of information record
>>> +object received for an event.

s.n.

>>> +The value of this field can be set to one of the following:
>>> +.BR FAN_EVENT_INFO_TYPE_FID ,
>>> +.BR FAN_EVENT_INFO_TYPE_DFID ,
>>> +.B FAN_EVENT_INFO_TYPE_DFID_NAME
>>> +or
>>> +.BR FAN_EVENT_INFO_TYPE_PIDFD .
>>
>> Use Oxford-style commas.  See:
>>
>> $ git show 3ded684c1a4b6104e1d4b7400015f8bf76dc75b9 | head -n25
>> commit 3ded684c1a4b6104e1d4b7400015f8bf76dc75b9
>> Author: Michael Kerrisk <mtk.manpages@gmail.com>
>> Date:   Sat Jan 9 11:14:08 2021 +0100
>>
>>      Various pages: tfix (Oxford comma)
>>
>>      Found using:
>>
>>          git grep -lE '^[^.].*,.*,.*[^,] (and|or)\>'
>>
>>      Signed-off-by: Michael Kerrisk <mtk.manpages@gmail.com>
>>
>> diff --git a/man1/intro.1 b/man1/intro.1
>> index 57023b652..9bf7190df 100644
>> --- a/man1/intro.1
>> +++ b/man1/intro.1
>> @@ -220,7 +220,7 @@ and
>>   .I pwd
>>   commands and explore
>>   .I cd
>> -usage: "cd", "cd .", "cd ..", "cd /" and "cd \(ti".
>> +usage: "cd", "cd .", "cd ..", "cd /", and "cd \(ti".
>>   .SS Directories
>>   The command
>>   .I mkdir
>>
>>
>>> +The value set for this field is dependent on the flags that have been
>>> +supplied to
>>> +.BR fanotify_init (2) .
>>
>> Spurious space.  Should be:
>>
>> .BR fanotify_init (2).
>>
>>> +Refer to the field details of each information record object type
>>> +below to understand the different cases in which the
>>> +.I info_type
>>> +values can be set.
>>> +.TP
>>> +.I pad
>>> +This field is currently not used by any information record object type
>>> +and therefore is set to zero.
>>> +.TP
>>> +.I len
>>> +The value of
>>> +.I len
>>> +is set to the size of the information record object, including the
>>> +.IR fanotify_event_info_header .
>>> +The total size of all additional information records is not expected
>>> +to be larger than
>>> +(
>>> +.I event_len
>>> +\-
>>> +.I metadata_len
>>> +).
>>
>> The above can be simplified to:
>>
>> .RI ( event_len
>> \-
>> .IR metadata_len ).
>>
>>> +.PP
>>>    The fields of the
>>>    .I fanotify_event_info_fid
>>>    structure are as follows:
>>> @@ -392,8 +501,6 @@ structure are as follows:
>>>    .I hdr
>>>    This is a structure of type
>>>    .IR fanotify_event_info_header .
>>> -It is a generic header that contains information used to describe an
>>> -additional information record attached to the event.
>>>    For example, when an fanotify file descriptor is created using
>>>    .BR FAN_REPORT_FID ,
>>>    a single information record is expected to be attached to the event with
>>> @@ -414,23 +521,6 @@ identifying a parent directory object, and one with
>>>    field value of
>>>    .BR FAN_EVENT_INFO_TYPE_FID ,
>>>    identifying a non-directory object.
>>> -The
>>> -.I fanotify_event_info_header
>>> -contains a
>>> -.I len
>>> -field.
>>> -The value of
>>> -.I len
>>> -is the size of the additional information record including the
>>> -.I fanotify_event_info_header
>>> -itself.
>>> -The total size of all additional information records is not expected
>>> -to be bigger than
>>> -(
>>> -.I event_len
>>> -\-
>>> -.I metadata_len
>>> -).
>>>    .TP
>>>    .I fsid
>>>    This is a unique identifier of the filesystem containing the object
>>> @@ -498,6 +588,48 @@ and the file handle is followed by a null terminated string that identifies the
>>>    name of a directory entry in that directory, or '.' to identify the directory
>>>    object itself.
>>>    .PP
>>> +The fields of the
>>> +.I fanotify_event_info_pidfd
>>> +structure are as follows:
>>> +.TP
>>> +.I hdr
>>> +This is a structure of type
>>> +.IR fanotify_event_info_header .
>>> +When an fanotify group is initialized using
>>> +.BR FAN_REPORT_PIDFD ,
>>> +the
>>> +.I info_type
>>> +field value of the
>>> +.I fanotify_event_info_header
>>> +is set to
>>> +.BR FAN_EVENT_INFO_TYPE_PIDFD .
>>> +.TP
>>> +.I pidfd
>>> +This is a process file descriptor that refers to the process
>>> +responsible for generating the event.
>>> +The returned process file descriptor is no different from one which
>>> +could be obtained manually if
>>> +.BR pidfd_open(2)
>>
>> Missing a space before "(2)".
>>
>>> +were to be called on
>>> +.IR fanotify_event_metadata.pid .
>>> +In the instance that an error is encountered during pidfd creation for
>>> +one of two possible error types represented by a negative integer
>>> +value may be returned in this

s.n.

>>> +.I pidfd
>>> +field.
>>> +In cases where the process responsible for generating the event has
>>> +terminated prior to the event listener being able to read events from the
>>> +notification queue,
>>> +.B FAN_NOPIDFD
>>> +is returned.
>>> +The pidfd creation for an event is only performed at the time the
>>> +events are read from the notification queue.
>>> +All other possible pidfd creation failures are represented by
>>> +.BR FAN_EPIDFD .
>>> +Once the event listener has dealt with an event and the pidfd is no
>>> +longer required, the pidfd should be closed via

s.n.

>>> +.BR close(2) .
>>
>> Space is misplaced.  Should be:
>>
>> .BR close (2).
>>
>>> +.PP
>>>    The following macros are provided to iterate over a buffer containing
>>>    fanotify event metadata returned by a
>>>    .BR read (2)
> 
> /M

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] fanotify: Document FAN_REPORT_PIDFD Feature
  2022-04-25 20:18     ` Alejandro Colomar
@ 2022-04-26 21:23       ` Matthew Bobrowski
  0 siblings, 0 replies; 5+ messages in thread
From: Matthew Bobrowski @ 2022-04-26 21:23 UTC (permalink / raw)
  To: Alejandro Colomar; +Cc: linux-man, jack, amir73il, mtk.manpages

On Mon, Apr 25, 2022 at 10:18:43PM +0200, Alejandro Colomar wrote:
> Hi Matthew,
> 
> On 4/14/22 01:40, Matthew Bobrowski wrote:
> > Haha, I created this patch using one of Amir's branches, as he
> > performed a rebase and handled some conflicts. It must've preserved
> > the display name "Amir Goldstein" in the "From:" header...
> 
> :)
> 
> > 
> > On Wed, Apr 13, 2022 at 08:24:21PM +0200, Alejandro Colomar wrote:
> > > Hi Amir!
> > > 
> > > On 4/12/22 01:17, Amir Goldstein wrote:
> > > > Update the fanotify API documentation to include details on the new
> > > > FAN_REPORT_PIDFD feature. This patch also includes a generic section
> > > > describing the concept of information records which are supported by
> > > > the fanotify API.
> > > > 
> > > > Signed-off-by: Matthew Bobrowski <repnop@google.com>
> > > > Reviewed-by: Jan Kara <jack@suse.cz>
> > > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > > 
> > > Thanks for the patch.  Please see some comments below.
> > 
> > Thanks for the review, I'll update and send through a follow up patch
> > shortly. I left a question on your comment about the use of semantic
> > newlines. I wasn't sure whether that comment was a general rule that
> > is to be applied across this entire patch (in which it definitely can,
> > I just wasn't aware of the rule prior to you explicitly pointing it
> > out), or whether there was a specific example you were referring to in
> > the code block directly above your comment.
> 
> General rule to be applied across the entire patch, if you do the favour.  I
> just mentioned it at a point where it is clear that it could be applied, to
> give some context.

Fair enough.

I've posted through an updated patch here [0] which I believe has
addressed all the feedback from this round of review.

[0] https://lore.kernel.org/linux-man/1af583adb1f368c51f1976db7bf3a27530cdc06f.1650408011.git.repnop@google.com/

/M

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-04-26 21:23 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-11 23:17 [PATCH v3] fanotify: Document FAN_REPORT_PIDFD Feature Amir Goldstein
2022-04-13 18:24 ` Alejandro Colomar
2022-04-13 23:40   ` Matthew Bobrowski
2022-04-25 20:18     ` Alejandro Colomar
2022-04-26 21:23       ` Matthew Bobrowski

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.