From: Brian Foster <bfoster@redhat.com>
To: Allison Collins <allison.henderson@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH v3 02/19] xfs: Embed struct xfs_name in xfs_da_args
Date: Wed, 18 Sep 2019 12:44:08 -0400 [thread overview]
Message-ID: <20190918164408.GF29377@bfoster> (raw)
In-Reply-To: <20190905221837.17388-3-allison.henderson@oracle.com>
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@oracle.com>
> ---
> 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@redhat.com>
> 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
>
next prev parent reply other threads:[~2019-09-18 16:44 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-05 22:18 [PATCH v3 00/19] Delayed Attributes Allison Collins
2019-09-05 22:18 ` [PATCH v3 01/19] xfs: Replace attribute parameters with struct xfs_name Allison Collins
2019-09-18 16:43 ` Brian Foster
2019-09-18 18:09 ` Allison Collins
2019-09-18 18:14 ` Brian Foster
2019-09-18 18:48 ` Allison Collins
2019-09-18 19:06 ` Darrick J. Wong
2019-09-05 22:18 ` [PATCH v3 02/19] xfs: Embed struct xfs_name in xfs_da_args Allison Collins
2019-09-18 16:44 ` Brian Foster [this message]
2019-09-18 16:47 ` Christoph Hellwig
2019-09-18 19:55 ` Allison Collins
2019-09-05 22:18 ` [PATCH v3 03/19] xfs: Add xfs_dabuf defines Allison Collins
2019-09-06 3:37 ` Darrick J. Wong
2019-09-05 22:18 ` [PATCH v3 04/19] xfs: Add xfs_has_attr and subroutines Allison Collins
2019-09-19 17:47 ` Brian Foster
2019-09-19 23:51 ` Allison Collins
2019-09-20 12:32 ` Brian Foster
2019-09-05 22:18 ` [PATCH v3 05/19] xfs: Factor out new helper functions xfs_attr_rmtval_set Allison Collins
2019-09-20 13:49 ` Brian Foster
2019-09-21 7:29 ` Allison Collins
2019-09-05 22:18 ` [PATCH v3 06/19] xfs: Factor up trans handling in xfs_attr3_leaf_flipflags Allison Collins
2019-09-20 13:49 ` Brian Foster
2019-09-21 7:00 ` Allison Collins
2019-09-05 22:18 ` [PATCH v3 07/19] xfs: Factor out xfs_attr_leaf_addname helper Allison Collins
2019-09-20 13:49 ` Brian Foster
2019-09-21 7:00 ` Allison Collins
2019-09-05 22:18 ` [PATCH v3 08/19] xfs: Factor up commit from xfs_attr_try_sf_addname Allison Collins
2019-09-20 13:50 ` Brian Foster
2019-09-21 1:25 ` Allison Collins
2019-09-23 12:04 ` Brian Foster
2019-09-05 22:18 ` [PATCH v3 09/19] xfs: Factor up trans roll from xfs_attr3_leaf_setflag Allison Collins
2019-09-20 13:50 ` Brian Foster
2019-09-05 22:18 ` [PATCH v3 10/19] xfs: Add xfs_attr3_leaf helper functions Allison Collins
2019-09-20 13:50 ` Brian Foster
2019-09-21 1:03 ` Allison Collins
2019-09-05 22:18 ` [PATCH v3 11/19] xfs: Factor out xfs_attr_rmtval_invalidate Allison Collins
2019-09-20 13:51 ` Brian Foster
2019-09-20 22:50 ` Allison Collins
2019-09-05 22:18 ` [PATCH v3 12/19] xfs: Factor up trans roll in xfs_attr3_leaf_clearflag Allison Collins
2019-09-20 13:51 ` Brian Foster
2019-09-20 22:49 ` Allison Collins
2019-09-05 22:18 ` [PATCH v3 13/19] xfs: Add delay context to xfs_da_args Allison Collins
2019-09-20 13:51 ` Brian Foster
2019-09-20 22:48 ` Allison Collins
2019-09-05 22:18 ` [PATCH v3 14/19] xfs: Add delayed attribute routines Allison Collins
2019-09-20 15:28 ` Brian Foster
2019-09-20 19:12 ` Allison Collins
2019-09-23 12:04 ` Brian Foster
2019-09-24 5:53 ` Allison Collins
2019-09-24 10:05 ` Brian Foster
2019-09-25 4:36 ` Darrick J. Wong
2019-09-25 11:53 ` Brian Foster
2019-09-25 20:28 ` Allison Collins
2019-09-05 22:18 ` [PATCH v3 15/19] xfs: Set up infastructure for deferred attribute operations Allison Collins
2019-09-05 22:18 ` [PATCH v3 16/19] xfs: Add xfs_attr_set_deferred and xfs_attr_remove_deferred Allison Collins
2019-09-05 22:18 ` [PATCH v3 17/19] xfs: Add feature bit XFS_SB_FEAT_INCOMPAT_LOG_DELATTR Allison Collins
2019-09-05 22:18 ` [PATCH v3 18/19] xfs: Enable delayed attributes Allison Collins
2019-09-05 22:18 ` [PATCH v3 19/19] xfs_io: Add delayed attributes error tag Allison Collins
2019-09-16 12:27 ` [PATCH v3 00/19] Delayed Attributes Brian Foster
2019-09-16 18:41 ` Allison Collins
2019-09-16 19:23 ` Brian Foster
2019-09-16 20:42 ` Allison Collins
2019-09-17 4:43 ` Darrick J. Wong
2019-09-17 12:17 ` Brian Foster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190918164408.GF29377@bfoster \
--to=bfoster@redhat.com \
--cc=allison.henderson@oracle.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).