Re: [PATCH v3 02/19] xfs: Embed struct xfs_name in xfs_da_args

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 


On 9/18/19 9:44 AM, Brian Foster wrote:

On Thu, Sep 05, 2019 at 03:18:20PM -0700, Allison Collins wrote:

This patch embeds an xfs_name in xfs_da_args, replacing the name,
namelen, and flags members.  This helps to clean up the xfs_da_args
structure and make it more uniform with the new xfs_name parameter
being passed around.

Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
---
  fs/xfs/libxfs/xfs_attr.c        |  34 ++++++-------
  fs/xfs/libxfs/xfs_attr_leaf.c   | 106 +++++++++++++++++++++-------------------
  fs/xfs/libxfs/xfs_attr_remote.c |   2 +-
  fs/xfs/libxfs/xfs_da_btree.c    |   5 +-
  fs/xfs/libxfs/xfs_da_btree.h    |   4 +-
  fs/xfs/libxfs/xfs_dir2.c        |  22 ++++-----
  fs/xfs/libxfs/xfs_dir2_block.c  |   6 +--
  fs/xfs/libxfs/xfs_dir2_leaf.c   |   6 +--
  fs/xfs/libxfs/xfs_dir2_node.c   |   8 +--
  fs/xfs/libxfs/xfs_dir2_sf.c     |  30 ++++++------
  fs/xfs/scrub/attr.c             |  12 ++---
  fs/xfs/xfs_trace.h              |  20 ++++----
  12 files changed, 130 insertions(+), 125 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index d0308d6..50e099f 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -71,13 +71,13 @@ xfs_attr_args_init(
  	args->geo = dp->i_mount->m_attr_geo;
  	args->whichfork = XFS_ATTR_FORK;
  	args->dp = dp;
-	args->flags = name->type;
-	args->name = name->name;
-	args->namelen = name->len;
-	if (args->namelen >= MAXNAMELEN)
+	args->name.type = name->type;
+	args->name.name = name->name;
+	args->name.len = name->len;


Looks like this could be a struct copy:

	args->name = *name;


+	if (args->name.len >= MAXNAMELEN)
  		return -EFAULT;		/* match IRIX behaviour */
- args->hashval = xfs_da_hashname(args->name, args->namelen); 
+	args->hashval = xfs_da_hashname(args->name.name, args->name.len);
  	return 0;
  }

...

diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index 867c5de..e8d6721 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c

...

