Re: [PATCH 4/8] vfs: Add superblock notifications

Darrick J. Wong
2020-12-08T00:57:37Z

On Mon, Dec 07, 2020 at 09:31:13PM -0300, Gabriel Krisman Bertazi wrote:
> From: David Howells <dhowells@redhat.com>
>
> Add a superblock event notification facility whereby notifications about
> superblock events, such as I/O errors (EIO), quota limits being hit
> (EDQUOT) and running out of space (ENOSPC) can be reported to a monitoring
> process asynchronously. Note that this does not cover vfsmount topology
> changes. watch_mount() is used for that.

<being a lazy reviewer and skipping straight to the data format>

> diff --git a/include/uapi/linux/watch_queue.h b/include/uapi/linux/watch_queue.h
> index c3d8320b5d3a..937363d9f7b3 100644
> --- a/include/uapi/linux/watch_queue.h
> +++ b/include/uapi/linux/watch_queue.h
> @@ -14,7 +14,8 @@
> enum watch_notification_type {
> WATCH_TYPE_META = 0, /* Special record */
> WATCH_TYPE_KEY_NOTIFY = 1, /* Key change event notification */
> - WATCH_TYPE__NR = 2
> + WATCH_TYPE_SB_NOTIFY = 2,
> + WATCH_TYPE__NR = 3
> };
>
> enum watch_meta_notification_subtype {
> @@ -101,4 +102,35 @@ struct key_notification {
> __u32 aux; /* Per-type auxiliary data */
> };
>
> +/*
> + * Type of superblock notification.
> + */
> +enum superblock_notification_type {
> + NOTIFY_SUPERBLOCK_READONLY = 0, /* Filesystem toggled between R/O and R/W */
> + NOTIFY_SUPERBLOCK_ERROR = 1, /* Error in filesystem or blockdev */
> + NOTIFY_SUPERBLOCK_EDQUOT = 2, /* EDQUOT notification */
> + NOTIFY_SUPERBLOCK_NETWORK = 3, /* Network status change */
> +};
> +
> +#define NOTIFY_SUPERBLOCK_IS_NOW_RO WATCH_INFO_FLAG_0 /* Superblock changed to R/O */
> +
> +/*
> + * Superblock notification record.
> + * - watch.type = WATCH_TYPE_MOUNT_NOTIFY
> + * - watch.subtype = enum superblock_notification_subtype
> + */
> +struct superblock_notification {
> + struct watch_notification watch; /* WATCH_TYPE_SB_NOTIFY */
> + __u64 sb_id; /* 64-bit superblock ID [fsinfo_ids::f_sb_id] */
> +};
> +
> +struct superblock_error_notification {
> + struct superblock_notification s; /* subtype = notify_superblock_error */
> + __u32 error_number;
> + __u32 error_cookie;
> + __u64 inode;
> + __u64 block;

Is this a file offset? In ... i_blocksize() units? What about
filesystems that have multiple file offset mapping structures, like xfs?

IOWs can we make a structure that covers enough of the "common"ly
desired features that we don't just end up with the per-fs but then
duplicated everywhere mess that is GETFLAGS/FSGETXATTR?

> + char desc[0];

If the end of this is a VLA then I guess we can't add new fields by
bumping the size and hoping userspace notices. I guess that implies the
need for some padding and a flags field that we can set bits in when we
start using that padding...

--D

> +};
> +
> #endif /* _UAPI_LINUX_WATCH_QUEUE_H */
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index f27ac94d5fa7..3e97984bc4c8 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -51,6 +51,9 @@ COND_SYSCALL_COMPAT(io_pgetevents);
> COND_SYSCALL(io_uring_setup);
> COND_SYSCALL(io_uring_enter);
> COND_SYSCALL(io_uring_register);
> +COND_SYSCALL(fsinfo);
> +COND_SYSCALL(watch_mount);
> +COND_SYSCALL(watch_sb);
>
> /* fs/xattr.c */
>
> --
> 2.29.2
>