Re: [PATCH 11/11] Documentation: Document the FAN_ERROR event

From: Amir Goldstein <amir73il@gmail.com>
To: Gabriel Krisman Bertazi <krisman@collabora.com>
Cc: kernel@collabora.com, "Darrick J . Wong" <djwong@kernel.org>,
	"Theodore Ts'o" <tytso@mit.edu>,
	Dave Chinner <david@fromorbit.com>, Jan Kara <jack@suse.com>,
	David Howells <dhowells@redhat.com>,
	Khazhismel Kumykov <khazhy@google.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Ext4 <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH 11/11] Documentation: Document the FAN_ERROR event
Date: Fri, 21 May 2021 12:54:06 +0300	[thread overview]
Message-ID: <CAOQ4uxhNm57vGTZ2vGZjyrnLCg8fCA-zE7hFBi6Mu1mHeJyOpw@mail.gmail.com> (raw)
In-Reply-To: <20210521024134.1032503-12-krisman@collabora.com>

On Fri, May 21, 2021 at 5:42 AM Gabriel Krisman Bertazi
<krisman@collabora.com> wrote:
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
> ---
>  .../admin-guide/filesystem-monitoring.rst     | 52 +++++++++++++++++++
>  Documentation/admin-guide/index.rst           |  1 +
>  2 files changed, 53 insertions(+)
>  create mode 100644 Documentation/admin-guide/filesystem-monitoring.rst
>
> diff --git a/Documentation/admin-guide/filesystem-monitoring.rst b/Documentation/admin-guide/filesystem-monitoring.rst
> new file mode 100644
> index 000000000000..81e632f8e1de
> --- /dev/null
> +++ b/Documentation/admin-guide/filesystem-monitoring.rst
> @@ -0,0 +1,52 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +====================================
> +File system Monitoring with fanotify
> +====================================
> +
> +fanotify supports the FAN_ERROR mark for file system-wide error
> +reporting.  It is meant to be used by file system health monitoring
> +daemons who listen on that interface and take actions (notify sysadmin,
> +start recovery) when a file system problem is detected by the kernel.
> +
> +By design, A FAN_ERROR notification exposes sufficient information for a
> +monitoring tool to know a problem in the file system has happened.  It
> +doesn't necessarily provide a user space application with semantics to
> +verify an IO operation was successfully executed.  That is outside of
> +scope of this feature. Instead, it is only meant as a framework for
> +early file system problem detection and reporting recovery tools.
> +
> +At the time of this writing, the only file system that emits this
> +FAN_ERROR notifications is ext4.
> +
> +A user space example code is provided at ``samples/fanotify/fs-monitor.c``.
> +
> +Usage
> +=====
> +
> +Notification structure
> +======================
> +
> +A FAN_ERROR Notification has the following format::
> +
> +  [ Notification Metadata (Mandatory) ]
> +  [ Generic Error Record  (Mandatory) ]
> +
> +With the exception of the notification metadata and the generic
> +information, all information records are optional.  Each record type is
> +identified by its unique ``struct fanotify_event_info_header.info_type``.

Out-dated. Unless you decide to add support for optional FID record.

> +
> +Generic error Location

'Location' seems irrelevant?

> +----------------------
> +
> +The Generic error record provides enough information for a file system
> +agnostic tool to learn about a problem in the file system, without
> +requiring any details about the problem.::
> +
> +  struct fanotify_event_info_error {
> +       struct fanotify_event_info_header hdr;
> +       int error;
> +       __kernel_fsid_t fsid;
> +       unsigned long inode;
> +       __u32 error_count;
> +  };

Maybe add some text about the fact the inode info is optional,
purpose of error_count and the fact that the info is related to
the first observed error.

Thanks,
Amir.