From 1ef7729df1f0c5f7bb63a121164f54d376d35835 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Fri, 23 Jan 2026 09:27:34 -0800 Subject: [PATCH 1/3] xfs: reduce xfs_attr_try_sf_addname parameters The dp parameter to this function is an alias of args->dp, so remove it for clarity before we go adding new callers. Signed-off-by: "Darrick J. Wong" Reviewed-by: Christoph Hellwig --- fs/xfs/libxfs/xfs_attr.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 9e6b18d6ae00..9a5402d1e9bf 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -350,16 +350,14 @@ xfs_attr_set_resv( */ STATIC int xfs_attr_try_sf_addname( - struct xfs_inode *dp, struct xfs_da_args *args) { - int error; /* * Build initial attribute list (if required). */ - if (dp->i_af.if_format == XFS_DINODE_FMT_EXTENTS) + if (args->dp->i_af.if_format == XFS_DINODE_FMT_EXTENTS) xfs_attr_shortform_create(args); error = xfs_attr_shortform_addname(args); @@ -371,9 +369,9 @@ xfs_attr_try_sf_addname( * NOTE: this is also the error path (EEXIST, etc). */ if (!error) - xfs_trans_ichgtime(args->trans, dp, XFS_ICHGTIME_CHG); + xfs_trans_ichgtime(args->trans, args->dp, XFS_ICHGTIME_CHG); - if (xfs_has_wsync(dp->i_mount)) + if (xfs_has_wsync(args->dp->i_mount)) xfs_trans_set_sync(args->trans); return error; @@ -384,10 +382,9 @@ xfs_attr_sf_addname( struct xfs_attr_intent *attr) { struct xfs_da_args *args = attr->xattri_da_args; - struct xfs_inode *dp = args->dp; int error = 0; - error = xfs_attr_try_sf_addname(dp, args); + error = xfs_attr_try_sf_addname(args); if (error != -ENOSPC) { ASSERT(!error || error == -EEXIST); attr->xattri_dela_state = XFS_DAS_DONE; From d693534513d8dcdaafcf855986d0fe0476a47462 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Fri, 23 Jan 2026 09:27:35 -0800 Subject: [PATCH 2/3] xfs: speed up parent pointer operations when possible After a recent fsmark benchmarking run, I observed that the overhead of parent pointers on file creation and deletion can be a bit high. On a machine with 20 CPUs, 128G of memory, and an NVME SSD capable of pushing 750000iops, I see the following results: $ mkfs.xfs -f -l logdev=/dev/nvme1n1,size=1g /dev/nvme0n1 -n parent=0 meta-data=/dev/nvme0n1 isize=512 agcount=40, agsize=9767586 blks = sectsz=4096 attr=2, projid32bit=1 = crc=1 finobt=1, sparse=1, rmapbt=1 = reflink=1 bigtime=1 inobtcount=1 nrext64=1 = exchange=0 metadir=0 data = bsize=4096 blocks=390703440, imaxpct=5 = sunit=0 swidth=0 blks naming =version 2 bsize=4096 ascii-ci=0, ftype=1, parent=0 log =/dev/nvme1n1 bsize=4096 blocks=262144, version=2 = sectsz=4096 sunit=1 blks, lazy-count=1 realtime =none extsz=4096 blocks=0, rtextents=0 = rgcount=0 rgsize=0 extents = zoned=0 start=0 reserved=0 So we created 40 AGs, one per CPU. Now we create 40 directories and run fsmark: $ time fs_mark -D 10000 -S 0 -n 100000 -s 0 -L 8 -d ... # Version 3.3, 40 thread(s) starting at Wed Dec 10 14:22:07 2025 # Sync method: NO SYNC: Test does not issue sync() or fsync() calls. # Directories: Time based hash between directories across 10000 subdirectories with 180 seconds per subdirectory. # File names: 40 bytes long, (16 initial bytes of time stamp with 24 random bytes at end of name) # Files info: size 0 bytes, written with an IO size of 16384 bytes per write # App overhead is time in microseconds spent in the test not doing file writing related system calls. parent=0 parent=1 ================== ================== real 0m57.573s real 1m2.934s user 3m53.578s user 3m53.508s sys 19m44.440s sys 25m14.810s $ time rm -rf ... parent=0 parent=1 ================== ================== real 0m59.649s real 1m12.505s user 0m41.196s user 0m47.489s sys 13m9.566s sys 20m33.844s Parent pointers increase the system time by 28% overhead to create 32 million files that are totally empty. Removing them incurs a system time increase of 56%. Wall time increases by 9% and 22%. For most filesystems, each file tends to have a single owner and not that many xattrs. If the xattr structure is shortform, then all xattr changes are logged with the inode and do not require the the xattr intent mechanism to persist the parent pointer. Therefore, we can speed up parent pointer operations by calling the shortform xattr functions directly if the child's xattr is in short format. Now the overhead looks like: $ time fs_mark -D 10000 -S 0 -n 100000 -s 0 -L 8 -d ... parent=0 parent=1 ================== ================== real 0m58.030s real 1m0.983s user 3m54.141s user 3m53.758s sys 19m57.003s sys 21m30.605s $ time rm -rf ... parent=0 parent=1 ================== ================== real 0m58.911s real 1m4.420s user 0m41.329s user 0m45.169s sys 13m27.857s sys 15m58.564s Now parent pointers only increase the system time by 8% for creation and 19% for deletion. Wall time increases by 5% and 9% now. Close the performance gap by creating helpers for the attr set, remove, and replace operations that will try to make direct shortform updates, and fall back to the attr intent machinery if that doesn't work. This works for regular xattrs and for parent pointers. Signed-off-by: "Darrick J. Wong" Reviewed-by: Christoph Hellwig --- fs/xfs/libxfs/xfs_attr.c | 99 ++++++++++++++++++++++++++++++++++++-- fs/xfs/libxfs/xfs_attr.h | 6 ++- fs/xfs/libxfs/xfs_parent.c | 14 +++--- 3 files changed, 109 insertions(+), 10 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 9a5402d1e9bf..54be75edb2eb 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -1028,6 +1028,91 @@ xfs_attr_add_fork( return error; } +/* + * Decide if it is theoretically possible to try to bypass the attr intent + * mechanism for better performance. Other constraints (e.g. available space + * in the existing structure) are not considered here. + */ +static inline bool +xfs_attr_can_shortcut( + const struct xfs_inode *ip) +{ + return xfs_inode_has_attr_fork(ip) && xfs_attr_is_shortform(ip); +} + +/* Try to set an attr in one transaction or fall back to attr intents. */ +int +xfs_attr_setname( + struct xfs_da_args *args, + int rmt_blks) +{ + int error; + + if (!rmt_blks && xfs_attr_can_shortcut(args->dp)) { + args->op_flags |= XFS_DA_OP_ADDNAME; + + error = xfs_attr_try_sf_addname(args); + if (error != -ENOSPC) + return error; + } + + xfs_attr_defer_add(args, XFS_ATTR_DEFER_SET); + return 0; +} + +/* Try to remove an attr in one transaction or fall back to attr intents. */ +int +xfs_attr_removename( + struct xfs_da_args *args) +{ + if (xfs_attr_can_shortcut(args->dp)) + return xfs_attr_sf_removename(args); + + xfs_attr_defer_add(args, XFS_ATTR_DEFER_REMOVE); + return 0; +} + +/* Try to replace an attr in one transaction or fall back to attr intents. */ +int +xfs_attr_replacename( + struct xfs_da_args *args, + int rmt_blks) +{ + int error; + + if (rmt_blks || !xfs_attr_can_shortcut(args->dp)) { + xfs_attr_defer_add(args, XFS_ATTR_DEFER_REPLACE); + return 0; + } + + args->op_flags |= XFS_DA_OP_ADDNAME | XFS_DA_OP_REPLACE; + + error = xfs_attr_sf_removename(args); + if (error) + return error; + + if (args->attr_filter & XFS_ATTR_PARENT) { + /* + * Move the new name/value to the regular name/value slots and + * zero out the new name/value slots because we don't need to + * log them for a PPTR_SET operation. + */ + xfs_attr_update_pptr_replace_args(args); + args->new_name = NULL; + args->new_namelen = 0; + args->new_value = NULL; + args->new_valuelen = 0; + } + args->op_flags &= ~XFS_DA_OP_REPLACE; + + error = xfs_attr_try_sf_addname(args); + if (error != -ENOSPC) + return error; + + xfs_attr_defer_add(args, XFS_ATTR_DEFER_SET); + return 0; +} + /* * Make a change to the xattr structure. * @@ -1108,14 +1193,19 @@ xfs_attr_set( case -EEXIST: if (op == XFS_ATTRUPDATE_REMOVE) { /* if no value, we are performing a remove operation */ - xfs_attr_defer_add(args, XFS_ATTR_DEFER_REMOVE); + error = xfs_attr_removename(args); + if (error) + goto out_trans_cancel; break; } /* Pure create fails if the attr already exists */ if (op == XFS_ATTRUPDATE_CREATE) goto out_trans_cancel; - xfs_attr_defer_add(args, XFS_ATTR_DEFER_REPLACE); + + error = xfs_attr_replacename(args, rmt_blks); + if (error) + goto out_trans_cancel; break; case -ENOATTR: /* Can't remove what isn't there. */ @@ -1125,7 +1215,10 @@ xfs_attr_set( /* Pure replace fails if no existing attr to replace. */ if (op == XFS_ATTRUPDATE_REPLACE) goto out_trans_cancel; - xfs_attr_defer_add(args, XFS_ATTR_DEFER_SET); + + error = xfs_attr_setname(args, rmt_blks); + if (error) + goto out_trans_cancel; break; default: goto out_trans_cancel; diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h index 0e51d0723f9a..8244305949de 100644 --- a/fs/xfs/libxfs/xfs_attr.h +++ b/fs/xfs/libxfs/xfs_attr.h @@ -573,7 +573,7 @@ struct xfs_trans_res xfs_attr_set_resv(const struct xfs_da_args *args); */ static inline bool xfs_attr_is_shortform( - struct xfs_inode *ip) + const struct xfs_inode *ip) { return ip->i_af.if_format == XFS_DINODE_FMT_LOCAL || (ip->i_af.if_format == XFS_DINODE_FMT_EXTENTS && @@ -649,4 +649,8 @@ void xfs_attr_intent_destroy_cache(void); int xfs_attr_sf_totsize(struct xfs_inode *dp); int xfs_attr_add_fork(struct xfs_inode *ip, int size, int rsvd); +int xfs_attr_setname(struct xfs_da_args *args, int rmt_blks); +int xfs_attr_removename(struct xfs_da_args *args); +int xfs_attr_replacename(struct xfs_da_args *args, int rmt_blks); + #endif /* __XFS_ATTR_H__ */ diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c index 6539f5adae2d..3509cc4b2175 100644 --- a/fs/xfs/libxfs/xfs_parent.c +++ b/fs/xfs/libxfs/xfs_parent.c @@ -29,6 +29,7 @@ #include "xfs_trans_space.h" #include "xfs_attr_item.h" #include "xfs_health.h" +#include "xfs_attr_leaf.h" struct kmem_cache *xfs_parent_args_cache; @@ -202,8 +203,8 @@ xfs_parent_addname( xfs_inode_to_parent_rec(&ppargs->rec, dp); xfs_parent_da_args_init(&ppargs->args, tp, &ppargs->rec, child, child->i_ino, parent_name); - xfs_attr_defer_add(&ppargs->args, XFS_ATTR_DEFER_SET); - return 0; + + return xfs_attr_setname(&ppargs->args, 0); } /* Remove a parent pointer to reflect a dirent removal. */ @@ -224,8 +225,8 @@ xfs_parent_removename( xfs_inode_to_parent_rec(&ppargs->rec, dp); xfs_parent_da_args_init(&ppargs->args, tp, &ppargs->rec, child, child->i_ino, parent_name); - xfs_attr_defer_add(&ppargs->args, XFS_ATTR_DEFER_REMOVE); - return 0; + + return xfs_attr_removename(&ppargs->args); } /* Replace one parent pointer with another to reflect a rename. */ @@ -250,12 +251,13 @@ xfs_parent_replacename( child->i_ino, old_name); xfs_inode_to_parent_rec(&ppargs->new_rec, new_dp); + ppargs->args.new_name = new_name->name; ppargs->args.new_namelen = new_name->len; ppargs->args.new_value = &ppargs->new_rec; ppargs->args.new_valuelen = sizeof(struct xfs_parent_rec); - xfs_attr_defer_add(&ppargs->args, XFS_ATTR_DEFER_REPLACE); - return 0; + + return xfs_attr_replacename(&ppargs->args, 0); } /* From eaec8aeff31d0679eadb27a13a62942ddbfd7b87 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Fri, 23 Jan 2026 09:27:36 -0800 Subject: [PATCH 3/3] xfs: add a method to replace shortform attrs If we're trying to replace an xattr in a shortform attr structure and the old entry fits the new entry, we can just memcpy and exit without having to delete, compact, and re-add the entry (or worse use the attr intent machinery). For parent pointers this only advantages renaming where the filename length stays the same (e.g. mv autoexec.bat scandisk.exe) but for regular xattrs it might be useful for updating security labels and the like. Signed-off-by: "Darrick J. Wong" Reviewed-by: Christoph Hellwig --- fs/xfs/libxfs/xfs_attr.c | 4 ++++ fs/xfs/libxfs/xfs_attr_leaf.c | 38 +++++++++++++++++++++++++++++++++++ fs/xfs/libxfs/xfs_attr_leaf.h | 1 + fs/xfs/xfs_trace.h | 1 + 4 files changed, 44 insertions(+) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 54be75edb2eb..93caa1dae501 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -1085,6 +1085,10 @@ xfs_attr_replacename( return 0; } + error = xfs_attr_shortform_replace(args); + if (error != -ENOSPC) + return error; + args->op_flags |= XFS_DA_OP_ADDNAME | XFS_DA_OP_REPLACE; error = xfs_attr_sf_removename(args); diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c index c3327b10709c..47f48ae555c0 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.c +++ b/fs/xfs/libxfs/xfs_attr_leaf.c @@ -842,6 +842,44 @@ xfs_attr_sf_findname( return NULL; } +/* + * Replace a shortform xattr if it's the right length. Returns 0 on success, + * -ENOSPC if the length is wrong, or -ENOATTR if the attr was not found. + */ +int +xfs_attr_shortform_replace( + struct xfs_da_args *args) +{ + struct xfs_attr_sf_entry *sfe; + + ASSERT(args->dp->i_af.if_format == XFS_DINODE_FMT_LOCAL); + + trace_xfs_attr_sf_replace(args); + + sfe = xfs_attr_sf_findname(args); + if (!sfe) + return -ENOATTR; + + if (args->attr_filter & XFS_ATTR_PARENT) { + if (sfe->namelen != args->new_namelen || + sfe->valuelen != args->new_valuelen) + return -ENOSPC; + + memcpy(sfe->nameval, args->new_name, sfe->namelen); + memcpy(&sfe->nameval[sfe->namelen], args->new_value, + sfe->valuelen); + } else { + if (sfe->valuelen != args->valuelen) + return -ENOSPC; + memcpy(&sfe->nameval[sfe->namelen], args->value, + sfe->valuelen); + } + + xfs_trans_log_inode(args->trans, args->dp, + XFS_ILOG_CORE | XFS_ILOG_ADATA); + return 0; +} + /* * Add a name/value pair to the shortform attribute list. * Overflow from the inode has already been checked for. diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h index 589f810eedc0..aca46da2bc50 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.h +++ b/fs/xfs/libxfs/xfs_attr_leaf.h @@ -46,6 +46,7 @@ struct xfs_attr3_icleaf_hdr { * Internal routines when attribute fork size < XFS_LITINO(mp). */ void xfs_attr_shortform_create(struct xfs_da_args *args); +int xfs_attr_shortform_replace(struct xfs_da_args *args); void xfs_attr_shortform_add(struct xfs_da_args *args, int forkoff); int xfs_attr_shortform_getvalue(struct xfs_da_args *args); int xfs_attr_shortform_to_leaf(struct xfs_da_args *args); diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index f70afbf3cb19..a8bea99e0024 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -2410,6 +2410,7 @@ DEFINE_ATTR_EVENT(xfs_attr_sf_addname); DEFINE_ATTR_EVENT(xfs_attr_sf_create); DEFINE_ATTR_EVENT(xfs_attr_sf_lookup); DEFINE_ATTR_EVENT(xfs_attr_sf_remove); +DEFINE_ATTR_EVENT(xfs_attr_sf_replace); DEFINE_ATTR_EVENT(xfs_attr_sf_to_leaf); DEFINE_ATTR_EVENT(xfs_attr_leaf_add);