From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH] xfs: devirtualize ->m_dirnameops
Date: Tue, 12 Nov 2019 20:22:47 -0800 [thread overview]
Message-ID: <20191113042247.GH6219@magnolia> (raw)
In-Reply-To: <20191111180415.22975-1-hch@lst.de>
On Mon, Nov 11, 2019 at 07:04:15PM +0100, Christoph Hellwig wrote:
> Instead of causing a relatively expensive indirect call for each
> hashing and comparism of a file name in a directory just use an
> inline function and a simple branch on the ASCII CI bit.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_da_btree.c | 14 +-------------
> fs/xfs/libxfs/xfs_da_btree.h | 11 -----------
> fs/xfs/libxfs/xfs_dir2.c | 33 +++++++++++----------------------
> fs/xfs/libxfs/xfs_dir2_block.c | 5 ++---
> fs/xfs/libxfs/xfs_dir2_data.c | 2 +-
> fs/xfs/libxfs/xfs_dir2_leaf.c | 2 +-
> fs/xfs/libxfs/xfs_dir2_node.c | 2 +-
> fs/xfs/libxfs/xfs_dir2_priv.h | 24 ++++++++++++++++++++++++
> fs/xfs/libxfs/xfs_dir2_sf.c | 3 +--
> fs/xfs/xfs_mount.h | 2 --
> 10 files changed, 42 insertions(+), 56 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 46b1c3fb305c..bbe883f04bda 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -12,9 +12,9 @@
> #include "xfs_trans_resv.h"
> #include "xfs_bit.h"
> #include "xfs_mount.h"
> +#include "xfs_inode.h"
> #include "xfs_dir2.h"
> #include "xfs_dir2_priv.h"
> -#include "xfs_inode.h"
> #include "xfs_trans.h"
> #include "xfs_bmap.h"
> #include "xfs_attr_leaf.h"
> @@ -2098,18 +2098,6 @@ xfs_da_compname(
> XFS_CMP_EXACT : XFS_CMP_DIFFERENT;
> }
>
> -static xfs_dahash_t
> -xfs_default_hashname(
> - struct xfs_name *name)
> -{
> - return xfs_da_hashname(name->name, name->len);
> -}
> -
> -const struct xfs_nameops xfs_default_nameops = {
> - .hashname = xfs_default_hashname,
> - .compname = xfs_da_compname
> -};
> -
> int
> xfs_da_grow_inode_int(
> struct xfs_da_args *args,
> diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
> index 5af4df71e92b..ed3b558a9c1a 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.h
> +++ b/fs/xfs/libxfs/xfs_da_btree.h
> @@ -158,16 +158,6 @@ struct xfs_da3_icnode_hdr {
> (uint)(XFS_DA_LOGOFF(BASE, ADDR)), \
> (uint)(XFS_DA_LOGOFF(BASE, ADDR)+(SIZE)-1)
>
> -/*
> - * Name ops for directory and/or attr name operations
> - */
> -struct xfs_nameops {
> - xfs_dahash_t (*hashname)(struct xfs_name *);
> - enum xfs_dacmp (*compname)(struct xfs_da_args *,
> - const unsigned char *, int);
> -};
> -
> -
> /*========================================================================
> * Function prototypes.
> *========================================================================*/
> @@ -234,6 +224,5 @@ void xfs_da3_node_hdr_to_disk(struct xfs_mount *mp,
> struct xfs_da_intnode *to, struct xfs_da3_icnode_hdr *from);
>
> extern struct kmem_zone *xfs_da_state_zone;
> -extern const struct xfs_nameops xfs_default_nameops;
>
> #endif /* __XFS_DA_BTREE_H__ */
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index 624c05e77ab4..05182f2ebef4 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -52,7 +52,7 @@ xfs_mode_to_ftype(
> * ASCII case-insensitive (ie. A-Z) support for directories that was
> * used in IRIX.
> */
> -STATIC xfs_dahash_t
> +xfs_dahash_t
> xfs_ascii_ci_hashname(
> struct xfs_name *name)
> {
> @@ -65,14 +65,14 @@ xfs_ascii_ci_hashname(
> return hash;
> }
>
> -STATIC enum xfs_dacmp
> +enum xfs_dacmp
> xfs_ascii_ci_compname(
> - struct xfs_da_args *args,
> - const unsigned char *name,
> - int len)
> + struct xfs_da_args *args,
> + const unsigned char *name,
> + int len)
> {
> - enum xfs_dacmp result;
> - int i;
> + enum xfs_dacmp result;
> + int i;
>
> if (args->namelen != len)
> return XFS_CMP_DIFFERENT;
> @@ -89,11 +89,6 @@ xfs_ascii_ci_compname(
> return result;
> }
>
> -static const struct xfs_nameops xfs_ascii_ci_nameops = {
> - .hashname = xfs_ascii_ci_hashname,
> - .compname = xfs_ascii_ci_compname,
> -};
> -
> int
> xfs_da_mount(
> struct xfs_mount *mp)
> @@ -163,12 +158,6 @@ xfs_da_mount(
> dageo->node_ents = (dageo->blksize - dageo->node_hdr_size) /
> (uint)sizeof(xfs_da_node_entry_t);
> dageo->magicpct = (dageo->blksize * 37) / 100;
> -
> - if (xfs_sb_version_hasasciici(&mp->m_sb))
> - mp->m_dirnameops = &xfs_ascii_ci_nameops;
> - else
> - mp->m_dirnameops = &xfs_default_nameops;
> -
> return 0;
> }
>
> @@ -279,7 +268,7 @@ xfs_dir_createname(
> args->name = name->name;
> args->namelen = name->len;
> args->filetype = name->type;
> - args->hashval = dp->i_mount->m_dirnameops->hashname(name);
> + args->hashval = xfs_dir2_hashname(dp->i_mount, name);
> args->inumber = inum;
> args->dp = dp;
> args->total = total;
> @@ -375,7 +364,7 @@ xfs_dir_lookup(
> args->name = name->name;
> args->namelen = name->len;
> args->filetype = name->type;
> - args->hashval = dp->i_mount->m_dirnameops->hashname(name);
> + args->hashval = xfs_dir2_hashname(dp->i_mount, name);
> args->dp = dp;
> args->whichfork = XFS_DATA_FORK;
> args->trans = tp;
> @@ -447,7 +436,7 @@ xfs_dir_removename(
> args->name = name->name;
> args->namelen = name->len;
> args->filetype = name->type;
> - args->hashval = dp->i_mount->m_dirnameops->hashname(name);
> + args->hashval = xfs_dir2_hashname(dp->i_mount, name);
> args->inumber = ino;
> args->dp = dp;
> args->total = total;
> @@ -508,7 +497,7 @@ xfs_dir_replace(
> args->name = name->name;
> args->namelen = name->len;
> args->filetype = name->type;
> - args->hashval = dp->i_mount->m_dirnameops->hashname(name);
> + args->hashval = xfs_dir2_hashname(dp->i_mount, name);
> args->inumber = inum;
> args->dp = dp;
> args->total = total;
> diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
> index 358151ddfa75..f9d83205659e 100644
> --- a/fs/xfs/libxfs/xfs_dir2_block.c
> +++ b/fs/xfs/libxfs/xfs_dir2_block.c
> @@ -718,7 +718,7 @@ xfs_dir2_block_lookup_int(
> * and buffer. If it's the first case-insensitive match, store
> * the index and buffer and continue looking for an exact match.
> */
> - cmp = mp->m_dirnameops->compname(args, dep->name, dep->namelen);
> + cmp = xfs_dir2_compname(args, dep->name, dep->namelen);
gcc complains about the unused @mp variable here. With that fixed the
rest looks ok, so:
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
> args->cmpresult = cmp;
> *bpp = bp;
> @@ -1218,8 +1218,7 @@ xfs_dir2_sf_to_block(
> xfs_dir2_data_log_entry(args, bp, dep);
> name.name = sfep->name;
> name.len = sfep->namelen;
> - blp[2 + i].hashval =
> - cpu_to_be32(mp->m_dirnameops->hashname(&name));
> + blp[2 + i].hashval = cpu_to_be32(xfs_dir2_hashname(mp, &name));
> blp[2 + i].address =
> cpu_to_be32(xfs_dir2_byte_to_dataptr(newoffset));
> offset = (int)((char *)(tagp + 1) - (char *)hdr);
> diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
> index 9e471a28b6c6..11b1f3021e66 100644
> --- a/fs/xfs/libxfs/xfs_dir2_data.c
> +++ b/fs/xfs/libxfs/xfs_dir2_data.c
> @@ -236,7 +236,7 @@ __xfs_dir3_data_check(
> ((char *)dep - (char *)hdr));
> name.name = dep->name;
> name.len = dep->namelen;
> - hash = mp->m_dirnameops->hashname(&name);
> + hash = xfs_dir2_hashname(mp, &name);
> for (i = 0; i < be32_to_cpu(btp->count); i++) {
> if (be32_to_cpu(lep[i].address) == addr &&
> be32_to_cpu(lep[i].hashval) == hash)
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index e2e4b2c6d6c2..73edd96ce0ac 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -1288,7 +1288,7 @@ xfs_dir2_leaf_lookup_int(
> * and buffer. If it's the first case-insensitive match, store
> * the index and buffer and continue looking for an exact match.
> */
> - cmp = mp->m_dirnameops->compname(args, dep->name, dep->namelen);
> + cmp = xfs_dir2_compname(args, dep->name, dep->namelen);
> if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
> args->cmpresult = cmp;
> *indexp = index;
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index 5f30a1953a52..f0f67b7eb171 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -876,7 +876,7 @@ xfs_dir2_leafn_lookup_for_entry(
> * EEXIST immediately. If it's the first case-insensitive
> * match, store the block & inode number and continue looking.
> */
> - cmp = mp->m_dirnameops->compname(args, dep->name, dep->namelen);
> + cmp = xfs_dir2_compname(args, dep->name, dep->namelen);
> if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
> /* If there is a CI match block, drop it */
> if (args->cmpresult != XFS_CMP_DIFFERENT &&
> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index a22222df4bf2..eb6af7daf803 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -40,6 +40,9 @@ struct xfs_dir3_icfree_hdr {
> };
>
> /* xfs_dir2.c */
> +xfs_dahash_t xfs_ascii_ci_hashname(struct xfs_name *name);
> +enum xfs_dacmp xfs_ascii_ci_compname(struct xfs_da_args *args,
> + const unsigned char *name, int len);
> extern int xfs_dir2_grow_inode(struct xfs_da_args *args, int space,
> xfs_dir2_db_t *dbp);
> extern int xfs_dir_cilookup_result(struct xfs_da_args *args,
> @@ -191,4 +194,25 @@ xfs_dir2_data_entsize(
> return round_up(len, XFS_DIR2_DATA_ALIGN);
> }
>
> +static inline xfs_dahash_t
> +xfs_dir2_hashname(
> + struct xfs_mount *mp,
> + struct xfs_name *name)
> +{
> + if (unlikely(xfs_sb_version_hasasciici(&mp->m_sb)))
> + return xfs_ascii_ci_hashname(name);
> + return xfs_da_hashname(name->name, name->len);
> +}
> +
> +static inline enum xfs_dacmp
> +xfs_dir2_compname(
> + struct xfs_da_args *args,
> + const unsigned char *name,
> + int len)
> +{
> + if (unlikely(xfs_sb_version_hasasciici(&args->dp->i_mount->m_sb)))
> + return xfs_ascii_ci_compname(args, name, len);
> + return xfs_da_compname(args, name, len);
> +}
> +
> #endif /* __XFS_DIR2_PRIV_H__ */
> diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
> index db1a82972d9e..41eb8a676bf3 100644
> --- a/fs/xfs/libxfs/xfs_dir2_sf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
> @@ -914,8 +914,7 @@ xfs_dir2_sf_lookup(
> * number. If it's the first case-insensitive match, store the
> * inode number and continue looking for an exact match.
> */
> - cmp = dp->i_mount->m_dirnameops->compname(args, sfep->name,
> - sfep->namelen);
> + cmp = xfs_dir2_compname(args, sfep->name, sfep->namelen);
> if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
> args->cmpresult = cmp;
> args->inumber = xfs_dir2_sf_get_ino(mp, sfp, sfep);
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 43145a4ab690..247c2b15a22c 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -9,7 +9,6 @@
> struct xlog;
> struct xfs_inode;
> struct xfs_mru_cache;
> -struct xfs_nameops;
> struct xfs_ail;
> struct xfs_quotainfo;
> struct xfs_da_geometry;
> @@ -154,7 +153,6 @@ typedef struct xfs_mount {
> int m_dalign; /* stripe unit */
> int m_swidth; /* stripe width */
> uint8_t m_sectbb_log; /* sectlog - BBSHIFT */
> - const struct xfs_nameops *m_dirnameops; /* vector of dir name ops */
> atomic_t m_active_trans; /* number trans frozen */
> struct xfs_mru_cache *m_filestream; /* per-mount filestream data */
> struct delayed_work m_reclaim_work; /* background inode reclaim */
> --
> 2.20.1
>
next prev parent reply other threads:[~2019-11-13 4:22 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-11 18:04 [PATCH] xfs: devirtualize ->m_dirnameops Christoph Hellwig
2019-11-11 18:10 ` Christoph Hellwig
2019-11-13 4:22 ` Darrick J. Wong [this message]
2019-11-13 6:59 ` Christoph Hellwig
2019-11-13 16:27 ` Darrick J. Wong
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=20191113042247.GH6219@magnolia \
--to=darrick.wong@oracle.com \
--cc=hch@lst.de \
--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