From: Darrick J. Wong <djwong@xxxxxxxxxx>
Reduce the size and performance impacts of parent pointer name hashes by
using the dirent name as the hash if the dirent name is shorter than a
sha512 hash would be. IOWs, we only use sha512 for names longer than 63
bytes.
Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_da_format.h | 21 +++++++++-
fs/xfs/libxfs/xfs_parent.c | 85 +++++++++++++++++++++++++++--------------
fs/xfs/libxfs/xfs_parent.h | 8 ++--
fs/xfs/scrub/dir.c | 12 ++++--
fs/xfs/scrub/dir_repair.c | 2 -
fs/xfs/scrub/parent.c | 16 +++++---
fs/xfs/scrub/parent_repair.c | 2 -
fs/xfs/xfs_parent_utils.c | 2 -
8 files changed, 101 insertions(+), 47 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
index 386f63b262d5..275357506394 100644
--- a/fs/xfs/libxfs/xfs_da_format.h
+++ b/fs/xfs/libxfs/xfs_da_format.h
@@ -831,8 +831,11 @@ xfs_failaddr_t xfs_da3_blkinfo_verify(struct xfs_buf *bp,
* Parent pointer attribute format definition
*
* The EA name encodes the parent inode number, generation and a collision
- * resistant hash computed from the dirent name. The hash is defined to be the
- * sha512 of the child inode generation and the dirent name.
+ * resistant hash computed from the dirent name. The hash is defined to be:
+ *
+ * - The dirent name if it fits within the EA name.
+ *
+ * - The sha512 of the child inode generation and the dirent name.
*
* The EA value contains the same name as the dirent in the parent directory.
*/
@@ -842,4 +845,18 @@ struct xfs_parent_name_rec {
__u8 p_namehash[XFS_PARENT_NAME_HASH_SIZE];
} __attribute__((packed));
+static inline unsigned int
+xfs_parent_name_rec_sizeof(
+ unsigned int hashlen)
+{
+ return offsetof(struct xfs_parent_name_rec, p_namehash) + hashlen;
+}
+
+static inline unsigned int
+xfs_parent_name_hashlen(
+ unsigned int rec_sizeof)
+{
+ return rec_sizeof - offsetof(struct xfs_parent_name_rec, p_namehash);
+}
+
#endif /* __XFS_DA_FORMAT_H__ */
diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c
index a28dcf18cb4d..32235a0e9e0d 100644
--- a/fs/xfs/libxfs/xfs_parent.c
+++ b/fs/xfs/libxfs/xfs_parent.c
@@ -56,7 +56,8 @@ xfs_parent_namecheck(
{
xfs_ino_t p_ino;
- if (reclen != sizeof(struct xfs_parent_name_rec))
+ if (reclen <= xfs_parent_name_rec_sizeof(0) ||
+ reclen > xfs_parent_name_rec_sizeof(XFS_PARENT_NAME_HASH_SIZE))
return false;
/* Only one namespace bit allowed. */
@@ -108,12 +109,16 @@ void
xfs_parent_irec_from_disk(
struct xfs_parent_name_irec *irec,
const struct xfs_parent_name_rec *rec,
+ int reclen,
const void *value,
int valuelen)
{
irec->p_ino = be64_to_cpu(rec->p_ino);
irec->p_gen = be32_to_cpu(rec->p_gen);
- memcpy(irec->p_namehash, rec->p_namehash, sizeof(irec->p_namehash));
+ irec->hashlen = xfs_parent_name_hashlen(reclen);
+ memcpy(irec->p_namehash, rec->p_namehash, irec->hashlen);
+ memset(irec->p_namehash + irec->hashlen, 0,
+ sizeof(irec->p_namehash) - irec->hashlen);
if (!value) {
irec->p_namelen = 0;
@@ -137,13 +142,15 @@ xfs_parent_irec_from_disk(
void
xfs_parent_irec_to_disk(
struct xfs_parent_name_rec *rec,
+ int *reclen,
void *value,
int *valuelen,
const struct xfs_parent_name_irec *irec)
{
rec->p_ino = cpu_to_be64(irec->p_ino);
rec->p_gen = cpu_to_be32(irec->p_gen);
- memcpy(rec->p_namehash, irec->p_namehash, sizeof(rec->p_namehash));
+ *reclen = xfs_parent_name_rec_sizeof(irec->hashlen);
+ memcpy(rec->p_namehash, irec->p_namehash, irec->hashlen);
if (valuelen) {
ASSERT(*valuelen > 0);
@@ -206,12 +213,14 @@ xfs_parent_add(
struct xfs_inode *child)
{
struct xfs_da_args *args = &parent->args;
- int error;
+ int hashlen;
- error = xfs_init_parent_name_rec(&parent->rec, dp, parent_name, child);
- if (error)
- return error;
+ hashlen = xfs_init_parent_name_rec(&parent->rec, dp, parent_name,
+ child);
+ if (hashlen < 0)
+ return hashlen;
+ args->namelen = xfs_parent_name_rec_sizeof(hashlen);
args->hashval = xfs_da_hashname(args->name, args->namelen);
args->trans = tp;
@@ -234,12 +243,13 @@ xfs_parent_remove(
struct xfs_inode *child)
{
struct xfs_da_args *args = &parent->args;
- int error;
+ int hashlen;
- error = xfs_init_parent_name_rec(&parent->rec, dp, name, child);
- if (error)
- return error;
+ hashlen = xfs_init_parent_name_rec(&parent->rec, dp, name, child);
+ if (hashlen < 0)
+ return hashlen;
+ args->namelen = xfs_parent_name_rec_sizeof(hashlen);
args->trans = tp;
args->dp = child;
args->hashval = xfs_da_hashname(args->name, args->namelen);
@@ -258,21 +268,21 @@ xfs_parent_replace(
struct xfs_inode *child)
{
struct xfs_da_args *args = &new_parent->args;
- int error;
+ int old_hashlen, new_hashlen;
- error = xfs_init_parent_name_rec(&new_parent->old_rec, old_dp,
+ old_hashlen = xfs_init_parent_name_rec(&new_parent->old_rec, old_dp,
old_name, child);
- if (error)
- return error;
- error = xfs_init_parent_name_rec(&new_parent->rec, new_dp, new_name,
- child);
- if (error)
- return error;
+ if (old_hashlen < 0)
+ return old_hashlen;
+ new_hashlen = xfs_init_parent_name_rec(&new_parent->rec, new_dp,
+ new_name, child);
+ if (new_hashlen < 0)
+ return new_hashlen;
new_parent->args.name = (const uint8_t *)&new_parent->old_rec;
- new_parent->args.namelen = sizeof(struct xfs_parent_name_rec);
+ new_parent->args.namelen = xfs_parent_name_rec_sizeof(old_hashlen);
new_parent->args.new_name = (const uint8_t *)&new_parent->rec;
- new_parent->args.new_namelen = sizeof(struct xfs_parent_name_rec);
+ new_parent->args.new_namelen = xfs_parent_name_rec_sizeof(new_hashlen);
args->trans = tp;
args->dp = child;
@@ -320,16 +330,17 @@ xfs_parent_lookup(
unsigned int namelen,
struct xfs_parent_scratch *scr)
{
+ int reclen;
int error;
- xfs_parent_irec_to_disk(&scr->rec, NULL, NULL, pptr);
+ xfs_parent_irec_to_disk(&scr->rec, &reclen, NULL, NULL, pptr);
memset(&scr->args, 0, sizeof(struct xfs_da_args));
scr->args.attr_filter = XFS_ATTR_PARENT;
scr->args.dp = ip;
scr->args.geo = ip->i_mount->m_attr_geo;
scr->args.name = (const unsigned char *)&scr->rec;
- scr->args.namelen = sizeof(struct xfs_parent_name_rec);
+ scr->args.namelen = reclen;
scr->args.op_flags = XFS_DA_OP_OKNOENT;
scr->args.trans = tp;
scr->args.valuelen = namelen;
@@ -357,14 +368,16 @@ xfs_parent_set(
const struct xfs_parent_name_irec *pptr,
struct xfs_parent_scratch *scr)
{
- xfs_parent_irec_to_disk(&scr->rec, NULL, NULL, pptr);
+ int reclen;
+
+ xfs_parent_irec_to_disk(&scr->rec, &reclen, NULL, NULL, pptr);
memset(&scr->args, 0, sizeof(struct xfs_da_args));
scr->args.attr_filter = XFS_ATTR_PARENT;
scr->args.dp = ip;
scr->args.geo = ip->i_mount->m_attr_geo;
scr->args.name = (const unsigned char *)&scr->rec;
- scr->args.namelen = sizeof(struct xfs_parent_name_rec);
+ scr->args.namelen = reclen;
scr->args.valuelen = pptr->p_namelen;
scr->args.value = (void *)pptr->p_name;
scr->args.whichfork = XFS_ATTR_FORK;
@@ -384,14 +397,16 @@ xfs_parent_unset(
const struct xfs_parent_name_irec *pptr,
struct xfs_parent_scratch *scr)
{
- xfs_parent_irec_to_disk(&scr->rec, NULL, NULL, pptr);
+ int reclen;
+
+ xfs_parent_irec_to_disk(&scr->rec, &reclen, NULL, NULL, pptr);
memset(&scr->args, 0, sizeof(struct xfs_da_args));
scr->args.attr_filter = XFS_ATTR_PARENT;
scr->args.dp = ip;
scr->args.geo = ip->i_mount->m_attr_geo;
scr->args.name = (const unsigned char *)&scr->rec;
- scr->args.namelen = sizeof(struct xfs_parent_name_rec);
+ scr->args.namelen = reclen;
scr->args.whichfork = XFS_ATTR_FORK;
return xfs_attr_set(&scr->args);
@@ -399,7 +414,7 @@ xfs_parent_unset(
/*
* Compute the parent pointer namehash for the given child file and dirent
- * name.
+ * name. Returns the length of the hash in bytes, or a negative errno.
*/
int
xfs_parent_namehash(
@@ -420,6 +435,12 @@ xfs_parent_namehash(
return -EINVAL;
}
+ if (name->len < namehash_len) {
+ memcpy(namehash, name->name, name->len);
+ memset(namehash + name->len, 0, namehash_len - name->len);
+ return name->len;
+ }
+
error = sha512_init(&shash);
if (error)
goto out;
@@ -436,6 +457,7 @@ xfs_parent_namehash(
if (error)
goto out;
+ error = SHA512_DIGEST_SIZE;
out:
sha512_erase(&shash);
return error;
@@ -451,7 +473,12 @@ xfs_parent_irec_hash(
.name = pptr->p_name,
.len = pptr->p_namelen,
};
+ int hashlen;
- return xfs_parent_namehash(ip, &xname, &pptr->p_namehash,
+ hashlen = xfs_parent_namehash(ip, &xname, &pptr->p_namehash,
sizeof(pptr->p_namehash));
+ if (hashlen < 0)
+ return hashlen;
+ pptr->hashlen = hashlen;
+ return 0;
}
diff --git a/fs/xfs/libxfs/xfs_parent.h b/fs/xfs/libxfs/xfs_parent.h
index d3f2841e0f6e..4c3100760bba 100644
--- a/fs/xfs/libxfs/xfs_parent.h
+++ b/fs/xfs/libxfs/xfs_parent.h
@@ -23,6 +23,7 @@ struct xfs_parent_name_irec {
/* Key fields for looking up a particular parent pointer. */
xfs_ino_t p_ino;
uint32_t p_gen;
+ uint8_t hashlen;
uint8_t p_namehash[XFS_PARENT_NAME_HASH_SIZE];
/* Attributes of a parent pointer. */
@@ -31,10 +32,11 @@ struct xfs_parent_name_irec {
};
void xfs_parent_irec_from_disk(struct xfs_parent_name_irec *irec,
- const struct xfs_parent_name_rec *rec,
+ const struct xfs_parent_name_rec *rec, int reclen,
const void *value, int valuelen);
-void xfs_parent_irec_to_disk(struct xfs_parent_name_rec *rec, void *value,
- int *valuelen, const struct xfs_parent_name_irec *irec);
+void xfs_parent_irec_to_disk(struct xfs_parent_name_rec *rec, int *reclen,
+ void *value, int *valuelen,
+ const struct xfs_parent_name_irec *irec);
/*
* Dynamically allocd structure used to wrap the needed data to pass around
diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index 2494947a0c93..87cff40b15f1 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -144,15 +144,19 @@ xchk_dir_parent_pointer(
{
struct xfs_scrub *sc = sd->sc;
int pptr_namelen;
- int error;
+ int hashlen;
sd->pptr.p_ino = sc->ip->i_ino;
sd->pptr.p_gen = VFS_I(sc->ip)->i_generation;
- error = xfs_parent_namehash(ip, name, &sd->pptr.p_namehash,
+ hashlen = xfs_parent_namehash(ip, name, &sd->pptr.p_namehash,
sizeof(sd->pptr.p_namehash));
- if (error)
- return error;
+ if (hashlen < 0) {
+ xchk_fblock_xref_process_error(sc, XFS_DATA_FORK, 0,
+ &hashlen);
+ return hashlen;
+ }
+ sd->pptr.hashlen = hashlen;
pptr_namelen = xfs_parent_lookup(sc->tp, ip, &sd->pptr, sd->namebuf,
MAXNAMELEN, &sd->pptr_scratch);
diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index c0b2b78da277..b12548787321 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -540,7 +540,7 @@ xrep_dir_scan_parent_pointer(
!xfs_parent_valuecheck(sc->mp, value, valuelen))
return -EFSCORRUPTED;
- xfs_parent_irec_from_disk(&rd->pptr, rec, value, valuelen);
+ xfs_parent_irec_from_disk(&rd->pptr, rec, namelen, value, valuelen);
/* Ignore parent pointers that point back to a different dir. */
if (rd->pptr.p_ino != sc->ip->i_ino ||
diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c
index 53872a7be942..b47f0bcef690 100644
--- a/fs/xfs/scrub/parent.c
+++ b/fs/xfs/scrub/parent.c
@@ -531,6 +531,7 @@ xchk_parent_scan_attr(
struct xfs_inode *dp = NULL;
const struct xfs_parent_name_rec *rec = (const void *)name;
unsigned int lockmode;
+ int hashlen;
int error;
/* Ignore incomplete xattrs */
@@ -552,7 +553,7 @@ xchk_parent_scan_attr(
return -ECANCELED;
}
- xfs_parent_irec_from_disk(&pp->pptr, rec, value, valuelen);
+ xfs_parent_irec_from_disk(&pp->pptr, rec, namelen, value, valuelen);
xname.name = pp->pptr.p_name;
xname.len = pp->pptr.p_namelen;
@@ -561,13 +562,16 @@ xchk_parent_scan_attr(
* Does the namehash in the parent pointer match the actual name?
* If not, there's no point in checking further.
*/
- error = xfs_parent_namehash(sc->ip, &xname, pp->child_namehash,
+ hashlen = xfs_parent_namehash(sc->ip, &xname, pp->child_namehash,
sizeof(pp->child_namehash));
- if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, &error))
- return error;
+ if (hashlen < 0) {
+ xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, &hashlen);
+ return hashlen;
+ }
- if (memcmp(pp->pptr.p_namehash, pp->child_namehash,
- sizeof(pp->pptr.p_namehash))) {
+ if (hashlen != pp->pptr.hashlen ||
+ memcmp(pp->pptr.p_namehash, pp->child_namehash,
+ pp->pptr.hashlen)) {
trace_xchk_parent_bad_namehash(sc->ip, pp->pptr.p_ino,
xname.name, xname.len);
xchk_fblock_xref_set_corrupt(sc, XFS_ATTR_FORK, 0);
diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c
index 51432ab61c94..7d3b9c82bd05 100644
--- a/fs/xfs/scrub/parent_repair.c
+++ b/fs/xfs/scrub/parent_repair.c
@@ -522,7 +522,7 @@ xrep_pptr_dump_tempptr(
else
return -EFSCORRUPTED;
- xfs_parent_irec_from_disk(&rp->pptr, rec, value, valuelen);
+ xfs_parent_irec_from_disk(&rp->pptr, rec, namelen, value, valuelen);
trace_xrep_pptr_dumpname(sc->tempip, &rp->pptr);
diff --git a/fs/xfs/xfs_parent_utils.c b/fs/xfs/xfs_parent_utils.c
index 65bec3875308..284ca3c14a0f 100644
--- a/fs/xfs/xfs_parent_utils.c
+++ b/fs/xfs/xfs_parent_utils.c
@@ -74,7 +74,7 @@ xfs_getparent_listent(
return;
}
- xfs_parent_irec_from_disk(&gp->pptr_irec, (void *)name, value,
+ xfs_parent_irec_from_disk(&gp->pptr_irec, (void *)name, namelen, value,
valuelen);
trace_xfs_getparent_listent(context->dp, ppi, irec);
