* [PATCH v3 0/1] ceph: add support for getvxattr op @ 2022-01-17 3:59 Milind Changire 2022-01-17 3:59 ` [PATCH v3 1/1] ceph: add " Milind Changire 0 siblings, 1 reply; 10+ messages in thread From: Milind Changire @ 2022-01-17 3:59 UTC (permalink / raw) To: Jeff Layton, Ilya Dryomov, ceph-devel; +Cc: Milind Changire Adds support for getting ceph virtual xattrs using the new getvxattr op. Milind Changire (1): ceph: add getvxattr op fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ fs/ceph/mds_client.c | 27 ++++++++++++++++++- fs/ceph/mds_client.h | 12 ++++++++- fs/ceph/strings.c | 1 + fs/ceph/super.h | 1 + fs/ceph/xattr.c | 34 ++++++++++++++++++++++++ include/linux/ceph/ceph_fs.h | 1 + 7 files changed, 125 insertions(+), 2 deletions(-) base-commit: fd84bfdddd169c219c3a637889a8b87f70a072c2 -- 2.31.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/1] ceph: add getvxattr op 2022-01-17 3:59 [PATCH v3 0/1] ceph: add support for getvxattr op Milind Changire @ 2022-01-17 3:59 ` Milind Changire 2022-01-17 6:23 ` kernel test robot ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Milind Changire @ 2022-01-17 3:59 UTC (permalink / raw) To: Jeff Layton, Ilya Dryomov, ceph-devel; +Cc: Milind Changire Problem: Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be propagated to the client as frequently to keep them updated. This creates vxattr availability problems. Solution: Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and ceph.file.layout* vxattrs. If the entire layout for a dir or a file is being set, then it is expected that the layout be set in standard JSON format. Individual field value retrieval is not wrapped in JSON. The JSON format also applies while setting the vxattr if the entire layout is being set in one go. As a temporary measure, setting a vxattr can also be done in the old format. The old format will be deprecated in the future. URL: https://tracker.ceph.com/issues/51062 Signed-off-by: Milind Changire <mchangir@redhat.com> --- fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ fs/ceph/mds_client.c | 27 ++++++++++++++++++- fs/ceph/mds_client.h | 12 ++++++++- fs/ceph/strings.c | 1 + fs/ceph/super.h | 1 + fs/ceph/xattr.c | 34 ++++++++++++++++++++++++ include/linux/ceph/ceph_fs.h | 1 + 7 files changed, 125 insertions(+), 2 deletions(-) diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c index e3322fcb2e8d..b63746a7a0e0 100644 --- a/fs/ceph/inode.c +++ b/fs/ceph/inode.c @@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page, return err; } +int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, + size_t size) +{ + struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb); + struct ceph_mds_client *mdsc = fsc->mdsc; + struct ceph_mds_request *req; + int mode = USE_AUTH_MDS; + int err; + char *xattr_value; + size_t xattr_value_len; + + req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode); + if (IS_ERR(req)) { + err = -ENOMEM; + goto out; + } + + req->r_path2 = kstrdup(name, GFP_NOFS); + if (!req->r_path2) { + err = -ENOMEM; + goto put; + } + + ihold(inode); + req->r_inode = inode; + err = ceph_mdsc_do_request(mdsc, NULL, req); + if (err < 0) + goto put; + + xattr_value = req->r_reply_info.xattr_info.xattr_value; + xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; + + dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); + + err = xattr_value_len; + if (size == 0) + goto put; + + if (xattr_value_len > size) { + err = -ERANGE; + goto put; + } + + memcpy(value, xattr_value, xattr_value_len); +put: + ceph_mdsc_put_request(req); +out: + dout("do_getvxattr result=%d\n", err); + return err; +} + /* * Check inode permissions. We verify we have a valid value for diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c index c30eefc0ac19..a5eafc71d976 100644 --- a/fs/ceph/mds_client.c +++ b/fs/ceph/mds_client.c @@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end, return -EIO; } +static int parse_reply_info_getvxattr(void **p, void *end, + struct ceph_mds_reply_info_parsed *info, + u64 features) +{ + u8 struct_v, struct_compat; + u32 struct_len; + u32 value_len; + + ceph_decode_8_safe(p, end, struct_v, bad); + ceph_decode_8_safe(p, end, struct_compat, bad); + ceph_decode_32_safe(p, end, struct_len, bad); + ceph_decode_32_safe(p, end, value_len, bad); + + if (value_len == end - *p) { + info->xattr_info.xattr_value = *p; + info->xattr_info.xattr_value_len = end - *p; + *p = end; + return info->xattr_info.xattr_value_len; + } +bad: + return -EIO; +} + /* * parse extra results */ @@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end, return parse_reply_info_readdir(p, end, info, features); else if (op == CEPH_MDS_OP_CREATE) return parse_reply_info_create(p, end, info, features, s); + else if (op == CEPH_MDS_OP_GETVXATTR) + return parse_reply_info_getvxattr(p, end, info, features); else return -EIO; } @@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg, if (p != end) goto bad; - return 0; + return err; bad: err = -EIO; diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h index 97c7f7bfa55f..f2a8e5af3c2e 100644 --- a/fs/ceph/mds_client.h +++ b/fs/ceph/mds_client.h @@ -29,8 +29,10 @@ enum ceph_feature_type { CEPHFS_FEATURE_MULTI_RECONNECT, CEPHFS_FEATURE_DELEG_INO, CEPHFS_FEATURE_METRIC_COLLECT, + CEPHFS_FEATURE_ALTERNATE_NAME, + CEPHFS_FEATURE_GETVXATTR, - CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT, + CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR, }; /* @@ -45,6 +47,8 @@ enum ceph_feature_type { CEPHFS_FEATURE_MULTI_RECONNECT, \ CEPHFS_FEATURE_DELEG_INO, \ CEPHFS_FEATURE_METRIC_COLLECT, \ + CEPHFS_FEATURE_ALTERNATE_NAME, \ + CEPHFS_FEATURE_GETVXATTR, \ \ CEPHFS_FEATURE_MAX, \ } @@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry { loff_t offset; }; +struct ceph_mds_reply_xattr { + char *xattr_value; + size_t xattr_value_len; +}; + /* * parsed info about an mds reply, including information about * either: 1) the target inode and/or its parent directory and dentry, @@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed { char *dname; u32 dname_len; struct ceph_mds_reply_lease *dlease; + struct ceph_mds_reply_xattr xattr_info; /* extra */ union { diff --git a/fs/ceph/strings.c b/fs/ceph/strings.c index 573bb9556fb5..e36e8948e728 100644 --- a/fs/ceph/strings.c +++ b/fs/ceph/strings.c @@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op) case CEPH_MDS_OP_LOOKUPINO: return "lookupino"; case CEPH_MDS_OP_LOOKUPNAME: return "lookupname"; case CEPH_MDS_OP_GETATTR: return "getattr"; + case CEPH_MDS_OP_GETVXATTR: return "getvxattr"; case CEPH_MDS_OP_SETXATTR: return "setxattr"; case CEPH_MDS_OP_SETATTR: return "setattr"; case CEPH_MDS_OP_RMXATTR: return "rmxattr"; diff --git a/fs/ceph/super.h b/fs/ceph/super.h index ac331aa07cfa..a627fa69668e 100644 --- a/fs/ceph/super.h +++ b/fs/ceph/super.h @@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode) /* xattr.c */ int __ceph_setxattr(struct inode *, const char *, const void *, size_t, int); +int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, size_t size); ssize_t __ceph_getxattr(struct inode *, const char *, void *, size_t); extern ssize_t ceph_listxattr(struct dentry *, char *, size_t); extern struct ceph_buffer *__ceph_build_xattrs_blob(struct ceph_inode_info *ci); diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c index fcf7dfdecf96..dc32876a541a 100644 --- a/fs/ceph/xattr.c +++ b/fs/ceph/xattr.c @@ -918,6 +918,30 @@ static inline int __get_request_mask(struct inode *in) { return mask; } +/* check if the entire cluster supports the given feature */ +static inline bool ceph_cluster_has_feature(struct inode *inode, int feature_bit) +{ + int64_t i; + struct ceph_fs_client *fsc = ceph_inode_to_client(inode); + struct ceph_mds_session **sessions = fsc->mdsc->sessions; + int64_t num_sessions = atomic_read(&fsc->mdsc->num_sessions); + + if (fsc->mdsc->stopping) + return false; + + if (!sessions) + return false; + + for (i = 0; i < num_sessions; i++) { + struct ceph_mds_session *session = sessions[i]; + if (!session) + return false; + if (!test_bit(feature_bit, &session->s_features)) + return false; + } + return true; +} + ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, size_t size) { @@ -927,6 +951,16 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, int req_mask; ssize_t err; + if (!strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) && + ceph_cluster_has_feature(inode, CEPHFS_FEATURE_GETVXATTR)) { + err = ceph_do_getvxattr(inode, name, value, size); + /* if cluster doesn't support xattr, we try to service it + * locally + */ + if (err >= 0) + return err; + } + /* let's see if a virtual xattr was requested */ vxattr = ceph_match_vxattr(inode, name); if (vxattr) { diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h index 7ad6c3d0db7d..66db21ac5f0c 100644 --- a/include/linux/ceph/ceph_fs.h +++ b/include/linux/ceph/ceph_fs.h @@ -328,6 +328,7 @@ enum { CEPH_MDS_OP_LOOKUPPARENT = 0x00103, CEPH_MDS_OP_LOOKUPINO = 0x00104, CEPH_MDS_OP_LOOKUPNAME = 0x00105, + CEPH_MDS_OP_GETVXATTR = 0x00106, CEPH_MDS_OP_SETXATTR = 0x01105, CEPH_MDS_OP_RMXATTR = 0x01106, -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/1] ceph: add getvxattr op 2022-01-17 3:59 ` [PATCH v3 1/1] ceph: add " Milind Changire @ 2022-01-17 6:23 ` kernel test robot 2022-01-17 7:19 ` Xiubo Li 2022-01-17 7:58 ` kernel test robot 2 siblings, 0 replies; 10+ messages in thread From: kernel test robot @ 2022-01-17 6:23 UTC (permalink / raw) To: Milind Changire, Jeff Layton, Ilya Dryomov, ceph-devel Cc: kbuild-all, Milind Changire Hi Milind, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on fd84bfdddd169c219c3a637889a8b87f70a072c2] url: https://github.com/0day-ci/linux/commits/Milind-Changire/ceph-add-support-for-getvxattr-op/20220117-120129 base: fd84bfdddd169c219c3a637889a8b87f70a072c2 config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20220117/202201171456.wqUIG50D-lkp@intel.com/config) compiler: riscv64-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/2c3b424994ab41a8d52471eb5a6721f466d515dc git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Milind-Changire/ceph-add-support-for-getvxattr-op/20220117-120129 git checkout 2c3b424994ab41a8d52471eb5a6721f466d515dc # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=riscv SHELL=/bin/bash fs/ceph/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): fs/ceph/inode.c: In function 'ceph_do_getvxattr': >> <command-line>: warning: format '%u' expects argument of type 'unsigned int', but argument 7 has type 'size_t' {aka 'long unsigned int'} [-Wformat=] include/linux/ceph/ceph_debug.h:5:21: note: in expansion of macro 'KBUILD_MODNAME' 5 | #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt | ^~~~~~~~~~~~~~ include/linux/dynamic_debug.h:134:29: note: in expansion of macro 'pr_fmt' 134 | func(&id, ##__VA_ARGS__); \ | ^~~~~~~~~~~ include/linux/dynamic_debug.h:152:9: note: in expansion of macro '__dynamic_func_call' 152 | __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~~~~ include/linux/dynamic_debug.h:162:9: note: in expansion of macro '_dynamic_func_call' 162 | _dynamic_func_call(fmt, __dynamic_pr_debug, \ | ^~~~~~~~~~~~~~~~~~ include/linux/printk.h:574:9: note: in expansion of macro 'dynamic_pr_debug' 574 | dynamic_pr_debug(fmt, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~ include/linux/ceph/ceph_debug.h:19:9: note: in expansion of macro 'pr_debug' 19 | pr_debug("%.*s %12.12s:%-4d : " fmt, \ | ^~~~~~~~ fs/ceph/inode.c:2326:9: note: in expansion of macro 'dout' 2326 | dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); | ^~~~ <command-line>: warning: format '%u' expects argument of type 'unsigned int', but argument 8 has type 'size_t' {aka 'long unsigned int'} [-Wformat=] include/linux/ceph/ceph_debug.h:5:21: note: in expansion of macro 'KBUILD_MODNAME' 5 | #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt | ^~~~~~~~~~~~~~ include/linux/dynamic_debug.h:134:29: note: in expansion of macro 'pr_fmt' 134 | func(&id, ##__VA_ARGS__); \ | ^~~~~~~~~~~ include/linux/dynamic_debug.h:152:9: note: in expansion of macro '__dynamic_func_call' 152 | __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~~~~ include/linux/dynamic_debug.h:162:9: note: in expansion of macro '_dynamic_func_call' 162 | _dynamic_func_call(fmt, __dynamic_pr_debug, \ | ^~~~~~~~~~~~~~~~~~ include/linux/printk.h:574:9: note: in expansion of macro 'dynamic_pr_debug' 574 | dynamic_pr_debug(fmt, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~ include/linux/ceph/ceph_debug.h:19:9: note: in expansion of macro 'pr_debug' 19 | pr_debug("%.*s %12.12s:%-4d : " fmt, \ | ^~~~~~~~ fs/ceph/inode.c:2326:9: note: in expansion of macro 'dout' 2326 | dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); | ^~~~ --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/1] ceph: add getvxattr op @ 2022-01-17 6:23 ` kernel test robot 0 siblings, 0 replies; 10+ messages in thread From: kernel test robot @ 2022-01-17 6:23 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 4560 bytes --] Hi Milind, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on fd84bfdddd169c219c3a637889a8b87f70a072c2] url: https://github.com/0day-ci/linux/commits/Milind-Changire/ceph-add-support-for-getvxattr-op/20220117-120129 base: fd84bfdddd169c219c3a637889a8b87f70a072c2 config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20220117/202201171456.wqUIG50D-lkp(a)intel.com/config) compiler: riscv64-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/2c3b424994ab41a8d52471eb5a6721f466d515dc git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Milind-Changire/ceph-add-support-for-getvxattr-op/20220117-120129 git checkout 2c3b424994ab41a8d52471eb5a6721f466d515dc # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=riscv SHELL=/bin/bash fs/ceph/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): fs/ceph/inode.c: In function 'ceph_do_getvxattr': >> <command-line>: warning: format '%u' expects argument of type 'unsigned int', but argument 7 has type 'size_t' {aka 'long unsigned int'} [-Wformat=] include/linux/ceph/ceph_debug.h:5:21: note: in expansion of macro 'KBUILD_MODNAME' 5 | #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt | ^~~~~~~~~~~~~~ include/linux/dynamic_debug.h:134:29: note: in expansion of macro 'pr_fmt' 134 | func(&id, ##__VA_ARGS__); \ | ^~~~~~~~~~~ include/linux/dynamic_debug.h:152:9: note: in expansion of macro '__dynamic_func_call' 152 | __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~~~~ include/linux/dynamic_debug.h:162:9: note: in expansion of macro '_dynamic_func_call' 162 | _dynamic_func_call(fmt, __dynamic_pr_debug, \ | ^~~~~~~~~~~~~~~~~~ include/linux/printk.h:574:9: note: in expansion of macro 'dynamic_pr_debug' 574 | dynamic_pr_debug(fmt, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~ include/linux/ceph/ceph_debug.h:19:9: note: in expansion of macro 'pr_debug' 19 | pr_debug("%.*s %12.12s:%-4d : " fmt, \ | ^~~~~~~~ fs/ceph/inode.c:2326:9: note: in expansion of macro 'dout' 2326 | dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); | ^~~~ <command-line>: warning: format '%u' expects argument of type 'unsigned int', but argument 8 has type 'size_t' {aka 'long unsigned int'} [-Wformat=] include/linux/ceph/ceph_debug.h:5:21: note: in expansion of macro 'KBUILD_MODNAME' 5 | #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt | ^~~~~~~~~~~~~~ include/linux/dynamic_debug.h:134:29: note: in expansion of macro 'pr_fmt' 134 | func(&id, ##__VA_ARGS__); \ | ^~~~~~~~~~~ include/linux/dynamic_debug.h:152:9: note: in expansion of macro '__dynamic_func_call' 152 | __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~~~~ include/linux/dynamic_debug.h:162:9: note: in expansion of macro '_dynamic_func_call' 162 | _dynamic_func_call(fmt, __dynamic_pr_debug, \ | ^~~~~~~~~~~~~~~~~~ include/linux/printk.h:574:9: note: in expansion of macro 'dynamic_pr_debug' 574 | dynamic_pr_debug(fmt, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~ include/linux/ceph/ceph_debug.h:19:9: note: in expansion of macro 'pr_debug' 19 | pr_debug("%.*s %12.12s:%-4d : " fmt, \ | ^~~~~~~~ fs/ceph/inode.c:2326:9: note: in expansion of macro 'dout' 2326 | dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); | ^~~~ --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/1] ceph: add getvxattr op 2022-01-17 3:59 ` [PATCH v3 1/1] ceph: add " Milind Changire 2022-01-17 6:23 ` kernel test robot @ 2022-01-17 7:19 ` Xiubo Li 2022-01-17 7:36 ` Milind Changire 2022-01-17 7:58 ` kernel test robot 2 siblings, 1 reply; 10+ messages in thread From: Xiubo Li @ 2022-01-17 7:19 UTC (permalink / raw) To: Milind Changire, Jeff Layton, Ilya Dryomov, ceph-devel; +Cc: Milind Changire On 1/17/22 11:59 AM, Milind Changire wrote: > Problem: > Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be > propagated to the client as frequently to keep them updated. This > creates vxattr availability problems. > > Solution: > Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and > ceph.file.layout* vxattrs. > If the entire layout for a dir or a file is being set, then it is > expected that the layout be set in standard JSON format. Individual > field value retrieval is not wrapped in JSON. The JSON format also > applies while setting the vxattr if the entire layout is being set in > one go. > As a temporary measure, setting a vxattr can also be done in the old > format. The old format will be deprecated in the future. > > URL: https://tracker.ceph.com/issues/51062 > Signed-off-by: Milind Changire <mchangir@redhat.com> > --- > fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ > fs/ceph/mds_client.c | 27 ++++++++++++++++++- > fs/ceph/mds_client.h | 12 ++++++++- > fs/ceph/strings.c | 1 + > fs/ceph/super.h | 1 + > fs/ceph/xattr.c | 34 ++++++++++++++++++++++++ > include/linux/ceph/ceph_fs.h | 1 + > 7 files changed, 125 insertions(+), 2 deletions(-) > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c > index e3322fcb2e8d..b63746a7a0e0 100644 > --- a/fs/ceph/inode.c > +++ b/fs/ceph/inode.c > @@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page, > return err; > } > > +int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, > + size_t size) > +{ > + struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb); > + struct ceph_mds_client *mdsc = fsc->mdsc; > + struct ceph_mds_request *req; > + int mode = USE_AUTH_MDS; > + int err; > + char *xattr_value; > + size_t xattr_value_len; > + > + req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode); > + if (IS_ERR(req)) { > + err = -ENOMEM; > + goto out; > + } > + > + req->r_path2 = kstrdup(name, GFP_NOFS); > + if (!req->r_path2) { > + err = -ENOMEM; > + goto put; > + } > + > + ihold(inode); > + req->r_inode = inode; > + err = ceph_mdsc_do_request(mdsc, NULL, req); > + if (err < 0) > + goto put; > + > + xattr_value = req->r_reply_info.xattr_info.xattr_value; > + xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; > + > + dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); > + > + err = xattr_value_len; > + if (size == 0) > + goto put; > + > + if (xattr_value_len > size) { > + err = -ERANGE; > + goto put; > + } > + > + memcpy(value, xattr_value, xattr_value_len); > +put: > + ceph_mdsc_put_request(req); > +out: > + dout("do_getvxattr result=%d\n", err); > + return err; > +} > + > > /* > * Check inode permissions. We verify we have a valid value for > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > index c30eefc0ac19..a5eafc71d976 100644 > --- a/fs/ceph/mds_client.c > +++ b/fs/ceph/mds_client.c > @@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end, > return -EIO; > } > > +static int parse_reply_info_getvxattr(void **p, void *end, > + struct ceph_mds_reply_info_parsed *info, > + u64 features) > +{ > + u8 struct_v, struct_compat; > + u32 struct_len; > + u32 value_len; > + > + ceph_decode_8_safe(p, end, struct_v, bad); > + ceph_decode_8_safe(p, end, struct_compat, bad); > + ceph_decode_32_safe(p, end, struct_len, bad); > + ceph_decode_32_safe(p, end, value_len, bad); > + > + if (value_len == end - *p) { > + info->xattr_info.xattr_value = *p; > + info->xattr_info.xattr_value_len = end - *p; > + *p = end; > + return info->xattr_info.xattr_value_len; > + } > +bad: > + return -EIO; > +} > + > /* > * parse extra results > */ > @@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end, > return parse_reply_info_readdir(p, end, info, features); > else if (op == CEPH_MDS_OP_CREATE) > return parse_reply_info_create(p, end, info, features, s); > + else if (op == CEPH_MDS_OP_GETVXATTR) > + return parse_reply_info_getvxattr(p, end, info, features); > else > return -EIO; > } > @@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg, > > if (p != end) > goto bad; > - return 0; > + return err; > > bad: > err = -EIO; > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h > index 97c7f7bfa55f..f2a8e5af3c2e 100644 > --- a/fs/ceph/mds_client.h > +++ b/fs/ceph/mds_client.h > @@ -29,8 +29,10 @@ enum ceph_feature_type { > CEPHFS_FEATURE_MULTI_RECONNECT, > CEPHFS_FEATURE_DELEG_INO, > CEPHFS_FEATURE_METRIC_COLLECT, > + CEPHFS_FEATURE_ALTERNATE_NAME, > + CEPHFS_FEATURE_GETVXATTR, > > - CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT, > + CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR, > }; > > /* > @@ -45,6 +47,8 @@ enum ceph_feature_type { > CEPHFS_FEATURE_MULTI_RECONNECT, \ > CEPHFS_FEATURE_DELEG_INO, \ > CEPHFS_FEATURE_METRIC_COLLECT, \ > + CEPHFS_FEATURE_ALTERNATE_NAME, \ > + CEPHFS_FEATURE_GETVXATTR, \ > \ > CEPHFS_FEATURE_MAX, \ > } > @@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry { > loff_t offset; > }; > > +struct ceph_mds_reply_xattr { > + char *xattr_value; > + size_t xattr_value_len; > +}; > + > /* > * parsed info about an mds reply, including information about > * either: 1) the target inode and/or its parent directory and dentry, > @@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed { > char *dname; > u32 dname_len; > struct ceph_mds_reply_lease *dlease; > + struct ceph_mds_reply_xattr xattr_info; > > /* extra */ > union { > diff --git a/fs/ceph/strings.c b/fs/ceph/strings.c > index 573bb9556fb5..e36e8948e728 100644 > --- a/fs/ceph/strings.c > +++ b/fs/ceph/strings.c > @@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op) > case CEPH_MDS_OP_LOOKUPINO: return "lookupino"; > case CEPH_MDS_OP_LOOKUPNAME: return "lookupname"; > case CEPH_MDS_OP_GETATTR: return "getattr"; > + case CEPH_MDS_OP_GETVXATTR: return "getvxattr"; > case CEPH_MDS_OP_SETXATTR: return "setxattr"; > case CEPH_MDS_OP_SETATTR: return "setattr"; > case CEPH_MDS_OP_RMXATTR: return "rmxattr"; > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > index ac331aa07cfa..a627fa69668e 100644 > --- a/fs/ceph/super.h > +++ b/fs/ceph/super.h > @@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode) > > /* xattr.c */ > int __ceph_setxattr(struct inode *, const char *, const void *, size_t, int); > +int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, size_t size); > ssize_t __ceph_getxattr(struct inode *, const char *, void *, size_t); > extern ssize_t ceph_listxattr(struct dentry *, char *, size_t); > extern struct ceph_buffer *__ceph_build_xattrs_blob(struct ceph_inode_info *ci); > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c > index fcf7dfdecf96..dc32876a541a 100644 > --- a/fs/ceph/xattr.c > +++ b/fs/ceph/xattr.c > @@ -918,6 +918,30 @@ static inline int __get_request_mask(struct inode *in) { > return mask; > } > > +/* check if the entire cluster supports the given feature */ > +static inline bool ceph_cluster_has_feature(struct inode *inode, int feature_bit) > +{ > + int64_t i; > + struct ceph_fs_client *fsc = ceph_inode_to_client(inode); > + struct ceph_mds_session **sessions = fsc->mdsc->sessions; > + int64_t num_sessions = atomic_read(&fsc->mdsc->num_sessions); > + > + if (fsc->mdsc->stopping) > + return false; > + > + if (!sessions) > + return false; > + > + for (i = 0; i < num_sessions; i++) { > + struct ceph_mds_session *session = sessions[i]; > + if (!session) > + return false; > + if (!test_bit(feature_bit, &session->s_features)) This will be possibly cause crash because, and you should put the whole for loop and 'fsc->mdsc->xxx' above under the 'mdsc->mutex'. > + return false; > + } > + return true; > +} > + > ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, > size_t size) > { > @@ -927,6 +951,16 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, > int req_mask; > ssize_t err; > > + if (!strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) && > + ceph_cluster_has_feature(inode, CEPHFS_FEATURE_GETVXATTR)) { > + err = ceph_do_getvxattr(inode, name, value, size); > + /* if cluster doesn't support xattr, we try to service it > + * locally > + */ > + if (err >= 0) > + return err; > + } > + If the 'Fa' or 'Fr' caps is issued to kclient, could we get this vxattr locally instead of getting it from MDS every time ? > /* let's see if a virtual xattr was requested */ > vxattr = ceph_match_vxattr(inode, name); > if (vxattr) { > diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h > index 7ad6c3d0db7d..66db21ac5f0c 100644 > --- a/include/linux/ceph/ceph_fs.h > +++ b/include/linux/ceph/ceph_fs.h > @@ -328,6 +328,7 @@ enum { > CEPH_MDS_OP_LOOKUPPARENT = 0x00103, > CEPH_MDS_OP_LOOKUPINO = 0x00104, > CEPH_MDS_OP_LOOKUPNAME = 0x00105, > + CEPH_MDS_OP_GETVXATTR = 0x00106, > > CEPH_MDS_OP_SETXATTR = 0x01105, > CEPH_MDS_OP_RMXATTR = 0x01106, ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/1] ceph: add getvxattr op 2022-01-17 7:19 ` Xiubo Li @ 2022-01-17 7:36 ` Milind Changire 2022-01-17 8:10 ` Xiubo Li 2022-01-17 11:01 ` Jeff Layton 0 siblings, 2 replies; 10+ messages in thread From: Milind Changire @ 2022-01-17 7:36 UTC (permalink / raw) To: Xiubo Li; +Cc: Jeff Layton, Ilya Dryomov, ceph-devel, Milind Changire On Mon, Jan 17, 2022 at 12:49 PM Xiubo Li <xiubli@redhat.com> wrote: > > > On 1/17/22 11:59 AM, Milind Changire wrote: > > Problem: > > Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be > > propagated to the client as frequently to keep them updated. This > > creates vxattr availability problems. > > > > Solution: > > Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and > > ceph.file.layout* vxattrs. > > If the entire layout for a dir or a file is being set, then it is > > expected that the layout be set in standard JSON format. Individual > > field value retrieval is not wrapped in JSON. The JSON format also > > applies while setting the vxattr if the entire layout is being set in > > one go. > > As a temporary measure, setting a vxattr can also be done in the old > > format. The old format will be deprecated in the future. > > > > URL: https://tracker.ceph.com/issues/51062 > > Signed-off-by: Milind Changire <mchangir@redhat.com> > > --- > > fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ > > fs/ceph/mds_client.c | 27 ++++++++++++++++++- > > fs/ceph/mds_client.h | 12 ++++++++- > > fs/ceph/strings.c | 1 + > > fs/ceph/super.h | 1 + > > fs/ceph/xattr.c | 34 ++++++++++++++++++++++++ > > include/linux/ceph/ceph_fs.h | 1 + > > 7 files changed, 125 insertions(+), 2 deletions(-) > > > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c > > index e3322fcb2e8d..b63746a7a0e0 100644 > > --- a/fs/ceph/inode.c > > +++ b/fs/ceph/inode.c > > @@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page, > > return err; > > } > > > > +int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, > > + size_t size) > > +{ > > + struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb); > > + struct ceph_mds_client *mdsc = fsc->mdsc; > > + struct ceph_mds_request *req; > > + int mode = USE_AUTH_MDS; > > + int err; > > + char *xattr_value; > > + size_t xattr_value_len; > > + > > + req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode); > > + if (IS_ERR(req)) { > > + err = -ENOMEM; > > + goto out; > > + } > > + > > + req->r_path2 = kstrdup(name, GFP_NOFS); > > + if (!req->r_path2) { > > + err = -ENOMEM; > > + goto put; > > + } > > + > > + ihold(inode); > > + req->r_inode = inode; > > + err = ceph_mdsc_do_request(mdsc, NULL, req); > > + if (err < 0) > > + goto put; > > + > > + xattr_value = req->r_reply_info.xattr_info.xattr_value; > > + xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; > > + > > + dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); Need some help here. The kernel CI reported the following warnings: 1. for i386 build that size_t is unsigned int 2. for riscv build that size_t is unsigned long int So the above (dout) statement gets a warning either way if I change the expected arguments to be either %u or %lu. > > + > > + err = xattr_value_len; > > + if (size == 0) > > + goto put; > > + > > + if (xattr_value_len > size) { > > + err = -ERANGE; > > + goto put; > > + } > > + > > + memcpy(value, xattr_value, xattr_value_len); > > +put: > > + ceph_mdsc_put_request(req); > > +out: > > + dout("do_getvxattr result=%d\n", err); > > + return err; > > +} > > + > > > > /* > > * Check inode permissions. We verify we have a valid value for > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > > index c30eefc0ac19..a5eafc71d976 100644 > > --- a/fs/ceph/mds_client.c > > +++ b/fs/ceph/mds_client.c > > @@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end, > > return -EIO; > > } > > > > +static int parse_reply_info_getvxattr(void **p, void *end, > > + struct ceph_mds_reply_info_parsed *info, > > + u64 features) > > +{ > > + u8 struct_v, struct_compat; > > + u32 struct_len; > > + u32 value_len; > > + > > + ceph_decode_8_safe(p, end, struct_v, bad); > > + ceph_decode_8_safe(p, end, struct_compat, bad); > > + ceph_decode_32_safe(p, end, struct_len, bad); > > + ceph_decode_32_safe(p, end, value_len, bad); > > + > > + if (value_len == end - *p) { > > + info->xattr_info.xattr_value = *p; > > + info->xattr_info.xattr_value_len = end - *p; > > + *p = end; > > + return info->xattr_info.xattr_value_len; > > + } > > +bad: > > + return -EIO; > > +} > > + > > /* > > * parse extra results > > */ > > @@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end, > > return parse_reply_info_readdir(p, end, info, features); > > else if (op == CEPH_MDS_OP_CREATE) > > return parse_reply_info_create(p, end, info, features, s); > > + else if (op == CEPH_MDS_OP_GETVXATTR) > > + return parse_reply_info_getvxattr(p, end, info, features); > > else > > return -EIO; > > } > > @@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg, > > > > if (p != end) > > goto bad; > > - return 0; > > + return err; > > > > bad: > > err = -EIO; > > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h > > index 97c7f7bfa55f..f2a8e5af3c2e 100644 > > --- a/fs/ceph/mds_client.h > > +++ b/fs/ceph/mds_client.h > > @@ -29,8 +29,10 @@ enum ceph_feature_type { > > CEPHFS_FEATURE_MULTI_RECONNECT, > > CEPHFS_FEATURE_DELEG_INO, > > CEPHFS_FEATURE_METRIC_COLLECT, > > + CEPHFS_FEATURE_ALTERNATE_NAME, > > + CEPHFS_FEATURE_GETVXATTR, > > > > - CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT, > > + CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR, > > }; > > > > /* > > @@ -45,6 +47,8 @@ enum ceph_feature_type { > > CEPHFS_FEATURE_MULTI_RECONNECT, \ > > CEPHFS_FEATURE_DELEG_INO, \ > > CEPHFS_FEATURE_METRIC_COLLECT, \ > > + CEPHFS_FEATURE_ALTERNATE_NAME, \ > > + CEPHFS_FEATURE_GETVXATTR, \ > > \ > > CEPHFS_FEATURE_MAX, \ > > } > > @@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry { > > loff_t offset; > > }; > > > > +struct ceph_mds_reply_xattr { > > + char *xattr_value; > > + size_t xattr_value_len; > > +}; > > + > > /* > > * parsed info about an mds reply, including information about > > * either: 1) the target inode and/or its parent directory and dentry, > > @@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed { > > char *dname; > > u32 dname_len; > > struct ceph_mds_reply_lease *dlease; > > + struct ceph_mds_reply_xattr xattr_info; > > > > /* extra */ > > union { > > diff --git a/fs/ceph/strings.c b/fs/ceph/strings.c > > index 573bb9556fb5..e36e8948e728 100644 > > --- a/fs/ceph/strings.c > > +++ b/fs/ceph/strings.c > > @@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op) > > case CEPH_MDS_OP_LOOKUPINO: return "lookupino"; > > case CEPH_MDS_OP_LOOKUPNAME: return "lookupname"; > > case CEPH_MDS_OP_GETATTR: return "getattr"; > > + case CEPH_MDS_OP_GETVXATTR: return "getvxattr"; > > case CEPH_MDS_OP_SETXATTR: return "setxattr"; > > case CEPH_MDS_OP_SETATTR: return "setattr"; > > case CEPH_MDS_OP_RMXATTR: return "rmxattr"; > > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > > index ac331aa07cfa..a627fa69668e 100644 > > --- a/fs/ceph/super.h > > +++ b/fs/ceph/super.h > > @@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode) > > > > /* xattr.c */ > > int __ceph_setxattr(struct inode *, const char *, const void *, size_t, int); > > +int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, size_t size); > > ssize_t __ceph_getxattr(struct inode *, const char *, void *, size_t); > > extern ssize_t ceph_listxattr(struct dentry *, char *, size_t); > > extern struct ceph_buffer *__ceph_build_xattrs_blob(struct ceph_inode_info *ci); > > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c > > index fcf7dfdecf96..dc32876a541a 100644 > > --- a/fs/ceph/xattr.c > > +++ b/fs/ceph/xattr.c > > @@ -918,6 +918,30 @@ static inline int __get_request_mask(struct inode *in) { > > return mask; > > } > > > > +/* check if the entire cluster supports the given feature */ > > +static inline bool ceph_cluster_has_feature(struct inode *inode, int feature_bit) > > +{ > > + int64_t i; > > + struct ceph_fs_client *fsc = ceph_inode_to_client(inode); > > + struct ceph_mds_session **sessions = fsc->mdsc->sessions; > > + int64_t num_sessions = atomic_read(&fsc->mdsc->num_sessions); > > + > > + if (fsc->mdsc->stopping) > > + return false; > > + > > + if (!sessions) > > + return false; > > + > > + for (i = 0; i < num_sessions; i++) { > > + struct ceph_mds_session *session = sessions[i]; > > + if (!session) > > + return false; > > + if (!test_bit(feature_bit, &session->s_features)) > > This will be possibly cause crash because, and you should put the whole > for loop and 'fsc->mdsc->xxx' above under the 'mdsc->mutex'. Thanks for pointing this out. I had a suspicion that the code needed to be wrapped within some lock guard. > > > > + return false; > > + } > > + return true; > > +} > > + > > ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, > > size_t size) > > { > > @@ -927,6 +951,16 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, > > int req_mask; > > ssize_t err; > > > > + if (!strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) && > > + ceph_cluster_has_feature(inode, CEPHFS_FEATURE_GETVXATTR)) { > > + err = ceph_do_getvxattr(inode, name, value, size); > > + /* if cluster doesn't support xattr, we try to service it > > + * locally > > + */ > > + if (err >= 0) > > + return err; > > + } > > + > > If the 'Fa' or 'Fr' caps is issued to kclient, could we get this vxattr > locally instead of getting it from MDS every time ? The new mechanism is meant to supersede the old one. 1. The new layout output format is JSON 2. The new mechanism also recursively resolves the layout to the closest ancestor 3. The new mechanism deals with ceph vxattrs exclusively at the server end (currently getvxattr handles only a limited subset of all the vxattrs) 4. There's no way to fetch ceph.dir.pin* vxattrs locally (see https://github.com/ceph/ceph/pull/42001 for userspace work) > > > > > /* let's see if a virtual xattr was requested */ > > vxattr = ceph_match_vxattr(inode, name); > > if (vxattr) { > > diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h > > index 7ad6c3d0db7d..66db21ac5f0c 100644 > > --- a/include/linux/ceph/ceph_fs.h > > +++ b/include/linux/ceph/ceph_fs.h > > @@ -328,6 +328,7 @@ enum { > > CEPH_MDS_OP_LOOKUPPARENT = 0x00103, > > CEPH_MDS_OP_LOOKUPINO = 0x00104, > > CEPH_MDS_OP_LOOKUPNAME = 0x00105, > > + CEPH_MDS_OP_GETVXATTR = 0x00106, > > > > CEPH_MDS_OP_SETXATTR = 0x01105, > > CEPH_MDS_OP_RMXATTR = 0x01106, > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/1] ceph: add getvxattr op 2022-01-17 7:36 ` Milind Changire @ 2022-01-17 8:10 ` Xiubo Li 2022-01-17 11:01 ` Jeff Layton 1 sibling, 0 replies; 10+ messages in thread From: Xiubo Li @ 2022-01-17 8:10 UTC (permalink / raw) To: Milind Changire; +Cc: Jeff Layton, Ilya Dryomov, ceph-devel, Milind Changire On 1/17/22 3:36 PM, Milind Changire wrote: > On Mon, Jan 17, 2022 at 12:49 PM Xiubo Li <xiubli@redhat.com> wrote: >> >> On 1/17/22 11:59 AM, Milind Changire wrote: >>> Problem: >>> Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be >>> propagated to the client as frequently to keep them updated. This >>> creates vxattr availability problems. >>> >>> Solution: >>> Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and >>> ceph.file.layout* vxattrs. >>> If the entire layout for a dir or a file is being set, then it is >>> expected that the layout be set in standard JSON format. Individual >>> field value retrieval is not wrapped in JSON. The JSON format also >>> applies while setting the vxattr if the entire layout is being set in >>> one go. >>> As a temporary measure, setting a vxattr can also be done in the old >>> format. The old format will be deprecated in the future. >>> >>> URL: https://tracker.ceph.com/issues/51062 >>> Signed-off-by: Milind Changire <mchangir@redhat.com> >>> --- >>> fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ >>> fs/ceph/mds_client.c | 27 ++++++++++++++++++- >>> fs/ceph/mds_client.h | 12 ++++++++- >>> fs/ceph/strings.c | 1 + >>> fs/ceph/super.h | 1 + >>> fs/ceph/xattr.c | 34 ++++++++++++++++++++++++ >>> include/linux/ceph/ceph_fs.h | 1 + >>> 7 files changed, 125 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c >>> index e3322fcb2e8d..b63746a7a0e0 100644 >>> --- a/fs/ceph/inode.c >>> +++ b/fs/ceph/inode.c >>> @@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page, >>> return err; >>> } >>> >>> +int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, >>> + size_t size) >>> +{ >>> + struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb); >>> + struct ceph_mds_client *mdsc = fsc->mdsc; >>> + struct ceph_mds_request *req; >>> + int mode = USE_AUTH_MDS; >>> + int err; >>> + char *xattr_value; >>> + size_t xattr_value_len; >>> + >>> + req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode); >>> + if (IS_ERR(req)) { >>> + err = -ENOMEM; >>> + goto out; >>> + } >>> + >>> + req->r_path2 = kstrdup(name, GFP_NOFS); >>> + if (!req->r_path2) { >>> + err = -ENOMEM; >>> + goto put; >>> + } >>> + >>> + ihold(inode); >>> + req->r_inode = inode; >>> + err = ceph_mdsc_do_request(mdsc, NULL, req); >>> + if (err < 0) >>> + goto put; >>> + >>> + xattr_value = req->r_reply_info.xattr_info.xattr_value; >>> + xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; >>> + >>> + dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); > Need some help here. > The kernel CI reported the following warnings: > 1. for i386 build that size_t is unsigned int > 2. for riscv build that size_t is unsigned long int > > So the above (dout) statement gets a warning either way if I change the expected > arguments to be either %u or %lu. > You can use the %zu, more detail please see https://www.kernel.org/doc/Documentation/printk-formats.txt. >>> + >>> + err = xattr_value_len; >>> + if (size == 0) >>> + goto put; >>> + >>> + if (xattr_value_len > size) { >>> + err = -ERANGE; >>> + goto put; >>> + } >>> + [...] >>> + return false; >>> + } >>> + return true; >>> +} >>> + >>> ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, >>> size_t size) >>> { >>> @@ -927,6 +951,16 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, >>> int req_mask; >>> ssize_t err; >>> >>> + if (!strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) && >>> + ceph_cluster_has_feature(inode, CEPHFS_FEATURE_GETVXATTR)) { >>> + err = ceph_do_getvxattr(inode, name, value, size); >>> + /* if cluster doesn't support xattr, we try to service it >>> + * locally >>> + */ >>> + if (err >= 0) >>> + return err; >>> + } >>> + >> If the 'Fa' or 'Fr' caps is issued to kclient, could we get this vxattr >> locally instead of getting it from MDS every time ? > The new mechanism is meant to supersede the old one. > 1. The new layout output format is JSON > 2. The new mechanism also recursively resolves the layout to the > closest ancestor > 3. The new mechanism deals with ceph vxattrs exclusively at the server end > (currently getvxattr handles only a limited subset of all the vxattrs) > 4. There's no way to fetch ceph.dir.pin* vxattrs locally > (see https://github.com/ceph/ceph/pull/42001 for userspace work) > Okay. >> >> >>> /* let's see if a virtual xattr was requested */ >>> vxattr = ceph_match_vxattr(inode, name); >>> if (vxattr) { >>> diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h >>> index 7ad6c3d0db7d..66db21ac5f0c 100644 >>> --- a/include/linux/ceph/ceph_fs.h >>> +++ b/include/linux/ceph/ceph_fs.h >>> @@ -328,6 +328,7 @@ enum { >>> CEPH_MDS_OP_LOOKUPPARENT = 0x00103, >>> CEPH_MDS_OP_LOOKUPINO = 0x00104, >>> CEPH_MDS_OP_LOOKUPNAME = 0x00105, >>> + CEPH_MDS_OP_GETVXATTR = 0x00106, >>> >>> CEPH_MDS_OP_SETXATTR = 0x01105, >>> CEPH_MDS_OP_RMXATTR = 0x01106, ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/1] ceph: add getvxattr op 2022-01-17 7:36 ` Milind Changire 2022-01-17 8:10 ` Xiubo Li @ 2022-01-17 11:01 ` Jeff Layton 1 sibling, 0 replies; 10+ messages in thread From: Jeff Layton @ 2022-01-17 11:01 UTC (permalink / raw) To: Milind Changire, Xiubo Li; +Cc: Ilya Dryomov, ceph-devel, Milind Changire On Mon, 2022-01-17 at 13:06 +0530, Milind Changire wrote: > On Mon, Jan 17, 2022 at 12:49 PM Xiubo Li <xiubli@redhat.com> wrote: > > > > > > On 1/17/22 11:59 AM, Milind Changire wrote: > > > Problem: > > > Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be > > > propagated to the client as frequently to keep them updated. This > > > creates vxattr availability problems. > > > > > > Solution: > > > Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and > > > ceph.file.layout* vxattrs. > > > If the entire layout for a dir or a file is being set, then it is > > > expected that the layout be set in standard JSON format. Individual > > > field value retrieval is not wrapped in JSON. The JSON format also > > > applies while setting the vxattr if the entire layout is being set in > > > one go. > > > As a temporary measure, setting a vxattr can also be done in the old > > > format. The old format will be deprecated in the future. > > > > > > URL: https://tracker.ceph.com/issues/51062 > > > Signed-off-by: Milind Changire <mchangir@redhat.com> > > > --- > > > fs/ceph/inode.c | 51 ++++++++++++++++++++++++++++++++++++ > > > fs/ceph/mds_client.c | 27 ++++++++++++++++++- > > > fs/ceph/mds_client.h | 12 ++++++++- > > > fs/ceph/strings.c | 1 + > > > fs/ceph/super.h | 1 + > > > fs/ceph/xattr.c | 34 ++++++++++++++++++++++++ > > > include/linux/ceph/ceph_fs.h | 1 + > > > 7 files changed, 125 insertions(+), 2 deletions(-) > > > > > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c > > > index e3322fcb2e8d..b63746a7a0e0 100644 > > > --- a/fs/ceph/inode.c > > > +++ b/fs/ceph/inode.c > > > @@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page, > > > return err; > > > } > > > > > > +int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, > > > + size_t size) > > > +{ > > > + struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb); > > > + struct ceph_mds_client *mdsc = fsc->mdsc; > > > + struct ceph_mds_request *req; > > > + int mode = USE_AUTH_MDS; > > > + int err; > > > + char *xattr_value; > > > + size_t xattr_value_len; > > > + > > > + req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode); > > > + if (IS_ERR(req)) { > > > + err = -ENOMEM; > > > + goto out; > > > + } > > > + > > > + req->r_path2 = kstrdup(name, GFP_NOFS); > > > + if (!req->r_path2) { > > > + err = -ENOMEM; > > > + goto put; > > > + } > > > + > > > + ihold(inode); > > > + req->r_inode = inode; > > > + err = ceph_mdsc_do_request(mdsc, NULL, req); > > > + if (err < 0) > > > + goto put; > > > + > > > + xattr_value = req->r_reply_info.xattr_info.xattr_value; > > > + xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; > > > + > > > + dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); > > Need some help here. > The kernel CI reported the following warnings: > 1. for i386 build that size_t is unsigned int > 2. for riscv build that size_t is unsigned long int > > So the above (dout) statement gets a warning either way if I change the expected > arguments to be either %u or %lu. > > > > + > > > + err = xattr_value_len; > > > + if (size == 0) > > > + goto put; > > > + > > > + if (xattr_value_len > size) { > > > + err = -ERANGE; > > > + goto put; > > > + } > > > + > > > + memcpy(value, xattr_value, xattr_value_len); > > > +put: > > > + ceph_mdsc_put_request(req); > > > +out: > > > + dout("do_getvxattr result=%d\n", err); > > > + return err; > > > +} > > > + > > > > > > /* > > > * Check inode permissions. We verify we have a valid value for > > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > > > index c30eefc0ac19..a5eafc71d976 100644 > > > --- a/fs/ceph/mds_client.c > > > +++ b/fs/ceph/mds_client.c > > > @@ -555,6 +555,29 @@ static int parse_reply_info_create(void **p, void *end, > > > return -EIO; > > > } > > > > > > +static int parse_reply_info_getvxattr(void **p, void *end, > > > + struct ceph_mds_reply_info_parsed *info, > > > + u64 features) > > > +{ > > > + u8 struct_v, struct_compat; > > > + u32 struct_len; > > > + u32 value_len; > > > + > > > + ceph_decode_8_safe(p, end, struct_v, bad); > > > + ceph_decode_8_safe(p, end, struct_compat, bad); > > > + ceph_decode_32_safe(p, end, struct_len, bad); > > > + ceph_decode_32_safe(p, end, value_len, bad); > > > + > > > + if (value_len == end - *p) { > > > + info->xattr_info.xattr_value = *p; > > > + info->xattr_info.xattr_value_len = end - *p; > > > + *p = end; > > > + return info->xattr_info.xattr_value_len; > > > + } > > > +bad: > > > + return -EIO; > > > +} > > > + > > > /* > > > * parse extra results > > > */ > > > @@ -570,6 +593,8 @@ static int parse_reply_info_extra(void **p, void *end, > > > return parse_reply_info_readdir(p, end, info, features); > > > else if (op == CEPH_MDS_OP_CREATE) > > > return parse_reply_info_create(p, end, info, features, s); > > > + else if (op == CEPH_MDS_OP_GETVXATTR) > > > + return parse_reply_info_getvxattr(p, end, info, features); > > > else > > > return -EIO; > > > } > > > @@ -615,7 +640,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg, > > > > > > if (p != end) > > > goto bad; > > > - return 0; > > > + return err; > > > > > > bad: > > > err = -EIO; > > > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h > > > index 97c7f7bfa55f..f2a8e5af3c2e 100644 > > > --- a/fs/ceph/mds_client.h > > > +++ b/fs/ceph/mds_client.h > > > @@ -29,8 +29,10 @@ enum ceph_feature_type { > > > CEPHFS_FEATURE_MULTI_RECONNECT, > > > CEPHFS_FEATURE_DELEG_INO, > > > CEPHFS_FEATURE_METRIC_COLLECT, > > > + CEPHFS_FEATURE_ALTERNATE_NAME, > > > + CEPHFS_FEATURE_GETVXATTR, > > > > > > - CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_METRIC_COLLECT, > > > + CEPHFS_FEATURE_MAX = CEPHFS_FEATURE_GETVXATTR, > > > }; > > > > > > /* > > > @@ -45,6 +47,8 @@ enum ceph_feature_type { > > > CEPHFS_FEATURE_MULTI_RECONNECT, \ > > > CEPHFS_FEATURE_DELEG_INO, \ > > > CEPHFS_FEATURE_METRIC_COLLECT, \ > > > + CEPHFS_FEATURE_ALTERNATE_NAME, \ > > > + CEPHFS_FEATURE_GETVXATTR, \ > > > \ > > > CEPHFS_FEATURE_MAX, \ > > > } > > > @@ -100,6 +104,11 @@ struct ceph_mds_reply_dir_entry { > > > loff_t offset; > > > }; > > > > > > +struct ceph_mds_reply_xattr { > > > + char *xattr_value; > > > + size_t xattr_value_len; > > > +}; > > > + > > > /* > > > * parsed info about an mds reply, including information about > > > * either: 1) the target inode and/or its parent directory and dentry, > > > @@ -115,6 +124,7 @@ struct ceph_mds_reply_info_parsed { > > > char *dname; > > > u32 dname_len; > > > struct ceph_mds_reply_lease *dlease; > > > + struct ceph_mds_reply_xattr xattr_info; > > > > > > /* extra */ > > > union { > > > diff --git a/fs/ceph/strings.c b/fs/ceph/strings.c > > > index 573bb9556fb5..e36e8948e728 100644 > > > --- a/fs/ceph/strings.c > > > +++ b/fs/ceph/strings.c > > > @@ -60,6 +60,7 @@ const char *ceph_mds_op_name(int op) > > > case CEPH_MDS_OP_LOOKUPINO: return "lookupino"; > > > case CEPH_MDS_OP_LOOKUPNAME: return "lookupname"; > > > case CEPH_MDS_OP_GETATTR: return "getattr"; > > > + case CEPH_MDS_OP_GETVXATTR: return "getvxattr"; > > > case CEPH_MDS_OP_SETXATTR: return "setxattr"; > > > case CEPH_MDS_OP_SETATTR: return "setattr"; > > > case CEPH_MDS_OP_RMXATTR: return "rmxattr"; > > > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > > > index ac331aa07cfa..a627fa69668e 100644 > > > --- a/fs/ceph/super.h > > > +++ b/fs/ceph/super.h > > > @@ -1043,6 +1043,7 @@ static inline bool ceph_inode_is_shutdown(struct inode *inode) > > > > > > /* xattr.c */ > > > int __ceph_setxattr(struct inode *, const char *, const void *, size_t, int); > > > +int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, size_t size); > > > ssize_t __ceph_getxattr(struct inode *, const char *, void *, size_t); > > > extern ssize_t ceph_listxattr(struct dentry *, char *, size_t); > > > extern struct ceph_buffer *__ceph_build_xattrs_blob(struct ceph_inode_info *ci); > > > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c > > > index fcf7dfdecf96..dc32876a541a 100644 > > > --- a/fs/ceph/xattr.c > > > +++ b/fs/ceph/xattr.c > > > @@ -918,6 +918,30 @@ static inline int __get_request_mask(struct inode *in) { > > > return mask; > > > } > > > > > > +/* check if the entire cluster supports the given feature */ > > > +static inline bool ceph_cluster_has_feature(struct inode *inode, int feature_bit) > > > +{ > > > + int64_t i; > > > + struct ceph_fs_client *fsc = ceph_inode_to_client(inode); > > > + struct ceph_mds_session **sessions = fsc->mdsc->sessions; > > > + int64_t num_sessions = atomic_read(&fsc->mdsc->num_sessions); > > > + > > > + if (fsc->mdsc->stopping) > > > + return false; > > > + > > > + if (!sessions) > > > + return false; > > > + > > > + for (i = 0; i < num_sessions; i++) { > > > + struct ceph_mds_session *session = sessions[i]; > > > + if (!session) > > > + return false; > > > + if (!test_bit(feature_bit, &session->s_features)) > > > > This will be possibly cause crash because, and you should put the whole > > for loop and 'fsc->mdsc->xxx' above under the 'mdsc->mutex'. > > Thanks for pointing this out. I had a suspicion that the code needed > to be wrapped > within some lock guard. > > > > > > > > + return false; > > > + } > > > + return true; > > > +} > > > + > > > ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, > > > size_t size) > > > { > > > @@ -927,6 +951,16 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value, > > > int req_mask; > > > ssize_t err; > > > > > > + if (!strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) && > > > + ceph_cluster_has_feature(inode, CEPHFS_FEATURE_GETVXATTR)) { > > > + err = ceph_do_getvxattr(inode, name, value, size); > > > + /* if cluster doesn't support xattr, we try to service it > > > + * locally > > > + */ > > > + if (err >= 0) > > > + return err; > > > + } > > > + > > > > If the 'Fa' or 'Fr' caps is issued to kclient, could we get this vxattr > > locally instead of getting it from MDS every time ? > > The new mechanism is meant to supersede the old one. > 1. The new layout output format is JSON NAK: We can't just change the format of the vxattrs from kernel release to kernel release. This is a userland interface and is therefore part of the ABI. It needs to continue displaying this as it did before, no matter the source of the information. If you want to switch this to some sort of json output then you'll need to add a new xattr, and mark the old one for deprecation in a few releases. What problem are you solving by changing the xattr format? I don't think that's necessary in order to fix this bug. > 2. The new mechanism also recursively resolves the layout to the > closest ancestor How do you tell whether the layout comes from the inode you're querying or was inherited from one of its parents? This is a major behavior change, and I'm not sure it's one we want. > 3. The new mechanism deals with ceph vxattrs exclusively at the server end > (currently getvxattr handles only a limited subset of all the vxattrs) > 4. There's no way to fetch ceph.dir.pin* vxattrs locally > (see https://github.com/ceph/ceph/pull/42001 for userspace work) > > > > > > > > > > > /* let's see if a virtual xattr was requested */ > > > vxattr = ceph_match_vxattr(inode, name); > > > if (vxattr) { > > > diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h > > > index 7ad6c3d0db7d..66db21ac5f0c 100644 > > > --- a/include/linux/ceph/ceph_fs.h > > > +++ b/include/linux/ceph/ceph_fs.h > > > @@ -328,6 +328,7 @@ enum { > > > CEPH_MDS_OP_LOOKUPPARENT = 0x00103, > > > CEPH_MDS_OP_LOOKUPINO = 0x00104, > > > CEPH_MDS_OP_LOOKUPNAME = 0x00105, > > > + CEPH_MDS_OP_GETVXATTR = 0x00106, > > > > > > CEPH_MDS_OP_SETXATTR = 0x01105, > > > CEPH_MDS_OP_RMXATTR = 0x01106, > > -- Jeff Layton <jlayton@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/1] ceph: add getvxattr op 2022-01-17 3:59 ` [PATCH v3 1/1] ceph: add " Milind Changire @ 2022-01-17 7:58 ` kernel test robot 2022-01-17 7:19 ` Xiubo Li 2022-01-17 7:58 ` kernel test robot 2 siblings, 0 replies; 10+ messages in thread From: kernel test robot @ 2022-01-17 7:58 UTC (permalink / raw) To: Milind Changire, Jeff Layton, Ilya Dryomov, ceph-devel Cc: llvm, kbuild-all, Milind Changire Hi Milind, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on fd84bfdddd169c219c3a637889a8b87f70a072c2] url: https://github.com/0day-ci/linux/commits/Milind-Changire/ceph-add-support-for-getvxattr-op/20220117-120129 base: fd84bfdddd169c219c3a637889a8b87f70a072c2 config: x86_64-randconfig-r002-20220117 (https://download.01.org/0day-ci/archive/20220117/202201171516.ilKzMFxt-lkp@intel.com/config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5f782d25a742302d25ef3c8b84b54f7483c2deb9) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/2c3b424994ab41a8d52471eb5a6721f466d515dc git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Milind-Changire/ceph-add-support-for-getvxattr-op/20220117-120129 git checkout 2c3b424994ab41a8d52471eb5a6721f466d515dc # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/ceph/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> fs/ceph/inode.c:2326:53: warning: format specifies type 'unsigned int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat] dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); ~~ ^~~~~~~~~~~~~~~ %lu include/linux/ceph/ceph_debug.h:26:29: note: expanded from macro 'dout' printk(KERN_DEBUG fmt, ##__VA_ARGS__); \ ~~~ ^~~~~~~~~~~ include/linux/printk.h:450:60: note: expanded from macro 'printk' #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ include/linux/printk.h:422:19: note: expanded from macro 'printk_index_wrap' _p_func(_fmt, ##__VA_ARGS__); \ ~~~~ ^~~~~~~~~~~ fs/ceph/inode.c:2326:70: warning: format specifies type 'unsigned int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat] dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); ~~ ^~~~ %lu include/linux/ceph/ceph_debug.h:26:29: note: expanded from macro 'dout' printk(KERN_DEBUG fmt, ##__VA_ARGS__); \ ~~~ ^~~~~~~~~~~ include/linux/printk.h:450:60: note: expanded from macro 'printk' #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ include/linux/printk.h:422:19: note: expanded from macro 'printk_index_wrap' _p_func(_fmt, ##__VA_ARGS__); \ ~~~~ ^~~~~~~~~~~ 2 warnings generated. vim +2326 fs/ceph/inode.c 2293 2294 int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, 2295 size_t size) 2296 { 2297 struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb); 2298 struct ceph_mds_client *mdsc = fsc->mdsc; 2299 struct ceph_mds_request *req; 2300 int mode = USE_AUTH_MDS; 2301 int err; 2302 char *xattr_value; 2303 size_t xattr_value_len; 2304 2305 req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode); 2306 if (IS_ERR(req)) { 2307 err = -ENOMEM; 2308 goto out; 2309 } 2310 2311 req->r_path2 = kstrdup(name, GFP_NOFS); 2312 if (!req->r_path2) { 2313 err = -ENOMEM; 2314 goto put; 2315 } 2316 2317 ihold(inode); 2318 req->r_inode = inode; 2319 err = ceph_mdsc_do_request(mdsc, NULL, req); 2320 if (err < 0) 2321 goto put; 2322 2323 xattr_value = req->r_reply_info.xattr_info.xattr_value; 2324 xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; 2325 > 2326 dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); 2327 2328 err = xattr_value_len; 2329 if (size == 0) 2330 goto put; 2331 2332 if (xattr_value_len > size) { 2333 err = -ERANGE; 2334 goto put; 2335 } 2336 2337 memcpy(value, xattr_value, xattr_value_len); 2338 put: 2339 ceph_mdsc_put_request(req); 2340 out: 2341 dout("do_getvxattr result=%d\n", err); 2342 return err; 2343 } 2344 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/1] ceph: add getvxattr op @ 2022-01-17 7:58 ` kernel test robot 0 siblings, 0 replies; 10+ messages in thread From: kernel test robot @ 2022-01-17 7:58 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 5133 bytes --] Hi Milind, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on fd84bfdddd169c219c3a637889a8b87f70a072c2] url: https://github.com/0day-ci/linux/commits/Milind-Changire/ceph-add-support-for-getvxattr-op/20220117-120129 base: fd84bfdddd169c219c3a637889a8b87f70a072c2 config: x86_64-randconfig-r002-20220117 (https://download.01.org/0day-ci/archive/20220117/202201171516.ilKzMFxt-lkp(a)intel.com/config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5f782d25a742302d25ef3c8b84b54f7483c2deb9) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/2c3b424994ab41a8d52471eb5a6721f466d515dc git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Milind-Changire/ceph-add-support-for-getvxattr-op/20220117-120129 git checkout 2c3b424994ab41a8d52471eb5a6721f466d515dc # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/ceph/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> fs/ceph/inode.c:2326:53: warning: format specifies type 'unsigned int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat] dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); ~~ ^~~~~~~~~~~~~~~ %lu include/linux/ceph/ceph_debug.h:26:29: note: expanded from macro 'dout' printk(KERN_DEBUG fmt, ##__VA_ARGS__); \ ~~~ ^~~~~~~~~~~ include/linux/printk.h:450:60: note: expanded from macro 'printk' #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ include/linux/printk.h:422:19: note: expanded from macro 'printk_index_wrap' _p_func(_fmt, ##__VA_ARGS__); \ ~~~~ ^~~~~~~~~~~ fs/ceph/inode.c:2326:70: warning: format specifies type 'unsigned int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat] dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); ~~ ^~~~ %lu include/linux/ceph/ceph_debug.h:26:29: note: expanded from macro 'dout' printk(KERN_DEBUG fmt, ##__VA_ARGS__); \ ~~~ ^~~~~~~~~~~ include/linux/printk.h:450:60: note: expanded from macro 'printk' #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ include/linux/printk.h:422:19: note: expanded from macro 'printk_index_wrap' _p_func(_fmt, ##__VA_ARGS__); \ ~~~~ ^~~~~~~~~~~ 2 warnings generated. vim +2326 fs/ceph/inode.c 2293 2294 int ceph_do_getvxattr(struct inode *inode, const char *name, void *value, 2295 size_t size) 2296 { 2297 struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb); 2298 struct ceph_mds_client *mdsc = fsc->mdsc; 2299 struct ceph_mds_request *req; 2300 int mode = USE_AUTH_MDS; 2301 int err; 2302 char *xattr_value; 2303 size_t xattr_value_len; 2304 2305 req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode); 2306 if (IS_ERR(req)) { 2307 err = -ENOMEM; 2308 goto out; 2309 } 2310 2311 req->r_path2 = kstrdup(name, GFP_NOFS); 2312 if (!req->r_path2) { 2313 err = -ENOMEM; 2314 goto put; 2315 } 2316 2317 ihold(inode); 2318 req->r_inode = inode; 2319 err = ceph_mdsc_do_request(mdsc, NULL, req); 2320 if (err < 0) 2321 goto put; 2322 2323 xattr_value = req->r_reply_info.xattr_info.xattr_value; 2324 xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len; 2325 > 2326 dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size); 2327 2328 err = xattr_value_len; 2329 if (size == 0) 2330 goto put; 2331 2332 if (xattr_value_len > size) { 2333 err = -ERANGE; 2334 goto put; 2335 } 2336 2337 memcpy(value, xattr_value, xattr_value_len); 2338 put: 2339 ceph_mdsc_put_request(req); 2340 out: 2341 dout("do_getvxattr result=%d\n", err); 2342 return err; 2343 } 2344 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-01-17 11:01 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-17 3:59 [PATCH v3 0/1] ceph: add support for getvxattr op Milind Changire 2022-01-17 3:59 ` [PATCH v3 1/1] ceph: add " Milind Changire 2022-01-17 6:23 ` kernel test robot 2022-01-17 6:23 ` kernel test robot 2022-01-17 7:19 ` Xiubo Li 2022-01-17 7:36 ` Milind Changire 2022-01-17 8:10 ` Xiubo Li 2022-01-17 11:01 ` Jeff Layton 2022-01-17 7:58 ` kernel test robot 2022-01-17 7:58 ` kernel test robot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.