@@ -259,8 +259,8 @@ xfs_dir_createname(
  		return -ENOMEM;
args->geo = dp->i_mount->m_dir_geo; 
-	args->name = name->name;
-	args->namelen = name->len;
+	args->name.name = name->name;
+	args->name.len = name->len;
  	args->filetype = name->type;
  	args->hashval = dp->i_mount->m_dirnameops->hashname(name);
  	args->inumber = inum;
@@ -355,8 +355,8 @@ xfs_dir_lookup(
  	 */
  	args = kmem_zalloc(sizeof(*args), KM_NOFS);
  	args->geo = dp->i_mount->m_dir_geo;
-	args->name = name->name;
-	args->namelen = name->len;
+	args->name.name = name->name;
+	args->name.len = name->len;
  	args->filetype = name->type;
  	args->hashval = dp->i_mount->m_dirnameops->hashname(name);
  	args->dp = dp;
@@ -427,8 +427,8 @@ xfs_dir_removename(
  		return -ENOMEM;
args->geo = dp->i_mount->m_dir_geo; 
-	args->name = name->name;
-	args->namelen = name->len;
+	args->name.name = name->name;
+	args->name.len = name->len;
  	args->filetype = name->type;
  	args->hashval = dp->i_mount->m_dirnameops->hashname(name);
  	args->inumber = ino;
@@ -488,8 +488,8 @@ xfs_dir_replace(
  		return -ENOMEM;
args->geo = dp->i_mount->m_dir_geo; 
-	args->name = name->name;
-	args->namelen = name->len;
+	args->name.name = name->name;
+	args->name.len = name->len;
  	args->filetype = name->type;
  	args->hashval = dp->i_mount->m_dirnameops->hashname(name);
  	args->inumber = inum;


More struct copy candidates above. Modulo that and the comments on the
previous patch, the rest LGTM:

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

i

Alrighty, will do.  Thanks for the review ;-)

Allison




diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index 9595ced..94269b9 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -355,7 +355,7 @@ xfs_dir2_block_addname(
  	if (error)
  		return error;
- len = dp->d_ops->data_entsize(args->namelen); 
+	len = dp->d_ops->data_entsize(args->name.len);
/* 
  	 * Set up pointers to parts of the block.
@@ -539,8 +539,8 @@ xfs_dir2_block_addname(
  	 * Create the new data entry.
  	 */
  	dep->inumber = cpu_to_be64(args->inumber);
-	dep->namelen = args->namelen;
-	memcpy(dep->name, args->name, args->namelen);
+	dep->namelen = args->name.len;
+	memcpy(dep->name, args->name.name, args->name.len);
  	dp->d_ops->data_put_ftype(dep, args->filetype);
  	tagp = dp->d_ops->data_entry_tag_p(dep);
  	*tagp = cpu_to_be16((char *)dep - (char *)hdr);
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index a53e458..b7046e2 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -610,7 +610,7 @@ xfs_dir2_leaf_addname(
  	ents = dp->d_ops->leaf_ents_p(leaf);
  	dp->d_ops->leaf_hdr_from_disk(&leafhdr, leaf);
  	bestsp = xfs_dir2_leaf_bests_p(ltp);
-	length = dp->d_ops->data_entsize(args->namelen);
+	length = dp->d_ops->data_entsize(args->name.len);
/* 
  	 * See if there are any entries with the same hash value
@@ -813,8 +813,8 @@ xfs_dir2_leaf_addname(
  	 */
  	dep = (xfs_dir2_data_entry_t *)dup;
  	dep->inumber = cpu_to_be64(args->inumber);
-	dep->namelen = args->namelen;
-	memcpy(dep->name, args->name, dep->namelen);
+	dep->namelen = args->name.len;
+	memcpy(dep->name, args->name.name, dep->namelen);
  	dp->d_ops->data_put_ftype(dep, args->filetype);
  	tagp = dp->d_ops->data_entry_tag_p(dep);
  	*tagp = cpu_to_be16((char *)dep - (char *)hdr);
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 705c4f5..8bbd742 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -604,7 +604,7 @@ xfs_dir2_leafn_lookup_for_addname(
  		ASSERT(free->hdr.magic == cpu_to_be32(XFS_DIR2_FREE_MAGIC) ||
  		       free->hdr.magic == cpu_to_be32(XFS_DIR3_FREE_MAGIC));
  	}
-	length = dp->d_ops->data_entsize(args->namelen);
+	length = dp->d_ops->data_entsize(args->name.len);
  	/*
  	 * Loop over leaf entries with the right hash value.
  	 */
@@ -1869,7 +1869,7 @@ xfs_dir2_node_addname_int(
  	__be16			*tagp;		/* data entry tag pointer */
  	__be16			*bests;
- length = dp->d_ops->data_entsize(args->namelen); 
+	length = dp->d_ops->data_entsize(args->name.len);
  	error = xfs_dir2_node_find_freeblk(args, fblk, &dbno, &fbp, &findex,
  					   length);
  	if (error)
@@ -1924,8 +1924,8 @@ xfs_dir2_node_addname_int(
  	/* Fill in the new entry and log it. */
  	dep = (xfs_dir2_data_entry_t *)dup;
  	dep->inumber = cpu_to_be64(args->inumber);
-	dep->namelen = args->namelen;
-	memcpy(dep->name, args->name, dep->namelen);
+	dep->namelen = args->name.len;
+	memcpy(dep->name, args->name.name, dep->namelen);
  	dp->d_ops->data_put_ftype(dep, args->filetype);
  	tagp = dp->d_ops->data_entry_tag_p(dep);
  	*tagp = cpu_to_be16((char *)dep - (char *)hdr);
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index 85f14fc..fdc1431 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -291,7 +291,7 @@ xfs_dir2_sf_addname(
  	/*
  	 * Compute entry (and change in) size.
  	 */
-	incr_isize = dp->d_ops->sf_entsize(sfp, args->namelen);
+	incr_isize = dp->d_ops->sf_entsize(sfp, args->name.len);
  	objchange = 0;
/* 
@@ -375,7 +375,7 @@ xfs_dir2_sf_addname_easy(
  	/*
  	 * Grow the in-inode space.
  	 */
-	xfs_idata_realloc(dp, dp->d_ops->sf_entsize(sfp, args->namelen),
+	xfs_idata_realloc(dp, dp->d_ops->sf_entsize(sfp, args->name.len),
  			  XFS_DATA_FORK);
  	/*
  	 * Need to set up again due to realloc of the inode data.
@@ -385,9 +385,9 @@ xfs_dir2_sf_addname_easy(
  	/*
  	 * Fill in the new entry.
  	 */
-	sfep->namelen = args->namelen;
+	sfep->namelen = args->name.len;
  	xfs_dir2_sf_put_offset(sfep, offset);
-	memcpy(sfep->name, args->name, sfep->namelen);
+	memcpy(sfep->name, args->name.name, sfep->namelen);
  	dp->d_ops->sf_put_ino(sfp, sfep, args->inumber);
  	dp->d_ops->sf_put_ftype(sfep, args->filetype);
@@ -446,7 +446,7 @@ xfs_dir2_sf_addname_hard( 
  	 */
  	for (offset = dp->d_ops->data_first_offset,
  	      oldsfep = xfs_dir2_sf_firstentry(oldsfp),
-	      add_datasize = dp->d_ops->data_entsize(args->namelen),
+	      add_datasize = dp->d_ops->data_entsize(args->name.len),
  	      eof = (char *)oldsfep == &buf[old_isize];
  	     !eof;
  	     offset = new_offset + dp->d_ops->data_entsize(oldsfep->namelen),
@@ -476,9 +476,9 @@ xfs_dir2_sf_addname_hard(
  	/*
  	 * Fill in the new entry, and update the header counts.
  	 */
-	sfep->namelen = args->namelen;
+	sfep->namelen = args->name.len;
  	xfs_dir2_sf_put_offset(sfep, offset);
-	memcpy(sfep->name, args->name, sfep->namelen);
+	memcpy(sfep->name, args->name.name, sfep->namelen);
  	dp->d_ops->sf_put_ino(sfp, sfep, args->inumber);
  	dp->d_ops->sf_put_ftype(sfep, args->filetype);
  	sfp->count++;
@@ -522,7 +522,7 @@ xfs_dir2_sf_addname_pick(
  	dp = args->dp;
sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data; 
-	size = dp->d_ops->data_entsize(args->namelen);
+	size = dp->d_ops->data_entsize(args->name.len);
  	offset = dp->d_ops->data_first_offset;
  	sfep = xfs_dir2_sf_firstentry(sfp);
  	holefit = 0;
@@ -807,7 +807,7 @@ xfs_dir2_sf_lookup(
  	/*
  	 * Special case for .
  	 */
-	if (args->namelen == 1 && args->name[0] == '.') {
+	if (args->name.len == 1 && args->name.name[0] == '.') {
  		args->inumber = dp->i_ino;
  		args->cmpresult = XFS_CMP_EXACT;
  		args->filetype = XFS_DIR3_FT_DIR;
@@ -816,8 +816,8 @@ xfs_dir2_sf_lookup(
  	/*
  	 * Special case for ..
  	 */
-	if (args->namelen == 2 &&
-	    args->name[0] == '.' && args->name[1] == '.') {
+	if (args->name.len == 2 &&
+	    args->name.name[0] == '.' && args->name.name[1] == '.') {
  		args->inumber = dp->d_ops->sf_get_parent_ino(sfp);
  		args->cmpresult = XFS_CMP_EXACT;
  		args->filetype = XFS_DIR3_FT_DIR;
@@ -912,7 +912,7 @@ xfs_dir2_sf_removename(
  	 * Calculate sizes.
  	 */
  	byteoff = (int)((char *)sfep - (char *)sfp);
-	entsize = dp->d_ops->sf_entsize(sfp, args->namelen);
+	entsize = dp->d_ops->sf_entsize(sfp, args->name.len);
  	newsize = oldsize - entsize;
  	/*
  	 * Copy the part if any after the removed entry, sliding it down.
@@ -1002,12 +1002,12 @@ xfs_dir2_sf_replace(
  	} else
  		i8elevated = 0;
- ASSERT(args->namelen != 1 || args->name[0] != '.'); 
+	ASSERT(args->name.len != 1 || args->name.name[0] != '.');
  	/*
  	 * Replace ..'s entry.
  	 */
-	if (args->namelen == 2 &&
-	    args->name[0] == '.' && args->name[1] == '.') {
+	if (args->name.len == 2 &&
+	    args->name.name[0] == '.' && args->name.name[1] == '.') {
  		ino = dp->d_ops->sf_get_parent_ino(sfp);
  		ASSERT(args->inumber != ino);
  		dp->d_ops->sf_put_parent_ino(sfp, args->inumber);
diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
index 0edc7f8..42f7c07 100644
--- a/fs/xfs/scrub/attr.c
+++ b/fs/xfs/scrub/attr.c
@@ -147,17 +147,17 @@ xchk_xattr_listent(
  		return;
  	}
- args.flags = ATTR_KERNOTIME; 
+	args.name.type = ATTR_KERNOTIME;
  	if (flags & XFS_ATTR_ROOT)
-		args.flags |= ATTR_ROOT;
+		args.name.type |= ATTR_ROOT;
  	else if (flags & XFS_ATTR_SECURE)
-		args.flags |= ATTR_SECURE;
+		args.name.type |= ATTR_SECURE;
  	args.geo = context->dp->i_mount->m_attr_geo;
  	args.whichfork = XFS_ATTR_FORK;
  	args.dp = context->dp;
-	args.name = name;
-	args.namelen = namelen;
-	args.hashval = xfs_da_hashname(args.name, args.namelen);
+	args.name.name = name;
+	args.name.len = namelen;
+	args.hashval = xfs_da_hashname(args.name.name, args.name.len);
  	args.trans = context->tp;
  	args.value = xchk_xattr_valuebuf(sx->sc);
  	args.valuelen = valuelen;
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index eaae275..e0f524d 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -1669,7 +1669,7 @@ DECLARE_EVENT_CLASS(xfs_da_class,
  	TP_STRUCT__entry(
  		__field(dev_t, dev)
  		__field(xfs_ino_t, ino)
-		__dynamic_array(char, name, args->namelen)
+		__dynamic_array(char, name, args->name.len)
  		__field(int, namelen)
  		__field(xfs_dahash_t, hashval)
  		__field(xfs_ino_t, inumber)
@@ -1678,9 +1678,10 @@ DECLARE_EVENT_CLASS(xfs_da_class,
  	TP_fast_assign(
  		__entry->dev = VFS_I(args->dp)->i_sb->s_dev;
  		__entry->ino = args->dp->i_ino;
-		if (args->namelen)
-			memcpy(__get_str(name), args->name, args->namelen);
-		__entry->namelen = args->namelen;
+		if (args->name.len)
+			memcpy(__get_str(name), args->name.name,
+			       args->name.len);
+		__entry->namelen = args->name.len;
  		__entry->hashval = args->hashval;
  		__entry->inumber = args->inumber;
  		__entry->op_flags = args->op_flags;
@@ -1733,7 +1734,7 @@ DECLARE_EVENT_CLASS(xfs_attr_class,
  	TP_STRUCT__entry(
  		__field(dev_t, dev)
  		__field(xfs_ino_t, ino)
-		__dynamic_array(char, name, args->namelen)
+		__dynamic_array(char, name, args->name.len)
  		__field(int, namelen)
  		__field(int, valuelen)
  		__field(xfs_dahash_t, hashval)
@@ -1743,12 +1744,13 @@ DECLARE_EVENT_CLASS(xfs_attr_class,
  	TP_fast_assign(
  		__entry->dev = VFS_I(args->dp)->i_sb->s_dev;
  		__entry->ino = args->dp->i_ino;
-		if (args->namelen)
-			memcpy(__get_str(name), args->name, args->namelen);
-		__entry->namelen = args->namelen;
+		if (args->name.len)
+			memcpy(__get_str(name), args->name.name,
+			       args->name.len);
+		__entry->namelen = args->name.len;
  		__entry->valuelen = args->valuelen;
  		__entry->hashval = args->hashval;
-		__entry->flags = args->flags;
+		__entry->flags = args->name.type;
  		__entry->op_flags = args->op_flags;
  	),
  	TP_printk("dev %d:%d ino 0x%llx name %.*s namelen %d valuelen %d "
--
2.7.4
[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]

  Powered by Linux