Re: [RFC][PATCH] fanotify_mark.2: Document FAN_MARK_IGNORE

[Matthew Bobrowski <repnop@google.com>]

Maybe some useful nits.

On Sun, Sep 04, 2022 at 06:51:13PM +0300, Amir Goldstein wrote:
> diff --git a/man2/fanotify_mark.2 b/man2/fanotify_mark.2
> index 757ad9159..e9303827c 100644
> --- a/man2/fanotify_mark.2
> +++ b/man2/fanotify_mark.2
> @@ -146,12 +146,102 @@ capability.
>  The events in
>  .I mask
>  shall be added to or removed from the ignore mask.
> +Note that the flags
> +.B FAN_ONDIR ,
> +and
> +.B FAN_EVENT_ON_CHILD
> +have no effect in combination with this flag.
> +The effect of setting those flags in the mask

Is there anything wrong with explicitly specifying the exact flags
you're referring to here? Like, "is it those flags there?", or "is it
those flags over there?". Oh wait, you mean "it is those flags
there?".

I think it's best not describe flag usage and behavior of something
based on spatial proximity.

> +on ignoring events that are set in the ignore mask
> +is undefined and depend on the Linux kernel version.

Reading this sentence made me a little confused at first. Just
checking, what I understood was that the use of FAN_ONDIR or
FAN_EVENT_ON_CHILD when setting the ignore mask via
FAN_MARK_IGNORED_MASK leads to undefined behavior. Is my understanding
correct?

> +Specifically, prior to Linux 5.9,
> +.\" commit 497b0c5a7c0688c1b100a9c2e267337f677c198e
> +setting a mark mask on a file
> +and a mark with ignore mask on its parent directory
> +would not result in ignoring events on the file,
> +regardless of the
> +.B FAN_EVENT_ON_CHILD
> +flag in the parent directory's mark mask.
> +When the ignore mask is updated with the
> +.B FAN_MARK_IGNORED_MASK
> +flag,

I'd just allow this sentence to flow without the use of comma after
flag.

> +on a mark that was previously updated with the
> +.B FAN_MARK_IGNORE
> +flag,
> +the update fails with
> +.B EEXIST
> +error.
> +.TP
> +.BR FAN_MARK_IGNORE " (since Linux 6.0)"
> +.\" commit e252f2ed1c8c6c3884ab5dd34e003ed21f1fe6e0
> +This flag has a similar effect as setting the
> +.B FAN_MARK_IGNORED_MASK
> +flag.
> +The events in
> +.I mask
> +shall be added to or removed from the ignore mask.
> +Unlike the
> +.B FAN_MARK_IGNORED_MASK
> +flag,
> +this flag also has the effect that the
> +.B FAN_ONDIR ,
> +and
> +.B FAN_EVENT_ON_CHILD
> +flags take effect on the ignore mask.
> +Specifically, unless
			^
			the

> +.B FAN_ONDIR
> +flag is set with
> +.BR FAN_MARK_IGNORE ,
> +events on directories will not be ignored
					    ^
					    .
					    
> +and if the flag

I'd start a new sentence here.

> +.B FAN_EVENT_ON_CHILD
> +is set,
> +events on children will be ignored.
> +For example,
> +a mark on a directory with combination of
> +a mask with
> +.B FAN_CREATE
> +event
> +and
> +.B FAN_ONDIR
> +flag
> +and an ignore mask with
> +.B FAN_CREATE
> +event
> +and without
> +.B FAN_ONDIR
> +flag,
> +will result in getting only events for creation of sub-directories.
  	       	  	       	      	 ^
					 the
					 
> +When using this flag to add to an ignore mask
> +of a mount, filesystem, or directory inode mark,
> +the
> +.B FAN_MARK_IGNORED_SURV_MODIFY
> +flag must be specified.
> +Failure to do so will results with
> +.B EINVAL
> +or
> +.B EISDIR
> +error.
>  .TP
>  .B FAN_MARK_IGNORED_SURV_MODIFY
>  The ignore mask shall survive modify events.
>  If this flag is not set,
>  the ignore mask is cleared when a modify event occurs
>  for the ignored file or directory.
> +This flag cannot be removed from a mark once set.
> +When the ignore mask is updated without this flag,
						    ^
Don't think the comma is needed here.

> +on a mark that was previously updated with the
> +.B FAN_MARK_IGNORE
> +and
> +.B FAN_MARK_IGNORED_SURV_MODIFY
> +flags,
> +the update fails with
> +.B EEXIST
> +error.
> +.TP
> +.B FAN_MARK_IGNORE_SURV
> +This is a synonym for
> +.RB ( FAN_MARK_IGNORE | FAN_MARK_IGNORED_SURV_MODIFY ).
>  .PP
>  .TP
>  .BR FAN_MARK_EVICTABLE " (since Linux 5.19)"
> @@ -459,6 +549,32 @@ and the user attempted to update the mark with
>  .B FAN_MARK_EVICTABLE
>  flag.
>  .TP
> +.B EEXIST
> +The filesystem object indicated by
> +.I dirfd
> +and
> +.I pathname
> +has a mark that was updated with the
> +.B FAN_MARK_IGNORE
> +flag,
> +and the user attempted to update the mark with
> +.B FAN_MARK_IGNORED_MASK
> +flag.
> +.TP
> +.B EEXIST
> +The filesystem object indicated by
> +.I dirfd
> +and
> +.I pathname
> +has a mark that was updated with the
> +.B FAN_MARK_IGNORE
> +and
> +.B FAN_MARK_IGNORED_SURV_MODIFY
> +flags,
> +and the user attempted to update the mark only with
> +.B FAN_MARK_IGNORE
> +flag.
> +.TP
>  .B EINVAL
>  An invalid value was passed in
>  .I flags
> @@ -484,8 +600,33 @@ but one or more event types specified in the
>  .I mask
>  require it.
>  .TP
> +.B EINVAL
> +.I flags
> +contains
> +.BR FAN_MARK_IGNORE ,
> +and either
> +.B FAN_MARK_MOUNT
> +or
> +.BR FAN_MARK_FILESYSTEM ,
> +but does not contain
> +.BR FAN_MARK_IGNORED_SURV_MODIFY .
> +.TP
> +.B EISDIR
> +.I flags
> +contains
> +.BR FAN_MARK_IGNORE ,
> +and
> +.BR FAN_MARK_IGNORED_SURV_MODIFY ,
> +and
> +.I dirfd
> +and
> +.I pathname
> +specify a directory.
> +.TP
>  .B ENODEV
>  The filesystem object indicated by
> +.I dirfd
> +and
>  .I pathname
>  is not associated with a filesystem that supports
>  .I fsid
> @@ -547,8 +688,12 @@ and
>  do not specify a directory.
>  .TP
>  .B ENOTDIR
> -The fanotify group was initialized with flag
> +.I flags
> +contains
> +.BR FAN_MARK_IGNORE ,
> +or the fanotify group was initialized with flag
>  .BR FAN_REPORT_TARGET_FID ,
> +and
>  .I mask
>  contains directory entry modification events
>
> -- 
> 2.25.1
>

/M