linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] xfs: Add new name to attri/d
@ 2022-08-29 21:36 Allison Henderson
  2022-08-30 16:31 ` kernel test robot
  0 siblings, 1 reply; 3+ messages in thread
From: Allison Henderson @ 2022-08-29 21:36 UTC (permalink / raw)
  To: linux-xfs

This patch adds two new fields to the atti/d.  They are nname and
nnamelen.  This will be used for parent pointer updates since a
rename operation may cause the parent pointer to update both the
name and value.  So we need to carry both the new name as well as
the target name in the attri/d.

updates since v1:
Renamed XFS_ATTRI_OP_FLAGS_NREPLACE to XFS_ATTRI_OP_FLAGS_NVREPLACE
Rearranged new xfs_da_args fields to group same sized fields together
New alfi_nname_len field now displaces the __pad field
Added extra validation checks to xfs_attri_validate
  and xlog_recover_attri_commit_pass2

Feedback appreciated.  Thanks all!

Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
 fs/xfs/libxfs/xfs_attr.c       |  12 +++-
 fs/xfs/libxfs/xfs_attr.h       |   4 +-
 fs/xfs/libxfs/xfs_da_btree.h   |   2 +
 fs/xfs/libxfs/xfs_log_format.h |   6 +-
 fs/xfs/xfs_attr_item.c         | 109 ++++++++++++++++++++++++++++-----
 fs/xfs/xfs_attr_item.h         |   1 +
 6 files changed, 114 insertions(+), 20 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index e28d93d232de..b1dbed7655e8 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -423,6 +423,12 @@ xfs_attr_complete_op(
 	args->op_flags &= ~XFS_DA_OP_REPLACE;
 	if (do_replace) {
 		args->attr_filter &= ~XFS_ATTR_INCOMPLETE;
+		if (args->new_namelen > 0) {
+			args->name = args->new_name;
+			args->namelen = args->new_namelen;
+			args->hashval = xfs_da_hashname(args->name,
+							args->namelen);
+		}
 		return replace_state;
 	}
 	return XFS_DAS_DONE;
@@ -922,9 +928,13 @@ xfs_attr_defer_replace(
 	struct xfs_da_args	*args)
 {
 	struct xfs_attr_intent	*new;
+	int			op_flag;
 	int			error = 0;
 
-	error = xfs_attr_intent_init(args, XFS_ATTRI_OP_FLAGS_REPLACE, &new);
+	op_flag = args->new_namelen == 0 ? XFS_ATTRI_OP_FLAGS_REPLACE :
+		  XFS_ATTRI_OP_FLAGS_NVREPLACE;
+
+	error = xfs_attr_intent_init(args, op_flag, &new);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
index 81be9b3e4004..3e81f3f48560 100644
--- a/fs/xfs/libxfs/xfs_attr.h
+++ b/fs/xfs/libxfs/xfs_attr.h
@@ -510,8 +510,8 @@ struct xfs_attr_intent {
 	struct xfs_da_args		*xattri_da_args;
 
 	/*
-	 * Shared buffer containing the attr name and value so that the logging
-	 * code can share large memory buffers between log items.
+	 * Shared buffer containing the attr name, new name, and value so that
+	 * the logging code can share large memory buffers between log items.
 	 */
 	struct xfs_attri_log_nameval	*xattri_nameval;
 
diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index ffa3df5b2893..a4b29827603f 100644
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -55,7 +55,9 @@ enum xfs_dacmp {
 typedef struct xfs_da_args {
 	struct xfs_da_geometry *geo;	/* da block geometry */
 	const uint8_t		*name;		/* string (maybe not NULL terminated) */
+	const uint8_t	*new_name;	/* new attr name */
 	int		namelen;	/* length of string (maybe no NULL) */
+	int		new_namelen;	/* new attr name len */
 	uint8_t		filetype;	/* filetype of inode for directories */
 	void		*value;		/* set of bytes (maybe contain NULLs) */
 	int		valuelen;	/* length of value */
diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index b351b9dc6561..62f40e6353c2 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -117,7 +117,8 @@ struct xfs_unmount_log_format {
 #define XLOG_REG_TYPE_ATTRD_FORMAT	28
 #define XLOG_REG_TYPE_ATTR_NAME	29
 #define XLOG_REG_TYPE_ATTR_VALUE	30
-#define XLOG_REG_TYPE_MAX		30
+#define XLOG_REG_TYPE_ATTR_NNAME	31
+#define XLOG_REG_TYPE_MAX		31
 
 
 /*
@@ -909,6 +910,7 @@ struct xfs_icreate_log {
 #define XFS_ATTRI_OP_FLAGS_SET		1	/* Set the attribute */
 #define XFS_ATTRI_OP_FLAGS_REMOVE	2	/* Remove the attribute */
 #define XFS_ATTRI_OP_FLAGS_REPLACE	3	/* Replace the attribute */
+#define XFS_ATTRI_OP_FLAGS_NVREPLACE	4	/* Replace attr name and val */
 #define XFS_ATTRI_OP_FLAGS_TYPE_MASK	0xFF	/* Flags type mask */
 
 /*
@@ -926,7 +928,7 @@ struct xfs_icreate_log {
 struct xfs_attri_log_format {
 	uint16_t	alfi_type;	/* attri log item type */
 	uint16_t	alfi_size;	/* size of this item */
-	uint32_t	__pad;		/* pad to 64 bit aligned */
+	uint32_t	alfi_nname_len;	/* attr new name length */
 	uint64_t	alfi_id;	/* attri identifier */
 	uint64_t	alfi_ino;	/* the inode for this attr operation */
 	uint32_t	alfi_op_flags;	/* marks the op as a set or remove */
diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index 5077a7ad5646..76eb454859f7 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -75,6 +75,8 @@ static inline struct xfs_attri_log_nameval *
 xfs_attri_log_nameval_alloc(
 	const void			*name,
 	unsigned int			name_len,
+	const void			*nname,
+	unsigned int			nname_len,
 	const void			*value,
 	unsigned int			value_len)
 {
@@ -85,7 +87,7 @@ xfs_attri_log_nameval_alloc(
 	 * this. But kvmalloc() utterly sucks, so we use our own version.
 	 */
 	nv = xlog_kvmalloc(sizeof(struct xfs_attri_log_nameval) +
-					name_len + value_len);
+					name_len + nname_len + value_len);
 	if (!nv)
 		return nv;
 
@@ -94,8 +96,18 @@ xfs_attri_log_nameval_alloc(
 	nv->name.i_type = XLOG_REG_TYPE_ATTR_NAME;
 	memcpy(nv->name.i_addr, name, name_len);
 
+	if (nname_len) {
+		nv->nname.i_addr = nv->name.i_addr + name_len;
+		nv->nname.i_len = nname_len;
+		memcpy(nv->nname.i_addr, nname, nname_len);
+	} else {
+		nv->nname.i_addr = NULL;
+		nv->nname.i_len = 0;
+	}
+	nv->nname.i_type = XLOG_REG_TYPE_ATTR_NNAME;
+
 	if (value_len) {
-		nv->value.i_addr = nv->name.i_addr + name_len;
+		nv->value.i_addr = nv->name.i_addr + nname_len + name_len;
 		nv->value.i_len = value_len;
 		memcpy(nv->value.i_addr, value, value_len);
 	} else {
@@ -149,11 +161,15 @@ xfs_attri_item_size(
 	*nbytes += sizeof(struct xfs_attri_log_format) +
 			xlog_calc_iovec_len(nv->name.i_len);
 
-	if (!nv->value.i_len)
-		return;
+	if (nv->nname.i_len) {
+		*nvecs += 1;
+		*nbytes += xlog_calc_iovec_len(nv->nname.i_len);
+	}
 
-	*nvecs += 1;
-	*nbytes += xlog_calc_iovec_len(nv->value.i_len);
+	if (nv->value.i_len) {
+		*nvecs += 1;
+		*nbytes += xlog_calc_iovec_len(nv->value.i_len);
+	}
 }
 
 /*
@@ -183,6 +199,9 @@ xfs_attri_item_format(
 	ASSERT(nv->name.i_len > 0);
 	attrip->attri_format.alfi_size++;
 
+	if (nv->nname.i_len > 0)
+		attrip->attri_format.alfi_size++;
+
 	if (nv->value.i_len > 0)
 		attrip->attri_format.alfi_size++;
 
@@ -190,6 +209,10 @@ xfs_attri_item_format(
 			&attrip->attri_format,
 			sizeof(struct xfs_attri_log_format));
 	xlog_copy_from_iovec(lv, &vecp, &nv->name);
+
+	if (nv->nname.i_len > 0)
+		xlog_copy_from_iovec(lv, &vecp, &nv->nname);
+
 	if (nv->value.i_len > 0)
 		xlog_copy_from_iovec(lv, &vecp, &nv->value);
 }
@@ -398,6 +421,7 @@ xfs_attr_log_item(
 	attrp->alfi_op_flags = attr->xattri_op_flags;
 	attrp->alfi_value_len = attr->xattri_nameval->value.i_len;
 	attrp->alfi_name_len = attr->xattri_nameval->name.i_len;
+	attrp->alfi_nname_len = attr->xattri_nameval->nname.i_len;
 	ASSERT(!(attr->xattri_da_args->attr_filter & ~XFS_ATTRI_FILTER_MASK));
 	attrp->alfi_attr_filter = attr->xattri_da_args->attr_filter;
 }
@@ -439,7 +463,8 @@ xfs_attr_create_intent(
 		 * deferred work state structure.
 		 */
 		attr->xattri_nameval = xfs_attri_log_nameval_alloc(args->name,
-				args->namelen, args->value, args->valuelen);
+				args->namelen, args->new_name,
+				args->new_namelen, args->value, args->valuelen);
 	}
 	if (!attr->xattri_nameval)
 		return ERR_PTR(-ENOMEM);
@@ -529,9 +554,6 @@ xfs_attri_validate(
 	unsigned int			op = attrp->alfi_op_flags &
 					     XFS_ATTRI_OP_FLAGS_TYPE_MASK;
 
-	if (attrp->__pad != 0)
-		return false;
-
 	if (attrp->alfi_op_flags & ~XFS_ATTRI_OP_FLAGS_TYPE_MASK)
 		return false;
 
@@ -543,6 +565,7 @@ xfs_attri_validate(
 	case XFS_ATTRI_OP_FLAGS_SET:
 	case XFS_ATTRI_OP_FLAGS_REPLACE:
 	case XFS_ATTRI_OP_FLAGS_REMOVE:
+	case XFS_ATTRI_OP_FLAGS_NVREPLACE:
 		break;
 	default:
 		return false;
@@ -552,9 +575,14 @@ xfs_attri_validate(
 		return false;
 
 	if ((attrp->alfi_name_len > XATTR_NAME_MAX) ||
+	    (attrp->alfi_nname_len > XATTR_NAME_MAX) ||
 	    (attrp->alfi_name_len == 0))
 		return false;
 
+	if (op == XFS_ATTRI_OP_FLAGS_REMOVE &&
+	    attrp->alfi_value_len != 0)
+		return false;
+
 	return xfs_verify_ino(mp, attrp->alfi_ino);
 }
 
@@ -615,6 +643,8 @@ xfs_attri_item_recover(
 	args->whichfork = XFS_ATTR_FORK;
 	args->name = nv->name.i_addr;
 	args->namelen = nv->name.i_len;
+	args->new_name = nv->nname.i_addr;
+	args->new_namelen = nv->nname.i_len;
 	args->hashval = xfs_da_hashname(args->name, args->namelen);
 	args->attr_filter = attrp->alfi_attr_filter & XFS_ATTRI_FILTER_MASK;
 	args->op_flags = XFS_DA_OP_RECOVERY | XFS_DA_OP_OKNOENT |
@@ -625,6 +655,7 @@ xfs_attri_item_recover(
 	switch (attr->xattri_op_flags) {
 	case XFS_ATTRI_OP_FLAGS_SET:
 	case XFS_ATTRI_OP_FLAGS_REPLACE:
+	case XFS_ATTRI_OP_FLAGS_NVREPLACE:
 		args->value = nv->value.i_addr;
 		args->valuelen = nv->value.i_len;
 		args->total = xfs_attr_calc_size(args, &local);
@@ -714,6 +745,7 @@ xfs_attri_item_relog(
 	new_attrp->alfi_op_flags = old_attrp->alfi_op_flags;
 	new_attrp->alfi_value_len = old_attrp->alfi_value_len;
 	new_attrp->alfi_name_len = old_attrp->alfi_name_len;
+	new_attrp->alfi_nname_len = old_attrp->alfi_nname_len;
 	new_attrp->alfi_attr_filter = old_attrp->alfi_attr_filter;
 
 	xfs_trans_add_item(tp, &new_attrip->attri_item);
@@ -735,10 +767,41 @@ xlog_recover_attri_commit_pass2(
 	struct xfs_attri_log_nameval	*nv;
 	const void			*attr_value = NULL;
 	const void			*attr_name;
-	int                             error;
+	const void			*attr_nname = NULL;
+	int				i = 0;
+	int                             op, error = 0;
 
-	attri_formatp = item->ri_buf[0].i_addr;
-	attr_name = item->ri_buf[1].i_addr;
+	if (item->ri_total == 0) {
+		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+		return -EFSCORRUPTED;
+	}
+
+	attri_formatp = item->ri_buf[i].i_addr;
+	i++;
+
+	op = attri_formatp->alfi_op_flags & XFS_ATTRI_OP_FLAGS_TYPE_MASK;
+	switch (op) {
+	case XFS_ATTRI_OP_FLAGS_SET:
+	case XFS_ATTRI_OP_FLAGS_REPLACE:
+		if (item->ri_total != 3)
+			error = -EFSCORRUPTED;
+		break;
+	case XFS_ATTRI_OP_FLAGS_REMOVE:
+		if (item->ri_total != 2)
+			error = -EFSCORRUPTED;
+		break;
+	case XFS_ATTRI_OP_FLAGS_NVREPLACE:
+		if (item->ri_total != 4)
+			error = -EFSCORRUPTED;
+		break;
+	default:
+		error = -EFSCORRUPTED;
+	}
+
+	if (error) {
+		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+		return error;
+	}
 
 	/* Validate xfs_attri_log_format before the large memory allocation */
 	if (!xfs_attri_validate(mp, attri_formatp)) {
@@ -746,13 +809,28 @@ xlog_recover_attri_commit_pass2(
 		return -EFSCORRUPTED;
 	}
 
+	attr_name = item->ri_buf[i].i_addr;
+	i++;
+
 	if (!xfs_attr_namecheck(attr_name, attri_formatp->alfi_name_len)) {
 		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
 		return -EFSCORRUPTED;
 	}
 
+	if (attri_formatp->alfi_nname_len) {
+		attr_nname = item->ri_buf[i].i_addr;
+		i++;
+
+		if (!xfs_attr_namecheck(mp, attr_nname,
+				attri_formatp->alfi_nname_len,
+				attri_formatp->alfi_attr_filter)) {
+			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+			return -EFSCORRUPTED;
+		}
+	}
+
 	if (attri_formatp->alfi_value_len)
-		attr_value = item->ri_buf[2].i_addr;
+		attr_value = item->ri_buf[i].i_addr;
 
 	/*
 	 * Memory alloc failure will cause replay to abort.  We attach the
@@ -760,7 +838,8 @@ xlog_recover_attri_commit_pass2(
 	 * reference.
 	 */
 	nv = xfs_attri_log_nameval_alloc(attr_name,
-			attri_formatp->alfi_name_len, attr_value,
+			attri_formatp->alfi_name_len, attr_nname,
+			attri_formatp->alfi_nname_len, attr_value,
 			attri_formatp->alfi_value_len);
 	if (!nv)
 		return -ENOMEM;
diff --git a/fs/xfs/xfs_attr_item.h b/fs/xfs/xfs_attr_item.h
index 3280a7930287..24d4968dd6cc 100644
--- a/fs/xfs/xfs_attr_item.h
+++ b/fs/xfs/xfs_attr_item.h
@@ -13,6 +13,7 @@ struct kmem_zone;
 
 struct xfs_attri_log_nameval {
 	struct xfs_log_iovec	name;
+	struct xfs_log_iovec	nname;
 	struct xfs_log_iovec	value;
 	refcount_t		refcount;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] xfs: Add new name to attri/d
  2022-08-29 21:36 [PATCH v2] xfs: Add new name to attri/d Allison Henderson
@ 2022-08-30 16:31 ` kernel test robot
  2022-08-30 18:05   ` Alli
  0 siblings, 1 reply; 3+ messages in thread
From: kernel test robot @ 2022-08-30 16:31 UTC (permalink / raw)
  To: Allison Henderson, linux-xfs; +Cc: kbuild-all

Hi Allison,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v6.0-rc3]
[also build test ERROR on linus/master next-20220830]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Allison-Henderson/xfs-Add-new-name-to-attri-d/20220830-053816
base:    b90cb1053190353cc30f0fef0ef1f378ccc063c5
config: x86_64-rhel-8.3-kselftests (https://download.01.org/0day-ci/archive/20220831/202208310018.1wKCQHzH-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/68f33e68647f25b811773b237669cf26e6b43382
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Allison-Henderson/xfs-Add-new-name-to-attri-d/20220830-053816
        git checkout 68f33e68647f25b811773b237669cf26e6b43382
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash arch/x86/entry/vdso/ fs/xfs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   fs/xfs/xfs_attr_item.c: In function 'xlog_recover_attri_commit_pass2':
   fs/xfs/xfs_attr_item.c:824:45: warning: passing argument 2 of 'xfs_attr_namecheck' makes integer from pointer without a cast [-Wint-conversion]
     824 |                 if (!xfs_attr_namecheck(mp, attr_nname,
         |                                             ^~~~~~~~~~
         |                                             |
         |                                             const void *
   In file included from fs/xfs/xfs_attr_item.c:22:
   fs/xfs/libxfs/xfs_attr.h:550:50: note: expected 'size_t' {aka 'long unsigned int'} but argument is of type 'const void *'
     550 | bool xfs_attr_namecheck(const void *name, size_t length);
         |                                           ~~~~~~~^~~~~~
>> fs/xfs/xfs_attr_item.c:824:22: error: too many arguments to function 'xfs_attr_namecheck'
     824 |                 if (!xfs_attr_namecheck(mp, attr_nname,
         |                      ^~~~~~~~~~~~~~~~~~
   In file included from fs/xfs/xfs_attr_item.c:22:
   fs/xfs/libxfs/xfs_attr.h:550:6: note: declared here
     550 | bool xfs_attr_namecheck(const void *name, size_t length);
         |      ^~~~~~~~~~~~~~~~~~


vim +/xfs_attr_namecheck +824 fs/xfs/xfs_attr_item.c

   756	
   757	STATIC int
   758	xlog_recover_attri_commit_pass2(
   759		struct xlog                     *log,
   760		struct list_head		*buffer_list,
   761		struct xlog_recover_item        *item,
   762		xfs_lsn_t                       lsn)
   763	{
   764		struct xfs_mount                *mp = log->l_mp;
   765		struct xfs_attri_log_item       *attrip;
   766		struct xfs_attri_log_format     *attri_formatp;
   767		struct xfs_attri_log_nameval	*nv;
   768		const void			*attr_value = NULL;
   769		const void			*attr_name;
   770		const void			*attr_nname = NULL;
   771		int				i = 0;
   772		int                             op, error = 0;
   773	
   774		if (item->ri_total == 0) {
   775			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
   776			return -EFSCORRUPTED;
   777		}
   778	
   779		attri_formatp = item->ri_buf[i].i_addr;
   780		i++;
   781	
   782		op = attri_formatp->alfi_op_flags & XFS_ATTRI_OP_FLAGS_TYPE_MASK;
   783		switch (op) {
   784		case XFS_ATTRI_OP_FLAGS_SET:
   785		case XFS_ATTRI_OP_FLAGS_REPLACE:
   786			if (item->ri_total != 3)
   787				error = -EFSCORRUPTED;
   788			break;
   789		case XFS_ATTRI_OP_FLAGS_REMOVE:
   790			if (item->ri_total != 2)
   791				error = -EFSCORRUPTED;
   792			break;
   793		case XFS_ATTRI_OP_FLAGS_NVREPLACE:
   794			if (item->ri_total != 4)
   795				error = -EFSCORRUPTED;
   796			break;
   797		default:
   798			error = -EFSCORRUPTED;
   799		}
   800	
   801		if (error) {
   802			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
   803			return error;
   804		}
   805	
   806		/* Validate xfs_attri_log_format before the large memory allocation */
   807		if (!xfs_attri_validate(mp, attri_formatp)) {
   808			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
   809			return -EFSCORRUPTED;
   810		}
   811	
   812		attr_name = item->ri_buf[i].i_addr;
   813		i++;
   814	
   815		if (!xfs_attr_namecheck(attr_name, attri_formatp->alfi_name_len)) {
   816			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
   817			return -EFSCORRUPTED;
   818		}
   819	
   820		if (attri_formatp->alfi_nname_len) {
   821			attr_nname = item->ri_buf[i].i_addr;
   822			i++;
   823	
 > 824			if (!xfs_attr_namecheck(mp, attr_nname,
   825					attri_formatp->alfi_nname_len,
   826					attri_formatp->alfi_attr_filter)) {
   827				XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
   828				return -EFSCORRUPTED;
   829			}
   830		}
   831	
   832		if (attri_formatp->alfi_value_len)
   833			attr_value = item->ri_buf[i].i_addr;
   834	
   835		/*
   836		 * Memory alloc failure will cause replay to abort.  We attach the
   837		 * name/value buffer to the recovered incore log item and drop our
   838		 * reference.
   839		 */
   840		nv = xfs_attri_log_nameval_alloc(attr_name,
   841				attri_formatp->alfi_name_len, attr_nname,
   842				attri_formatp->alfi_nname_len, attr_value,
   843				attri_formatp->alfi_value_len);
   844		if (!nv)
   845			return -ENOMEM;
   846	
   847		attrip = xfs_attri_init(mp, nv);
   848		error = xfs_attri_copy_format(&item->ri_buf[0], &attrip->attri_format);
   849		if (error)
   850			goto out;
   851	
   852		/*
   853		 * The ATTRI has two references. One for the ATTRD and one for ATTRI to
   854		 * ensure it makes it into the AIL. Insert the ATTRI into the AIL
   855		 * directly and drop the ATTRI reference. Note that
   856		 * xfs_trans_ail_update() drops the AIL lock.
   857		 */
   858		xfs_trans_ail_insert(log->l_ailp, &attrip->attri_item, lsn);
   859		xfs_attri_release(attrip);
   860		xfs_attri_log_nameval_put(nv);
   861		return 0;
   862	out:
   863		xfs_attri_item_free(attrip);
   864		xfs_attri_log_nameval_put(nv);
   865		return error;
   866	}
   867	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] xfs: Add new name to attri/d
  2022-08-30 16:31 ` kernel test robot
