Re: [PATCH v4] ceph: do not print the whole xattr value if it's too long

[Ilya Dryomov <idryomov@gmail.com>]

On Thu, Mar 2, 2023 at 1:26 PM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> If the xattr's value size is long enough the kernel will warn and
> then will fail the xfstests test case.
>
> Just print part of the value string if it's too long.
>
> At the same time fix the function name issue in the debug logs.
>
> URL: https://tracker.ceph.com/issues/58404
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>
> V4:
> - fix the function name issue in the debug logs
> - make the logs to be more compact.
>
>  fs/ceph/xattr.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index b10d459c2326..5ab4aed2eecc 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -561,6 +561,8 @@ static struct ceph_vxattr *ceph_match_vxattr(struct inode *inode,
>         return NULL;
>  }
>
> +#define MAX_XATTR_VAL_PRINT_LEN 256
> +
>  static int __set_xattr(struct ceph_inode_info *ci,
>                            const char *name, int name_len,
>                            const char *val, int val_len,
> @@ -623,7 +625,7 @@ static int __set_xattr(struct ceph_inode_info *ci,
>                 xattr->should_free_name = update_xattr;
>
>                 ci->i_xattrs.count++;
> -               dout("__set_xattr count=%d\n", ci->i_xattrs.count);
> +               dout("%s count=%d\n", __func__, ci->i_xattrs.count);
>         } else {
>                 kfree(*newxattr);
>                 *newxattr = NULL;
> @@ -651,11 +653,13 @@ static int __set_xattr(struct ceph_inode_info *ci,
>         if (new) {
>                 rb_link_node(&xattr->node, parent, p);
>                 rb_insert_color(&xattr->node, &ci->i_xattrs.index);
> -               dout("__set_xattr_val p=%p\n", p);
> +               dout("%s p=%p\n", __func__, p);
>         }
>
> -       dout("__set_xattr_val added %llx.%llx xattr %p %.*s=%.*s\n",
> -            ceph_vinop(&ci->netfs.inode), xattr, name_len, name, val_len, val);
> +       dout("%s added %llx.%llx xattr %p %.*s=%.*s%s\n", __func__,
> +            ceph_vinop(&ci->netfs.inode), xattr, name_len, name,
> +            min(val_len, MAX_XATTR_VAL_PRINT_LEN), val,
> +            val_len > MAX_XATTR_VAL_PRINT_LEN ? "..." : "");
>
>         return 0;
>  }
> @@ -681,13 +685,15 @@ static struct ceph_inode_xattr *__get_xattr(struct ceph_inode_info *ci,
>                 else if (c > 0)
>                         p = &(*p)->rb_right;
>                 else {
> -                       dout("__get_xattr %s: found %.*s\n", name,
> -                            xattr->val_len, xattr->val);
> +                       int len = min(xattr->val_len, MAX_XATTR_VAL_PRINT_LEN);
> +
> +                       dout("%s %s: found %.*s%s\n", __func__, name, len,
> +                            xattr->val, xattr->val_len > len ? "..." : "");
>                         return xattr;
>                 }
>         }
>
> -       dout("__get_xattr %s: not found\n", name);
> +       dout("%s %s: not found\n", __func__, name);
>
>         return NULL;
>  }
> --
> 2.31.1
>

Reviewed-by: Ilya Dryomov <idryomov@gmail.com>

Thanks,

                Ilya