Re: [PATCH v4] fanotify.7, fanotify_mark.2: Document FAN_FS_ERROR

[Amir Goldstein <amir73il@gmail.com>]

On Wed, Apr 13, 2022 at 11:27 AM Jan Kara <jack@suse.cz> wrote:
>
> On Tue 12-04-22 14:50:28, Gabriel Krisman Bertazi wrote:
> > FAN_FS_ERROR is a new event for fanotify to report filesystem errors.
> > This documents how to use the feature and specific caveats.
> >
> > Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
> >
> > ---
> > + Michael, linux-man
> >
> > Matthew,
> >
> > as discussed, this is rebased on top of the PIDFD documentation: commit
> > 207080c7f7f5 ("fanotify: Document FAN_REPORT_PIDFD Feature").
> >
> > Changes since v3:
> >  (Matthew)
> >  - Rewording and fixes from github)
> >  (amir)
> >  - 5.15 -> 5.16
> >
> > Changes since v2:
> >   (matthew)
> >     - Grammar
> >     - List filesystems that support the feature
> >     - file system -> filesystem
> > Changes since v1:
> > (Matthew)
> >   - Grammar fixes
> >   - Don't use the term "submitted" for events sent to the listener
> >   - Clarify the kind of information that is file system specific
>
> Thanks for the manpage! Couple of notes below.
>
> > diff --git a/man2/fanotify_mark.2 b/man2/fanotify_mark.2
> > index 9a45cbb77893..8f9bb863980b 100644
> > --- a/man2/fanotify_mark.2
> > +++ b/man2/fanotify_mark.2
> > @@ -203,6 +203,27 @@ Create an event when a marked file or directory itself is deleted.
> >  An fanotify group that identifies filesystem objects by file handles
> >  is required.
> >  .TP
> > +.BR FAN_FS_ERROR " (since Linux 5.16)"
> > +.\" commit 9709bd548f11a092d124698118013f66e1740f9b
> > +Create an event when a filesystem error is detected.
>
> Maybe we could specify here a bit more what "filesystem error" means?
> Because we don't generate the event e.g. on ENOSPC which could be
> considered filesystem error. We don't generate it on EIO in data block
> either. So maybe something like: "Create an event when a filesystem error
> leading to inconsistent filesystem metadata is detected." Because that's
> closer to what we currently do as far as I remember.
>
> > +An fanotify group that identifies filesystem objects by file handles
> > +is required.
> > +.IP
> > +An additional information record of type
> > +.BR FAN_EVENT_INFO_TYPE_ERROR
> > +is returned for each event in the read buffer.
> > +.IP
> > +Events of such type are dependent on support
> > +from the underlying filesystem.
> > +At the time of this writing,
> > +only the
> > +.B ext4
> > +filesystem supports this feature.
> > +.IP
> > +See
> > +.BR fanotify (7)
> > +for additional details.
> > +.TP
>
> BTW any plans to add support to XFS or btrfs? I guess it would be good to
> spread the use a bit more so that it does not end up as a niche ext4
> feature not very useful to programmers...
>

I would like to point that the term "filesystem supports this feature"
is somewhat
ambiguous here.

This same man page says: "...tmpfs(5) did not support fsid prior to Linux 5.13."
meaning that an attempt to setup a mark with FAN_REPORT_FID group on
tmpfs would result in an error.

OTOH, trying to setup a mark with FAN_FS_ERROR on tmpfs/xfs is not going to
result in any error, it is just going to result in no FAN_FS_ERROR
errors reported.

I think that we should make that more clear  by saying something like:
"At the time of this writing, only the ext4 filesystem reports
FAN_FS_ERROR events.

With that you may add:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

And I will update your v5 patch in my patch queue.

Thanks,
Amir.