@ 2022-08-30 18:05   ` Alli
  0 siblings, 0 replies; 3+ messages in thread
From: Alli @ 2022-08-30 18:05 UTC (permalink / raw)
  To: linux-xfs

On Wed, 2022-08-31 at 00:31 +0800, kernel test robot wrote:
> Hi Allison,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on v6.0-rc3]
> [also build test ERROR on linus/master next-20220830]
> [If your patch is applied to the wrong git tree, kindly drop us a
> note.
> And when submitting patch, we suggest to use '--base' as documented
> in
> https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch*_base_tree_information__;Iw!!ACWV5N9M2RV99hQ!P_ML_BJdQByB_s-n0ArO4YhQL5J5odeU-wn7Z_WCCcblbKqP8g51z8T-Yrx9v7evOofMkoANqhTd_uQ$  ]
> 
> url:    
> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Allison-Henderson/xfs-Add-new-name-to-attri-d/20220830-053816__;!!ACWV5N9M2RV99hQ!P_ML_BJdQByB_s-n0ArO4YhQL5J5odeU-wn7Z_WCCcblbKqP8g51z8T-Yrx9v7evOofMkoANAw_F4ME$
>   
> base:    b90cb1053190353cc30f0fef0ef1f378ccc063c5
> config: x86_64-rhel-8.3-kselftests (
> https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20220831/202208310018.1wKCQHzH-lkp@intel.com/config__;!!ACWV5N9M2RV99hQ!P_ML_BJdQByB_s-n0ArO4YhQL5J5odeU-wn7Z_WCCcblbKqP8g51z8T-Yrx9v7evOofMkoANc7fxoPo$
>   )
> compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
> reproduce (this is a W=1 build):
>         # 
> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commit/68f33e68647f25b811773b237669cf26e6b43382__;!!ACWV5N9M2RV99hQ!P_ML_BJdQByB_s-n0ArO4YhQL5J5odeU-wn7Z_WCCcblbKqP8g51z8T-Yrx9v7evOofMkoANaktysnU$
>   
>         git remote add linux-review 
> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux__;!!ACWV5N9M2RV99hQ!P_ML_BJdQByB_s-n0ArO4YhQL5J5odeU-wn7Z_WCCcblbKqP8g51z8T-Yrx9v7evOofMkoANKKi62kA$
>   
>         git fetch --no-tags linux-review Allison-Henderson/xfs-Add-
> new-name-to-attri-d/20220830-053816
>         git checkout 68f33e68647f25b811773b237669cf26e6b43382
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
> arch/x86/entry/vdso/ fs/xfs/
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    fs/xfs/xfs_attr_item.c: In function
> 'xlog_recover_attri_commit_pass2':
>    fs/xfs/xfs_attr_item.c:824:45: warning: passing argument 2 of
> 'xfs_attr_namecheck' makes integer from pointer without a cast [-
> Wint-conversion]
>      824 |                 if (!xfs_attr_namecheck(mp, attr_nname,
>          |                                             ^~~~~~~~~~
>          |                                             |
>          |                                             const void *
>    In file included from fs/xfs/xfs_attr_item.c:22:
>    fs/xfs/libxfs/xfs_attr.h:550:50: note: expected 'size_t' {aka
> 'long unsigned int'} but argument is of type 'const void *'
>      550 | bool xfs_attr_namecheck(const void *name, size_t length);
>          |                                           ~~~~~~~^~~~~~
> > > fs/xfs/xfs_attr_item.c:824:22: error: too many arguments to
> > > function 'xfs_attr_namecheck'
>      824 |                 if (!xfs_attr_namecheck(mp, attr_nname,
>          |                      ^~~~~~~~~~~~~~~~~~
>    In file included from fs/xfs/xfs_attr_item.c:22:
>    fs/xfs/libxfs/xfs_attr.h:550:6: note: declared here
>      550 | bool xfs_attr_namecheck(const void *name, size_t length);
>          |      ^~~~~~~~~~~~~~~~~~
> 
> 
> vim +/xfs_attr_namecheck +824 fs/xfs/xfs_attr_item.c
> 
>    756	
>    757	STATIC int
>    758	xlog_recover_attri_commit_pass2(
>    759		struct xlog                     *log,
>    760		struct list_head		*buffer_list,
>    761		struct xlog_recover_item        *item,
>    762		xfs_lsn_t                       lsn)
>    763	{
>    764		struct xfs_mount                *mp = log-
> >l_mp;
>    765		struct xfs_attri_log_item       *attrip;
>    766		struct xfs_attri_log_format     *attri_formatp;
>    767		struct xfs_attri_log_nameval	*nv;
>    768		const void			*attr_value =
> NULL;
>    769		const void			*attr_name;
>    770		const void			*attr_nname =
> NULL;
>    771		int				i = 0;
>    772		int                             op, error = 0;
>    773	
>    774		if (item->ri_total == 0) {
>    775			XFS_ERROR_REPORT(__func__,
> XFS_ERRLEVEL_LOW, mp);
>    776			return -EFSCORRUPTED;
>    777		}
>    778	
>    779		attri_formatp = item->ri_buf[i].i_addr;
>    780		i++;
>    781	
>    782		op = attri_formatp->alfi_op_flags &
> XFS_ATTRI_OP_FLAGS_TYPE_MASK;
>    783		switch (op) {
>    784		case XFS_ATTRI_OP_FLAGS_SET:
>    785		case XFS_ATTRI_OP_FLAGS_REPLACE:
>    786			if (item->ri_total != 3)
>    787				error = -EFSCORRUPTED;
>    788			break;
>    789		case XFS_ATTRI_OP_FLAGS_REMOVE:
>    790			if (item->ri_total != 2)
>    791				error = -EFSCORRUPTED;
>    792			break;
>    793		case XFS_ATTRI_OP_FLAGS_NVREPLACE:
>    794			if (item->ri_total != 4)
>    795				error = -EFSCORRUPTED;
>    796			break;
>    797		default:
>    798			error = -EFSCORRUPTED;
>    799		}
>    800	
>    801		if (error) {
>    802			XFS_ERROR_REPORT(__func__,
> XFS_ERRLEVEL_LOW, mp);
>    803			return error;
>    804		}
>    805	
>    806		/* Validate xfs_attri_log_format before the
> large memory allocation */
>    807		if (!xfs_attri_validate(mp, attri_formatp)) {
>    808			XFS_ERROR_REPORT(__func__,
> XFS_ERRLEVEL_LOW, mp);
>    809			return -EFSCORRUPTED;
>    810		}
>    811	
>    812		attr_name = item->ri_buf[i].i_addr;
>    813		i++;
>    814	
>    815		if (!xfs_attr_namecheck(attr_name,
> attri_formatp->alfi_name_len)) {
>    816			XFS_ERROR_REPORT(__func__,
> XFS_ERRLEVEL_LOW, mp);
>    817			return -EFSCORRUPTED;
>    818		}
>    819	
>    820		if (attri_formatp->alfi_nname_len) {
>    821			attr_nname = item->ri_buf[i].i_addr;
>    822			i++;
>    823	
>  > 824			if (!xfs_attr_namecheck(mp, attr_nname,
>    825					attri_formatp-
> >alfi_nname_len,
>    826					attri_formatp-
> >alfi_attr_filter)) {
Oops, this signature change belongs to parent pointers.  Will
separate...


>    827				XFS_ERROR_REPORT(__func__,
> XFS_ERRLEVEL_LOW, mp);
>    828				return -EFSCORRUPTED;
>    829			}
>    830		}
>    831	
>    832		if (attri_formatp->alfi_value_len)
>    833			attr_value = item->ri_buf[i].i_addr;
>    834	
>    835		/*
>    836		 * Memory alloc failure will cause replay to
> abort.  We attach the
>    837		 * name/value buffer to the recovered incore
> log item and drop our
>    838		 * reference.
>    839		 */
>    840		nv = xfs_attri_log_nameval_alloc(attr_name,
>    841				attri_formatp->alfi_name_len,
> attr_nname,
>    842				attri_formatp->alfi_nname_len,
> attr_value,
>    843				attri_formatp->alfi_value_len);
>    844		if (!nv)
>    845			return -ENOMEM;
>    846	
>    847		attrip = xfs_attri_init(mp, nv);
>    848		error = xfs_attri_copy_format(&item->ri_buf[0], 
> &attrip->attri_format);
>    849		if (error)
>    850			goto out;
>    851	
>    852		/*
>    853		 * The ATTRI has two references. One for the
> ATTRD and one for ATTRI to
>    854		 * ensure it makes it into the AIL. Insert the
> ATTRI into the AIL
>    855		 * directly and drop the ATTRI reference. Note
> that
>    856		 * xfs_trans_ail_update() drops the AIL lock.
>    857		 */
>    858		xfs_trans_ail_insert(log->l_ailp, &attrip-
> >attri_item, lsn);
>    859		xfs_attri_release(attrip);
>    860		xfs_attri_log_nameval_put(nv);
>    861		return 0;
>    862	out:
>    863		xfs_attri_item_free(attrip);
>    864		xfs_attri_log_nameval_put(nv);
>    865		return error;
>    866	}
>    867	
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-08-30 18:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-29 21:36 [PATCH v2] xfs: Add new name to attri/d Allison Henderson
2022-08-30 16:31 ` kernel test robot
2022-08-30 18:05   ` Alli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).