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 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.