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>
---
db/attr.c | 8 +++--
db/attrshort.c | 2 +
libxfs/xfs_da_format.h | 21 +++++++++++-
libxfs/xfs_parent.c | 85 ++++++++++++++++++++++++++++++++----------------
libxfs/xfs_parent.h | 8 +++--
logprint/log_redo.c | 5 ++-
repair/pptr.c | 10 +++---
7 files changed, 96 insertions(+), 43 deletions(-)
diff --git a/db/attr.c b/db/attr.c
index bacdc6d9..798a7e1a 100644
--- a/db/attr.c
+++ b/db/attr.c
@@ -309,8 +309,12 @@ __attr_leaf_name_pptr_namehashlen(
struct xfs_attr_leaf_entry *e,
int i)
{
- if (e->flags & XFS_ATTR_PARENT)
- return XFS_PARENT_NAME_HASH_SIZE;
+ struct xfs_attr_leaf_name_local *lname;
+
+ if (e->flags & XFS_ATTR_PARENT) {
+ lname = xfs_attr3_leaf_name_local(leaf, i);
+ return xfs_parent_name_hashlen(lname->namelen);
+ }
return 0;
}
diff --git a/db/attrshort.c b/db/attrshort.c
index be15f4ee..2fcf44c1 100644
--- a/db/attrshort.c
+++ b/db/attrshort.c
@@ -88,7 +88,7 @@ attr_sf_entry_pptr_namehashlen(
ASSERT(bitoffs(startoff) == 0);
e = (struct xfs_attr_sf_entry *)((char *)obj + byteize(startoff));
if (e->flags & XFS_ATTR_PARENT)
- return XFS_PARENT_NAME_HASH_SIZE;
+ return xfs_parent_name_hashlen(e->namelen);
return 0;
}
diff --git a/libxfs/xfs_da_format.h b/libxfs/xfs_da_format.h
index 386f63b2..27535750 100644
--- a/libxfs/xfs_da_format.h
+++ b/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/libxfs/xfs_parent.c b/libxfs/xfs_parent.c
index 05c1e032..064f2f40 100644
--- a/libxfs/xfs_parent.c
+++ b/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/libxfs/xfs_parent.h b/libxfs/xfs_parent.h
index d3f2841e..4c310076 100644
--- a/libxfs/xfs_parent.h
+++ b/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/logprint/log_redo.c b/logprint/log_redo.c
index ca6b2641..339d4815 100644
--- a/logprint/log_redo.c
+++ b/logprint/log_redo.c
@@ -709,11 +709,12 @@ dump_pptr(
{
struct xfs_parent_name_irec irec;
- libxfs_parent_irec_from_disk(&irec, name, value, valuelen);
+ libxfs_parent_irec_from_disk(&irec, name, namelen, value, valuelen);
printf("PPTR: %s attr_namelen %u value_namelen %u\n", tag, namelen, valuelen);
- printf("PPTR: %s parent_ino %llu parent_gen %u namelen %u name '%.*s'\n",
+ printf("PPTR: %s parent_ino %llu parent_gen %u hashlen %u namelen %u name '%.*s'\n",
tag, (unsigned long long)irec.p_ino, irec.p_gen,
+ irec.hashlen,
irec.p_namelen, irec.p_namelen, irec.p_name);
}
diff --git a/repair/pptr.c b/repair/pptr.c
index ca5fe7e3..12382ad7 100644
--- a/repair/pptr.c
+++ b/repair/pptr.c
@@ -515,6 +515,7 @@ examine_xattr(
struct xfs_mount *mp = ip->i_mount;
struct file_scan *fscan = priv;
const struct xfs_parent_name_rec *rec = (const void *)name;
+ int hashlen;
int error;
/* Ignore anything that isn't a parent pointer. */
@@ -530,7 +531,7 @@ examine_xattr(
!xfs_parent_valuecheck(mp, value, valuelen))
goto corrupt;
- libxfs_parent_irec_from_disk(&irec, rec, value, valuelen);
+ libxfs_parent_irec_from_disk(&irec, rec, namelen, value, valuelen);
file_pptr.parent_ino = irec.p_ino;
file_pptr.parent_gen = irec.p_gen;
@@ -543,12 +544,13 @@ examine_xattr(
* Does the namehash in the attr key match the name in the attr value?
* If not, there's no point in checking further.
*/
- error = -libxfs_parent_namehash(ip, &xname, namehash,
+ hashlen = libxfs_parent_namehash(ip, &xname, namehash,
sizeof(namehash));
- if (error)
+ if (hashlen < 0)
goto corrupt;
- if (memcmp(irec.p_namehash, namehash, sizeof(irec.p_namehash)))
+ if (namelen != xfs_parent_name_rec_sizeof(hashlen) ||
+ memcmp(irec.p_namehash, namehash, hashlen))
goto corrupt;
error = store_file_pptr_name(fscan, &file_pptr, &irec);
