On Mon, Dec 07, 2020 at 09:31:14PM -0300, Gabriel Krisman Bertazi wrote:
> When reporting a filesystem error, we really need to know where the
> error came from, therefore, include "function:line" information in the
> notification sent to userspace. There is no current users of notify_sb
> in the kernel, so there are no callers to update.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@xxxxxxxxxxxxx>
> ---
> include/linux/fs.h | 11 +++++++++--
> include/uapi/linux/watch_queue.h | 3 +++
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index df588edc0a34..864d86fcc68c 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3514,14 +3514,17 @@ static inline void notify_sb(struct super_block *s,
> /**
> * notify_sb_error: Post superblock error notification.
> * @s: The superblock the notification is about.
> + * @function: function name reported as source of the warning.
> + * @line: source code line reported as source of the warning.
> * @error: The error number to be recorded.
> * @inode: The inode the error refers to (if available, 0 otherwise)
> * @block: The block the error refers to (if available, 0 otherwise)
> * @fmt: Formating string for extra information appended to the notification
> * @args: arguments for extra information string appended to the notification
> */
> -static inline int notify_sb_error(struct super_block *s, int error, u64 inode,
> - u64 block, const char *fmt, va_list *args)
> +static inline int notify_sb_error(struct super_block *s, const char *function, int line,
> + int error, u64 inode, u64 block,
> + const char *fmt, va_list *args)
> {
> #ifdef CONFIG_SB_NOTIFICATIONS
> if (unlikely(s->s_watchers)) {
> @@ -3534,8 +3537,12 @@ static inline int notify_sb_error(struct super_block *s, int error, u64 inode,
> .error_cookie = 0,
> .inode = inode,
> .block = block,
> + .line = line,
> };
>
> + memcpy(&n.function, function, SB_NOTIFICATION_FNAME_LEN);
> + n.function[SB_NOTIFICATION_FNAME_LEN-1] = '\0';
> +
> post_sb_notification(s, &n.s, fmt, args);
> }
> #endif
> diff --git a/include/uapi/linux/watch_queue.h b/include/uapi/linux/watch_queue.h
> index 937363d9f7b3..5fa5286c5cc7 100644
> --- a/include/uapi/linux/watch_queue.h
> +++ b/include/uapi/linux/watch_queue.h
> @@ -114,6 +114,7 @@ enum superblock_notification_type {
>
> #define NOTIFY_SUPERBLOCK_IS_NOW_RO WATCH_INFO_FLAG_0 /* Superblock changed to R/O */
>
> +#define SB_NOTIFICATION_FNAME_LEN 30
> /*
> * Superblock notification record.
> * - watch.type = WATCH_TYPE_MOUNT_NOTIFY
> @@ -130,6 +131,8 @@ struct superblock_error_notification {
> __u32 error_cookie;
> __u64 inode;
> __u64 block;
> + char function[SB_NOTIFICATION_FNAME_LEN];
> + __u16 line;
Er... this is enlarging a structure in the userspace ABI, right? Which
will break userspace that latched on to the structure definition in the
previous patch, and therefore isn't expecting a function name here.
If you're gonna put a character string(?) at the end then I guess you
have to pre-pad the notification structure so that we can add things
later, or.... bump the type code every time you add a field?
(Maybe I misread that? But this is include/uapi/...)
--D
> char desc[0];
> };
>
> --
> 2.29.2
>
