* [PATCH] xfs: devirtualize ->m_dirnameops
@ 2019-11-11 18:04 Christoph Hellwig
2019-11-11 18:10 ` Christoph Hellwig
2019-11-13 4:22 ` Darrick J. Wong
0 siblings, 2 replies; 5+ messages in thread
From: Christoph Hellwig @ 2019-11-11 18:04 UTC (permalink / raw)
To: linux-xfs
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);
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: devirtualize ->m_dirnameops
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
1 sibling, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2019-11-11 18:10 UTC (permalink / raw)
To: linux-xfs
Sorry, this needs to go on top of the trivial
"xfs: remove the unused m_chsize field" patch I sent out just now.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: devirtualize ->m_dirnameops
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
2019-11-13 6:59 ` Christoph Hellwig
1 sibling, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2019-11-13 4:22 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: devirtualize ->m_dirnameops
2019-11-13 4:22 ` Darrick J. Wong
@ 2019-11-13 6:59 ` Christoph Hellwig
2019-11-13 16:27 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2019-11-13 6:59 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Tue, Nov 12, 2019 at 08:22:47PM -0800, Darrick J. Wong wrote:
> > @@ -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:
What gcc version do you use? I see a consistent pattern lately that
yours (correctly) find initialized but unused variable, but neither my
local one nor the build bot does..
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: devirtualize ->m_dirnameops
2019-11-13 6:59 ` Christoph Hellwig
@ 2019-11-13 16:27 ` Darrick J. Wong
0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2019-11-13 16:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Wed, Nov 13, 2019 at 07:59:18AM +0100, Christoph Hellwig wrote:
> On Tue, Nov 12, 2019 at 08:22:47PM -0800, Darrick J. Wong wrote:
> > > @@ -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:
>
> What gcc version do you use? I see a consistent pattern lately that
> yours (correctly) find initialized but unused variable, but neither my
> local one nor the build bot does..
$ gcc --version
gcc (Ubuntu 8.3.0-6ubuntu1~18.04.1) 8.3.0
AHA, I remember now that I kludged up the xfs and iomap makefiles to
include the following, which turns on more warnings and debuginfo:
ccflags-$(CONFIG_KASAN) += -Wno-error
ccflags-y += -g \
-Werror \
-femit-struct-debug-detailed=any \
-Wunused-but-set-variable \
-Wuninitialized \
-Wno-pointer-sign \
-Wall \
-Wextra \
-Wno-unused-parameter \
-fstack-usage \
-Wno-sign-compare \
-Wno-ignored-qualifiers \
-Wno-error=unused-but-set-variable \
-Wno-error=format=
UBSAN_SANITIZE := y
At this point I suspect -Wall -Wextra cover a lot of these.
--D
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-11-13 16:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2019-11-13 6:59 ` Christoph Hellwig
2019-11-13 16:27 ` Darrick J. Wong
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).