On Wed, Sep 07, 2022 at 01:55:30PM +0300, Amir Goldstein wrote: > On Wed, Sep 7, 2022 at 9:46 AM Matthew Bobrowski <repnop@xxxxxxxxxx> wrote: > > > > Maybe some useful nits. > > Definitely useful. > Thanks! > > > > > 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. > > > > ok. > > > > +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 > > Yeh, it's a badly phrased sentence. > I tried to improve it a bit: > > .B FAN_MARK_IGNORED_MASK > 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 the flags > +.B FAN_ONDIR , > +and > +.B FAN_EVENT_ON_CHILD > +in the mark mask > +on the events that are set in the ignore mask > +is undefined and depends on the Linux kernel version. > +Specifically, prior to Linux 5.9, > > I would love to get suggestions for improvement. > > > 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? > > > > Not exactly. > Setting those flags in the ignore mask has no effect > (as mentioned above). > Setting those flags in the mask has undefined behavior > w.r.t whether the events on a directory or on a child will > be ignored, when said events are in the ignore mask. > > See the "Specifically" clause below. > > > > +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. > > ok. > > > > > > +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 > > > > ok. > > > > +.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. > > ok. > > > > > > +.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 > > ok. > > > > > > +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. > > ok. > > FYI, I pushed the fixes to: > > https://github.com/amir73il/man-pages/commits/fan_mark_ignore One optional suggestion, but apart from that the refactoring LGTM. Reviewed-by: Matthew Bobrowski <repnop@xxxxxxxxxx> /M
