xfs: separate the icreate logic around INIT_XATTRS
authorDarrick J. Wong <djwong@kernel.org>
Wed, 3 Jul 2024 21:21:35 +0000 (14:21 -0700)
committerDarrick J. Wong <djwong@kernel.org>
Wed, 31 Jul 2024 01:46:42 +0000 (18:46 -0700)
INIT_XATTRS is overloaded here -- it's set during the creat process when
we think that we're immediately going to set some ACL xattrs to save
time.  However, it's also used by the parent pointers code to enable the
attr fork in preparation to receive ppptr xattrs.  This results in
xfs_has_parent() branches scattered around the codebase to turn on
INIT_XATTRS.

Linkable files are created far more commonly than unlinkable temporary
files or directory tree roots, so we should centralize this logic in
xfs_inode_init.  For the three callers that don't want parent pointers
(online repiar tempfiles, unlinkable tempfiles, rootdir creation) we
provide an UNLINKABLE flag to skip attr fork initialization.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
db/iunlink.c
libxfs/xfs_inode_util.c
libxfs/xfs_inode_util.h
mkfs/proto.c
repair/phase6.c

index 47e14ed24d744dd5323d7704c19a571c3ce88a42..b3356d73b346d611dc63dce8fd9547dd2d1d98e9 100644 (file)
@@ -312,7 +312,7 @@ create_unlinked(
        struct xfs_icreate_args args = {
                .idmap          = libxfs_nop_idmap,
                .mode           = S_IFREG | 0600,
-               .flags          = XFS_ICREATE_TMPFILE,
+               .flags          = XFS_ICREATE_TMPFILE | XFS_ICREATE_UNLINKABLE,
        };
        struct xfs_inode        *ip;
        struct xfs_trans        *tp;
index 62af002b2140dc59784e6faa32e6add256ef83fa..13c32d114cb9a81144792a1f514c0ac620f6d82e 100644 (file)
@@ -231,6 +231,31 @@ xfs_inode_inherit_flags2(
        }
 }
 
+/*
+ * If we need to create attributes immediately after allocating the inode,
+ * initialise an empty attribute fork right now. We use the default fork offset
+ * for attributes here as we don't know exactly what size or how many
+ * attributes we might be adding. We can do this safely here because we know
+ * the data fork is completely empty and this saves us from needing to run a
+ * separate transaction to set the fork offset in the immediate future.
+ *
+ * If we have parent pointers and the caller hasn't told us that the file will
+ * never be linked into a directory tree, we /must/ create the attr fork.
+ */
+static inline bool
+xfs_icreate_want_attrfork(
+       struct xfs_mount                *mp,
+       const struct xfs_icreate_args   *args)
+{
+       if (args->flags & XFS_ICREATE_INIT_XATTRS)
+               return true;
+
+       if (!(args->flags & XFS_ICREATE_UNLINKABLE) && xfs_has_parent(mp))
+               return true;
+
+       return false;
+}
+
 /* Initialise an inode's attributes. */
 void
 xfs_inode_init(
@@ -323,16 +348,7 @@ xfs_inode_init(
                ASSERT(0);
        }
 
-       /*
-        * If we need to create attributes immediately after allocating the
-        * inode, initialise an empty attribute fork right now. We use the
-        * default fork offset for attributes here as we don't know exactly what
-        * size or how many attributes we might be adding. We can do this
-        * safely here because we know the data fork is completely empty and
-        * this saves us from needing to run a separate transaction to set the
-        * fork offset in the immediate future.
-        */
-       if (args->flags & XFS_ICREATE_INIT_XATTRS) {
+       if (xfs_icreate_want_attrfork(mp, args)) {
                ip->i_forkoff = xfs_default_attroffset(ip) >> 3;
                xfs_ifork_init_attr(ip, XFS_DINODE_FMT_EXTENTS, 0);
 
index 50c14ba6ca5a2c7b7988a4ea00726f208f9f87df..1c54c3b0cf2626ed74884a8569eb5d9caa9bd500 100644 (file)
@@ -32,6 +32,7 @@ struct xfs_icreate_args {
 
 #define XFS_ICREATE_TMPFILE    (1U << 0)  /* create an unlinked file */
 #define XFS_ICREATE_INIT_XATTRS        (1U << 1)  /* will set xattrs immediately */
+#define XFS_ICREATE_UNLINKABLE (1U << 2)  /* cannot link into dir tree */
        uint16_t                flags;
 };
 
index 96cb9f8544bf853adf5228cc736992750fcf1c34..251e3a9ee019e98d00505500f9b827f0875bd45d 100644 (file)
@@ -432,8 +432,9 @@ creatproto(
        xfs_ino_t               ino;
        int                     error;
 
-       if (dp && xfs_has_parent(dp->i_mount))
-               args.flags |= XFS_ICREATE_INIT_XATTRS;
+       /* Root directories cannot be linked to a parent. */
+       if (!dp)
+               args.flags |= XFS_ICREATE_UNLINKABLE;
 
        /*
         * Call the space management code to pick the on-disk inode to be
index 52e42d4c0bc8893619f70fa92540cf704c33eedd..85f122ec79ae5f9e74a1a9ae919c44cb545f8787 100644 (file)
@@ -909,9 +909,6 @@ mk_orphanage(
        struct xfs_name         xname;
        struct xfs_parent_args  *ppargs = NULL;
 
-       if (xfs_has_parent(mp))
-               args.flags |= XFS_ICREATE_INIT_XATTRS;
-
        i = -libxfs_parent_start(mp, &ppargs);
        if (i)
                do_error(_("%d - couldn't allocate parent pointer for %s\n"),