All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/22] xfs: hoist inode operations to libxfs
@ 2019-01-01  2:19 Darrick J. Wong
  2019-01-01  2:19 ` [PATCH 01/22] xfs: hoist extent size helpers " Darrick J. Wong
                   ` (21 more replies)
  0 siblings, 22 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:19 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

Hi all,

This series hoists inode creation, renaming, and deletion operations to
libxfs in anticipation of the metadata inode directory feature, which
maintains a directory tree of metadata inodes.  This will be necessary
for further enhancements to the realtime feature, subvolume support.

There aren't supposed to be any functional changes in this intense
refactoring -- we just split the functions into pieces that are generic
and pieces that are specific to libxfs clients.  As a bonus, we can
remove various open-coded pieces of mkfs.xfs and xfs_repair when this
series gets to xfsprogs.

If you're going to start using this mess, you probably ought to just
pull from my git trees.  The kernel patches[1] should apply against
4.20.  xfsprogs[2] and xfstests[3] can be found in their usual
places.  The git trees contain all four series' worth of changes.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

[1] https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=djwong-devel
[2] https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=djwong-devel
[3] https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=djwong-devel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 01/22] xfs: hoist extent size helpers to libxfs
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
@ 2019-01-01  2:19 ` Darrick J. Wong
  2019-01-17 14:18   ` Christoph Hellwig
  2019-01-01  2:19 ` [PATCH 02/22] xfs: hoist inode flag conversion functions Darrick J. Wong
                   ` (20 subsequent siblings)
  21 siblings, 1 reply; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:19 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a new xfs_inode_utils.c file in libxfs and hoist the extent size
helper functions to libxfs, since they pertain directly to inode
functionality.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/Makefile                |    1 +
 fs/xfs/libxfs/xfs_bmap.c       |    2 +-
 fs/xfs/libxfs/xfs_inode_util.c |   52 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_inode_util.h |   12 +++++++++
 fs/xfs/xfs_inode.c             |   37 ----------------------------
 fs/xfs/xfs_inode.h             |    4 +--
 6 files changed, 67 insertions(+), 41 deletions(-)
 create mode 100644 fs/xfs/libxfs/xfs_inode_util.c
 create mode 100644 fs/xfs/libxfs/xfs_inode_util.h


diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
index fd658fce9a63..ab373aeb0c37 100644
--- a/fs/xfs/Makefile
+++ b/fs/xfs/Makefile
@@ -48,6 +48,7 @@ xfs-y				+= $(addprefix libxfs/, \
 				   xfs_iext_tree.o \
 				   xfs_inode_fork.o \
 				   xfs_inode_buf.o \
+				   xfs_inode_util.o \
 				   xfs_log_rlimit.o \
 				   xfs_ag_resv.o \
 				   xfs_rmap.o \
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 6471fdd843a1..e151685b57d3 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -39,7 +39,7 @@
 #include "xfs_ag_resv.h"
 #include "xfs_refcount.h"
 #include "xfs_icache.h"
-
+#include "xfs_inode_util.h"
 
 kmem_zone_t		*xfs_bmap_free_item_zone;
 
diff --git a/fs/xfs/libxfs/xfs_inode_util.c b/fs/xfs/libxfs/xfs_inode_util.c
new file mode 100644
index 000000000000..f9fc2e60c91c
--- /dev/null
+++ b/fs/xfs/libxfs/xfs_inode_util.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000-2006 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ */
+#include "xfs.h"
+#include "xfs_fs.h"
+#include "xfs_shared.h"
+#include "xfs_format.h"
+#include "xfs_log_format.h"
+#include "xfs_trans_resv.h"
+#include "xfs_sb.h"
+#include "xfs_mount.h"
+#include "xfs_inode.h"
+#include "xfs_inode_util.h"
+
+/*
+ * helper function to extract extent size hint from inode
+ */
+xfs_extlen_t
+xfs_get_extsz_hint(
+	struct xfs_inode	*ip)
+{
+	if ((ip->i_d.di_flags & XFS_DIFLAG_EXTSIZE) && ip->i_d.di_extsize)
+		return ip->i_d.di_extsize;
+	if (XFS_IS_REALTIME_INODE(ip))
+		return ip->i_mount->m_sb.sb_rextsize;
+	return 0;
+}
+
+/*
+ * Helper function to extract CoW extent size hint from inode.
+ * Between the extent size hint and the CoW extent size hint, we
+ * return the greater of the two.  If the value is zero (automatic),
+ * use the default size.
+ */
+xfs_extlen_t
+xfs_get_cowextsz_hint(
+	struct xfs_inode	*ip)
+{
+	xfs_extlen_t		a, b;
+
+	a = 0;
+	if (ip->i_d.di_flags2 & XFS_DIFLAG2_COWEXTSIZE)
+		a = ip->i_d.di_cowextsize;
+	b = xfs_get_extsz_hint(ip);
+
+	a = max(a, b);
+	if (a == 0)
+		return XFS_DEFAULT_COWEXTSZ_HINT;
+	return a;
+}
diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
new file mode 100644
index 000000000000..98bae709dee8
--- /dev/null
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000-2003,2005 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ */
+#ifndef	__XFS_INODE_UTIL_H__
+#define	__XFS_INODE_UTIL_H__
+
+xfs_extlen_t	xfs_get_extsz_hint(struct xfs_inode *ip);
+xfs_extlen_t	xfs_get_cowextsz_hint(struct xfs_inode *ip);
+
+#endif /* __XFS_INODE_UTIL_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 2c7f8aeffa92..9774f1ecdd35 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -50,43 +50,6 @@ STATIC int xfs_iflush_int(struct xfs_inode *, struct xfs_buf *);
 STATIC int xfs_iunlink(struct xfs_trans *, struct xfs_inode *);
 STATIC int xfs_iunlink_remove(struct xfs_trans *, struct xfs_inode *);
 
-/*
- * helper function to extract extent size hint from inode
- */
-xfs_extlen_t
-xfs_get_extsz_hint(
-	struct xfs_inode	*ip)
-{
-	if ((ip->i_d.di_flags & XFS_DIFLAG_EXTSIZE) && ip->i_d.di_extsize)
-		return ip->i_d.di_extsize;
-	if (XFS_IS_REALTIME_INODE(ip))
-		return ip->i_mount->m_sb.sb_rextsize;
-	return 0;
-}
-
-/*
- * Helper function to extract CoW extent size hint from inode.
- * Between the extent size hint and the CoW extent size hint, we
- * return the greater of the two.  If the value is zero (automatic),
- * use the default size.
- */
-xfs_extlen_t
-xfs_get_cowextsz_hint(
-	struct xfs_inode	*ip)
-{
-	xfs_extlen_t		a, b;
-
-	a = 0;
-	if (ip->i_d.di_flags2 & XFS_DIFLAG2_COWEXTSIZE)
-		a = ip->i_d.di_cowextsize;
-	b = xfs_get_extsz_hint(ip);
-
-	a = max(a, b);
-	if (a == 0)
-		return XFS_DEFAULT_COWEXTSZ_HINT;
-	return a;
-}
-
 /*
  * These two are wrapper routines around the xfs_ilock() routine used to
  * centralize some grungy code.  They are used in places that wish to lock the
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index e95e30a94243..29aedfdf3d05 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -8,6 +8,7 @@
 
 #include "xfs_inode_buf.h"
 #include "xfs_inode_fork.h"
+#include "xfs_inode_util.h"
 
 /*
  * Kernel only inode definitions
@@ -446,9 +447,6 @@ int		xfs_iflush(struct xfs_inode *, struct xfs_buf **);
 void		xfs_lock_two_inodes(struct xfs_inode *ip0, uint ip0_mode,
 				struct xfs_inode *ip1, uint ip1_mode);
 
-xfs_extlen_t	xfs_get_extsz_hint(struct xfs_inode *ip);
-xfs_extlen_t	xfs_get_cowextsz_hint(struct xfs_inode *ip);
-
 int		xfs_dir_ialloc(struct xfs_trans **, struct xfs_inode *, umode_t,
 			       xfs_nlink_t, dev_t, prid_t,
 			       struct xfs_inode **);

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 02/22] xfs: hoist inode flag conversion functions
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
  2019-01-01  2:19 ` [PATCH 01/22] xfs: hoist extent size helpers " Darrick J. Wong
@ 2019-01-01  2:19 ` Darrick J. Wong
  2019-01-01  2:19 ` [PATCH 03/22] xfs: convert projid get/set functions Darrick J. Wong
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:19 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Hoist the inode flag conversion functions into libxfs so that we can
keep them in sync.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_util.c |  110 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_inode_util.h |    5 ++
 fs/xfs/xfs_inode.c             |   54 --------------------
 fs/xfs/xfs_ioctl.c             |   58 ---------------------
 4 files changed, 116 insertions(+), 111 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_util.c b/fs/xfs/libxfs/xfs_inode_util.c
index f9fc2e60c91c..6cc749b67d48 100644
--- a/fs/xfs/libxfs/xfs_inode_util.c
+++ b/fs/xfs/libxfs/xfs_inode_util.c
@@ -50,3 +50,113 @@ xfs_get_cowextsz_hint(
 		return XFS_DEFAULT_COWEXTSZ_HINT;
 	return a;
 }
+
+uint16_t
+xfs_flags2diflags(
+	struct xfs_inode	*ip,
+	unsigned int		xflags)
+{
+	/* can't set PREALLOC this way, just preserve it */
+	uint16_t		di_flags =
+		(ip->i_d.di_flags & XFS_DIFLAG_PREALLOC);
+
+	if (xflags & FS_XFLAG_IMMUTABLE)
+		di_flags |= XFS_DIFLAG_IMMUTABLE;
+	if (xflags & FS_XFLAG_APPEND)
+		di_flags |= XFS_DIFLAG_APPEND;
+	if (xflags & FS_XFLAG_SYNC)
+		di_flags |= XFS_DIFLAG_SYNC;
+	if (xflags & FS_XFLAG_NOATIME)
+		di_flags |= XFS_DIFLAG_NOATIME;
+	if (xflags & FS_XFLAG_NODUMP)
+		di_flags |= XFS_DIFLAG_NODUMP;
+	if (xflags & FS_XFLAG_NODEFRAG)
+		di_flags |= XFS_DIFLAG_NODEFRAG;
+	if (xflags & FS_XFLAG_FILESTREAM)
+		di_flags |= XFS_DIFLAG_FILESTREAM;
+	if (S_ISDIR(VFS_I(ip)->i_mode)) {
+		if (xflags & FS_XFLAG_RTINHERIT)
+			di_flags |= XFS_DIFLAG_RTINHERIT;
+		if (xflags & FS_XFLAG_NOSYMLINKS)
+			di_flags |= XFS_DIFLAG_NOSYMLINKS;
+		if (xflags & FS_XFLAG_EXTSZINHERIT)
+			di_flags |= XFS_DIFLAG_EXTSZINHERIT;
+		if (xflags & FS_XFLAG_PROJINHERIT)
+			di_flags |= XFS_DIFLAG_PROJINHERIT;
+	} else if (S_ISREG(VFS_I(ip)->i_mode)) {
+		if (xflags & FS_XFLAG_REALTIME)
+			di_flags |= XFS_DIFLAG_REALTIME;
+		if (xflags & FS_XFLAG_EXTSIZE)
+			di_flags |= XFS_DIFLAG_EXTSIZE;
+	}
+
+	return di_flags;
+}
+
+uint64_t
+xfs_flags2diflags2(
+	struct xfs_inode	*ip,
+	unsigned int		xflags)
+{
+	uint64_t		di_flags2 =
+		(ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK);
+
+	if (xflags & FS_XFLAG_DAX)
+		di_flags2 |= XFS_DIFLAG2_DAX;
+	if (xflags & FS_XFLAG_COWEXTSIZE)
+		di_flags2 |= XFS_DIFLAG2_COWEXTSIZE;
+
+	return di_flags2;
+}
+
+uint32_t
+xfs_dic2xflags(
+	uint16_t		di_flags,
+	uint64_t		di_flags2,
+	bool			has_attr)
+{
+	uint			flags = 0;
+
+	if (di_flags & XFS_DIFLAG_ANY) {
+		if (di_flags & XFS_DIFLAG_REALTIME)
+			flags |= FS_XFLAG_REALTIME;
+		if (di_flags & XFS_DIFLAG_PREALLOC)
+			flags |= FS_XFLAG_PREALLOC;
+		if (di_flags & XFS_DIFLAG_IMMUTABLE)
+			flags |= FS_XFLAG_IMMUTABLE;
+		if (di_flags & XFS_DIFLAG_APPEND)
+			flags |= FS_XFLAG_APPEND;
+		if (di_flags & XFS_DIFLAG_SYNC)
+			flags |= FS_XFLAG_SYNC;
+		if (di_flags & XFS_DIFLAG_NOATIME)
+			flags |= FS_XFLAG_NOATIME;
+		if (di_flags & XFS_DIFLAG_NODUMP)
+			flags |= FS_XFLAG_NODUMP;
+		if (di_flags & XFS_DIFLAG_RTINHERIT)
+			flags |= FS_XFLAG_RTINHERIT;
+		if (di_flags & XFS_DIFLAG_PROJINHERIT)
+			flags |= FS_XFLAG_PROJINHERIT;
+		if (di_flags & XFS_DIFLAG_NOSYMLINKS)
+			flags |= FS_XFLAG_NOSYMLINKS;
+		if (di_flags & XFS_DIFLAG_EXTSIZE)
+			flags |= FS_XFLAG_EXTSIZE;
+		if (di_flags & XFS_DIFLAG_EXTSZINHERIT)
+			flags |= FS_XFLAG_EXTSZINHERIT;
+		if (di_flags & XFS_DIFLAG_NODEFRAG)
+			flags |= FS_XFLAG_NODEFRAG;
+		if (di_flags & XFS_DIFLAG_FILESTREAM)
+			flags |= FS_XFLAG_FILESTREAM;
+	}
+
+	if (di_flags2 & XFS_DIFLAG2_ANY) {
+		if (di_flags2 & XFS_DIFLAG2_DAX)
+			flags |= FS_XFLAG_DAX;
+		if (di_flags2 & XFS_DIFLAG2_COWEXTSIZE)
+			flags |= FS_XFLAG_COWEXTSIZE;
+	}
+
+	if (has_attr)
+		flags |= FS_XFLAG_HASATTR;
+
+	return flags;
+}
diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index 98bae709dee8..45f8c4869c4b 100644
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -9,4 +9,9 @@
 xfs_extlen_t	xfs_get_extsz_hint(struct xfs_inode *ip);
 xfs_extlen_t	xfs_get_cowextsz_hint(struct xfs_inode *ip);
 
+uint16_t	xfs_flags2diflags(struct xfs_inode *ip, unsigned int xflags);
+uint64_t	xfs_flags2diflags2(struct xfs_inode *ip, unsigned int xflags);
+uint32_t	xfs_dic2xflags(uint16_t di_flags, uint64_t di_flags2,
+			       bool has_attr);
+
 #endif /* __XFS_INODE_UTIL_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 9774f1ecdd35..1df5c5ecac41 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -573,65 +573,13 @@ __xfs_iflock(
 	finish_wait(wq, &wait.wq_entry);
 }
 
-STATIC uint
-_xfs_dic2xflags(
-	uint16_t		di_flags,
-	uint64_t		di_flags2,
-	bool			has_attr)
-{
-	uint			flags = 0;
-
-	if (di_flags & XFS_DIFLAG_ANY) {
-		if (di_flags & XFS_DIFLAG_REALTIME)
-			flags |= FS_XFLAG_REALTIME;
-		if (di_flags & XFS_DIFLAG_PREALLOC)
-			flags |= FS_XFLAG_PREALLOC;
-		if (di_flags & XFS_DIFLAG_IMMUTABLE)
-			flags |= FS_XFLAG_IMMUTABLE;
-		if (di_flags & XFS_DIFLAG_APPEND)
-			flags |= FS_XFLAG_APPEND;
-		if (di_flags & XFS_DIFLAG_SYNC)
-			flags |= FS_XFLAG_SYNC;
-		if (di_flags & XFS_DIFLAG_NOATIME)
-			flags |= FS_XFLAG_NOATIME;
-		if (di_flags & XFS_DIFLAG_NODUMP)
-			flags |= FS_XFLAG_NODUMP;
-		if (di_flags & XFS_DIFLAG_RTINHERIT)
-			flags |= FS_XFLAG_RTINHERIT;
-		if (di_flags & XFS_DIFLAG_PROJINHERIT)
-			flags |= FS_XFLAG_PROJINHERIT;
-		if (di_flags & XFS_DIFLAG_NOSYMLINKS)
-			flags |= FS_XFLAG_NOSYMLINKS;
-		if (di_flags & XFS_DIFLAG_EXTSIZE)
-			flags |= FS_XFLAG_EXTSIZE;
-		if (di_flags & XFS_DIFLAG_EXTSZINHERIT)
-			flags |= FS_XFLAG_EXTSZINHERIT;
-		if (di_flags & XFS_DIFLAG_NODEFRAG)
-			flags |= FS_XFLAG_NODEFRAG;
-		if (di_flags & XFS_DIFLAG_FILESTREAM)
-			flags |= FS_XFLAG_FILESTREAM;
-	}
-
-	if (di_flags2 & XFS_DIFLAG2_ANY) {
-		if (di_flags2 & XFS_DIFLAG2_DAX)
-			flags |= FS_XFLAG_DAX;
-		if (di_flags2 & XFS_DIFLAG2_COWEXTSIZE)
-			flags |= FS_XFLAG_COWEXTSIZE;
-	}
-
-	if (has_attr)
-		flags |= FS_XFLAG_HASATTR;
-
-	return flags;
-}
-
 uint
 xfs_ip2xflags(
 	struct xfs_inode	*ip)
 {
 	struct xfs_icdinode	*dic = &ip->i_d;
 
-	return _xfs_dic2xflags(dic->di_flags, dic->di_flags2, XFS_IFORK_Q(ip));
+	return xfs_dic2xflags(dic->di_flags, dic->di_flags2, XFS_IFORK_Q(ip));
 }
 
 /*
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 6ecdbb3af7de..78a3d315fa82 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -909,64 +909,6 @@ xfs_ioc_fsgetxattr(
 	return 0;
 }
 
-STATIC uint16_t
-xfs_flags2diflags(
-	struct xfs_inode	*ip,
-	unsigned int		xflags)
-{
-	/* can't set PREALLOC this way, just preserve it */
-	uint16_t		di_flags =
-		(ip->i_d.di_flags & XFS_DIFLAG_PREALLOC);
-
-	if (xflags & FS_XFLAG_IMMUTABLE)
-		di_flags |= XFS_DIFLAG_IMMUTABLE;
-	if (xflags & FS_XFLAG_APPEND)
-		di_flags |= XFS_DIFLAG_APPEND;
-	if (xflags & FS_XFLAG_SYNC)
-		di_flags |= XFS_DIFLAG_SYNC;
-	if (xflags & FS_XFLAG_NOATIME)
-		di_flags |= XFS_DIFLAG_NOATIME;
-	if (xflags & FS_XFLAG_NODUMP)
-		di_flags |= XFS_DIFLAG_NODUMP;
-	if (xflags & FS_XFLAG_NODEFRAG)
-		di_flags |= XFS_DIFLAG_NODEFRAG;
-	if (xflags & FS_XFLAG_FILESTREAM)
-		di_flags |= XFS_DIFLAG_FILESTREAM;
-	if (S_ISDIR(VFS_I(ip)->i_mode)) {
-		if (xflags & FS_XFLAG_RTINHERIT)
-			di_flags |= XFS_DIFLAG_RTINHERIT;
-		if (xflags & FS_XFLAG_NOSYMLINKS)
-			di_flags |= XFS_DIFLAG_NOSYMLINKS;
-		if (xflags & FS_XFLAG_EXTSZINHERIT)
-			di_flags |= XFS_DIFLAG_EXTSZINHERIT;
-		if (xflags & FS_XFLAG_PROJINHERIT)
-			di_flags |= XFS_DIFLAG_PROJINHERIT;
-	} else if (S_ISREG(VFS_I(ip)->i_mode)) {
-		if (xflags & FS_XFLAG_REALTIME)
-			di_flags |= XFS_DIFLAG_REALTIME;
-		if (xflags & FS_XFLAG_EXTSIZE)
-			di_flags |= XFS_DIFLAG_EXTSIZE;
-	}
-
-	return di_flags;
-}
-
-STATIC uint64_t
-xfs_flags2diflags2(
-	struct xfs_inode	*ip,
-	unsigned int		xflags)
-{
-	uint64_t		di_flags2 =
-		(ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK);
-
-	if (xflags & FS_XFLAG_DAX)
-		di_flags2 |= XFS_DIFLAG2_DAX;
-	if (xflags & FS_XFLAG_COWEXTSIZE)
-		di_flags2 |= XFS_DIFLAG2_COWEXTSIZE;
-
-	return di_flags2;
-}
-
 STATIC void
 xfs_diflags_to_linux(
 	struct xfs_inode	*ip)

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 03/22] xfs: convert projid get/set functions
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
  2019-01-01  2:19 ` [PATCH 01/22] xfs: hoist extent size helpers " Darrick J. Wong
  2019-01-01  2:19 ` [PATCH 02/22] xfs: hoist inode flag conversion functions Darrick J. Wong
@ 2019-01-01  2:19 ` Darrick J. Wong
  2019-01-17 14:19   ` Christoph Hellwig
  2019-01-01  2:19 ` [PATCH 04/22] xfs: hoist project id " Darrick J. Wong
                   ` (18 subsequent siblings)
  21 siblings, 1 reply; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:19 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Convert the projid get and set functions to work on xfs_icdinode like
they do in userspace.  xfs_db needs the icdinode version.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_dquot.c   |    2 +-
 fs/xfs/xfs_icache.c  |    4 ++--
 fs/xfs/xfs_inode.c   |   12 +++++++-----
 fs/xfs/xfs_inode.h   |   17 ++++++++---------
 fs/xfs/xfs_ioctl.c   |   10 +++++-----
 fs/xfs/xfs_iops.c    |    2 +-
 fs/xfs/xfs_qm.c      |    9 +++++----
 fs/xfs/xfs_qm_bhv.c  |    3 ++-
 fs/xfs/xfs_symlink.c |    2 +-
 9 files changed, 32 insertions(+), 29 deletions(-)


diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 978ce0c2e0aa..d67c3449a846 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -883,7 +883,7 @@ xfs_qm_id_for_quotatype(
 	case XFS_DQ_GROUP:
 		return ip->i_d.di_gid;
 	case XFS_DQ_PROJ:
-		return xfs_get_projid(ip);
+		return xfs_get_projid(&ip->i_d);
 	}
 	ASSERT(0);
 	return 0;
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 064c5de9dce3..f00c6f4bc515 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -41,7 +41,7 @@ xfs_inode_match_id(
 		return 0;
 
 	if ((eofb->eof_flags & XFS_EOF_FLAGS_PRID) &&
-	    xfs_get_projid(ip) != eofb->eof_prid)
+	    xfs_get_projid(&ip->i_d) != eofb->eof_prid)
 		return 0;
 
 	return 1;
@@ -65,7 +65,7 @@ xfs_inode_match_id_union(
 		return 1;
 
 	if ((eofb->eof_flags & XFS_EOF_FLAGS_PRID) &&
-	    xfs_get_projid(ip) == eofb->eof_prid)
+	    xfs_get_projid(&ip->i_d) == eofb->eof_prid)
 		return 1;
 
 	return 0;
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 1df5c5ecac41..79c8876e9f61 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -722,7 +722,7 @@ xfs_ialloc(
 	ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
 	ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
 	inode->i_rdev = rdev;
-	xfs_set_projid(ip, prid);
+	xfs_set_projid(&ip->i_d, prid);
 
 	if (pip && XFS_INHERIT_GID(pip)) {
 		ip->i_d.di_gid = pip->i_d.di_gid;
@@ -1062,7 +1062,7 @@ xfs_create(
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
 
-	prid = xfs_get_initial_prid(dp);
+	prid = xfs_get_initial_prid(&dp->i_d);
 
 	/*
 	 * Make sure that we have allocated dquot(s) on disk.
@@ -1216,7 +1216,7 @@ xfs_create_tmpfile(
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
 
-	prid = xfs_get_initial_prid(dp);
+	prid = xfs_get_initial_prid(&dp->i_d);
 
 	/*
 	 * Make sure that we have allocated dquot(s) on disk.
@@ -1335,7 +1335,8 @@ xfs_link(
 	 * the tree quota mechanism could be circumvented.
 	 */
 	if (unlikely((tdp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) &&
-		     (xfs_get_projid(tdp) != xfs_get_projid(sip)))) {
+		     (xfs_get_projid(&tdp->i_d) !=
+		      xfs_get_projid(&sip->i_d)))) {
 		error = -EXDEV;
 		goto error_return;
 	}
@@ -2997,7 +2998,8 @@ xfs_rename(
 	 * tree quota mechanism would be circumvented.
 	 */
 	if (unlikely((target_dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) &&
-		     (xfs_get_projid(target_dp) != xfs_get_projid(src_ip)))) {
+		     (xfs_get_projid(&target_dp->i_d) !=
+		      xfs_get_projid(&src_ip->i_d)))) {
 		error = -EXDEV;
 		goto out_trans_cancel;
 	}
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 29aedfdf3d05..ca2623e0dd4f 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -171,24 +171,23 @@ xfs_iflags_test_and_set(xfs_inode_t *ip, unsigned short flags)
  * to retain compatibility with "old" filesystems).
  */
 static inline prid_t
-xfs_get_projid(struct xfs_inode *ip)
+xfs_get_projid(struct xfs_icdinode *id)
 {
-	return (prid_t)ip->i_d.di_projid_hi << 16 | ip->i_d.di_projid_lo;
+	return (prid_t)id->di_projid_hi << 16 | id->di_projid_lo;
 }
 
 static inline void
-xfs_set_projid(struct xfs_inode *ip,
-		prid_t projid)
+xfs_set_projid(struct xfs_icdinode *id, prid_t projid)
 {
-	ip->i_d.di_projid_hi = (uint16_t) (projid >> 16);
-	ip->i_d.di_projid_lo = (uint16_t) (projid & 0xffff);
+	id->di_projid_hi = (uint16_t) (projid >> 16);
+	id->di_projid_lo = (uint16_t) (projid & 0xffff);
 }
 
 static inline prid_t
-xfs_get_initial_prid(struct xfs_inode *dp)
+xfs_get_initial_prid(struct xfs_icdinode *id)
 {
-	if (dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
-		return xfs_get_projid(dp);
+	if (id->di_flags & XFS_DIFLAG_PROJINHERIT)
+		return xfs_get_projid(id);
 
 	return XFS_PROJID_DEFAULT;
 }
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 78a3d315fa82..8cd9f5757732 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -886,7 +886,7 @@ xfs_ioc_fsgetxattr(
 	fa.fsx_extsize = ip->i_d.di_extsize << ip->i_mount->m_sb.sb_blocklog;
 	fa.fsx_cowextsize = ip->i_d.di_cowextsize <<
 			ip->i_mount->m_sb.sb_blocklog;
-	fa.fsx_projid = xfs_get_projid(ip);
+	fa.fsx_projid = xfs_get_projid(&ip->i_d);
 
 	if (attr) {
 		if (ip->i_afp) {
@@ -1243,7 +1243,7 @@ xfs_ioctl_setattr_check_projid(
 	if (current_user_ns() == &init_user_ns)
 		return 0;
 
-	if (xfs_get_projid(ip) != fa->fsx_projid)
+	if (xfs_get_projid(&ip->i_d) != fa->fsx_projid)
 		return -EINVAL;
 	if ((fa->fsx_xflags & FS_XFLAG_PROJINHERIT) !=
 	    (ip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT))
@@ -1306,7 +1306,7 @@ xfs_ioctl_setattr(
 
 
 	if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp) &&
-	    xfs_get_projid(ip) != fa->fsx_projid) {
+	    xfs_get_projid(&ip->i_d) != fa->fsx_projid) {
 		code = xfs_qm_vop_chown_reserve(tp, ip, udqp, NULL, pdqp,
 				capable(CAP_FOWNER) ?  XFS_QMOPT_FORCE_RES : 0);
 		if (code)	/* out of quota */
@@ -1338,13 +1338,13 @@ xfs_ioctl_setattr(
 		VFS_I(ip)->i_mode &= ~(S_ISUID|S_ISGID);
 
 	/* Change the ownerships and register project quota modifications */
-	if (xfs_get_projid(ip) != fa->fsx_projid) {
+	if (xfs_get_projid(&ip->i_d) != fa->fsx_projid) {
 		if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp)) {
 			olddquot = xfs_qm_vop_chown(tp, ip,
 						&ip->i_pdquot, pdqp);
 		}
 		ASSERT(ip->i_d.di_version > 1);
-		xfs_set_projid(ip, fa->fsx_projid);
+		xfs_set_projid(&ip->i_d, fa->fsx_projid);
 	}
 
 	/*
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index f48ffd7a8d3e..1081dd376f86 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -657,7 +657,7 @@ xfs_setattr_nonsize(
 		ASSERT(gdqp == NULL);
 		error = xfs_qm_vop_dqalloc(ip, xfs_kuid_to_uid(uid),
 					   xfs_kgid_to_gid(gid),
-					   xfs_get_projid(ip),
+					   xfs_get_projid(&ip->i_d),
 					   qflags, &udqp, &gdqp, NULL);
 		if (error)
 			return error;
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 116b24c980fd..32f400867727 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -345,7 +345,7 @@ xfs_qm_dqattach_locked(
 	}
 
 	if (XFS_IS_PQUOTA_ON(mp) && !ip->i_pdquot) {
-		error = xfs_qm_dqattach_one(ip, xfs_get_projid(ip), XFS_DQ_PROJ,
+		error = xfs_qm_dqattach_one(ip, xfs_get_projid(&ip->i_d), XFS_DQ_PROJ,
 				doalloc, &ip->i_pdquot);
 		if (error)
 			goto done;
@@ -1732,7 +1732,7 @@ xfs_qm_vop_dqalloc(
 		}
 	}
 	if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
-		if (xfs_get_projid(ip) != prid) {
+		if (xfs_get_projid(&ip->i_d) != prid) {
 			xfs_iunlock(ip, lockflags);
 			error = xfs_qm_dqget(mp, (xfs_dqid_t)prid, XFS_DQ_PROJ,
 					true, &pq);
@@ -1865,7 +1865,7 @@ xfs_qm_vop_chown_reserve(
 	}
 
 	if (XFS_IS_PQUOTA_ON(ip->i_mount) && pdqp &&
-	    xfs_get_projid(ip) != be32_to_cpu(pdqp->q_core.d_id)) {
+	    xfs_get_projid(&ip->i_d) != be32_to_cpu(pdqp->q_core.d_id)) {
 		prjflags = XFS_QMOPT_ENOSPC;
 		pdq_delblks = pdqp;
 		if (delblks) {
@@ -1966,7 +1966,8 @@ xfs_qm_vop_create_dqattach(
 	}
 	if (pdqp && XFS_IS_PQUOTA_ON(mp)) {
 		ASSERT(ip->i_pdquot == NULL);
-		ASSERT(xfs_get_projid(ip) == be32_to_cpu(pdqp->q_core.d_id));
+		ASSERT(xfs_get_projid(&ip->i_d) ==
+				be32_to_cpu(pdqp->q_core.d_id));
 
 		ip->i_pdquot = xfs_qm_dqhold(pdqp);
 		xfs_trans_mod_dquot(tp, pdqp, XFS_TRANS_DQ_ICOUNT, 1);
diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
index 3091e4bc04ef..2032fe6bb22a 100644
--- a/fs/xfs/xfs_qm_bhv.c
+++ b/fs/xfs/xfs_qm_bhv.c
@@ -60,7 +60,8 @@ xfs_qm_statvfs(
 	xfs_mount_t		*mp = ip->i_mount;
 	xfs_dquot_t		*dqp;
 
-	if (!xfs_qm_dqget(mp, xfs_get_projid(ip), XFS_DQ_PROJ, false, &dqp)) {
+	if (!xfs_qm_dqget(mp, xfs_get_projid(&ip->i_d), XFS_DQ_PROJ,
+				false, &dqp)) {
 		xfs_fill_statvfs_from_dquot(statp, dqp);
 		xfs_qm_dqput(dqp);
 	}
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 8e554e8264b6..0096cdbad34e 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -267,7 +267,7 @@ xfs_symlink(
 	ASSERT(pathlen > 0);
 
 	udqp = gdqp = NULL;
-	prid = xfs_get_initial_prid(dp);
+	prid = xfs_get_initial_prid(&dp->i_d);
 
 	/*
 	 * Make sure that we have allocated dquot(s) on disk.

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 04/22] xfs: hoist project id get/set functions
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (2 preceding siblings ...)
  2019-01-01  2:19 ` [PATCH 03/22] xfs: convert projid get/set functions Darrick J. Wong
@ 2019-01-01  2:19 ` Darrick J. Wong
  2019-01-01  2:19 ` [PATCH 05/22] xfs: pack inode allocation parameters into a separate structure Darrick J. Wong
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:19 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Move the project id get and set functions into libxfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_util.h |   29 +++++++++++++++++++++++++++++
 fs/xfs/xfs_inode.h             |   27 ---------------------------
 fs/xfs/xfs_linux.h             |    2 --
 3 files changed, 29 insertions(+), 29 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index 45f8c4869c4b..808e4322106e 100644
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -14,4 +14,33 @@ uint64_t	xfs_flags2diflags2(struct xfs_inode *ip, unsigned int xflags);
 uint32_t	xfs_dic2xflags(uint16_t di_flags, uint64_t di_flags2,
 			       bool has_attr);
 
+/*
+ * Project quota id helpers (previously projid was 16bit only and using two
+ * 16bit values to hold new 32bit projid was chosen to retain compatibility
+ * with "old" filesystems).
+ */
+static inline prid_t
+xfs_get_projid(struct xfs_icdinode *id)
+{
+	return (prid_t)id->di_projid_hi << 16 | id->di_projid_lo;
+}
+
+static inline void
+xfs_set_projid(struct xfs_icdinode *id, prid_t projid)
+{
+	id->di_projid_hi = (uint16_t) (projid >> 16);
+	id->di_projid_lo = (uint16_t) (projid & 0xffff);
+}
+
+#define XFS_PROJID_DEFAULT		0
+
+static inline prid_t
+xfs_get_initial_prid(struct xfs_icdinode *id)
+{
+	if (id->di_flags & XFS_DIFLAG_PROJINHERIT)
+		return xfs_get_projid(id);
+
+	return XFS_PROJID_DEFAULT;
+}
+
 #endif /* __XFS_INODE_UTIL_H__ */
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index ca2623e0dd4f..fb55e3b74b34 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -165,33 +165,6 @@ xfs_iflags_test_and_set(xfs_inode_t *ip, unsigned short flags)
 	return ret;
 }
 
-/*
- * Project quota id helpers (previously projid was 16bit only
- * and using two 16bit values to hold new 32bit projid was chosen
- * to retain compatibility with "old" filesystems).
- */
-static inline prid_t
-xfs_get_projid(struct xfs_icdinode *id)
-{
-	return (prid_t)id->di_projid_hi << 16 | id->di_projid_lo;
-}
-
-static inline void
-xfs_set_projid(struct xfs_icdinode *id, prid_t projid)
-{
-	id->di_projid_hi = (uint16_t) (projid >> 16);
-	id->di_projid_lo = (uint16_t) (projid & 0xffff);
-}
-
-static inline prid_t
-xfs_get_initial_prid(struct xfs_icdinode *id)
-{
-	if (id->di_flags & XFS_DIFLAG_PROJINHERIT)
-		return xfs_get_projid(id);
-
-	return XFS_PROJID_DEFAULT;
-}
-
 static inline bool xfs_is_reflink_inode(struct xfs_inode *ip)
 {
 	return ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK;
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index edbd5a210df2..f2835a9f0a16 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -138,8 +138,6 @@ typedef __u32			xfs_nlink_t;
  */
 #define __this_address	({ __label__ __here; __here: barrier(); &&__here; })
 
-#define XFS_PROJID_DEFAULT	0
-
 #define howmany(x, y)	(((x)+((y)-1))/(y))
 
 static inline void delay(long ticks)

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 05/22] xfs: pack inode allocation parameters into a separate structure
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (3 preceding siblings ...)
  2019-01-01  2:19 ` [PATCH 04/22] xfs: hoist project id " Darrick J. Wong
@ 2019-01-01  2:19 ` Darrick J. Wong
  2019-01-17 14:21   ` Christoph Hellwig
  2019-01-01  2:19 ` [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc Darrick J. Wong
                   ` (16 subsequent siblings)
  21 siblings, 1 reply; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:19 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Instead of open-coding new inode parameters through xfs_dir_ialloc and
xfs_Ialloc, put everything into a structure and pass that instead.  This
will make it easier to share code with xfsprogs while maintaining the
ability for xfsprogs to supply extra new inode parameters.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_util.h |   14 +++++++++
 fs/xfs/xfs_inode.c             |   64 +++++++++++++++++++++-------------------
 2 files changed, 48 insertions(+), 30 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index 808e4322106e..942524b59cf6 100644
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -43,4 +43,18 @@ xfs_get_initial_prid(struct xfs_icdinode *id)
 	return XFS_PROJID_DEFAULT;
 }
 
+/* Initial ids, link count, device number, and mode of a new inode. */
+struct xfs_ialloc_args {
+	struct xfs_inode		*pip;	/* parent inode or null */
+
+	uint32_t			uid;
+	uint32_t			gid;
+	prid_t				prid;
+
+	xfs_nlink_t			nlink;
+	dev_t				rdev;
+
+	umode_t				mode;
+};
+
 #endif /* __XFS_INODE_UTIL_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 79c8876e9f61..403eb8fa2f1f 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -654,28 +654,25 @@ xfs_lookup(
  */
 static int
 xfs_ialloc(
-	xfs_trans_t	*tp,
-	xfs_inode_t	*pip,
-	umode_t		mode,
-	xfs_nlink_t	nlink,
-	dev_t		rdev,
-	prid_t		prid,
-	xfs_buf_t	**ialloc_context,
-	xfs_inode_t	**ipp)
+	struct xfs_trans		*tp,
+	const struct xfs_ialloc_args	*args,
+	struct xfs_buf			**ialloc_context,
+	struct xfs_inode		**ipp)
 {
-	struct xfs_mount *mp = tp->t_mountp;
-	xfs_ino_t	ino;
-	xfs_inode_t	*ip;
-	uint		flags;
-	int		error;
-	struct timespec64 tv;
-	struct inode	*inode;
+	struct xfs_mount		*mp = tp->t_mountp;
+	struct xfs_inode		*pip = args->pip;
+	struct xfs_inode		*ip;
+	struct inode			*inode;
+	xfs_ino_t			ino;
+	uint				flags;
+	int				error;
+	struct timespec64		tv;
 
 	/*
 	 * Call the space management code to pick
 	 * the on-disk inode to be allocated.
 	 */
-	error = xfs_dialloc(tp, pip ? pip->i_ino : 0, mode,
+	error = xfs_dialloc(tp, pip ? pip->i_ino : 0, args->mode,
 			    ialloc_context, &ino);
 	if (error)
 		return error;
@@ -717,16 +714,16 @@ xfs_ialloc(
 	if (ip->i_d.di_version == 1)
 		ip->i_d.di_version = 2;
 
-	inode->i_mode = mode;
-	set_nlink(inode, nlink);
-	ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
-	ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
-	inode->i_rdev = rdev;
-	xfs_set_projid(&ip->i_d, prid);
+	inode->i_mode = args->mode;
+	set_nlink(inode, args->nlink);
+	ip->i_d.di_uid = args->uid;
+	ip->i_d.di_gid = args->gid;
+	inode->i_rdev = args->rdev;
+	xfs_set_projid(&ip->i_d, args->prid);
 
 	if (pip && XFS_INHERIT_GID(pip)) {
 		ip->i_d.di_gid = pip->i_d.di_gid;
-		if ((VFS_I(pip)->i_mode & S_ISGID) && S_ISDIR(mode))
+		if ((VFS_I(pip)->i_mode & S_ISGID) && S_ISDIR(args->mode))
 			inode->i_mode |= S_ISGID;
 	}
 
@@ -764,7 +761,7 @@ xfs_ialloc(
 
 
 	flags = XFS_ILOG_CORE;
-	switch (mode & S_IFMT) {
+	switch (args->mode & S_IFMT) {
 	case S_IFIFO:
 	case S_IFCHR:
 	case S_IFBLK:
@@ -778,7 +775,7 @@ xfs_ialloc(
 		if (pip && (pip->i_d.di_flags & XFS_DIFLAG_ANY)) {
 			uint		di_flags = 0;
 
-			if (S_ISDIR(mode)) {
+			if (S_ISDIR(args->mode)) {
 				if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
 					di_flags |= XFS_DIFLAG_RTINHERIT;
 				if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
@@ -787,7 +784,7 @@ xfs_ialloc(
 				}
 				if (pip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
 					di_flags |= XFS_DIFLAG_PROJINHERIT;
-			} else if (S_ISREG(mode)) {
+			} else if (S_ISREG(args->mode)) {
 				if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
 					di_flags |= XFS_DIFLAG_REALTIME;
 				if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
@@ -882,6 +879,15 @@ xfs_dir_ialloc(
 	xfs_inode_t	**ipp)		/* pointer to inode; it will be
 					   locked. */
 {
+	struct xfs_ialloc_args	args = {
+		.pip	= dp,
+		.uid	= xfs_kuid_to_uid(current_fsuid()),
+		.gid	= xfs_kgid_to_gid(current_fsgid()),
+		.prid	= prid,
+		.nlink	= nlink,
+		.rdev	= rdev,
+		.mode	= mode,
+	};
 	xfs_trans_t	*tp;
 	xfs_inode_t	*ip;
 	xfs_buf_t	*ialloc_context = NULL;
@@ -907,8 +913,7 @@ xfs_dir_ialloc(
 	 * transaction commit so that no other process can steal
 	 * the inode(s) that we've just allocated.
 	 */
-	code = xfs_ialloc(tp, dp, mode, nlink, rdev, prid, &ialloc_context,
-			&ip);
+	code = xfs_ialloc(tp, &args, &ialloc_context, &ip);
 
 	/*
 	 * Return an error if we were unable to allocate a new inode.
@@ -977,8 +982,7 @@ xfs_dir_ialloc(
 		 * other allocations in this allocation group,
 		 * this call should always succeed.
 		 */
-		code = xfs_ialloc(tp, dp, mode, nlink, rdev, prid,
-				  &ialloc_context, &ip);
+		code = xfs_ialloc(tp, &args, &ialloc_context, &ip);
 
 		/*
 		 * If we get an error at this point, return to the caller

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (4 preceding siblings ...)
  2019-01-01  2:19 ` [PATCH 05/22] xfs: pack inode allocation parameters into a separate structure Darrick J. Wong
@ 2019-01-01  2:19 ` Darrick J. Wong
  2019-01-17 14:22   ` Christoph Hellwig
  2019-01-17 22:26   ` Dave Chinner
  2019-01-01  2:19 ` [PATCH 07/22] xfs: decouple platform-specific inode allocation functions Darrick J. Wong
                   ` (15 subsequent siblings)
  21 siblings, 2 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:19 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Move the kernel-specific parts of xfs_ialloc into a separate function in
preparation for hoisting xfs_ialloc to libxfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_inode.c |   45 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 9 deletions(-)


diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 403eb8fa2f1f..b635d43caeed 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -621,6 +621,32 @@ xfs_lookup(
 	return error;
 }
 
+/* Return an exclusive ILOCK'd in-core inode. */
+static int
+xfs_ialloc_iget(
+	struct xfs_mount	*mp,
+	struct xfs_trans	*tp,
+	xfs_ino_t		ino,
+	struct xfs_inode	**ipp)
+{
+	return xfs_iget(mp, tp, ino, XFS_IGET_CREATE, XFS_ILOCK_EXCL, ipp);
+}
+
+/* Propagate platform-specific inode properties into the new child. */
+static void
+xfs_ialloc_platform_init(
+	struct xfs_trans		*tp,
+	const struct xfs_ialloc_args	*args,
+	struct xfs_inode		*ip)
+{
+	struct timespec64		tv;
+
+	tv = current_time(VFS_I(ip));
+	VFS_I(ip)->i_mtime = tv;
+	VFS_I(ip)->i_atime = tv;
+	VFS_I(ip)->i_ctime = tv;
+}
+
 /*
  * Allocate an inode on disk and return a copy of its in-core version.
  * The in-core inode is locked exclusively.  Set mode, nlink, and rdev
@@ -666,7 +692,6 @@ xfs_ialloc(
 	xfs_ino_t			ino;
 	uint				flags;
 	int				error;
-	struct timespec64		tv;
 
 	/*
 	 * Call the space management code to pick
@@ -699,8 +724,7 @@ xfs_ialloc(
 	 * This is because we're setting fields here we need
 	 * to prevent others from looking at until we're done.
 	 */
-	error = xfs_iget(mp, tp, ino, XFS_IGET_CREATE,
-			 XFS_ILOCK_EXCL, &ip);
+	error = xfs_ialloc_iget(mp, tp, ino, &ip);
 	if (error)
 		return error;
 	ASSERT(ip != NULL);
@@ -741,10 +765,12 @@ xfs_ialloc(
 	ip->i_d.di_nextents = 0;
 	ASSERT(ip->i_d.di_nblocks == 0);
 
-	tv = current_time(inode);
-	inode->i_mtime = tv;
-	inode->i_atime = tv;
-	inode->i_ctime = tv;
+	inode->i_mtime.tv_sec = 0;
+	inode->i_mtime.tv_nsec = 0;
+	inode->i_atime.tv_sec = 0;
+	inode->i_atime.tv_nsec = 0;
+	inode->i_ctime.tv_sec = 0;
+	inode->i_ctime.tv_nsec = 0;
 
 	ip->i_d.di_extsize = 0;
 	ip->i_d.di_dmevmask = 0;
@@ -755,10 +781,11 @@ xfs_ialloc(
 		inode_set_iversion(inode, 1);
 		ip->i_d.di_flags2 = 0;
 		ip->i_d.di_cowextsize = 0;
-		ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
-		ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
+		ip->i_d.di_crtime.t_sec = 0;
+		ip->i_d.di_crtime.t_nsec = 0;
 	}
 
+	xfs_ialloc_platform_init(tp, args, ip);
 
 	flags = XFS_ILOG_CORE;
 	switch (args->mode & S_IFMT) {

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 07/22] xfs: decouple platform-specific inode allocation functions
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (5 preceding siblings ...)
  2019-01-01  2:19 ` [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc Darrick J. Wong
@ 2019-01-01  2:19 ` Darrick J. Wong
  2019-01-17 14:22   ` Christoph Hellwig
  2019-01-01  2:19 ` [PATCH 08/22] xfs: split inode allocation and initialization Darrick J. Wong
                   ` (14 subsequent siblings)
  21 siblings, 1 reply; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:19 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Decouple the xfs_ialloc function from the platform-specific pieces by
passing in an operations structure.  This prepares the function for
hoisting to libxfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_util.h |   30 ++++++++++++++++++++++++++++++
 fs/xfs/xfs_inode.c             |   13 ++++++++++---
 2 files changed, 40 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index 942524b59cf6..6318234003e1 100644
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -43,8 +43,11 @@ xfs_get_initial_prid(struct xfs_icdinode *id)
 	return XFS_PROJID_DEFAULT;
 }
 
+struct xfs_ialloc_ops;
+
 /* Initial ids, link count, device number, and mode of a new inode. */
 struct xfs_ialloc_args {
+	const struct xfs_ialloc_ops	*ops;
 	struct xfs_inode		*pip;	/* parent inode or null */
 
 	uint32_t			uid;
@@ -57,4 +60,31 @@ struct xfs_ialloc_args {
 	umode_t				mode;
 };
 
+/* Platform-specific inode allocation functions. */
+struct xfs_ialloc_ops {
+	/*
+	 * Load in-core inode given our new inode number, returning the inode
+	 * with ILOCK_EXCL held to prevent anyone else from seeing the inode
+	 * until we're done.
+	 */
+	int (*iget)(struct xfs_mount *mp, struct xfs_trans *tp, xfs_ino_t ino,
+		    struct xfs_inode **ipp);
+
+	/*
+	 * Do whatever platform-specific initialization is necessary to bring
+	 * up this new inode.  This is called after we've set up default values
+	 * on the inode but before propagating required fields from the parent
+	 * inode.
+	 */
+	void (*platform_init)(struct xfs_trans *tp,
+			      const struct xfs_ialloc_args *args,
+			      struct xfs_inode *ip);
+
+	/* Do any final setup needed before we return the inode. */
+	void (*setup)(struct xfs_inode *ip);
+};
+
+/* The libxfs client must provide this symbol. */
+extern const struct xfs_ialloc_ops xfs_default_ialloc_ops;
+
 #endif /* __XFS_INODE_UTIL_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index b635d43caeed..41fab97d1270 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -647,6 +647,12 @@ xfs_ialloc_platform_init(
 	VFS_I(ip)->i_ctime = tv;
 }
 
+const struct xfs_ialloc_ops xfs_default_ialloc_ops = {
+	.iget		= xfs_ialloc_iget,
+	.platform_init	= xfs_ialloc_platform_init,
+	.setup		= xfs_setup_inode,
+};
+
 /*
  * Allocate an inode on disk and return a copy of its in-core version.
  * The in-core inode is locked exclusively.  Set mode, nlink, and rdev
@@ -724,7 +730,7 @@ xfs_ialloc(
 	 * This is because we're setting fields here we need
 	 * to prevent others from looking at until we're done.
 	 */
-	error = xfs_ialloc_iget(mp, tp, ino, &ip);
+	error = args->ops->iget(mp, tp, ino, &ip);
 	if (error)
 		return error;
 	ASSERT(ip != NULL);
@@ -785,7 +791,7 @@ xfs_ialloc(
 		ip->i_d.di_crtime.t_nsec = 0;
 	}
 
-	xfs_ialloc_platform_init(tp, args, ip);
+	args->ops->platform_init(tp, args, ip);
 
 	flags = XFS_ILOG_CORE;
 	switch (args->mode & S_IFMT) {
@@ -877,7 +883,7 @@ xfs_ialloc(
 	xfs_trans_log_inode(tp, ip, flags);
 
 	/* now that we have an i_mode we can setup the inode structure */
-	xfs_setup_inode(ip);
+	args->ops->setup(ip);
 
 	*ipp = ip;
 	return 0;
@@ -907,6 +913,7 @@ xfs_dir_ialloc(
 					   locked. */
 {
 	struct xfs_ialloc_args	args = {
+		.ops	= &xfs_default_ialloc_ops,
 		.pip	= dp,
 		.uid	= xfs_kuid_to_uid(current_fsuid()),
 		.gid	= xfs_kgid_to_gid(current_fsgid()),

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 08/22] xfs: split inode allocation and initialization
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (6 preceding siblings ...)
  2019-01-01  2:19 ` [PATCH 07/22] xfs: decouple platform-specific inode allocation functions Darrick J. Wong
@ 2019-01-01  2:19 ` Darrick J. Wong
  2019-01-01  2:19 ` [PATCH 09/22] xfs: hoist inode allocation function Darrick J. Wong
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:19 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Split new inode allocation and initialization into separate helpers.
Eventually we'll supply a force-reinitialization function so that
xfs_repair can use libxfs to reset the root directory and friends.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_inode.c |  164 ++++++++++++++++++++++++++++------------------------
 1 file changed, 89 insertions(+), 75 deletions(-)


diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 41fab97d1270..c053e1d422d5 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -654,87 +654,19 @@ const struct xfs_ialloc_ops xfs_default_ialloc_ops = {
 };
 
 /*
- * Allocate an inode on disk and return a copy of its in-core version.
- * The in-core inode is locked exclusively.  Set mode, nlink, and rdev
- * appropriately within the inode.  The uid and gid for the inode are
- * set according to the contents of the given cred structure.
- *
- * Use xfs_dialloc() to allocate the on-disk inode. If xfs_dialloc()
- * has a free inode available, call xfs_iget() to obtain the in-core
- * version of the allocated inode.  Finally, fill in the inode and
- * log its initial contents.  In this case, ialloc_context would be
- * set to NULL.
- *
- * If xfs_dialloc() does not have an available inode, it will replenish
- * its supply by doing an allocation. Since we can only do one
- * allocation within a transaction without deadlocks, we must commit
- * the current transaction before returning the inode itself.
- * In this case, therefore, we will set ialloc_context and return.
- * The caller should then commit the current transaction, start a new
- * transaction, and call xfs_ialloc() again to actually get the inode.
- *
- * To ensure that some other process does not grab the inode that
- * was allocated during the first call to xfs_ialloc(), this routine
- * also returns the [locked] bp pointing to the head of the freelist
- * as ialloc_context.  The caller should hold this buffer across
- * the commit and pass it back into this routine on the second call.
- *
- * If we are allocating quota inodes, we do not have a parent inode
- * to attach to or associate with (i.e. pip == NULL) because they
- * are not linked into the directory structure - they are attached
- * directly to the superblock - and so have no parent.
+ * Initialize a newly allocated inode with the given arguments.  Heritable
+ * inode properties will be copied from the parent if one is supplied and the
+ * appropriate inode flags are set on the parent.
  */
-static int
-xfs_ialloc(
+STATIC void
+xfs_inode_init(
 	struct xfs_trans		*tp,
 	const struct xfs_ialloc_args	*args,
-	struct xfs_buf			**ialloc_context,
-	struct xfs_inode		**ipp)
+	struct xfs_inode		*ip)
 {
-	struct xfs_mount		*mp = tp->t_mountp;
 	struct xfs_inode		*pip = args->pip;
-	struct xfs_inode		*ip;
-	struct inode			*inode;
-	xfs_ino_t			ino;
+	struct inode			*inode = VFS_I(ip);
 	uint				flags;
-	int				error;
-
-	/*
-	 * Call the space management code to pick
-	 * the on-disk inode to be allocated.
-	 */
-	error = xfs_dialloc(tp, pip ? pip->i_ino : 0, args->mode,
-			    ialloc_context, &ino);
-	if (error)
-		return error;
-	if (*ialloc_context || ino == NULLFSINO) {
-		*ipp = NULL;
-		return 0;
-	}
-	ASSERT(*ialloc_context == NULL);
-
-	/*
-	 * Protect against obviously corrupt allocation btree records. Later
-	 * xfs_iget checks will catch re-allocation of other active in-memory
-	 * and on-disk inodes. If we don't catch reallocating the parent inode
-	 * here we will deadlock in xfs_iget() so we have to do these checks
-	 * first.
-	 */
-	if ((pip && ino == pip->i_ino) || !xfs_verify_dir_ino(mp, ino)) {
-		xfs_alert(mp, "Allocated a known in-use inode 0x%llx!", ino);
-		return -EFSCORRUPTED;
-	}
-
-	/*
-	 * Get the in-core inode with the lock held exclusively.
-	 * This is because we're setting fields here we need
-	 * to prevent others from looking at until we're done.
-	 */
-	error = args->ops->iget(mp, tp, ino, &ip);
-	if (error)
-		return error;
-	ASSERT(ip != NULL);
-	inode = VFS_I(ip);
 
 	/*
 	 * We always convert v1 inodes to v2 now - we only support filesystems
@@ -884,7 +816,89 @@ xfs_ialloc(
 
 	/* now that we have an i_mode we can setup the inode structure */
 	args->ops->setup(ip);
+}
+
+/*
+ * Allocate an inode on disk and return a copy of its in-core version.
+ * The in-core inode is locked exclusively.  Set mode, nlink, and rdev
+ * appropriately within the inode.  The uid and gid for the inode are
+ * set according to the contents of the given cred structure.
+ *
+ * Use xfs_dialloc() to allocate the on-disk inode. If xfs_dialloc()
+ * has a free inode available, call xfs_iget() to obtain the in-core
+ * version of the allocated inode.  Finally, fill in the inode and
+ * log its initial contents.  In this case, ialloc_context would be
+ * set to NULL.
+ *
+ * If xfs_dialloc() does not have an available inode, it will replenish
+ * its supply by doing an allocation. Since we can only do one
+ * allocation within a transaction without deadlocks, we must commit
+ * the current transaction before returning the inode itself.
+ * In this case, therefore, we will set ialloc_context and return.
+ * The caller should then commit the current transaction, start a new
+ * transaction, and call xfs_ialloc() again to actually get the inode.
+ *
+ * To ensure that some other process does not grab the inode that
+ * was allocated during the first call to xfs_ialloc(), this routine
+ * also returns the [locked] bp pointing to the head of the freelist
+ * as ialloc_context.  The caller should hold this buffer across
+ * the commit and pass it back into this routine on the second call.
+ *
+ * If we are allocating quota inodes, we do not have a parent inode
+ * to attach to or associate with (i.e. pip == NULL) because they
+ * are not linked into the directory structure - they are attached
+ * directly to the superblock - and so have no parent.
+ */
+static int
+xfs_ialloc(
+	struct xfs_trans		*tp,
+	const struct xfs_ialloc_args	*args,
+	struct xfs_buf			**ialloc_context,
+	struct xfs_inode		**ipp)
+{
+	struct xfs_mount		*mp = tp->t_mountp;
+	struct xfs_inode		*pip = args->pip;
+	struct xfs_inode		*ip;
+	xfs_ino_t			ino;
+	int				error;
+
+	/*
+	 * Call the space management code to pick
+	 * the on-disk inode to be allocated.
+	 */
+	error = xfs_dialloc(tp, pip ? pip->i_ino : 0, args->mode,
+			    ialloc_context, &ino);
+	if (error)
+		return error;
+	if (*ialloc_context || ino == NULLFSINO) {
+		*ipp = NULL;
+		return 0;
+	}
+	ASSERT(*ialloc_context == NULL);
+
+	/*
+	 * Protect against obviously corrupt allocation btree records. Later
+	 * xfs_iget checks will catch re-allocation of other active in-memory
+	 * and on-disk inodes. If we don't catch reallocating the parent inode
+	 * here we will deadlock in xfs_iget() so we have to do these checks
+	 * first.
+	 */
+	if ((pip && ino == pip->i_ino) || !xfs_verify_dir_ino(mp, ino)) {
+		xfs_alert(mp, "Allocated a known in-use inode 0x%llx!", ino);
+		return -EFSCORRUPTED;
+	}
+
+	/*
+	 * Get the in-core inode with the lock held exclusively.
+	 * This is because we're setting fields here we need
+	 * to prevent others from looking at until we're done.
+	 */
+	error = args->ops->iget(mp, tp, ino, &ip);
+	if (error)
+		return error;
+	ASSERT(ip != NULL);
 
+	xfs_inode_init(tp, args, ip);
 	*ipp = ip;
 	return 0;
 }

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 09/22] xfs: hoist inode allocation function
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (7 preceding siblings ...)
  2019-01-01  2:19 ` [PATCH 08/22] xfs: split inode allocation and initialization Darrick J. Wong
@ 2019-01-01  2:19 ` Darrick J. Wong
  2019-01-01  2:20 ` [PATCH 10/22] xfs: push xfs_ialloc_args creation out of xfs_dir_ialloc Darrick J. Wong
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:19 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Move the inode allocation function into libxfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_util.c |  253 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_inode_util.h |    3 
 fs/xfs/xfs_inode.c             |  250 ----------------------------------------
 3 files changed, 256 insertions(+), 250 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_util.c b/fs/xfs/libxfs/xfs_inode_util.c
index 6cc749b67d48..6c548015a882 100644
--- a/fs/xfs/libxfs/xfs_inode_util.c
+++ b/fs/xfs/libxfs/xfs_inode_util.c
@@ -3,6 +3,7 @@
  * Copyright (c) 2000-2006 Silicon Graphics, Inc.
  * All Rights Reserved.
  */
+#include <linux/iversion.h>
 #include "xfs.h"
 #include "xfs_fs.h"
 #include "xfs_shared.h"
@@ -13,6 +14,8 @@
 #include "xfs_mount.h"
 #include "xfs_inode.h"
 #include "xfs_inode_util.h"
+#include "xfs_trans.h"
+#include "xfs_ialloc.h"
 
 /*
  * helper function to extract extent size hint from inode
@@ -160,3 +163,253 @@ xfs_dic2xflags(
 
 	return flags;
 }
+
+/*
+ * Initialize a newly allocated inode with the given arguments.  Heritable
+ * inode properties will be copied from the parent if one is supplied and the
+ * appropriate inode flags are set on the parent.
+ */
+STATIC void
+xfs_inode_init(
+	struct xfs_trans		*tp,
+	const struct xfs_ialloc_args	*args,
+	struct xfs_inode		*ip)
+{
+	struct xfs_inode		*pip = args->pip;
+	struct inode			*inode = VFS_I(ip);
+	uint				flags;
+
+	/*
+	 * We always convert v1 inodes to v2 now - we only support filesystems
+	 * with >= v2 inode capability, so there is no reason for ever leaving
+	 * an inode in v1 format.
+	 */
+	if (ip->i_d.di_version == 1)
+		ip->i_d.di_version = 2;
+
+	inode->i_mode = args->mode;
+	set_nlink(inode, args->nlink);
+	ip->i_d.di_uid = args->uid;
+	ip->i_d.di_gid = args->gid;
+	inode->i_rdev = args->rdev;
+	xfs_set_projid(&ip->i_d, args->prid);
+
+	if (pip && XFS_INHERIT_GID(pip)) {
+		ip->i_d.di_gid = pip->i_d.di_gid;
+		if ((VFS_I(pip)->i_mode & S_ISGID) && S_ISDIR(args->mode))
+			inode->i_mode |= S_ISGID;
+	}
+
+	/*
+	 * If the group ID of the new file does not match the effective group
+	 * ID or one of the supplementary group IDs, the S_ISGID bit is cleared
+	 * (and only if the irix_sgid_inherit compatibility variable is set).
+	 */
+	if ((irix_sgid_inherit) &&
+	    (inode->i_mode & S_ISGID) &&
+	    (!in_group_p(xfs_gid_to_kgid(ip->i_d.di_gid))))
+		inode->i_mode &= ~S_ISGID;
+
+	ip->i_d.di_size = 0;
+	ip->i_d.di_nextents = 0;
+	ASSERT(ip->i_d.di_nblocks == 0);
+
+	inode->i_mtime.tv_sec = 0;
+	inode->i_mtime.tv_nsec = 0;
+	inode->i_atime.tv_sec = 0;
+	inode->i_atime.tv_nsec = 0;
+	inode->i_ctime.tv_sec = 0;
+	inode->i_ctime.tv_nsec = 0;
+
+	ip->i_d.di_extsize = 0;
+	ip->i_d.di_dmevmask = 0;
+	ip->i_d.di_dmstate = 0;
+	ip->i_d.di_flags = 0;
+
+	if (ip->i_d.di_version == 3) {
+		inode_set_iversion(inode, 1);
+		ip->i_d.di_flags2 = 0;
+		ip->i_d.di_cowextsize = 0;
+		ip->i_d.di_crtime.t_sec = 0;
+		ip->i_d.di_crtime.t_nsec = 0;
+	}
+
+	args->ops->platform_init(tp, args, ip);
+
+	flags = XFS_ILOG_CORE;
+	switch (args->mode & S_IFMT) {
+	case S_IFIFO:
+	case S_IFCHR:
+	case S_IFBLK:
+	case S_IFSOCK:
+		ip->i_d.di_format = XFS_DINODE_FMT_DEV;
+		ip->i_df.if_flags = 0;
+		flags |= XFS_ILOG_DEV;
+		break;
+	case S_IFREG:
+	case S_IFDIR:
+		if (pip && (pip->i_d.di_flags & XFS_DIFLAG_ANY)) {
+			uint		di_flags = 0;
+
+			if (S_ISDIR(args->mode)) {
+				if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
+					di_flags |= XFS_DIFLAG_RTINHERIT;
+				if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
+					di_flags |= XFS_DIFLAG_EXTSZINHERIT;
+					ip->i_d.di_extsize = pip->i_d.di_extsize;
+				}
+				if (pip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
+					di_flags |= XFS_DIFLAG_PROJINHERIT;
+			} else if (S_ISREG(args->mode)) {
+				if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
+					di_flags |= XFS_DIFLAG_REALTIME;
+				if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
+					di_flags |= XFS_DIFLAG_EXTSIZE;
+					ip->i_d.di_extsize = pip->i_d.di_extsize;
+				}
+			}
+			if ((pip->i_d.di_flags & XFS_DIFLAG_NOATIME) &&
+			    xfs_inherit_noatime)
+				di_flags |= XFS_DIFLAG_NOATIME;
+			if ((pip->i_d.di_flags & XFS_DIFLAG_NODUMP) &&
+			    xfs_inherit_nodump)
+				di_flags |= XFS_DIFLAG_NODUMP;
+			if ((pip->i_d.di_flags & XFS_DIFLAG_SYNC) &&
+			    xfs_inherit_sync)
+				di_flags |= XFS_DIFLAG_SYNC;
+			if ((pip->i_d.di_flags & XFS_DIFLAG_NOSYMLINKS) &&
+			    xfs_inherit_nosymlinks)
+				di_flags |= XFS_DIFLAG_NOSYMLINKS;
+			if ((pip->i_d.di_flags & XFS_DIFLAG_NODEFRAG) &&
+			    xfs_inherit_nodefrag)
+				di_flags |= XFS_DIFLAG_NODEFRAG;
+			if (pip->i_d.di_flags & XFS_DIFLAG_FILESTREAM)
+				di_flags |= XFS_DIFLAG_FILESTREAM;
+
+			ip->i_d.di_flags |= di_flags;
+		}
+		if (pip &&
+		    (pip->i_d.di_flags2 & XFS_DIFLAG2_ANY) &&
+		    pip->i_d.di_version == 3 &&
+		    ip->i_d.di_version == 3) {
+			uint64_t	di_flags2 = 0;
+
+			if (pip->i_d.di_flags2 & XFS_DIFLAG2_COWEXTSIZE) {
+				di_flags2 |= XFS_DIFLAG2_COWEXTSIZE;
+				ip->i_d.di_cowextsize = pip->i_d.di_cowextsize;
+			}
+			if (pip->i_d.di_flags2 & XFS_DIFLAG2_DAX)
+				di_flags2 |= XFS_DIFLAG2_DAX;
+
+			ip->i_d.di_flags2 |= di_flags2;
+		}
+		/* FALLTHROUGH */
+	case S_IFLNK:
+		ip->i_d.di_format = XFS_DINODE_FMT_EXTENTS;
+		ip->i_df.if_flags = XFS_IFEXTENTS;
+		ip->i_df.if_bytes = 0;
+		ip->i_df.if_u1.if_root = NULL;
+		break;
+	default:
+		ASSERT(0);
+	}
+	/*
+	 * Attribute fork settings for new inode.
+	 */
+	ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS;
+	ip->i_d.di_anextents = 0;
+
+	/*
+	 * Log the new values stuffed into the inode.
+	 */
+	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
+	xfs_trans_log_inode(tp, ip, flags);
+
+	/* now that we have an i_mode we can setup the inode structure */
+	args->ops->setup(ip);
+}
+
+/*
+ * Allocate an inode on disk and return a copy of its in-core version.
+ * The in-core inode is locked exclusively.  Set mode, nlink, and rdev
+ * appropriately within the inode.  The uid and gid for the inode are
+ * set according to the contents of the given cred structure.
+ *
+ * Use xfs_dialloc() to allocate the on-disk inode. If xfs_dialloc()
+ * has a free inode available, call xfs_iget() to obtain the in-core
+ * version of the allocated inode.  Finally, fill in the inode and
+ * log its initial contents.  In this case, ialloc_context would be
+ * set to NULL.
+ *
+ * If xfs_dialloc() does not have an available inode, it will replenish
+ * its supply by doing an allocation. Since we can only do one
+ * allocation within a transaction without deadlocks, we must commit
+ * the current transaction before returning the inode itself.
+ * In this case, therefore, we will set ialloc_context and return.
+ * The caller should then commit the current transaction, start a new
+ * transaction, and call xfs_ialloc() again to actually get the inode.
+ *
+ * To ensure that some other process does not grab the inode that
+ * was allocated during the first call to xfs_ialloc(), this routine
+ * also returns the [locked] bp pointing to the head of the freelist
+ * as ialloc_context.  The caller should hold this buffer across
+ * the commit and pass it back into this routine on the second call.
+ *
+ * If we are allocating quota inodes, we do not have a parent inode
+ * to attach to or associate with (i.e. pip == NULL) because they
+ * are not linked into the directory structure - they are attached
+ * directly to the superblock - and so have no parent.
+ */
+int
+xfs_ialloc(
+	struct xfs_trans		*tp,
+	const struct xfs_ialloc_args	*args,
+	struct xfs_buf			**ialloc_context,
+	struct xfs_inode		**ipp)
+{
+	struct xfs_mount		*mp = tp->t_mountp;
+	struct xfs_inode		*pip = args->pip;
+	struct xfs_inode		*ip;
+	xfs_ino_t			ino;
+	int				error;
+
+	/*
+	 * Call the space management code to pick
+	 * the on-disk inode to be allocated.
+	 */
+	error = xfs_dialloc(tp, pip ? pip->i_ino : 0, args->mode,
+			    ialloc_context, &ino);
+	if (error)
+		return error;
+	if (*ialloc_context || ino == NULLFSINO) {
+		*ipp = NULL;
+		return 0;
+	}
+	ASSERT(*ialloc_context == NULL);
+
+	/*
+	 * Protect against obviously corrupt allocation btree records. Later
+	 * xfs_iget checks will catch re-allocation of other active in-memory
+	 * and on-disk inodes. If we don't catch reallocating the parent inode
+	 * here we will deadlock in xfs_iget() so we have to do these checks
+	 * first.
+	 */
+	if ((pip && ino == pip->i_ino) || !xfs_verify_dir_ino(mp, ino)) {
+		xfs_alert(mp, "Allocated a known in-use inode 0x%llx!", ino);
+		return -EFSCORRUPTED;
+	}
+
+	/*
+	 * Get the in-core inode with the lock held exclusively.
+	 * This is because we're setting fields here we need
+	 * to prevent others from looking at until we're done.
+	 */
+	error = args->ops->iget(mp, tp, ino, &ip);
+	if (error)
+		return error;
+	ASSERT(ip != NULL);
+
+	xfs_inode_init(tp, args, ip);
+	*ipp = ip;
+	return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index 6318234003e1..5a1d98d1546d 100644
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -87,4 +87,7 @@ struct xfs_ialloc_ops {
 /* The libxfs client must provide this symbol. */
 extern const struct xfs_ialloc_ops xfs_default_ialloc_ops;
 
+int xfs_ialloc(struct xfs_trans *tp, const struct xfs_ialloc_args *args,
+	       struct xfs_buf **ialloc_context, struct xfs_inode **ipp);
+
 #endif /* __XFS_INODE_UTIL_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c053e1d422d5..28d794300cdd 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -653,256 +653,6 @@ const struct xfs_ialloc_ops xfs_default_ialloc_ops = {
 	.setup		= xfs_setup_inode,
 };
 
-/*
- * Initialize a newly allocated inode with the given arguments.  Heritable
- * inode properties will be copied from the parent if one is supplied and the
- * appropriate inode flags are set on the parent.
- */
-STATIC void
-xfs_inode_init(
-	struct xfs_trans		*tp,
-	const struct xfs_ialloc_args	*args,
-	struct xfs_inode		*ip)
-{
-	struct xfs_inode		*pip = args->pip;
-	struct inode			*inode = VFS_I(ip);
-	uint				flags;
-
-	/*
-	 * We always convert v1 inodes to v2 now - we only support filesystems
-	 * with >= v2 inode capability, so there is no reason for ever leaving
-	 * an inode in v1 format.
-	 */
-	if (ip->i_d.di_version == 1)
-		ip->i_d.di_version = 2;
-
-	inode->i_mode = args->mode;
-	set_nlink(inode, args->nlink);
-	ip->i_d.di_uid = args->uid;
-	ip->i_d.di_gid = args->gid;
-	inode->i_rdev = args->rdev;
-	xfs_set_projid(&ip->i_d, args->prid);
-
-	if (pip && XFS_INHERIT_GID(pip)) {
-		ip->i_d.di_gid = pip->i_d.di_gid;
-		if ((VFS_I(pip)->i_mode & S_ISGID) && S_ISDIR(args->mode))
-			inode->i_mode |= S_ISGID;
-	}
-
-	/*
-	 * If the group ID of the new file does not match the effective group
-	 * ID or one of the supplementary group IDs, the S_ISGID bit is cleared
-	 * (and only if the irix_sgid_inherit compatibility variable is set).
-	 */
-	if ((irix_sgid_inherit) &&
-	    (inode->i_mode & S_ISGID) &&
-	    (!in_group_p(xfs_gid_to_kgid(ip->i_d.di_gid))))
-		inode->i_mode &= ~S_ISGID;
-
-	ip->i_d.di_size = 0;
-	ip->i_d.di_nextents = 0;
-	ASSERT(ip->i_d.di_nblocks == 0);
-
-	inode->i_mtime.tv_sec = 0;
-	inode->i_mtime.tv_nsec = 0;
-	inode->i_atime.tv_sec = 0;
-	inode->i_atime.tv_nsec = 0;
-	inode->i_ctime.tv_sec = 0;
-	inode->i_ctime.tv_nsec = 0;
-
-	ip->i_d.di_extsize = 0;
-	ip->i_d.di_dmevmask = 0;
-	ip->i_d.di_dmstate = 0;
-	ip->i_d.di_flags = 0;
-
-	if (ip->i_d.di_version == 3) {
-		inode_set_iversion(inode, 1);
-		ip->i_d.di_flags2 = 0;
-		ip->i_d.di_cowextsize = 0;
-		ip->i_d.di_crtime.t_sec = 0;
-		ip->i_d.di_crtime.t_nsec = 0;
-	}
-
-	args->ops->platform_init(tp, args, ip);
-
-	flags = XFS_ILOG_CORE;
-	switch (args->mode & S_IFMT) {
-	case S_IFIFO:
-	case S_IFCHR:
-	case S_IFBLK:
-	case S_IFSOCK:
-		ip->i_d.di_format = XFS_DINODE_FMT_DEV;
-		ip->i_df.if_flags = 0;
-		flags |= XFS_ILOG_DEV;
-		break;
-	case S_IFREG:
-	case S_IFDIR:
-		if (pip && (pip->i_d.di_flags & XFS_DIFLAG_ANY)) {
-			uint		di_flags = 0;
-
-			if (S_ISDIR(args->mode)) {
-				if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
-					di_flags |= XFS_DIFLAG_RTINHERIT;
-				if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
-					di_flags |= XFS_DIFLAG_EXTSZINHERIT;
-					ip->i_d.di_extsize = pip->i_d.di_extsize;
-				}
-				if (pip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
-					di_flags |= XFS_DIFLAG_PROJINHERIT;
-			} else if (S_ISREG(args->mode)) {
-				if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
-					di_flags |= XFS_DIFLAG_REALTIME;
-				if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
-					di_flags |= XFS_DIFLAG_EXTSIZE;
-					ip->i_d.di_extsize = pip->i_d.di_extsize;
-				}
-			}
-			if ((pip->i_d.di_flags & XFS_DIFLAG_NOATIME) &&
-			    xfs_inherit_noatime)
-				di_flags |= XFS_DIFLAG_NOATIME;
-			if ((pip->i_d.di_flags & XFS_DIFLAG_NODUMP) &&
-			    xfs_inherit_nodump)
-				di_flags |= XFS_DIFLAG_NODUMP;
-			if ((pip->i_d.di_flags & XFS_DIFLAG_SYNC) &&
-			    xfs_inherit_sync)
-				di_flags |= XFS_DIFLAG_SYNC;
-			if ((pip->i_d.di_flags & XFS_DIFLAG_NOSYMLINKS) &&
-			    xfs_inherit_nosymlinks)
-				di_flags |= XFS_DIFLAG_NOSYMLINKS;
-			if ((pip->i_d.di_flags & XFS_DIFLAG_NODEFRAG) &&
-			    xfs_inherit_nodefrag)
-				di_flags |= XFS_DIFLAG_NODEFRAG;
-			if (pip->i_d.di_flags & XFS_DIFLAG_FILESTREAM)
-				di_flags |= XFS_DIFLAG_FILESTREAM;
-
-			ip->i_d.di_flags |= di_flags;
-		}
-		if (pip &&
-		    (pip->i_d.di_flags2 & XFS_DIFLAG2_ANY) &&
-		    pip->i_d.di_version == 3 &&
-		    ip->i_d.di_version == 3) {
-			uint64_t	di_flags2 = 0;
-
-			if (pip->i_d.di_flags2 & XFS_DIFLAG2_COWEXTSIZE) {
-				di_flags2 |= XFS_DIFLAG2_COWEXTSIZE;
-				ip->i_d.di_cowextsize = pip->i_d.di_cowextsize;
-			}
-			if (pip->i_d.di_flags2 & XFS_DIFLAG2_DAX)
-				di_flags2 |= XFS_DIFLAG2_DAX;
-
-			ip->i_d.di_flags2 |= di_flags2;
-		}
-		/* FALLTHROUGH */
-	case S_IFLNK:
-		ip->i_d.di_format = XFS_DINODE_FMT_EXTENTS;
-		ip->i_df.if_flags = XFS_IFEXTENTS;
-		ip->i_df.if_bytes = 0;
-		ip->i_df.if_u1.if_root = NULL;
-		break;
-	default:
-		ASSERT(0);
-	}
-	/*
-	 * Attribute fork settings for new inode.
-	 */
-	ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS;
-	ip->i_d.di_anextents = 0;
-
-	/*
-	 * Log the new values stuffed into the inode.
-	 */
-	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
-	xfs_trans_log_inode(tp, ip, flags);
-
-	/* now that we have an i_mode we can setup the inode structure */
-	args->ops->setup(ip);
-}
-
-/*
- * Allocate an inode on disk and return a copy of its in-core version.
- * The in-core inode is locked exclusively.  Set mode, nlink, and rdev
- * appropriately within the inode.  The uid and gid for the inode are
- * set according to the contents of the given cred structure.
- *
- * Use xfs_dialloc() to allocate the on-disk inode. If xfs_dialloc()
- * has a free inode available, call xfs_iget() to obtain the in-core
- * version of the allocated inode.  Finally, fill in the inode and
- * log its initial contents.  In this case, ialloc_context would be
- * set to NULL.
- *
- * If xfs_dialloc() does not have an available inode, it will replenish
- * its supply by doing an allocation. Since we can only do one
- * allocation within a transaction without deadlocks, we must commit
- * the current transaction before returning the inode itself.
- * In this case, therefore, we will set ialloc_context and return.
- * The caller should then commit the current transaction, start a new
- * transaction, and call xfs_ialloc() again to actually get the inode.
- *
- * To ensure that some other process does not grab the inode that
- * was allocated during the first call to xfs_ialloc(), this routine
- * also returns the [locked] bp pointing to the head of the freelist
- * as ialloc_context.  The caller should hold this buffer across
- * the commit and pass it back into this routine on the second call.
- *
- * If we are allocating quota inodes, we do not have a parent inode
- * to attach to or associate with (i.e. pip == NULL) because they
- * are not linked into the directory structure - they are attached
- * directly to the superblock - and so have no parent.
- */
-static int
-xfs_ialloc(
-	struct xfs_trans		*tp,
-	const struct xfs_ialloc_args	*args,
-	struct xfs_buf			**ialloc_context,
-	struct xfs_inode		**ipp)
-{
-	struct xfs_mount		*mp = tp->t_mountp;
-	struct xfs_inode		*pip = args->pip;
-	struct xfs_inode		*ip;
-	xfs_ino_t			ino;
-	int				error;
-
-	/*
-	 * Call the space management code to pick
-	 * the on-disk inode to be allocated.
-	 */
-	error = xfs_dialloc(tp, pip ? pip->i_ino : 0, args->mode,
-			    ialloc_context, &ino);
-	if (error)
-		return error;
-	if (*ialloc_context || ino == NULLFSINO) {
-		*ipp = NULL;
-		return 0;
-	}
-	ASSERT(*ialloc_context == NULL);
-
-	/*
-	 * Protect against obviously corrupt allocation btree records. Later
-	 * xfs_iget checks will catch re-allocation of other active in-memory
-	 * and on-disk inodes. If we don't catch reallocating the parent inode
-	 * here we will deadlock in xfs_iget() so we have to do these checks
-	 * first.
-	 */
-	if ((pip && ino == pip->i_ino) || !xfs_verify_dir_ino(mp, ino)) {
-		xfs_alert(mp, "Allocated a known in-use inode 0x%llx!", ino);
-		return -EFSCORRUPTED;
-	}
-
-	/*
-	 * Get the in-core inode with the lock held exclusively.
-	 * This is because we're setting fields here we need
-	 * to prevent others from looking at until we're done.
-	 */
-	error = args->ops->iget(mp, tp, ino, &ip);
-	if (error)
-		return error;
-	ASSERT(ip != NULL);
-
-	xfs_inode_init(tp, args, ip);
-	*ipp = ip;
-	return 0;
-}
-
 /*
  * Allocates a new inode from disk and return a pointer to the
  * incore copy. This routine will internally commit the current

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 10/22] xfs: push xfs_ialloc_args creation out of xfs_dir_ialloc
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (8 preceding siblings ...)
  2019-01-01  2:19 ` [PATCH 09/22] xfs: hoist inode allocation function Darrick J. Wong
@ 2019-01-01  2:20 ` Darrick J. Wong
  2019-01-01  2:20 ` [PATCH 11/22] xfs: refactor special inode roll " Darrick J. Wong
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:20 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Move the initialization of the xfs_ialloc_args structure out of
xfs_dir_ialloc into its callers' callers so that we can set the new
inode's parameters in one place and pass it through instead of open
coding the new uid/gid/prid all over the code.  This also prepares us
for moving xfs_dir_ialloc and xfs_create to libxfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_inode.c   |  127 ++++++++++++++++++++++----------------------------
 fs/xfs/xfs_inode.h   |   10 ++--
 fs/xfs/xfs_iops.c    |   45 +++++++++++-------
 fs/xfs/xfs_qm.c      |    7 ++-
 fs/xfs/xfs_symlink.c |   18 ++++---
 5 files changed, 108 insertions(+), 99 deletions(-)


diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 28d794300cdd..e6fda8777fe1 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -665,33 +665,16 @@ const struct xfs_ialloc_ops xfs_default_ialloc_ops = {
  */
 int
 xfs_dir_ialloc(
-	xfs_trans_t	**tpp,		/* input: current transaction;
-					   output: may be a new transaction. */
-	xfs_inode_t	*dp,		/* directory within whose allocate
-					   the inode. */
-	umode_t		mode,
-	xfs_nlink_t	nlink,
-	dev_t		rdev,
-	prid_t		prid,		/* project id */
-	xfs_inode_t	**ipp)		/* pointer to inode; it will be
-					   locked. */
+	struct xfs_trans		**tpp,
+	const struct xfs_ialloc_args	*args,
+	struct xfs_inode		**ipp)
 {
-	struct xfs_ialloc_args	args = {
-		.ops	= &xfs_default_ialloc_ops,
-		.pip	= dp,
-		.uid	= xfs_kuid_to_uid(current_fsuid()),
-		.gid	= xfs_kgid_to_gid(current_fsgid()),
-		.prid	= prid,
-		.nlink	= nlink,
-		.rdev	= rdev,
-		.mode	= mode,
-	};
-	xfs_trans_t	*tp;
-	xfs_inode_t	*ip;
-	xfs_buf_t	*ialloc_context = NULL;
-	int		code;
-	void		*dqinfo;
-	uint		tflags;
+	struct xfs_trans		*tp;
+	struct xfs_inode		*ip;
+	struct xfs_buf			*ialloc_context = NULL;
+	void				*dqinfo;
+	uint				tflags;
+	int				code;
 
 	tp = *tpp;
 	ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
@@ -711,7 +694,7 @@ xfs_dir_ialloc(
 	 * transaction commit so that no other process can steal
 	 * the inode(s) that we've just allocated.
 	 */
-	code = xfs_ialloc(tp, &args, &ialloc_context, &ip);
+	code = xfs_ialloc(tp, args, &ialloc_context, &ip);
 
 	/*
 	 * Return an error if we were unable to allocate a new inode.
@@ -780,7 +763,7 @@ xfs_dir_ialloc(
 		 * other allocations in this allocation group,
 		 * this call should always succeed.
 		 */
-		code = xfs_ialloc(tp, &args, &ialloc_context, &ip);
+		code = xfs_ialloc(tp, args, &ialloc_context, &ip);
 
 		/*
 		 * If we get an error at this point, return to the caller
@@ -840,37 +823,33 @@ xfs_bumplink(
 
 int
 xfs_create(
-	xfs_inode_t		*dp,
-	struct xfs_name		*name,
-	umode_t			mode,
-	dev_t			rdev,
-	xfs_inode_t		**ipp)
+	struct xfs_inode		*dp,
+	struct xfs_name			*name,
+	const struct xfs_ialloc_args	*args,
+	struct xfs_inode		**ipp)
 {
-	int			is_dir = S_ISDIR(mode);
-	struct xfs_mount	*mp = dp->i_mount;
-	struct xfs_inode	*ip = NULL;
-	struct xfs_trans	*tp = NULL;
-	int			error;
-	bool                    unlock_dp_on_error = false;
-	prid_t			prid;
-	struct xfs_dquot	*udqp = NULL;
-	struct xfs_dquot	*gdqp = NULL;
-	struct xfs_dquot	*pdqp = NULL;
-	struct xfs_trans_res	*tres;
-	uint			resblks;
-
+	struct xfs_mount		*mp = dp->i_mount;
+	struct xfs_inode		*ip = NULL;
+	struct xfs_trans		*tp = NULL;
+	struct xfs_dquot		*udqp = NULL;
+	struct xfs_dquot		*gdqp = NULL;
+	struct xfs_dquot		*pdqp = NULL;
+	struct xfs_trans_res		*tres;
+	uint				resblks;
+	bool				unlock_dp_on_error = false;
+	bool				is_dir = S_ISDIR(args->mode);
+	int				error;
+
+	ASSERT(args->pip == dp);
 	trace_xfs_create(dp, name);
 
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
 
-	prid = xfs_get_initial_prid(&dp->i_d);
-
 	/*
 	 * Make sure that we have allocated dquot(s) on disk.
 	 */
-	error = xfs_qm_vop_dqalloc(dp, xfs_kuid_to_uid(current_fsuid()),
-					xfs_kgid_to_gid(current_fsgid()), prid,
+	error = xfs_qm_vop_dqalloc(dp, args->uid, args->gid, args->prid,
 					XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
 					&udqp, &gdqp, &pdqp);
 	if (error)
@@ -916,7 +895,7 @@ xfs_create(
 	 * entry pointing to them, but a directory also the "." entry
 	 * pointing to itself.
 	 */
-	error = xfs_dir_ialloc(&tp, dp, mode, is_dir ? 2 : 1, rdev, prid, &ip);
+	error = xfs_dir_ialloc(&tp, args, &ip);
 	if (error)
 		goto out_trans_cancel;
 
@@ -1000,31 +979,30 @@ xfs_create(
 
 int
 xfs_create_tmpfile(
-	struct xfs_inode	*dp,
-	umode_t			mode,
-	struct xfs_inode	**ipp)
+	struct xfs_inode		*dp,
+	const struct xfs_ialloc_args	*args,
+	struct xfs_inode		**ipp)
 {
-	struct xfs_mount	*mp = dp->i_mount;
-	struct xfs_inode	*ip = NULL;
-	struct xfs_trans	*tp = NULL;
-	int			error;
-	prid_t                  prid;
-	struct xfs_dquot	*udqp = NULL;
-	struct xfs_dquot	*gdqp = NULL;
-	struct xfs_dquot	*pdqp = NULL;
-	struct xfs_trans_res	*tres;
-	uint			resblks;
+	struct xfs_mount		*mp = dp->i_mount;
+	struct xfs_inode		*ip = NULL;
+	struct xfs_trans		*tp = NULL;
+	struct xfs_dquot		*udqp = NULL;
+	struct xfs_dquot		*gdqp = NULL;
+	struct xfs_dquot		*pdqp = NULL;
+	struct xfs_trans_res		*tres;
+	uint				resblks;
+	int				error;
+
+	ASSERT(args->nlink == 1);
+	ASSERT(args->pip == dp);
 
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
 
-	prid = xfs_get_initial_prid(&dp->i_d);
-
 	/*
 	 * Make sure that we have allocated dquot(s) on disk.
 	 */
-	error = xfs_qm_vop_dqalloc(dp, xfs_kuid_to_uid(current_fsuid()),
-				xfs_kgid_to_gid(current_fsgid()), prid,
+	error = xfs_qm_vop_dqalloc(dp, args->uid, args->gid, args->prid,
 				XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
 				&udqp, &gdqp, &pdqp);
 	if (error)
@@ -1042,7 +1020,7 @@ xfs_create_tmpfile(
 	if (error)
 		goto out_trans_cancel;
 
-	error = xfs_dir_ialloc(&tp, dp, mode, 1, 0, prid, &ip);
+	error = xfs_dir_ialloc(&tp, args, &ip);
 	if (error)
 		goto out_trans_cancel;
 
@@ -2687,10 +2665,19 @@ xfs_rename_alloc_whiteout(
 	struct xfs_inode	*dp,
 	struct xfs_inode	**wip)
 {
+	struct xfs_ialloc_args	args = {
+		.ops	= &xfs_default_ialloc_ops,
+		.pip	= dp,
+		.uid	= xfs_kuid_to_uid(current_fsuid()),
+		.gid	= xfs_kgid_to_gid(current_fsgid()),
+		.prid	= xfs_get_initial_prid(&dp->i_d),
+		.nlink	= 1,
+		.mode	= S_IFCHR | WHITEOUT_MODE,
+	};
 	struct xfs_inode	*tmpfile;
 	int			error;
 
-	error = xfs_create_tmpfile(dp, S_IFCHR | WHITEOUT_MODE, &tmpfile);
+	error = xfs_create_tmpfile(dp, &args, &tmpfile);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index fb55e3b74b34..08aa747d07cf 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -386,8 +386,10 @@ void		xfs_inactive(struct xfs_inode *ip);
 int		xfs_lookup(struct xfs_inode *dp, struct xfs_name *name,
 			   struct xfs_inode **ipp, struct xfs_name *ci_name);
 int		xfs_create(struct xfs_inode *dp, struct xfs_name *name,
-			   umode_t mode, dev_t rdev, struct xfs_inode **ipp);
-int		xfs_create_tmpfile(struct xfs_inode *dp, umode_t mode,
+			   const struct xfs_ialloc_args *iargs,
+			   struct xfs_inode **ipp);
+int		xfs_create_tmpfile(struct xfs_inode *dp,
+			   const struct xfs_ialloc_args *iargs,
 			   struct xfs_inode **ipp);
 int		xfs_remove(struct xfs_inode *dp, struct xfs_name *name,
 			   struct xfs_inode *ip);
@@ -419,8 +421,8 @@ int		xfs_iflush(struct xfs_inode *, struct xfs_buf **);
 void		xfs_lock_two_inodes(struct xfs_inode *ip0, uint ip0_mode,
 				struct xfs_inode *ip1, uint ip1_mode);
 
-int		xfs_dir_ialloc(struct xfs_trans **, struct xfs_inode *, umode_t,
-			       xfs_nlink_t, dev_t, prid_t,
+int		xfs_dir_ialloc(struct xfs_trans **,
+			       const struct xfs_ialloc_args *,
 			       struct xfs_inode **);
 
 static inline int
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 1081dd376f86..2ea77189bc9f 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -130,42 +130,53 @@ xfs_cleanup_inode(
 
 STATIC int
 xfs_generic_create(
-	struct inode	*dir,
-	struct dentry	*dentry,
-	umode_t		mode,
-	dev_t		rdev,
-	bool		tmpfile)	/* unnamed file */
+	struct inode		*dir,
+	struct dentry		*dentry,
+	umode_t			mode,
+	dev_t			rdev,
+	bool			tmpfile)	/* unnamed file */
 {
-	struct inode	*inode;
-	struct xfs_inode *ip = NULL;
-	struct posix_acl *default_acl, *acl;
-	struct xfs_name	name;
-	int		error;
+	struct xfs_ialloc_args args = {
+		.ops		= &xfs_default_ialloc_ops,
+		.pip		= XFS_I(dir),
+		.uid		= xfs_kuid_to_uid(current_fsuid()),
+		.gid		= xfs_kgid_to_gid(current_fsgid()),
+		.prid		= xfs_get_initial_prid(&XFS_I(dir)->i_d),
+		.nlink		= S_ISDIR(mode) ? 2 : 1,
+		.rdev		= rdev,
+		.mode		= mode,
+	};
+	struct inode		*inode;
+	struct xfs_inode	*ip = NULL;
+	struct posix_acl	*default_acl, *acl;
+	struct xfs_name		name;
+	int			error;
 
 	/*
 	 * Irix uses Missed'em'V split, but doesn't want to see
 	 * the upper 5 bits of (14bit) major.
 	 */
-	if (S_ISCHR(mode) || S_ISBLK(mode)) {
-		if (unlikely(!sysv_valid_dev(rdev) || MAJOR(rdev) & ~0x1ff))
+	if (S_ISCHR(args.mode) || S_ISBLK(args.mode)) {
+		if (unlikely(!sysv_valid_dev(args.rdev) ||
+			     MAJOR(args.rdev) & ~0x1ff))
 			return -EINVAL;
 	} else {
-		rdev = 0;
+		args.rdev = 0;
 	}
 
-	error = posix_acl_create(dir, &mode, &default_acl, &acl);
+	error = posix_acl_create(dir, &args.mode, &default_acl, &acl);
 	if (error)
 		return error;
 
 	/* Verify mode is valid also for tmpfile case */
-	error = xfs_dentry_mode_to_name(&name, dentry, mode);
+	error = xfs_dentry_mode_to_name(&name, dentry, args.mode);
 	if (unlikely(error))
 		goto out_free_acl;
 
 	if (!tmpfile) {
-		error = xfs_create(XFS_I(dir), &name, mode, rdev, &ip);
+		error = xfs_create(XFS_I(dir), &name, &args, &ip);
 	} else {
-		error = xfs_create_tmpfile(XFS_I(dir), mode, &ip);
+		error = xfs_create_tmpfile(XFS_I(dir), &args, &ip);
 	}
 	if (unlikely(error))
 		goto out_free_acl;
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 32f400867727..58be2ef90351 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -752,6 +752,11 @@ xfs_qm_qino_alloc(
 	xfs_inode_t	**ip,
 	uint		flags)
 {
+	struct xfs_ialloc_args	args = {
+		.ops	= &xfs_default_ialloc_ops,
+		.nlink	= 1,
+		.mode	= S_IFREG,
+	};
 	xfs_trans_t	*tp;
 	int		error;
 	bool		need_alloc = true;
@@ -793,7 +798,7 @@ xfs_qm_qino_alloc(
 		return error;
 
 	if (need_alloc) {
-		error = xfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0, 0, ip);
+		error = xfs_dir_ialloc(&tp, &args, ip);
 		if (error) {
 			xfs_trans_cancel(tp);
 			return error;
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 0096cdbad34e..a34e79b57790 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -238,6 +238,15 @@ xfs_symlink(
 	umode_t			mode,
 	struct xfs_inode	**ipp)
 {
+	struct xfs_ialloc_args	args = {
+		.ops	= &xfs_default_ialloc_ops,
+		.pip	= dp,
+		.uid	= xfs_kuid_to_uid(current_fsuid()),
+		.gid	= xfs_kgid_to_gid(current_fsgid()),
+		.prid	= xfs_get_initial_prid(&dp->i_d),
+		.nlink	= 1,
+		.mode	= S_IFLNK | (mode & ~S_IFMT),
+	};
 	struct xfs_mount	*mp = dp->i_mount;
 	struct xfs_trans	*tp = NULL;
 	struct xfs_inode	*ip = NULL;
@@ -245,7 +254,6 @@ xfs_symlink(
 	int			pathlen;
 	bool                    unlock_dp_on_error = false;
 	xfs_filblks_t		fs_blocks;
-	prid_t			prid;
 	struct xfs_dquot	*udqp = NULL;
 	struct xfs_dquot	*gdqp = NULL;
 	struct xfs_dquot	*pdqp = NULL;
@@ -267,14 +275,11 @@ xfs_symlink(
 	ASSERT(pathlen > 0);
 
 	udqp = gdqp = NULL;
-	prid = xfs_get_initial_prid(&dp->i_d);
 
 	/*
 	 * Make sure that we have allocated dquot(s) on disk.
 	 */
-	error = xfs_qm_vop_dqalloc(dp,
-			xfs_kuid_to_uid(current_fsuid()),
-			xfs_kgid_to_gid(current_fsgid()), prid,
+	error = xfs_qm_vop_dqalloc(dp, args.uid, args.gid, args.prid,
 			XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
 			&udqp, &gdqp, &pdqp);
 	if (error)
@@ -316,8 +321,7 @@ xfs_symlink(
 	/*
 	 * Allocate an inode for the symlink.
 	 */
-	error = xfs_dir_ialloc(&tp, dp, S_IFLNK | (mode & ~S_IFMT), 1, 0,
-			       prid, &ip);
+	error = xfs_dir_ialloc(&tp, &args, &ip);
 	if (error)
 		goto out_trans_cancel;
 

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 11/22] xfs: refactor special inode roll out of xfs_dir_ialloc
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (9 preceding siblings ...)
  2019-01-01  2:20 ` [PATCH 10/22] xfs: push xfs_ialloc_args creation out of xfs_dir_ialloc Darrick J. Wong
@ 2019-01-01  2:20 ` Darrick J. Wong
  2019-01-17 14:24   ` Christoph Hellwig
  2019-01-01  2:20 ` [PATCH 12/22] xfs: move xfs_dir_ialloc to libxfs Darrick J. Wong
                   ` (10 subsequent siblings)
  21 siblings, 1 reply; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:20 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

In xfs_dir_ialloc, we roll the transaction if we had to allocate a new
inode chunk and before we actually initialize the inode.  In the kernel
this requires us to detach the transaction's quota charge information
from the ichunk allocation transaction and to attach it the ialloc
transaction because we don't charge quota for inode chunks.  This
doesn't exist in the userspace side of things, so pop it out into a
separately called function.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_util.h |    6 ++++
 fs/xfs/xfs_inode.c             |   66 ++++++++++++++++++++++++----------------
 2 files changed, 46 insertions(+), 26 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index 5a1d98d1546d..ee274d74b8d4 100644
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -82,6 +82,12 @@ struct xfs_ialloc_ops {
 
 	/* Do any final setup needed before we return the inode. */
 	void (*setup)(struct xfs_inode *ip);
+
+	/*
+	 * Roll the transaction between allocating a new ichunk and
+	 * initializing a new inode core.
+	 */
+	int (*ichunk_roll)(struct xfs_trans **tpp);
 };
 
 /* The libxfs client must provide this symbol. */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index e6fda8777fe1..82de92bc84b9 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -647,10 +647,49 @@ xfs_ialloc_platform_init(
 	VFS_I(ip)->i_ctime = tv;
 }
 
+/*
+ * Roll the transaction after allocating an inode chunk and before allocating
+ * the actual inode, moving the quota charge information to the second
+ * transaction.
+ */
+static int
+xfs_dir_ialloc_roll(
+	struct xfs_trans	**tpp)
+{
+	struct xfs_dquot_acct	*dqinfo = NULL;
+	unsigned int		tflags = 0;
+	int			error;
+
+	/*
+	 * We want the quota changes to be associated with the next
+	 * transaction, NOT this one. So, detach the dqinfo from this
+	 * and attach it to the next transaction.
+	 */
+	if ((*tpp)->t_dqinfo) {
+		dqinfo = (*tpp)->t_dqinfo;
+		(*tpp)->t_dqinfo = NULL;
+		tflags = (*tpp)->t_flags & XFS_TRANS_DQ_DIRTY;
+		(*tpp)->t_flags &= ~(XFS_TRANS_DQ_DIRTY);
+	}
+
+	error = xfs_trans_roll(tpp);
+
+	/*
+	 * Re-attach the quota info that we detached from prev trx.
+	 */
+	if (dqinfo) {
+		(*tpp)->t_dqinfo = dqinfo;
+		(*tpp)->t_flags |= tflags;
+	}
+
+	return error;
+}
+
 const struct xfs_ialloc_ops xfs_default_ialloc_ops = {
 	.iget		= xfs_ialloc_iget,
 	.platform_init	= xfs_ialloc_platform_init,
 	.setup		= xfs_setup_inode,
+	.ichunk_roll	= xfs_dir_ialloc_roll,
 };
 
 /*
@@ -672,8 +711,6 @@ xfs_dir_ialloc(
 	struct xfs_trans		*tp;
 	struct xfs_inode		*ip;
 	struct xfs_buf			*ialloc_context = NULL;
-	void				*dqinfo;
-	uint				tflags;
 	int				code;
 
 	tp = *tpp;
@@ -726,30 +763,7 @@ xfs_dir_ialloc(
 		 */
 		xfs_trans_bhold(tp, ialloc_context);
 
-		/*
-		 * We want the quota changes to be associated with the next
-		 * transaction, NOT this one. So, detach the dqinfo from this
-		 * and attach it to the next transaction.
-		 */
-		dqinfo = NULL;
-		tflags = 0;
-		if (tp->t_dqinfo) {
-			dqinfo = (void *)tp->t_dqinfo;
-			tp->t_dqinfo = NULL;
-			tflags = tp->t_flags & XFS_TRANS_DQ_DIRTY;
-			tp->t_flags &= ~(XFS_TRANS_DQ_DIRTY);
-		}
-
-		code = xfs_trans_roll(&tp);
-
-		/*
-		 * Re-attach the quota info that we detached from prev trx.
-		 */
-		if (dqinfo) {
-			tp->t_dqinfo = dqinfo;
-			tp->t_flags |= tflags;
-		}
-
+		code = args->ops->ichunk_roll(&tp);
 		if (code) {
 			xfs_buf_relse(ialloc_context);
 			*tpp = tp;

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 12/22] xfs: move xfs_dir_ialloc to libxfs
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (10 preceding siblings ...)
  2019-01-01  2:20 ` [PATCH 11/22] xfs: refactor special inode roll " Darrick J. Wong
@ 2019-01-01  2:20 ` Darrick J. Wong
  2019-01-01  2:20 ` [PATCH 13/22] xfs: hoist xfs_iunlink " Darrick J. Wong
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:20 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Move xfs_dir_ialloc to libxfs, and make xfs_ialloc static since we only
needed it to be non-static temporarily.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_util.c |  108 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_inode_util.h |    4 +
 fs/xfs/xfs_inode.c             |  106 ---------------------------------------
 fs/xfs/xfs_inode.h             |    4 -
 4 files changed, 109 insertions(+), 113 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_util.c b/fs/xfs/libxfs/xfs_inode_util.c
index 6c548015a882..fa9baea5be6d 100644
--- a/fs/xfs/libxfs/xfs_inode_util.c
+++ b/fs/xfs/libxfs/xfs_inode_util.c
@@ -360,7 +360,7 @@ xfs_inode_init(
  * are not linked into the directory structure - they are attached
  * directly to the superblock - and so have no parent.
  */
-int
+STATIC int
 xfs_ialloc(
 	struct xfs_trans		*tp,
 	const struct xfs_ialloc_args	*args,
@@ -413,3 +413,109 @@ xfs_ialloc(
 	*ipp = ip;
 	return 0;
 }
+
+/*
+ * Allocates a new inode from disk and return a pointer to the
+ * incore copy. This routine will internally commit the current
+ * transaction and allocate a new one if the Space Manager needed
+ * to do an allocation to replenish the inode free-list.
+ *
+ * This routine is designed to be called from xfs_create and
+ * xfs_create_dir.
+ *
+ */
+int
+xfs_dir_ialloc(
+	struct xfs_trans		**tpp,
+	const struct xfs_ialloc_args	*args,
+	struct xfs_inode		**ipp)
+{
+	struct xfs_trans		*tp;
+	struct xfs_inode		*ip;
+	struct xfs_buf			*ialloc_context = NULL;
+	int				error;
+
+	tp = *tpp;
+	ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
+
+	/*
+	 * xfs_ialloc will return a pointer to an incore inode if
+	 * the Space Manager has an available inode on the free
+	 * list. Otherwise, it will do an allocation and replenish
+	 * the freelist.  Since we can only do one allocation per
+	 * transaction without deadlocks, we will need to commit the
+	 * current transaction and start a new one.  We will then
+	 * need to call xfs_ialloc again to get the inode.
+	 *
+	 * If xfs_ialloc did an allocation to replenish the freelist,
+	 * it returns the bp containing the head of the freelist as
+	 * ialloc_context. We will hold a lock on it across the
+	 * transaction commit so that no other process can steal
+	 * the inode(s) that we've just allocated.
+	 */
+	error = xfs_ialloc(tp, args, &ialloc_context, &ip);
+
+	/*
+	 * Return an error if we were unable to allocate a new inode.
+	 * This should only happen if we run out of space on disk or
+	 * encounter a disk error.
+	 */
+	if (error) {
+		*ipp = NULL;
+		return error;
+	}
+	if (!ialloc_context && !ip) {
+		*ipp = NULL;
+		return -ENOSPC;
+	}
+
+	/*
+	 * If the AGI buffer is non-NULL, then we were unable to get an
+	 * inode in one operation.  We need to commit the current
+	 * transaction and call xfs_ialloc() again.  It is guaranteed
+	 * to succeed the second time.
+	 */
+	if (ialloc_context) {
+		/*
+		 * Normally, xfs_trans_commit releases all the locks.
+		 * We call bhold to hang on to the ialloc_context across
+		 * the commit.  Holding this buffer prevents any other
+		 * processes from doing any allocations in this
+		 * allocation group.
+		 */
+		xfs_trans_bhold(tp, ialloc_context);
+
+		error = args->ops->ichunk_roll(&tp);
+		if (error) {
+			xfs_buf_relse(ialloc_context);
+			*tpp = tp;
+			*ipp = NULL;
+			return error;
+		}
+		xfs_trans_bjoin(tp, ialloc_context);
+
+		/*
+		 * Call ialloc again. Since we've locked out all
+		 * other allocations in this allocation group,
+		 * this call should always succeed.
+		 */
+		error = xfs_ialloc(tp, args, &ialloc_context, &ip);
+
+		/*
+		 * If we get an error at this point, return to the caller
+		 * so that the current transaction can be aborted.
+		 */
+		if (error) {
+			*tpp = tp;
+			*ipp = NULL;
+			return error;
+		}
+		ASSERT(!ialloc_context && ip);
+
+	}
+
+	*ipp = ip;
+	*tpp = tp;
+
+	return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index ee274d74b8d4..5e2608f99fad 100644
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -93,7 +93,7 @@ struct xfs_ialloc_ops {
 /* The libxfs client must provide this symbol. */
 extern const struct xfs_ialloc_ops xfs_default_ialloc_ops;
 
-int xfs_ialloc(struct xfs_trans *tp, const struct xfs_ialloc_args *args,
-	       struct xfs_buf **ialloc_context, struct xfs_inode **ipp);
+int xfs_dir_ialloc(struct xfs_trans **tpp, const struct xfs_ialloc_args *args,
+		   struct xfs_inode **ipp);
 
 #endif /* __XFS_INODE_UTIL_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 82de92bc84b9..167fe4ec48bd 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -692,112 +692,6 @@ const struct xfs_ialloc_ops xfs_default_ialloc_ops = {
 	.ichunk_roll	= xfs_dir_ialloc_roll,
 };
 
-/*
- * Allocates a new inode from disk and return a pointer to the
- * incore copy. This routine will internally commit the current
- * transaction and allocate a new one if the Space Manager needed
- * to do an allocation to replenish the inode free-list.
- *
- * This routine is designed to be called from xfs_create and
- * xfs_create_dir.
- *
- */
-int
-xfs_dir_ialloc(
-	struct xfs_trans		**tpp,
-	const struct xfs_ialloc_args	*args,
-	struct xfs_inode		**ipp)
-{
-	struct xfs_trans		*tp;
-	struct xfs_inode		*ip;
-	struct xfs_buf			*ialloc_context = NULL;
-	int				code;
-
-	tp = *tpp;
-	ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
-
-	/*
-	 * xfs_ialloc will return a pointer to an incore inode if
-	 * the Space Manager has an available inode on the free
-	 * list. Otherwise, it will do an allocation and replenish
-	 * the freelist.  Since we can only do one allocation per
-	 * transaction without deadlocks, we will need to commit the
-	 * current transaction and start a new one.  We will then
-	 * need to call xfs_ialloc again to get the inode.
-	 *
-	 * If xfs_ialloc did an allocation to replenish the freelist,
-	 * it returns the bp containing the head of the freelist as
-	 * ialloc_context. We will hold a lock on it across the
-	 * transaction commit so that no other process can steal
-	 * the inode(s) that we've just allocated.
-	 */
-	code = xfs_ialloc(tp, args, &ialloc_context, &ip);
-
-	/*
-	 * Return an error if we were unable to allocate a new inode.
-	 * This should only happen if we run out of space on disk or
-	 * encounter a disk error.
-	 */
-	if (code) {
-		*ipp = NULL;
-		return code;
-	}
-	if (!ialloc_context && !ip) {
-		*ipp = NULL;
-		return -ENOSPC;
-	}
-
-	/*
-	 * If the AGI buffer is non-NULL, then we were unable to get an
-	 * inode in one operation.  We need to commit the current
-	 * transaction and call xfs_ialloc() again.  It is guaranteed
-	 * to succeed the second time.
-	 */
-	if (ialloc_context) {
-		/*
-		 * Normally, xfs_trans_commit releases all the locks.
-		 * We call bhold to hang on to the ialloc_context across
-		 * the commit.  Holding this buffer prevents any other
-		 * processes from doing any allocations in this
-		 * allocation group.
-		 */
-		xfs_trans_bhold(tp, ialloc_context);
-
-		code = args->ops->ichunk_roll(&tp);
-		if (code) {
-			xfs_buf_relse(ialloc_context);
-			*tpp = tp;
-			*ipp = NULL;
-			return code;
-		}
-		xfs_trans_bjoin(tp, ialloc_context);
-
-		/*
-		 * Call ialloc again. Since we've locked out all
-		 * other allocations in this allocation group,
-		 * this call should always succeed.
-		 */
-		code = xfs_ialloc(tp, args, &ialloc_context, &ip);
-
-		/*
-		 * If we get an error at this point, return to the caller
-		 * so that the current transaction can be aborted.
-		 */
-		if (code) {
-			*tpp = tp;
-			*ipp = NULL;
-			return code;
-		}
-		ASSERT(!ialloc_context && ip);
-
-	}
-
-	*ipp = ip;
-	*tpp = tp;
-
-	return 0;
-}
-
 /*
  * Decrement the link count on an inode & log the change.  If this causes the
  * link count to go to zero, move the inode to AGI unlinked list so that it can
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 08aa747d07cf..211b7d85bf62 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -421,10 +421,6 @@ int		xfs_iflush(struct xfs_inode *, struct xfs_buf **);
 void		xfs_lock_two_inodes(struct xfs_inode *ip0, uint ip0_mode,
 				struct xfs_inode *ip1, uint ip1_mode);
 
-int		xfs_dir_ialloc(struct xfs_trans **,
-			       const struct xfs_ialloc_args *,
-			       struct xfs_inode **);
-
 static inline int
 xfs_itruncate_extents(
 	struct xfs_trans	**tpp,

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 13/22] xfs: hoist xfs_iunlink to libxfs
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (11 preceding siblings ...)
  2019-01-01  2:20 ` [PATCH 12/22] xfs: move xfs_dir_ialloc to libxfs Darrick J. Wong
@ 2019-01-01  2:20 ` Darrick J. Wong
  2019-01-01  2:20 ` [PATCH 14/22] xfs: hoist xfs_{bump,drop}link " Darrick J. Wong
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:20 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Move xfs_iunlink and xfs_iunlink_remove to libxfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_util.c |  267 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_inode_util.h |    3 
 fs/xfs/xfs_inode.c             |  268 ----------------------------------------
 3 files changed, 270 insertions(+), 268 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_util.c b/fs/xfs/libxfs/xfs_inode_util.c
index fa9baea5be6d..b67d72cb8e38 100644
--- a/fs/xfs/libxfs/xfs_inode_util.c
+++ b/fs/xfs/libxfs/xfs_inode_util.c
@@ -16,6 +16,7 @@
 #include "xfs_inode_util.h"
 #include "xfs_trans.h"
 #include "xfs_ialloc.h"
+#include "xfs_error.h"
 
 /*
  * helper function to extract extent size hint from inode
@@ -519,3 +520,269 @@ xfs_dir_ialloc(
 
 	return 0;
 }
+
+/*
+ * This is called when the inode's link count goes to 0 or we are creating a
+ * tmpfile via O_TMPFILE. In the case of a tmpfile, @ignore_linkcount will be
+ * set to true as the link count is dropped to zero by the VFS after we've
+ * created the file successfully, so we have to add it to the unlinked list
+ * while the link count is non-zero.
+ *
+ * We place the on-disk inode on a list in the AGI.  It will be pulled from this
+ * list when the inode is freed.
+ */
+int
+xfs_iunlink(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip)
+{
+	struct xfs_mount	*mp = tp->t_mountp;
+	struct xfs_agi		*agi;
+	struct xfs_dinode	*dip;
+	struct xfs_buf		*agibp;
+	struct xfs_buf		*ibp;
+	xfs_agino_t		agino;
+	short			bucket_index;
+	int			offset;
+	int			error;
+
+	ASSERT(VFS_I(ip)->i_mode != 0);
+
+	/*
+	 * Get the agi buffer first.  It ensures lock ordering
+	 * on the list.
+	 */
+	error = xfs_read_agi(mp, tp, XFS_INO_TO_AGNO(mp, ip->i_ino), &agibp);
+	if (error)
+		return error;
+	agi = XFS_BUF_TO_AGI(agibp);
+
+	/*
+	 * Get the index into the agi hash table for the
+	 * list this inode will go on.
+	 */
+	agino = XFS_INO_TO_AGINO(mp, ip->i_ino);
+	ASSERT(agino != 0);
+	bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS;
+	ASSERT(agi->agi_unlinked[bucket_index]);
+	ASSERT(be32_to_cpu(agi->agi_unlinked[bucket_index]) != agino);
+
+	if (agi->agi_unlinked[bucket_index] != cpu_to_be32(NULLAGINO)) {
+		/*
+		 * There is already another inode in the bucket we need
+		 * to add ourselves to.  Add us at the front of the list.
+		 * Here we put the head pointer into our next pointer,
+		 * and then we fall through to point the head at us.
+		 */
+		error = xfs_imap_to_bp(mp, tp, &ip->i_imap, &dip, &ibp,
+				       0, 0);
+		if (error)
+			return error;
+
+		ASSERT(dip->di_next_unlinked == cpu_to_be32(NULLAGINO));
+		dip->di_next_unlinked = agi->agi_unlinked[bucket_index];
+		offset = ip->i_imap.im_boffset +
+			offsetof(xfs_dinode_t, di_next_unlinked);
+
+		/* need to recalc the inode CRC if appropriate */
+		xfs_dinode_calc_crc(mp, dip);
+
+		xfs_trans_inode_buf(tp, ibp);
+		xfs_trans_log_buf(tp, ibp, offset,
+				  (offset + sizeof(xfs_agino_t) - 1));
+		xfs_inobp_check(mp, ibp);
+	}
+
+	/*
+	 * Point the bucket head pointer at the inode being inserted.
+	 */
+	ASSERT(agino != 0);
+	agi->agi_unlinked[bucket_index] = cpu_to_be32(agino);
+	offset = offsetof(xfs_agi_t, agi_unlinked) +
+		(sizeof(xfs_agino_t) * bucket_index);
+	xfs_trans_log_buf(tp, agibp, offset,
+			  (offset + sizeof(xfs_agino_t) - 1));
+	return 0;
+}
+
+/*
+ * Pull the on-disk inode from the AGI unlinked list.
+ */
+int
+xfs_iunlink_remove(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip)
+{
+	struct xfs_mount	*mp;
+	struct xfs_agi		*agi;
+	struct xfs_dinode	*dip;
+	struct xfs_buf		*agibp;
+	struct xfs_buf		*ibp;
+	struct xfs_buf		*last_ibp;
+	struct xfs_dinode	*last_dip = NULL;
+	xfs_ino_t		next_ino;
+	xfs_agnumber_t		agno;
+	xfs_agino_t		agino;
+	xfs_agino_t		next_agino;
+	short			bucket_index;
+	int			offset, last_offset = 0;
+	int			error;
+
+	mp = tp->t_mountp;
+	agno = XFS_INO_TO_AGNO(mp, ip->i_ino);
+
+	/*
+	 * Get the agi buffer first.  It ensures lock ordering
+	 * on the list.
+	 */
+	error = xfs_read_agi(mp, tp, agno, &agibp);
+	if (error)
+		return error;
+
+	agi = XFS_BUF_TO_AGI(agibp);
+
+	/*
+	 * Get the index into the agi hash table for the
+	 * list this inode will go on.
+	 */
+	agino = XFS_INO_TO_AGINO(mp, ip->i_ino);
+	if (!xfs_verify_agino(mp, agno, agino))
+		return -EFSCORRUPTED;
+	bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS;
+	if (!xfs_verify_agino(mp, agno,
+			be32_to_cpu(agi->agi_unlinked[bucket_index]))) {
+		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+				agi, sizeof(*agi));
+		return -EFSCORRUPTED;
+	}
+
+	if (be32_to_cpu(agi->agi_unlinked[bucket_index]) == agino) {
+		/*
+		 * We're at the head of the list.  Get the inode's on-disk
+		 * buffer to see if there is anyone after us on the list.
+		 * Only modify our next pointer if it is not already NULLAGINO.
+		 * This saves us the overhead of dealing with the buffer when
+		 * there is no need to change it.
+		 */
+		error = xfs_imap_to_bp(mp, tp, &ip->i_imap, &dip, &ibp,
+				       0, 0);
+		if (error) {
+			xfs_warn(mp, "%s: xfs_imap_to_bp returned error %d.",
+				__func__, error);
+			return error;
+		}
+		next_agino = be32_to_cpu(dip->di_next_unlinked);
+		ASSERT(next_agino != 0);
+		if (next_agino != NULLAGINO) {
+			dip->di_next_unlinked = cpu_to_be32(NULLAGINO);
+			offset = ip->i_imap.im_boffset +
+				offsetof(xfs_dinode_t, di_next_unlinked);
+
+			/* need to recalc the inode CRC if appropriate */
+			xfs_dinode_calc_crc(mp, dip);
+
+			xfs_trans_inode_buf(tp, ibp);
+			xfs_trans_log_buf(tp, ibp, offset,
+					  (offset + sizeof(xfs_agino_t) - 1));
+			xfs_inobp_check(mp, ibp);
+		} else {
+			xfs_trans_brelse(tp, ibp);
+		}
+		/*
+		 * Point the bucket head pointer at the next inode.
+		 */
+		ASSERT(next_agino != 0);
+		ASSERT(next_agino != agino);
+		agi->agi_unlinked[bucket_index] = cpu_to_be32(next_agino);
+		offset = offsetof(xfs_agi_t, agi_unlinked) +
+			(sizeof(xfs_agino_t) * bucket_index);
+		xfs_trans_log_buf(tp, agibp, offset,
+				  (offset + sizeof(xfs_agino_t) - 1));
+	} else {
+		/*
+		 * We need to search the list for the inode being freed.
+		 */
+		next_agino = be32_to_cpu(agi->agi_unlinked[bucket_index]);
+		last_ibp = NULL;
+		while (next_agino != agino) {
+			struct xfs_imap	imap;
+
+			if (last_ibp)
+				xfs_trans_brelse(tp, last_ibp);
+
+			imap.im_blkno = 0;
+			next_ino = XFS_AGINO_TO_INO(mp, agno, next_agino);
+
+			error = xfs_imap(mp, tp, next_ino, &imap, 0);
+			if (error) {
+				xfs_warn(mp,
+	"%s: xfs_imap returned error %d.",
+					 __func__, error);
+				return error;
+			}
+
+			error = xfs_imap_to_bp(mp, tp, &imap, &last_dip,
+					       &last_ibp, 0, 0);
+			if (error) {
+				xfs_warn(mp,
+	"%s: xfs_imap_to_bp returned error %d.",
+					__func__, error);
+				return error;
+			}
+
+			last_offset = imap.im_boffset;
+			next_agino = be32_to_cpu(last_dip->di_next_unlinked);
+			if (!xfs_verify_agino(mp, agno, next_agino)) {
+				XFS_CORRUPTION_ERROR(__func__,
+						XFS_ERRLEVEL_LOW, mp,
+						last_dip, sizeof(*last_dip));
+				return -EFSCORRUPTED;
+			}
+		}
+
+		/*
+		 * Now last_ibp points to the buffer previous to us on the
+		 * unlinked list.  Pull us from the list.
+		 */
+		error = xfs_imap_to_bp(mp, tp, &ip->i_imap, &dip, &ibp,
+				       0, 0);
+		if (error) {
+			xfs_warn(mp, "%s: xfs_imap_to_bp(2) returned error %d.",
+				__func__, error);
+			return error;
+		}
+		next_agino = be32_to_cpu(dip->di_next_unlinked);
+		ASSERT(next_agino != 0);
+		ASSERT(next_agino != agino);
+		if (next_agino != NULLAGINO) {
+			dip->di_next_unlinked = cpu_to_be32(NULLAGINO);
+			offset = ip->i_imap.im_boffset +
+				offsetof(xfs_dinode_t, di_next_unlinked);
+
+			/* need to recalc the inode CRC if appropriate */
+			xfs_dinode_calc_crc(mp, dip);
+
+			xfs_trans_inode_buf(tp, ibp);
+			xfs_trans_log_buf(tp, ibp, offset,
+					  (offset + sizeof(xfs_agino_t) - 1));
+			xfs_inobp_check(mp, ibp);
+		} else {
+			xfs_trans_brelse(tp, ibp);
+		}
+		/*
+		 * Point the previous inode on the list to the next inode.
+		 */
+		last_dip->di_next_unlinked = cpu_to_be32(next_agino);
+		ASSERT(next_agino != 0);
+		offset = last_offset + offsetof(xfs_dinode_t, di_next_unlinked);
+
+		/* need to recalc the inode CRC if appropriate */
+		xfs_dinode_calc_crc(mp, last_dip);
+
+		xfs_trans_inode_buf(tp, last_ibp);
+		xfs_trans_log_buf(tp, last_ibp, offset,
+				  (offset + sizeof(xfs_agino_t) - 1));
+		xfs_inobp_check(mp, last_ibp);
+	}
+	return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index 5e2608f99fad..bb403d3386ad 100644
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -96,4 +96,7 @@ extern const struct xfs_ialloc_ops xfs_default_ialloc_ops;
 int xfs_dir_ialloc(struct xfs_trans **tpp, const struct xfs_ialloc_args *args,
 		   struct xfs_inode **ipp);
 
+int xfs_iunlink(struct xfs_trans *tp, struct xfs_inode *ip);
+int xfs_iunlink_remove(struct xfs_trans *tp, struct xfs_inode *ip);
+
 #endif /* __XFS_INODE_UTIL_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 167fe4ec48bd..30c2f1076db0 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -47,8 +47,6 @@
 kmem_zone_t *xfs_inode_zone;
 
 STATIC int xfs_iflush_int(struct xfs_inode *, struct xfs_buf *);
-STATIC int xfs_iunlink(struct xfs_trans *, struct xfs_inode *);
-STATIC int xfs_iunlink_remove(struct xfs_trans *, struct xfs_inode *);
 
 /*
  * These two are wrapper routines around the xfs_ilock() routine used to
@@ -1653,272 +1651,6 @@ xfs_inactive(
 	xfs_qm_dqdetach(ip);
 }
 
-/*
- * This is called when the inode's link count goes to 0 or we are creating a
- * tmpfile via O_TMPFILE. In the case of a tmpfile, @ignore_linkcount will be
- * set to true as the link count is dropped to zero by the VFS after we've
- * created the file successfully, so we have to add it to the unlinked list
- * while the link count is non-zero.
- *
- * We place the on-disk inode on a list in the AGI.  It will be pulled from this
- * list when the inode is freed.
- */
-STATIC int
-xfs_iunlink(
-	struct xfs_trans *tp,
-	struct xfs_inode *ip)
-{
-	xfs_mount_t	*mp = tp->t_mountp;
-	xfs_agi_t	*agi;
-	xfs_dinode_t	*dip;
-	xfs_buf_t	*agibp;
-	xfs_buf_t	*ibp;
-	xfs_agino_t	agino;
-	short		bucket_index;
-	int		offset;
-	int		error;
-
-	ASSERT(VFS_I(ip)->i_mode != 0);
-
-	/*
-	 * Get the agi buffer first.  It ensures lock ordering
-	 * on the list.
-	 */
-	error = xfs_read_agi(mp, tp, XFS_INO_TO_AGNO(mp, ip->i_ino), &agibp);
-	if (error)
-		return error;
-	agi = XFS_BUF_TO_AGI(agibp);
-
-	/*
-	 * Get the index into the agi hash table for the
-	 * list this inode will go on.
-	 */
-	agino = XFS_INO_TO_AGINO(mp, ip->i_ino);
-	ASSERT(agino != 0);
-	bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS;
-	ASSERT(agi->agi_unlinked[bucket_index]);
-	ASSERT(be32_to_cpu(agi->agi_unlinked[bucket_index]) != agino);
-
-	if (agi->agi_unlinked[bucket_index] != cpu_to_be32(NULLAGINO)) {
-		/*
-		 * There is already another inode in the bucket we need
-		 * to add ourselves to.  Add us at the front of the list.
-		 * Here we put the head pointer into our next pointer,
-		 * and then we fall through to point the head at us.
-		 */
-		error = xfs_imap_to_bp(mp, tp, &ip->i_imap, &dip, &ibp,
-				       0, 0);
-		if (error)
-			return error;
-
-		ASSERT(dip->di_next_unlinked == cpu_to_be32(NULLAGINO));
-		dip->di_next_unlinked = agi->agi_unlinked[bucket_index];
-		offset = ip->i_imap.im_boffset +
-			offsetof(xfs_dinode_t, di_next_unlinked);
-
-		/* need to recalc the inode CRC if appropriate */
-		xfs_dinode_calc_crc(mp, dip);
-
-		xfs_trans_inode_buf(tp, ibp);
-		xfs_trans_log_buf(tp, ibp, offset,
-				  (offset + sizeof(xfs_agino_t) - 1));
-		xfs_inobp_check(mp, ibp);
-	}
-
-	/*
-	 * Point the bucket head pointer at the inode being inserted.
-	 */
-	ASSERT(agino != 0);
-	agi->agi_unlinked[bucket_index] = cpu_to_be32(agino);
-	offset = offsetof(xfs_agi_t, agi_unlinked) +
-		(sizeof(xfs_agino_t) * bucket_index);
-	xfs_trans_log_buf(tp, agibp, offset,
-			  (offset + sizeof(xfs_agino_t) - 1));
-	return 0;
-}
-
-/*
- * Pull the on-disk inode from the AGI unlinked list.
- */
-STATIC int
-xfs_iunlink_remove(
-	xfs_trans_t	*tp,
-	xfs_inode_t	*ip)
-{
-	xfs_ino_t	next_ino;
-	xfs_mount_t	*mp;
-	xfs_agi_t	*agi;
-	xfs_dinode_t	*dip;
-	xfs_buf_t	*agibp;
-	xfs_buf_t	*ibp;
-	xfs_agnumber_t	agno;
-	xfs_agino_t	agino;
-	xfs_agino_t	next_agino;
-	xfs_buf_t	*last_ibp;
-	xfs_dinode_t	*last_dip = NULL;
-	short		bucket_index;
-	int		offset, last_offset = 0;
-	int		error;
-
-	mp = tp->t_mountp;
-	agno = XFS_INO_TO_AGNO(mp, ip->i_ino);
-
-	/*
-	 * Get the agi buffer first.  It ensures lock ordering
-	 * on the list.
-	 */
-	error = xfs_read_agi(mp, tp, agno, &agibp);
-	if (error)
-		return error;
-
-	agi = XFS_BUF_TO_AGI(agibp);
-
-	/*
-	 * Get the index into the agi hash table for the
-	 * list this inode will go on.
-	 */
-	agino = XFS_INO_TO_AGINO(mp, ip->i_ino);
-	if (!xfs_verify_agino(mp, agno, agino))
-		return -EFSCORRUPTED;
-	bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS;
-	if (!xfs_verify_agino(mp, agno,
-			be32_to_cpu(agi->agi_unlinked[bucket_index]))) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
-				agi, sizeof(*agi));
-		return -EFSCORRUPTED;
-	}
-
-	if (be32_to_cpu(agi->agi_unlinked[bucket_index]) == agino) {
-		/*
-		 * We're at the head of the list.  Get the inode's on-disk
-		 * buffer to see if there is anyone after us on the list.
-		 * Only modify our next pointer if it is not already NULLAGINO.
-		 * This saves us the overhead of dealing with the buffer when
-		 * there is no need to change it.
-		 */
-		error = xfs_imap_to_bp(mp, tp, &ip->i_imap, &dip, &ibp,
-				       0, 0);
-		if (error) {
-			xfs_warn(mp, "%s: xfs_imap_to_bp returned error %d.",
-				__func__, error);
-			return error;
-		}
-		next_agino = be32_to_cpu(dip->di_next_unlinked);
-		ASSERT(next_agino != 0);
-		if (next_agino != NULLAGINO) {
-			dip->di_next_unlinked = cpu_to_be32(NULLAGINO);
-			offset = ip->i_imap.im_boffset +
-				offsetof(xfs_dinode_t, di_next_unlinked);
-
-			/* need to recalc the inode CRC if appropriate */
-			xfs_dinode_calc_crc(mp, dip);
-
-			xfs_trans_inode_buf(tp, ibp);
-			xfs_trans_log_buf(tp, ibp, offset,
-					  (offset + sizeof(xfs_agino_t) - 1));
-			xfs_inobp_check(mp, ibp);
-		} else {
-			xfs_trans_brelse(tp, ibp);
-		}
-		/*
-		 * Point the bucket head pointer at the next inode.
-		 */
-		ASSERT(next_agino != 0);
-		ASSERT(next_agino != agino);
-		agi->agi_unlinked[bucket_index] = cpu_to_be32(next_agino);
-		offset = offsetof(xfs_agi_t, agi_unlinked) +
-			(sizeof(xfs_agino_t) * bucket_index);
-		xfs_trans_log_buf(tp, agibp, offset,
-				  (offset + sizeof(xfs_agino_t) - 1));
-	} else {
-		/*
-		 * We need to search the list for the inode being freed.
-		 */
-		next_agino = be32_to_cpu(agi->agi_unlinked[bucket_index]);
-		last_ibp = NULL;
-		while (next_agino != agino) {
-			struct xfs_imap	imap;
-
-			if (last_ibp)
-				xfs_trans_brelse(tp, last_ibp);
-
-			imap.im_blkno = 0;
-			next_ino = XFS_AGINO_TO_INO(mp, agno, next_agino);
-
-			error = xfs_imap(mp, tp, next_ino, &imap, 0);
-			if (error) {
-				xfs_warn(mp,
-	"%s: xfs_imap returned error %d.",
-					 __func__, error);
-				return error;
-			}
-
-			error = xfs_imap_to_bp(mp, tp, &imap, &last_dip,
-					       &last_ibp, 0, 0);
-			if (error) {
-				xfs_warn(mp,
-	"%s: xfs_imap_to_bp returned error %d.",
-					__func__, error);
-				return error;
-			}
-
-			last_offset = imap.im_boffset;
-			next_agino = be32_to_cpu(last_dip->di_next_unlinked);
-			if (!xfs_verify_agino(mp, agno, next_agino)) {
-				XFS_CORRUPTION_ERROR(__func__,
-						XFS_ERRLEVEL_LOW, mp,
-						last_dip, sizeof(*last_dip));
-				return -EFSCORRUPTED;
-			}
-		}
-
-		/*
-		 * Now last_ibp points to the buffer previous to us on the
-		 * unlinked list.  Pull us from the list.
-		 */
-		error = xfs_imap_to_bp(mp, tp, &ip->i_imap, &dip, &ibp,
-				       0, 0);
-		if (error) {
-			xfs_warn(mp, "%s: xfs_imap_to_bp(2) returned error %d.",
-				__func__, error);
-			return error;
-		}
-		next_agino = be32_to_cpu(dip->di_next_unlinked);
-		ASSERT(next_agino != 0);
-		ASSERT(next_agino != agino);
-		if (next_agino != NULLAGINO) {
-			dip->di_next_unlinked = cpu_to_be32(NULLAGINO);
-			offset = ip->i_imap.im_boffset +
-				offsetof(xfs_dinode_t, di_next_unlinked);
-
-			/* need to recalc the inode CRC if appropriate */
-			xfs_dinode_calc_crc(mp, dip);
-
-			xfs_trans_inode_buf(tp, ibp);
-			xfs_trans_log_buf(tp, ibp, offset,
-					  (offset + sizeof(xfs_agino_t) - 1));
-			xfs_inobp_check(mp, ibp);
-		} else {
-			xfs_trans_brelse(tp, ibp);
-		}
-		/*
-		 * Point the previous inode on the list to the next inode.
-		 */
-		last_dip->di_next_unlinked = cpu_to_be32(next_agino);
-		ASSERT(next_agino != 0);
-		offset = last_offset + offsetof(xfs_dinode_t, di_next_unlinked);
-
-		/* need to recalc the inode CRC if appropriate */
-		xfs_dinode_calc_crc(mp, last_dip);
-
-		xfs_trans_inode_buf(tp, last_ibp);
-		xfs_trans_log_buf(tp, last_ibp, offset,
-				  (offset + sizeof(xfs_agino_t) - 1));
-		xfs_inobp_check(mp, last_ibp);
-	}
-	return 0;
-}
-
 /*
  * A big issue when freeing the inode cluster is that we _cannot_ skip any
  * inodes that are in memory - they all must be marked stale and attached to

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 14/22] xfs: hoist xfs_{bump,drop}link to libxfs
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (12 preceding siblings ...)
  2019-01-01  2:20 ` [PATCH 13/22] xfs: hoist xfs_iunlink " Darrick J. Wong
@ 2019-01-01  2:20 ` Darrick J. Wong
  2019-01-01  2:20 ` [PATCH 15/22] xfs: create libxfs helper to link a new inode into a directory Darrick J. Wong
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:20 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Move xfs_bumplink and xfs_droplink to libxfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_util.c |   37 +++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_inode_util.h |    2 ++
 fs/xfs/xfs_inode.c             |   37 -------------------------------------
 3 files changed, 39 insertions(+), 37 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_util.c b/fs/xfs/libxfs/xfs_inode_util.c
index b67d72cb8e38..e7251f8fd96a 100644
--- a/fs/xfs/libxfs/xfs_inode_util.c
+++ b/fs/xfs/libxfs/xfs_inode_util.c
@@ -786,3 +786,40 @@ xfs_iunlink_remove(
 	}
 	return 0;
 }
+
+/*
+ * Decrement the link count on an inode & log the change.  If this causes the
+ * link count to go to zero, move the inode to AGI unlinked list so that it can
+ * be freed when the last active reference goes away via xfs_inactive().
+ */
+int
+xfs_droplink(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip)
+{
+	xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG);
+
+	drop_nlink(VFS_I(ip));
+	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+
+	if (VFS_I(ip)->i_nlink)
+		return 0;
+
+	return xfs_iunlink(tp, ip);
+}
+
+/*
+ * Increment the link count on an inode & log the change.
+ */
+int
+xfs_bumplink(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip)
+{
+	xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG);
+
+	ASSERT(ip->i_d.di_version > 1);
+	inc_nlink(VFS_I(ip));
+	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+	return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index bb403d3386ad..7258e18aeed0 100644
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -98,5 +98,7 @@ int xfs_dir_ialloc(struct xfs_trans **tpp, const struct xfs_ialloc_args *args,
 
 int xfs_iunlink(struct xfs_trans *tp, struct xfs_inode *ip);
 int xfs_iunlink_remove(struct xfs_trans *tp, struct xfs_inode *ip);
+int xfs_droplink(struct xfs_trans *tp, struct xfs_inode *ip);
+int xfs_bumplink(struct xfs_trans *tp, struct xfs_inode *ip);
 
 #endif /* __XFS_INODE_UTIL_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 30c2f1076db0..110ce01f183e 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -690,43 +690,6 @@ const struct xfs_ialloc_ops xfs_default_ialloc_ops = {
 	.ichunk_roll	= xfs_dir_ialloc_roll,
 };
 
-/*
- * Decrement the link count on an inode & log the change.  If this causes the
- * link count to go to zero, move the inode to AGI unlinked list so that it can
- * be freed when the last active reference goes away via xfs_inactive().
- */
-static int			/* error */
-xfs_droplink(
-	xfs_trans_t *tp,
-	xfs_inode_t *ip)
-{
-	xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG);
-
-	drop_nlink(VFS_I(ip));
-	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-
-	if (VFS_I(ip)->i_nlink)
-		return 0;
-
-	return xfs_iunlink(tp, ip);
-}
-
-/*
- * Increment the link count on an inode & log the change.
- */
-static int
-xfs_bumplink(
-	xfs_trans_t *tp,
-	xfs_inode_t *ip)
-{
-	xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG);
-
-	ASSERT(ip->i_d.di_version > 1);
-	inc_nlink(VFS_I(ip));
-	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-	return 0;
-}
-
 int
 xfs_create(
 	struct xfs_inode		*dp,

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 15/22] xfs: create libxfs helper to link a new inode into a directory
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (13 preceding siblings ...)
  2019-01-01  2:20 ` [PATCH 14/22] xfs: hoist xfs_{bump,drop}link " Darrick J. Wong
@ 2019-01-01  2:20 ` Darrick J. Wong
  2019-01-01  2:20 ` [PATCH 16/22] xfs: create libxfs helper to link an existing " Darrick J. Wong
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:20 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a new libxfs function to link a newly created inode into a
directory.  The upcoming metadata directory feature will need this to
create a metadata directory tree.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_dir2.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_dir2.h |    4 ++++
 fs/xfs/xfs_inode.c       |   20 ++------------------
 3 files changed, 51 insertions(+), 18 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index 229152cd1a24..aaac55668e9d 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -22,6 +22,9 @@
 #include "xfs_errortag.h"
 #include "xfs_error.h"
 #include "xfs_trace.h"
+#include "xfs_shared.h"
+#include "xfs_bmap_btree.h"
+#include "xfs_trans_space.h"
 
 struct xfs_name xfs_name_dotdot = { (unsigned char *)"..", 2, XFS_DIR3_FT_DIR };
 
@@ -703,3 +706,45 @@ xfs_dir2_shrink_inode(
 	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
 	return 0;
 }
+
+/*
+ * Given a directory @dp, a newly allocated inode @ip, and a @name, link @ip
+ * into @dp under the given @name.  If @ip is a directory, it will be
+ * initialized.  Both inodes must have the ILOCK held and the transaction must
+ * have sufficient blocks reserved.
+ */
+int
+xfs_dir_create_new_child(
+	struct xfs_trans		*tp,
+	uint				resblks,
+	struct xfs_inode		*dp,
+	struct xfs_name			*name,
+	struct xfs_inode		*ip)
+{
+	struct xfs_mount		*mp = tp->t_mountp;
+	int				error;
+
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+	ASSERT(xfs_isilocked(dp, XFS_ILOCK_EXCL));
+	ASSERT(resblks == 0 || resblks > XFS_IALLOC_SPACE_RES(mp));
+
+	error = xfs_dir_createname(tp, dp, name, ip->i_ino,
+				   resblks ?
+					resblks - XFS_IALLOC_SPACE_RES(mp) : 0);
+	if (error) {
+		ASSERT(error != -ENOSPC);
+		return error;
+	}
+
+	xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
+	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
+
+	if (!S_ISDIR(VFS_I(ip)->i_mode))
+		return 0;
+
+	error = xfs_dir_init(tp, ip, dp);
+	if (error)
+		return error;
+
+	return xfs_bumplink(tp, dp);
+}
diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
index c3e3f6b813d8..5219ca503012 100644
--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -327,4 +327,8 @@ unsigned char xfs_dir3_get_dtype(struct xfs_mount *mp, uint8_t filetype);
 void *xfs_dir3_data_endp(struct xfs_da_geometry *geo,
 		struct xfs_dir2_data_hdr *hdr);
 
+int xfs_dir_create_new_child(struct xfs_trans *tp, uint resblks,
+		struct xfs_inode *dp, struct xfs_name *name,
+		struct xfs_inode *ip);
+
 #endif	/* __XFS_DIR2_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 110ce01f183e..1469f905be36 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -778,25 +778,9 @@ xfs_create(
 	xfs_trans_ijoin(tp, dp, XFS_ILOCK_EXCL);
 	unlock_dp_on_error = false;
 
-	error = xfs_dir_createname(tp, dp, name, ip->i_ino,
-				   resblks ?
-					resblks - XFS_IALLOC_SPACE_RES(mp) : 0);
-	if (error) {
-		ASSERT(error != -ENOSPC);
+	error = xfs_dir_create_new_child(tp, resblks, dp, name, ip);
+	if (error)
 		goto out_trans_cancel;
-	}
-	xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
-
-	if (is_dir) {
-		error = xfs_dir_init(tp, ip, dp);
-		if (error)
-			goto out_trans_cancel;
-
-		error = xfs_bumplink(tp, dp);
-		if (error)
-			goto out_trans_cancel;
-	}
 
 	/*
 	 * If this is a synchronous mount, make sure that the

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 16/22] xfs: create libxfs helper to link an existing inode into a directory
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (14 preceding siblings ...)
  2019-01-01  2:20 ` [PATCH 15/22] xfs: create libxfs helper to link a new inode into a directory Darrick J. Wong
@ 2019-01-01  2:20 ` Darrick J. Wong
  2019-01-01  2:21 ` [PATCH 17/22] xfs: hoist inode free function to libxfs Darrick J. Wong
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:20 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a new libxfs function to link an existing inode into a directory.
The upcoming metadata directory feature will need this to create a
metadata directory tree.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_dir2.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_dir2.h |    3 +++
 fs/xfs/xfs_inode.c       |   24 +-----------------------
 3 files changed, 48 insertions(+), 23 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index aaac55668e9d..4cd2afa234ca 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -748,3 +748,47 @@ xfs_dir_create_new_child(
 
 	return xfs_bumplink(tp, dp);
 }
+
+/*
+ * Given a directory @dp, an existing non-directory inode @ip, and a @name,
+ * link @ip into @dp under the given @name.  Both inodes must have the ILOCK
+ * held.
+ */
+int
+xfs_dir_link_existing_child(
+	struct xfs_trans		*tp,
+	uint				resblks,
+	struct xfs_inode		*dp,
+	struct xfs_name			*name,
+	struct xfs_inode		*ip)
+{
+	int				error;
+
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+	ASSERT(xfs_isilocked(dp, XFS_ILOCK_EXCL));
+	ASSERT(!S_ISDIR(VFS_I(ip)->i_mode));
+
+	if (!resblks) {
+		error = xfs_dir_canenter(tp, dp, name);
+		if (error)
+			return error;
+	}
+
+	/*
+	 * Handle initial link state of O_TMPFILE inode
+	 */
+	if (VFS_I(ip)->i_nlink == 0) {
+		error = xfs_iunlink_remove(tp, ip);
+		if (error)
+			return error;
+	}
+
+	error = xfs_dir_createname(tp, dp, name, ip->i_ino, resblks);
+	if (error)
+		return error;
+
+	xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
+	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
+
+	return xfs_bumplink(tp, ip);
+}
diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
index 5219ca503012..7e713fcb4c40 100644
--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -330,5 +330,8 @@ void *xfs_dir3_data_endp(struct xfs_da_geometry *geo,
 int xfs_dir_create_new_child(struct xfs_trans *tp, uint resblks,
 		struct xfs_inode *dp, struct xfs_name *name,
 		struct xfs_inode *ip);
+int xfs_dir_link_existing_child(struct xfs_trans *tp, uint resblks,
+		struct xfs_inode *dp, struct xfs_name *name,
+		struct xfs_inode *ip);
 
 #endif	/* __XFS_DIR2_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 1469f905be36..189f11bba539 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -974,29 +974,7 @@ xfs_link(
 		goto error_return;
 	}
 
-	if (!resblks) {
-		error = xfs_dir_canenter(tp, tdp, target_name);
-		if (error)
-			goto error_return;
-	}
-
-	/*
-	 * Handle initial link state of O_TMPFILE inode
-	 */
-	if (VFS_I(sip)->i_nlink == 0) {
-		error = xfs_iunlink_remove(tp, sip);
-		if (error)
-			goto error_return;
-	}
-
-	error = xfs_dir_createname(tp, tdp, target_name, sip->i_ino,
-				   resblks);
-	if (error)
-		goto error_return;
-	xfs_trans_ichgtime(tp, tdp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-	xfs_trans_log_inode(tp, tdp, XFS_ILOG_CORE);
-
-	error = xfs_bumplink(tp, sip);
+	error = xfs_dir_link_existing_child(tp, resblks, tdp, target_name, sip);
 	if (error)
 		goto error_return;
 

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 17/22] xfs: hoist inode free function to libxfs
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (15 preceding siblings ...)
  2019-01-01  2:20 ` [PATCH 16/22] xfs: create libxfs helper to link an existing " Darrick J. Wong
@ 2019-01-01  2:21 ` Darrick J. Wong
  2019-01-01  2:21 ` [PATCH 18/22] xfs: create libxfs helper to remove an existing inode/name from a directory Darrick J. Wong
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:21 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a libxfs helper function that marks an inode free on disk.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_util.c |   63 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_inode_util.h |    4 +++
 fs/xfs/xfs_inode.c             |   48 +-----------------------------
 3 files changed, 68 insertions(+), 47 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_util.c b/fs/xfs/libxfs/xfs_inode_util.c
index e7251f8fd96a..01a320747ddf 100644
--- a/fs/xfs/libxfs/xfs_inode_util.c
+++ b/fs/xfs/libxfs/xfs_inode_util.c
@@ -17,6 +17,7 @@
 #include "xfs_trans.h"
 #include "xfs_ialloc.h"
 #include "xfs_error.h"
+#include "xfs_inode_item.h"
 
 /*
  * helper function to extract extent size hint from inode
@@ -823,3 +824,65 @@ xfs_bumplink(
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 	return 0;
 }
+
+/*
+ * Free any local-format buffers sitting around before we reset to
+ * extents format.
+ */
+static inline void
+xfs_ifree_local_data(
+	struct xfs_inode	*ip,
+	int			whichfork)
+{
+	struct xfs_ifork	*ifp;
+
+	if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
+		return;
+
+	ifp = XFS_IFORK_PTR(ip, whichfork);
+	xfs_idata_realloc(ip, -ifp->if_bytes, whichfork);
+}
+
+/* Mark an inode free on disk. */
+int
+xfs_dir_ifree(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip,
+	struct xfs_icluster	*xic)
+{
+	int			error;
+
+	/*
+	 * Pull the on-disk inode from the AGI unlinked list.
+	 */
+	error = xfs_iunlink_remove(tp, ip);
+	if (error)
+		return error;
+
+	error = xfs_difree(tp, ip->i_ino, xic);
+	if (error)
+		return error;
+
+	xfs_ifree_local_data(ip, XFS_DATA_FORK);
+	xfs_ifree_local_data(ip, XFS_ATTR_FORK);
+
+	VFS_I(ip)->i_mode = 0;		/* mark incore inode as free */
+	ip->i_d.di_flags = 0;
+	ip->i_d.di_flags2 = 0;
+	ip->i_d.di_dmevmask = 0;
+	ip->i_d.di_forkoff = 0;		/* mark the attr fork not in use */
+	ip->i_d.di_format = XFS_DINODE_FMT_EXTENTS;
+	ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS;
+
+	/* Don't attempt to replay owner changes for a deleted inode */
+	ip->i_itemp->ili_fields &= ~(XFS_ILOG_AOWNER | XFS_ILOG_DOWNER);
+
+	/*
+	 * Bump the generation count so no one will be confused
+	 * by reincarnations of this inode.
+	 */
+	VFS_I(ip)->i_generation++;
+	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+
+	return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index 7258e18aeed0..4918379b682e 100644
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -6,6 +6,8 @@
 #ifndef	__XFS_INODE_UTIL_H__
 #define	__XFS_INODE_UTIL_H__
 
+struct xfs_icluster;
+
 xfs_extlen_t	xfs_get_extsz_hint(struct xfs_inode *ip);
 xfs_extlen_t	xfs_get_cowextsz_hint(struct xfs_inode *ip);
 
@@ -100,5 +102,7 @@ int xfs_iunlink(struct xfs_trans *tp, struct xfs_inode *ip);
 int xfs_iunlink_remove(struct xfs_trans *tp, struct xfs_inode *ip);
 int xfs_droplink(struct xfs_trans *tp, struct xfs_inode *ip);
 int xfs_bumplink(struct xfs_trans *tp, struct xfs_inode *ip);
+int xfs_dir_ifree(struct xfs_trans *tp, struct xfs_inode *ip,
+		  struct xfs_icluster *xic);
 
 #endif /* __XFS_INODE_UTIL_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 189f11bba539..93a03cbbb9af 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1770,24 +1770,6 @@ xfs_ifree_cluster(
 	return 0;
 }
 
-/*
- * Free any local-format buffers sitting around before we reset to
- * extents format.
- */
-static inline void
-xfs_ifree_local_data(
-	struct xfs_inode	*ip,
-	int			whichfork)
-{
-	struct xfs_ifork	*ifp;
-
-	if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
-		return;
-
-	ifp = XFS_IFORK_PTR(ip, whichfork);
-	xfs_idata_realloc(ip, -ifp->if_bytes, whichfork);
-}
-
 /*
  * This is called to return an inode to the inode free list.
  * The inode should already be truncated to 0 length and have
@@ -1813,38 +1795,10 @@ xfs_ifree(
 	ASSERT(ip->i_d.di_size == 0 || !S_ISREG(VFS_I(ip)->i_mode));
 	ASSERT(ip->i_d.di_nblocks == 0);
 
-	/*
-	 * Pull the on-disk inode from the AGI unlinked list.
-	 */
-	error = xfs_iunlink_remove(tp, ip);
-	if (error)
-		return error;
-
-	error = xfs_difree(tp, ip->i_ino, &xic);
+	error = xfs_dir_ifree(tp, ip, &xic);
 	if (error)
 		return error;
 
-	xfs_ifree_local_data(ip, XFS_DATA_FORK);
-	xfs_ifree_local_data(ip, XFS_ATTR_FORK);
-
-	VFS_I(ip)->i_mode = 0;		/* mark incore inode as free */
-	ip->i_d.di_flags = 0;
-	ip->i_d.di_flags2 = 0;
-	ip->i_d.di_dmevmask = 0;
-	ip->i_d.di_forkoff = 0;		/* mark the attr fork not in use */
-	ip->i_d.di_format = XFS_DINODE_FMT_EXTENTS;
-	ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS;
-
-	/* Don't attempt to replay owner changes for a deleted inode */
-	ip->i_itemp->ili_fields &= ~(XFS_ILOG_AOWNER|XFS_ILOG_DOWNER);
-
-	/*
-	 * Bump the generation count so no one will be confused
-	 * by reincarnations of this inode.
-	 */
-	VFS_I(ip)->i_generation++;
-	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-
 	if (xic.deleted)
 		error = xfs_ifree_cluster(ip, tp, &xic);
 

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 18/22] xfs: create libxfs helper to remove an existing inode/name from a directory
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (16 preceding siblings ...)
  2019-01-01  2:21 ` [PATCH 17/22] xfs: hoist inode free function to libxfs Darrick J. Wong
@ 2019-01-01  2:21 ` Darrick J. Wong
  2019-01-01  2:21 ` [PATCH 19/22] xfs: create libxfs helper to exchange two directory entries Darrick J. Wong
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:21 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a new libxfs function to remove a (name, inode) entry from a
directory.  The upcoming metadata directory feature will need this to
create a metadata directory tree.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_dir2.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_dir2.h |    3 ++
 fs/xfs/xfs_inode.c       |   42 +-------------------------------
 3 files changed, 64 insertions(+), 41 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index 4cd2afa234ca..59f8d887f8ce 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -792,3 +792,63 @@ xfs_dir_link_existing_child(
 
 	return xfs_bumplink(tp, ip);
 }
+
+/*
+ * Given a directory @dp, a child @ip, and a @name, remove the (@name, @ip)
+ * entry from the directory.  Both inodes must have the ILOCK held.
+ */
+int
+xfs_dir_remove_child(
+	struct xfs_trans		*tp,
+	uint				resblks,
+	struct xfs_inode		*dp,
+	struct xfs_name			*name,
+	struct xfs_inode		*ip)
+{
+	int				error;
+
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+	ASSERT(xfs_isilocked(dp, XFS_ILOCK_EXCL));
+
+	/*
+	 * If we're removing a directory perform some additional validation.
+	 */
+	if (S_ISDIR(VFS_I(ip)->i_mode)) {
+		ASSERT(VFS_I(ip)->i_nlink >= 2);
+		if (VFS_I(ip)->i_nlink != 2)
+			return -ENOTEMPTY;
+		if (!xfs_dir_isempty(ip))
+			return -ENOTEMPTY;
+
+		/* Drop the link from ip's "..".  */
+		error = xfs_droplink(tp, dp);
+		if (error)
+			return error;
+
+		/* Drop the "." link from ip to self.  */
+		error = xfs_droplink(tp, ip);
+		if (error)
+			return error;
+	} else {
+		/*
+		 * When removing a non-directory we need to log the parent
+		 * inode here.  For a directory this is done implicitly
+		 * by the xfs_droplink call for the ".." entry.
+		 */
+		xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
+	}
+	xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
+
+	/* Drop the link from dp to ip. */
+	error = xfs_droplink(tp, ip);
+	if (error)
+		return error;
+
+	error = xfs_dir_removename(tp, dp, name, ip->i_ino, resblks);
+	if (error) {
+		ASSERT(error != -ENOENT);
+		return error;
+	}
+
+	return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
index 7e713fcb4c40..3da87780388e 100644
--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -333,5 +333,8 @@ int xfs_dir_create_new_child(struct xfs_trans *tp, uint resblks,
 int xfs_dir_link_existing_child(struct xfs_trans *tp, uint resblks,
 		struct xfs_inode *dp, struct xfs_name *name,
 		struct xfs_inode *ip);
+int xfs_dir_remove_child(struct xfs_trans *tp, uint resblks,
+		struct xfs_inode *dp, struct xfs_name *name,
+		struct xfs_inode *ip);
 
 #endif	/* __XFS_DIR2_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 93a03cbbb9af..a882563118c8 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1926,50 +1926,10 @@ xfs_remove(
 	xfs_trans_ijoin(tp, dp, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
 
-	/*
-	 * If we're removing a directory perform some additional validation.
-	 */
-	if (is_dir) {
-		ASSERT(VFS_I(ip)->i_nlink >= 2);
-		if (VFS_I(ip)->i_nlink != 2) {
-			error = -ENOTEMPTY;
-			goto out_trans_cancel;
-		}
-		if (!xfs_dir_isempty(ip)) {
-			error = -ENOTEMPTY;
-			goto out_trans_cancel;
-		}
-
-		/* Drop the link from ip's "..".  */
-		error = xfs_droplink(tp, dp);
-		if (error)
-			goto out_trans_cancel;
-
-		/* Drop the "." link from ip to self.  */
-		error = xfs_droplink(tp, ip);
-		if (error)
-			goto out_trans_cancel;
-	} else {
-		/*
-		 * When removing a non-directory we need to log the parent
-		 * inode here.  For a directory this is done implicitly
-		 * by the xfs_droplink call for the ".." entry.
-		 */
-		xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
-	}
-	xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-
-	/* Drop the link from dp to ip. */
-	error = xfs_droplink(tp, ip);
+	error = xfs_dir_remove_child(tp, resblks, dp, name, ip);
 	if (error)
 		goto out_trans_cancel;
 
-	error = xfs_dir_removename(tp, dp, name, ip->i_ino, resblks);
-	if (error) {
-		ASSERT(error != -ENOENT);
-		goto out_trans_cancel;
-	}
-
 	/*
 	 * If this is a synchronous mount, make sure that the
 	 * remove transaction goes to disk before returning to

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 19/22] xfs: create libxfs helper to exchange two directory entries
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (17 preceding siblings ...)
  2019-01-01  2:21 ` [PATCH 18/22] xfs: create libxfs helper to remove an existing inode/name from a directory Darrick J. Wong
@ 2019-01-01  2:21 ` Darrick J. Wong
  2019-01-01  2:21 ` [PATCH 20/22] xfs: create libxfs helper to rename " Darrick J. Wong
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:21 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a new libxfs function to exchange two directory entries.
The upcoming metadata directory feature will need this to replace a
metadata inode directory entry.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_dir2.c |  112 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_dir2.h |    4 ++
 fs/xfs/xfs_inode.c       |   89 +------------------------------------
 3 files changed, 119 insertions(+), 86 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index 59f8d887f8ce..fbd5ce580b4a 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -852,3 +852,115 @@ xfs_dir_remove_child(
 
 	return 0;
 }
+
+/*
+ * Exchange the entry (@name1, @ip1) in directory @dp1 with the entry (@name2,
+ * @ip2) in directory @dp2, and update '..' @ip1 and @ip2's entries as needed.
+ * @ip1 and @ip2 need not be of the same type.
+ *
+ * All inodes must have the ILOCK held, and both entries must already exist.
+ */
+int
+xfs_dir_exchange(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*dp1,
+	struct xfs_name		*name1,
+	struct xfs_inode	*ip1,
+	struct xfs_inode	*dp2,
+	struct xfs_name		*name2,
+	struct xfs_inode	*ip2,
+	unsigned int		spaceres)
+{
+	int			ip1_flags = 0;
+	int			ip2_flags = 0;
+	int			dp2_flags = 0;
+	int			error;
+
+	/* Swap inode number for dirent in first parent */
+	error = xfs_dir_replace(tp, dp1, name1, ip2->i_ino, spaceres);
+	if (error)
+		return error;
+
+	/* Swap inode number for dirent in second parent */
+	error = xfs_dir_replace(tp, dp2, name2, ip1->i_ino, spaceres);
+	if (error)
+		return error;
+
+	/*
+	 * If we're renaming one or more directories across different parents,
+	 * update the respective ".." entries (and link counts) to match the new
+	 * parents.
+	 */
+	if (dp1 != dp2) {
+		dp2_flags = XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
+
+		if (S_ISDIR(VFS_I(ip2)->i_mode)) {
+			error = xfs_dir_replace(tp, ip2, &xfs_name_dotdot,
+						dp1->i_ino, spaceres);
+			if (error)
+				return error;
+
+			/* transfer ip2 ".." reference to dp1 */
+			if (!S_ISDIR(VFS_I(ip1)->i_mode)) {
+				error = xfs_droplink(tp, dp2);
+				if (error)
+					return error;
+				error = xfs_bumplink(tp, dp1);
+				if (error)
+					return error;
+			}
+
+			/*
+			 * Although ip1 isn't changed here, userspace needs
+			 * to be warned about the change, so that applications
+			 * relying on it (like backup ones), will properly
+			 * notify the change
+			 */
+			ip1_flags |= XFS_ICHGTIME_CHG;
+			ip2_flags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
+		}
+
+		if (S_ISDIR(VFS_I(ip1)->i_mode)) {
+			error = xfs_dir_replace(tp, ip1, &xfs_name_dotdot,
+						dp2->i_ino, spaceres);
+			if (error)
+				return error;
+
+			/* transfer ip1 ".." reference to dp2 */
+			if (!S_ISDIR(VFS_I(ip2)->i_mode)) {
+				error = xfs_droplink(tp, dp1);
+				if (error)
+					return error;
+				error = xfs_bumplink(tp, dp2);
+				if (error)
+					return error;
+			}
+
+			/*
+			 * Although ip2 isn't changed here, userspace needs
+			 * to be warned about the change, so that applications
+			 * relying on it (like backup ones), will properly
+			 * notify the change
+			 */
+			ip1_flags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
+			ip2_flags |= XFS_ICHGTIME_CHG;
+		}
+	}
+
+	if (ip1_flags) {
+		xfs_trans_ichgtime(tp, ip1, ip1_flags);
+		xfs_trans_log_inode(tp, ip1, XFS_ILOG_CORE);
+	}
+	if (ip2_flags) {
+		xfs_trans_ichgtime(tp, ip2, ip2_flags);
+		xfs_trans_log_inode(tp, ip2, XFS_ILOG_CORE);
+	}
+	if (dp2_flags) {
+		xfs_trans_ichgtime(tp, dp2, dp2_flags);
+		xfs_trans_log_inode(tp, dp2, XFS_ILOG_CORE);
+	}
+	xfs_trans_ichgtime(tp, dp1, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
+	xfs_trans_log_inode(tp, dp1, XFS_ILOG_CORE);
+
+	return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
index 3da87780388e..09765f010f90 100644
--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -336,5 +336,9 @@ int xfs_dir_link_existing_child(struct xfs_trans *tp, uint resblks,
 int xfs_dir_remove_child(struct xfs_trans *tp, uint resblks,
 		struct xfs_inode *dp, struct xfs_name *name,
 		struct xfs_inode *ip);
+int xfs_dir_exchange(struct xfs_trans *tp, struct xfs_inode *dp1,
+		struct xfs_name *name1, struct xfs_inode *ip1,
+		struct xfs_inode *dp2, struct xfs_name *name2,
+		struct xfs_inode *ip2, unsigned int spaceres);
 
 #endif	/* __XFS_DIR2_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index a882563118c8..3bed0db609bd 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2034,96 +2034,13 @@ xfs_cross_rename(
 	struct xfs_inode	*ip2,
 	int			spaceres)
 {
-	int		error = 0;
-	int		ip1_flags = 0;
-	int		ip2_flags = 0;
-	int		dp2_flags = 0;
-
-	/* Swap inode number for dirent in first parent */
-	error = xfs_dir_replace(tp, dp1, name1, ip2->i_ino, spaceres);
-	if (error)
-		goto out_trans_abort;
+	int			error;
 
-	/* Swap inode number for dirent in second parent */
-	error = xfs_dir_replace(tp, dp2, name2, ip1->i_ino, spaceres);
+	error = xfs_dir_exchange(tp, dp1, name1, ip1, dp2, name2, ip2,
+			spaceres);
 	if (error)
 		goto out_trans_abort;
 
-	/*
-	 * If we're renaming one or more directories across different parents,
-	 * update the respective ".." entries (and link counts) to match the new
-	 * parents.
-	 */
-	if (dp1 != dp2) {
-		dp2_flags = XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
-
-		if (S_ISDIR(VFS_I(ip2)->i_mode)) {
-			error = xfs_dir_replace(tp, ip2, &xfs_name_dotdot,
-						dp1->i_ino, spaceres);
-			if (error)
-				goto out_trans_abort;
-
-			/* transfer ip2 ".." reference to dp1 */
-			if (!S_ISDIR(VFS_I(ip1)->i_mode)) {
-				error = xfs_droplink(tp, dp2);
-				if (error)
-					goto out_trans_abort;
-				error = xfs_bumplink(tp, dp1);
-				if (error)
-					goto out_trans_abort;
-			}
-
-			/*
-			 * Although ip1 isn't changed here, userspace needs
-			 * to be warned about the change, so that applications
-			 * relying on it (like backup ones), will properly
-			 * notify the change
-			 */
-			ip1_flags |= XFS_ICHGTIME_CHG;
-			ip2_flags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
-		}
-
-		if (S_ISDIR(VFS_I(ip1)->i_mode)) {
-			error = xfs_dir_replace(tp, ip1, &xfs_name_dotdot,
-						dp2->i_ino, spaceres);
-			if (error)
-				goto out_trans_abort;
-
-			/* transfer ip1 ".." reference to dp2 */
-			if (!S_ISDIR(VFS_I(ip2)->i_mode)) {
-				error = xfs_droplink(tp, dp1);
-				if (error)
-					goto out_trans_abort;
-				error = xfs_bumplink(tp, dp2);
-				if (error)
-					goto out_trans_abort;
-			}
-
-			/*
-			 * Although ip2 isn't changed here, userspace needs
-			 * to be warned about the change, so that applications
-			 * relying on it (like backup ones), will properly
-			 * notify the change
-			 */
-			ip1_flags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
-			ip2_flags |= XFS_ICHGTIME_CHG;
-		}
-	}
-
-	if (ip1_flags) {
-		xfs_trans_ichgtime(tp, ip1, ip1_flags);
-		xfs_trans_log_inode(tp, ip1, XFS_ILOG_CORE);
-	}
-	if (ip2_flags) {
-		xfs_trans_ichgtime(tp, ip2, ip2_flags);
-		xfs_trans_log_inode(tp, ip2, XFS_ILOG_CORE);
-	}
-	if (dp2_flags) {
-		xfs_trans_ichgtime(tp, dp2, dp2_flags);
-		xfs_trans_log_inode(tp, dp2, XFS_ILOG_CORE);
-	}
-	xfs_trans_ichgtime(tp, dp1, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-	xfs_trans_log_inode(tp, dp1, XFS_ILOG_CORE);
 	return xfs_finish_rename(tp);
 
 out_trans_abort:

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 20/22] xfs: create libxfs helper to rename two directory entries
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (18 preceding siblings ...)
  2019-01-01  2:21 ` [PATCH 19/22] xfs: create libxfs helper to exchange two directory entries Darrick J. Wong
@ 2019-01-01  2:21 ` Darrick J. Wong
  2019-01-01  2:21 ` [PATCH 21/22] xfs: get rid of cross_rename Darrick J. Wong
  2019-01-01  2:21 ` [PATCH 22/22] xfs: create library function to reset root inodes Darrick J. Wong
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:21 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a new libxfs function to rename two directory entries.  The
upcoming metadata directory feature will need this to replace a metadata
inode directory entry.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_dir2.c |  195 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_dir2.h |    5 +
 fs/xfs/xfs_inode.c       |  160 --------------------------------------
 3 files changed, 202 insertions(+), 158 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index fbd5ce580b4a..77286d4f6e72 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -964,3 +964,198 @@ xfs_dir_exchange(
 
 	return 0;
 }
+
+
+/*
+ * Given an entry (@src_name, @src_ip) in directory @src_dp, make the entry
+ * @target_name in directory @target_dp point to @src_ip and remove the
+ * original entry, cleaning up everything left behind.
+ *
+ * Cleanup involves dropping a link count on @target_ip, and either removing
+ * the (@src_name, @src_ip) entry from @src_dp or simply replacing the entry
+ * with (@src_name, @wip) if a whiteout inode @wip is supplied.
+ *
+ * All inodes must have the ILOCK held.  We assume that if @src_ip is a
+ * directory then its '..' doesn't already point to @target_dp, and that @wip
+ * is a freshly allocated whiteout.
+ */
+int
+xfs_dir_rename(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*src_dp,
+	struct xfs_name		*src_name,
+	struct xfs_inode	*src_ip,
+	struct xfs_inode	*target_dp,
+	struct xfs_name		*target_name,
+	struct xfs_inode	*target_ip,
+	unsigned int		spaceres,
+	struct xfs_inode	*wip)
+{
+	bool			new_parent = (src_dp != target_dp);
+	bool			src_is_directory;
+	int			error;
+
+	src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode);
+
+	/*
+	 * Set up the target.
+	 */
+	if (target_ip == NULL) {
+		/*
+		 * If there's no space reservation, check the entry will
+		 * fit before actually inserting it.
+		 */
+		if (!spaceres) {
+			error = xfs_dir_canenter(tp, target_dp, target_name);
+			if (error)
+				return error;
+		}
+		/*
+		 * If target does not exist and the rename crosses
+		 * directories, adjust the target directory link count
+		 * to account for the ".." reference from the new entry.
+		 */
+		error = xfs_dir_createname(tp, target_dp, target_name,
+					   src_ip->i_ino, spaceres);
+		if (error)
+			return error;
+
+		xfs_trans_ichgtime(tp, target_dp,
+					XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
+
+		if (new_parent && src_is_directory) {
+			error = xfs_bumplink(tp, target_dp);
+			if (error)
+				return error;
+		}
+	} else { /* target_ip != NULL */
+		/*
+		 * If target exists and it's a directory, check that both
+		 * target and source are directories and that target can be
+		 * destroyed, or that neither is a directory.
+		 */
+		if (S_ISDIR(VFS_I(target_ip)->i_mode)) {
+			/*
+			 * Make sure target dir is empty.
+			 */
+			if (!(xfs_dir_isempty(target_ip)) ||
+			    (VFS_I(target_ip)->i_nlink > 2))
+				return -EEXIST;
+		}
+
+		/*
+		 * Link the source inode under the target name.
+		 * If the source inode is a directory and we are moving
+		 * it across directories, its ".." entry will be
+		 * inconsistent until we replace that down below.
+		 *
+		 * In case there is already an entry with the same
+		 * name at the destination directory, remove it first.
+		 */
+		error = xfs_dir_replace(tp, target_dp, target_name,
+					src_ip->i_ino, spaceres);
+		if (error)
+			return error;
+
+		xfs_trans_ichgtime(tp, target_dp,
+					XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
+
+		/*
+		 * Decrement the link count on the target since the target
+		 * dir no longer points to it.
+		 */
+		error = xfs_droplink(tp, target_ip);
+		if (error)
+			return error;
+
+		if (src_is_directory) {
+			/*
+			 * Drop the link from the old "." entry.
+			 */
+			error = xfs_droplink(tp, target_ip);
+			if (error)
+				return error;
+		}
+	} /* target_ip != NULL */
+
+	/*
+	 * Remove the source.
+	 */
+	if (new_parent && src_is_directory) {
+		/*
+		 * Rewrite the ".." entry to point to the new
+		 * directory.
+		 */
+		error = xfs_dir_replace(tp, src_ip, &xfs_name_dotdot,
+					target_dp->i_ino, spaceres);
+		ASSERT(error != -EEXIST);
+		if (error)
+			return error;
+	}
+
+	/*
+	 * We always want to hit the ctime on the source inode.
+	 *
+	 * This isn't strictly required by the standards since the source
+	 * inode isn't really being changed, but old unix file systems did
+	 * it and some incremental backup programs won't work without it.
+	 */
+	xfs_trans_ichgtime(tp, src_ip, XFS_ICHGTIME_CHG);
+	xfs_trans_log_inode(tp, src_ip, XFS_ILOG_CORE);
+
+	/*
+	 * Adjust the link count on src_dp.  This is necessary when
+	 * renaming a directory, either within one parent when
+	 * the target existed, or across two parent directories.
+	 */
+	if (src_is_directory && (new_parent || target_ip != NULL)) {
+
+		/*
+		 * Decrement link count on src_directory since the
+		 * entry that's moved no longer points to it.
+		 */
+		error = xfs_droplink(tp, src_dp);
+		if (error)
+			return error;
+	}
+
+	/*
+	 * For whiteouts, we only need to update the source dirent with the
+	 * inode number of the whiteout inode rather than removing it
+	 * altogether.
+	 */
+	if (wip) {
+		error = xfs_dir_replace(tp, src_dp, src_name, wip->i_ino,
+					spaceres);
+	} else
+		error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino,
+					   spaceres);
+	if (error)
+		return error;
+
+	/*
+	 * For whiteouts, we need to bump the link count on the whiteout inode.
+	 * This means that failures all the way up to this point leave the inode
+	 * on the unlinked list and so cleanup is a simple matter of dropping
+	 * the remaining reference to it. If we fail here after bumping the link
+	 * count, we're shutting down the filesystem so we'll never see the
+	 * intermediate state on disk.
+	 */
+	if (wip) {
+		ASSERT(VFS_I(wip)->i_nlink == 0);
+		error = xfs_bumplink(tp, wip);
+		if (error)
+			return error;
+		error = xfs_iunlink_remove(tp, wip);
+		if (error)
+			return error;
+		xfs_trans_log_inode(tp, wip, XFS_ILOG_CORE);
+	}
+
+	xfs_trans_ichgtime(tp, src_dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
+	xfs_trans_log_inode(tp, src_dp, XFS_ILOG_CORE);
+	if (new_parent)
+		xfs_trans_log_inode(tp, target_dp, XFS_ILOG_CORE);
+
+	return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
index 09765f010f90..d89552b49791 100644
--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -340,5 +340,10 @@ int xfs_dir_exchange(struct xfs_trans *tp, struct xfs_inode *dp1,
 		struct xfs_name *name1, struct xfs_inode *ip1,
 		struct xfs_inode *dp2, struct xfs_name *name2,
 		struct xfs_inode *ip2, unsigned int spaceres);
+int xfs_dir_rename(struct xfs_trans *tp, struct xfs_inode *src_dp,
+		struct xfs_name *src_name, struct xfs_inode *src_ip,
+		struct xfs_inode *target_dp, struct xfs_name *target_name,
+		struct xfs_inode *target_ip, unsigned int spaceres,
+		struct xfs_inode *wip);
 
 #endif	/* __XFS_DIR2_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 3bed0db609bd..52e29a27f514 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2111,7 +2111,6 @@ xfs_rename(
 	struct xfs_inode	*inodes[__XFS_SORT_INODES];
 	int			num_inodes = __XFS_SORT_INODES;
 	bool			new_parent = (src_dp != target_dp);
-	bool			src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode);
 	int			spaceres;
 	int			error;
 
@@ -2195,162 +2194,12 @@ xfs_rename(
 					target_dp, target_name, target_ip,
 					spaceres);
 
-	/*
-	 * Set up the target.
-	 */
-	if (target_ip == NULL) {
-		/*
-		 * If there's no space reservation, check the entry will
-		 * fit before actually inserting it.
-		 */
-		if (!spaceres) {
-			error = xfs_dir_canenter(tp, target_dp, target_name);
-			if (error)
-				goto out_trans_cancel;
-		}
-		/*
-		 * If target does not exist and the rename crosses
-		 * directories, adjust the target directory link count
-		 * to account for the ".." reference from the new entry.
-		 */
-		error = xfs_dir_createname(tp, target_dp, target_name,
-					   src_ip->i_ino, spaceres);
-		if (error)
-			goto out_trans_cancel;
-
-		xfs_trans_ichgtime(tp, target_dp,
-					XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-
-		if (new_parent && src_is_directory) {
-			error = xfs_bumplink(tp, target_dp);
-			if (error)
-				goto out_trans_cancel;
-		}
-	} else { /* target_ip != NULL */
-		/*
-		 * If target exists and it's a directory, check that both
-		 * target and source are directories and that target can be
-		 * destroyed, or that neither is a directory.
-		 */
-		if (S_ISDIR(VFS_I(target_ip)->i_mode)) {
-			/*
-			 * Make sure target dir is empty.
-			 */
-			if (!(xfs_dir_isempty(target_ip)) ||
-			    (VFS_I(target_ip)->i_nlink > 2)) {
-				error = -EEXIST;
-				goto out_trans_cancel;
-			}
-		}
-
-		/*
-		 * Link the source inode under the target name.
-		 * If the source inode is a directory and we are moving
-		 * it across directories, its ".." entry will be
-		 * inconsistent until we replace that down below.
-		 *
-		 * In case there is already an entry with the same
-		 * name at the destination directory, remove it first.
-		 */
-		error = xfs_dir_replace(tp, target_dp, target_name,
-					src_ip->i_ino, spaceres);
-		if (error)
-			goto out_trans_cancel;
-
-		xfs_trans_ichgtime(tp, target_dp,
-					XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-
-		/*
-		 * Decrement the link count on the target since the target
-		 * dir no longer points to it.
-		 */
-		error = xfs_droplink(tp, target_ip);
-		if (error)
-			goto out_trans_cancel;
-
-		if (src_is_directory) {
-			/*
-			 * Drop the link from the old "." entry.
-			 */
-			error = xfs_droplink(tp, target_ip);
-			if (error)
-				goto out_trans_cancel;
-		}
-	} /* target_ip != NULL */
-
-	/*
-	 * Remove the source.
-	 */
-	if (new_parent && src_is_directory) {
-		/*
-		 * Rewrite the ".." entry to point to the new
-		 * directory.
-		 */
-		error = xfs_dir_replace(tp, src_ip, &xfs_name_dotdot,
-					target_dp->i_ino, spaceres);
-		ASSERT(error != -EEXIST);
-		if (error)
-			goto out_trans_cancel;
-	}
-
-	/*
-	 * We always want to hit the ctime on the source inode.
-	 *
-	 * This isn't strictly required by the standards since the source
-	 * inode isn't really being changed, but old unix file systems did
-	 * it and some incremental backup programs won't work without it.
-	 */
-	xfs_trans_ichgtime(tp, src_ip, XFS_ICHGTIME_CHG);
-	xfs_trans_log_inode(tp, src_ip, XFS_ILOG_CORE);
-
-	/*
-	 * Adjust the link count on src_dp.  This is necessary when
-	 * renaming a directory, either within one parent when
-	 * the target existed, or across two parent directories.
-	 */
-	if (src_is_directory && (new_parent || target_ip != NULL)) {
-
-		/*
-		 * Decrement link count on src_directory since the
-		 * entry that's moved no longer points to it.
-		 */
-		error = xfs_droplink(tp, src_dp);
-		if (error)
-			goto out_trans_cancel;
-	}
-
-	/*
-	 * For whiteouts, we only need to update the source dirent with the
-	 * inode number of the whiteout inode rather than removing it
-	 * altogether.
-	 */
-	if (wip) {
-		error = xfs_dir_replace(tp, src_dp, src_name, wip->i_ino,
-					spaceres);
-	} else
-		error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino,
-					   spaceres);
+	error = xfs_dir_rename(tp, src_dp, src_name, src_ip, target_dp,
+			target_name, target_ip, spaceres, wip);
 	if (error)
 		goto out_trans_cancel;
 
-	/*
-	 * For whiteouts, we need to bump the link count on the whiteout inode.
-	 * This means that failures all the way up to this point leave the inode
-	 * on the unlinked list and so cleanup is a simple matter of dropping
-	 * the remaining reference to it. If we fail here after bumping the link
-	 * count, we're shutting down the filesystem so we'll never see the
-	 * intermediate state on disk.
-	 */
 	if (wip) {
-		ASSERT(VFS_I(wip)->i_nlink == 0);
-		error = xfs_bumplink(tp, wip);
-		if (error)
-			goto out_trans_cancel;
-		error = xfs_iunlink_remove(tp, wip);
-		if (error)
-			goto out_trans_cancel;
-		xfs_trans_log_inode(tp, wip, XFS_ILOG_CORE);
-
 		/*
 		 * Now we have a real link, clear the "I'm a tmpfile" state
 		 * flag from the inode so it doesn't accidentally get misused in
@@ -2359,11 +2208,6 @@ xfs_rename(
 		VFS_I(wip)->i_state &= ~I_LINKABLE;
 	}
 
-	xfs_trans_ichgtime(tp, src_dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-	xfs_trans_log_inode(tp, src_dp, XFS_ILOG_CORE);
-	if (new_parent)
-		xfs_trans_log_inode(tp, target_dp, XFS_ILOG_CORE);
-
 	error = xfs_finish_rename(tp);
 	if (wip)
 		xfs_irele(wip);

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 21/22] xfs: get rid of cross_rename
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (19 preceding siblings ...)
  2019-01-01  2:21 ` [PATCH 20/22] xfs: create libxfs helper to rename " Darrick J. Wong
@ 2019-01-01  2:21 ` Darrick J. Wong
  2019-01-01  2:21 ` [PATCH 22/22] xfs: create library function to reset root inodes Darrick J. Wong
  21 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:21 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Get rid of the largely pointless xfs_cross_rename now that we've
refactored its parent.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_inode.c |   42 ++++++------------------------------------
 1 file changed, 6 insertions(+), 36 deletions(-)


diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 52e29a27f514..f177e0ded21c 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2018,36 +2018,6 @@ xfs_finish_rename(
 	return xfs_trans_commit(tp);
 }
 
-/*
- * xfs_cross_rename()
- *
- * responsible for handling RENAME_EXCHANGE flag in renameat2() sytemcall
- */
-STATIC int
-xfs_cross_rename(
-	struct xfs_trans	*tp,
-	struct xfs_inode	*dp1,
-	struct xfs_name		*name1,
-	struct xfs_inode	*ip1,
-	struct xfs_inode	*dp2,
-	struct xfs_name		*name2,
-	struct xfs_inode	*ip2,
-	int			spaceres)
-{
-	int			error;
-
-	error = xfs_dir_exchange(tp, dp1, name1, ip1, dp2, name2, ip2,
-			spaceres);
-	if (error)
-		goto out_trans_abort;
-
-	return xfs_finish_rename(tp);
-
-out_trans_abort:
-	xfs_trans_cancel(tp);
-	return error;
-}
-
 /*
  * xfs_rename_alloc_whiteout()
  *
@@ -2190,12 +2160,12 @@ xfs_rename(
 
 	/* RENAME_EXCHANGE is unique from here on. */
 	if (flags & RENAME_EXCHANGE)
-		return xfs_cross_rename(tp, src_dp, src_name, src_ip,
-					target_dp, target_name, target_ip,
-					spaceres);
-
-	error = xfs_dir_rename(tp, src_dp, src_name, src_ip, target_dp,
-			target_name, target_ip, spaceres, wip);
+		error = xfs_dir_exchange(tp, src_dp, src_name, src_ip,
+					 target_dp, target_name, target_ip,
+					 spaceres);
+	else
+		error = xfs_dir_rename(tp, src_dp, src_name, src_ip, target_dp,
+				       target_name, target_ip, spaceres, wip);
 	if (error)
 		goto out_trans_cancel;
 

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 22/22] xfs: create library function to reset root inodes
  2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
                   ` (20 preceding siblings ...)
  2019-01-01  2:21 ` [PATCH 21/22] xfs: get rid of cross_rename Darrick J. Wong
@ 2019-01-01  2:21 ` Darrick J. Wong
  2019-01-17 14:25   ` Christoph Hellwig
  21 siblings, 1 reply; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-01  2:21 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a library function to reset a root inode, which xfs_repair will
take advantage of in xfsprogs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_util.c |   24 ++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_inode_util.h |    1 +
 2 files changed, 25 insertions(+)


diff --git a/fs/xfs/libxfs/xfs_inode_util.c b/fs/xfs/libxfs/xfs_inode_util.c
index 01a320747ddf..00cda9046c8e 100644
--- a/fs/xfs/libxfs/xfs_inode_util.c
+++ b/fs/xfs/libxfs/xfs_inode_util.c
@@ -416,6 +416,30 @@ xfs_ialloc(
 	return 0;
 }
 
+/*
+ * Forcibly reinitialize a fixed-location inode, such as a filesystem root
+ * directory or the realtime metadata inodes.  The inode must not otherwise be
+ * in use, and the inode's forks must already be empty.
+ */
+int
+xfs_fixed_inode_reset(
+	struct xfs_trans		*tp,
+	umode_t				mode,
+	struct xfs_inode		*ip)
+{
+	struct xfs_ialloc_args	args = {
+		.ops			= &xfs_default_ialloc_ops,
+		.nlink			= S_ISDIR(mode) ? 2 : 1,
+		.mode			= mode,
+	};
+	struct xfs_mount		*mp = tp->t_mountp;
+
+	ip->i_d.di_version = xfs_sb_version_hascrc(&mp->m_sb) ? 3 : 2;
+
+	xfs_inode_init(tp, &args, ip);
+	return 0;
+}
+
 /*
  * Allocates a new inode from disk and return a pointer to the
  * incore copy. This routine will internally commit the current
diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index 4918379b682e..5a6f1a348a47 100644
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -97,6 +97,7 @@ extern const struct xfs_ialloc_ops xfs_default_ialloc_ops;
 
 int xfs_dir_ialloc(struct xfs_trans **tpp, const struct xfs_ialloc_args *args,
 		   struct xfs_inode **ipp);
+int xfs_fixed_inode_reset(struct xfs_trans *tp, umode_t mode, struct xfs_inode *ip);
 
 int xfs_iunlink(struct xfs_trans *tp, struct xfs_inode *ip);
 int xfs_iunlink_remove(struct xfs_trans *tp, struct xfs_inode *ip);

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/22] xfs: hoist extent size helpers to libxfs
  2019-01-01  2:19 ` [PATCH 01/22] xfs: hoist extent size helpers " Darrick J. Wong
@ 2019-01-17 14:18   ` Christoph Hellwig
  2019-01-17 19:11     ` Darrick J. Wong
  0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2019-01-17 14:18 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

Can we move thse to xfs_bmap.c, which includes a few users of these
little helpers?

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 03/22] xfs: convert projid get/set functions
  2019-01-01  2:19 ` [PATCH 03/22] xfs: convert projid get/set functions Darrick J. Wong
@ 2019-01-17 14:19   ` Christoph Hellwig
  2019-01-17 19:16     ` Darrick J. Wong
  0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2019-01-17 14:19 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Mon, Dec 31, 2018 at 06:19:19PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Convert the projid get and set functions to work on xfs_icdinode like
> they do in userspace.  xfs_db needs the icdinode version.

No new users of xfs_icdinode, please.  I actually have a series to
kill it off that I need to dust off once I've got a little time.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 05/22] xfs: pack inode allocation parameters into a separate structure
  2019-01-01  2:19 ` [PATCH 05/22] xfs: pack inode allocation parameters into a separate structure Darrick J. Wong
@ 2019-01-17 14:21   ` Christoph Hellwig
  2019-01-23  4:11     ` Darrick J. Wong
  0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2019-01-17 14:21 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Mon, Dec 31, 2018 at 06:19:32PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Instead of open-coding new inode parameters through xfs_dir_ialloc and
> xfs_Ialloc, put everything into a structure and pass that instead.  This
> will make it easier to share code with xfsprogs while maintaining the
> ability for xfsprogs to supply extra new inode parameters.

I find these ad-hoc arg structures rather cumberssome to be honest.

> +/* Initial ids, link count, device number, and mode of a new inode. */
> +struct xfs_ialloc_args {
> +	struct xfs_inode		*pip;	/* parent inode or null */
> +
> +	uint32_t			uid;
> +	uint32_t			gid;
> +	prid_t				prid;
> +
> +	xfs_nlink_t			nlink;
> +	dev_t				rdev;
> +
> +	umode_t				mode;

And except for the parent inode this is a significant subsystet of
stat data.  Can't we just pass a struct kstat, and a mask of attributes
from the kstat we want to set instead?  That is use a calling convention
similar to ->setattr?

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc
  2019-01-01  2:19 ` [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc Darrick J. Wong
@ 2019-01-17 14:22   ` Christoph Hellwig
  2019-01-17 23:28     ` Darrick J. Wong
  2019-01-17 22:26   ` Dave Chinner
  1 sibling, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2019-01-17 14:22 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Mon, Dec 31, 2018 at 06:19:38PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move the kernel-specific parts of xfs_ialloc into a separate function in
> preparation for hoisting xfs_ialloc to libxfs.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_inode.c |   45 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 36 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 403eb8fa2f1f..b635d43caeed 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -621,6 +621,32 @@ xfs_lookup(
>  	return error;
>  }
>  
> +/* Return an exclusive ILOCK'd in-core inode. */
> +static int
> +xfs_ialloc_iget(
> +	struct xfs_mount	*mp,
> +	struct xfs_trans	*tp,
> +	xfs_ino_t		ino,
> +	struct xfs_inode	**ipp)
> +{
> +	return xfs_iget(mp, tp, ino, XFS_IGET_CREATE, XFS_ILOCK_EXCL, ipp);
> +}

This is just too ugly and pointless.  Can you explain (or even better
show code) why we really need this for xfsprogs?

> +	xfs_ialloc_platform_init(tp, args, ip);

And that platform naming also is rather horrible.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 07/22] xfs: decouple platform-specific inode allocation functions
  2019-01-01  2:19 ` [PATCH 07/22] xfs: decouple platform-specific inode allocation functions Darrick J. Wong
@ 2019-01-17 14:22   ` Christoph Hellwig
  0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2019-01-17 14:22 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Mon, Dec 31, 2018 at 06:19:44PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Decouple the xfs_ialloc function from the platform-specific pieces by
> passing in an operations structure.  This prepares the function for
> hoisting to libxfs.

What "platform"?

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/22] xfs: refactor special inode roll out of xfs_dir_ialloc
  2019-01-01  2:20 ` [PATCH 11/22] xfs: refactor special inode roll " Darrick J. Wong
@ 2019-01-17 14:24   ` Christoph Hellwig
  2019-01-17 20:04     ` Darrick J. Wong
  0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2019-01-17 14:24 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

> diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
> index 5a1d98d1546d..ee274d74b8d4 100644
> --- a/fs/xfs/libxfs/xfs_inode_util.h
> +++ b/fs/xfs/libxfs/xfs_inode_util.h
> @@ -82,6 +82,12 @@ struct xfs_ialloc_ops {
>  
>  	/* Do any final setup needed before we return the inode. */
>  	void (*setup)(struct xfs_inode *ip);
> +
> +	/*
> +	 * Roll the transaction between allocating a new ichunk and
> +	 * initializing a new inode core.
> +	 */
> +	int (*ichunk_roll)(struct xfs_trans **tpp);

Sorry, but this whole idea to add gracious indirect calls is just
backwards.  They do have a non-trivial cost, and we should rather
get rid of pointless indirect calls instead of adding more.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 22/22] xfs: create library function to reset root inodes
  2019-01-01  2:21 ` [PATCH 22/22] xfs: create library function to reset root inodes Darrick J. Wong
@ 2019-01-17 14:25   ` Christoph Hellwig
  2019-01-17 23:29     ` Darrick J. Wong
  0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2019-01-17 14:25 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Mon, Dec 31, 2018 at 06:21:33PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Create a library function to reset a root inode, which xfs_repair will
> take advantage of in xfsprogs.

And which will just add dead code to the kernel?

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/22] xfs: hoist extent size helpers to libxfs
  2019-01-17 14:18   ` Christoph Hellwig
@ 2019-01-17 19:11     ` Darrick J. Wong
  0 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-17 19:11 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Thu, Jan 17, 2019 at 06:18:25AM -0800, Christoph Hellwig wrote:
> Can we move thse to xfs_bmap.c, which includes a few users of these
> little helpers?

Ok.

--D

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 03/22] xfs: convert projid get/set functions
  2019-01-17 14:19   ` Christoph Hellwig
@ 2019-01-17 19:16     ` Darrick J. Wong
  0 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-17 19:16 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Thu, Jan 17, 2019 at 06:19:23AM -0800, Christoph Hellwig wrote:
> On Mon, Dec 31, 2018 at 06:19:19PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Convert the projid get and set functions to work on xfs_icdinode like
> > they do in userspace.  xfs_db needs the icdinode version.
> 
> No new users of xfs_icdinode, please.  I actually have a series to
> kill it off that I need to dust off once I've got a little time.

Heh, I just realized that the xfs_db caller of xfs_get_projid actually
does have the corresponding (struct xfs_inode *) in local variable scope
so I can fix the xfsprogs version of this function.

--D

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/22] xfs: refactor special inode roll out of xfs_dir_ialloc
  2019-01-17 14:24   ` Christoph Hellwig
@ 2019-01-17 20:04     ` Darrick J. Wong
  0 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-17 20:04 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Thu, Jan 17, 2019 at 06:24:27AM -0800, Christoph Hellwig wrote:
> > diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
> > index 5a1d98d1546d..ee274d74b8d4 100644
> > --- a/fs/xfs/libxfs/xfs_inode_util.h
> > +++ b/fs/xfs/libxfs/xfs_inode_util.h
> > @@ -82,6 +82,12 @@ struct xfs_ialloc_ops {
> >  
> >  	/* Do any final setup needed before we return the inode. */
> >  	void (*setup)(struct xfs_inode *ip);
> > +
> > +	/*
> > +	 * Roll the transaction between allocating a new ichunk and
> > +	 * initializing a new inode core.
> > +	 */
> > +	int (*ichunk_roll)(struct xfs_trans **tpp);
> 
> Sorry, but this whole idea to add gracious indirect calls is just
> backwards.  They do have a non-trivial cost, and we should rather
> get rid of pointless indirect calls instead of adding more.

I built all this indirect call stuff so that mkfs protofile code would
be able to set parameters ... but seeing as Eric said he'll deprecate
all that, I think it's no longer necessary.

--D

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc
  2019-01-01  2:19 ` [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc Darrick J. Wong
  2019-01-17 14:22   ` Christoph Hellwig
@ 2019-01-17 22:26   ` Dave Chinner
  2019-01-17 23:02     ` Darrick J. Wong
  1 sibling, 1 reply; 40+ messages in thread
From: Dave Chinner @ 2019-01-17 22:26 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Mon, Dec 31, 2018 at 06:19:38PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move the kernel-specific parts of xfs_ialloc into a separate function in
> preparation for hoisting xfs_ialloc to libxfs.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_inode.c |   45 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 36 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 403eb8fa2f1f..b635d43caeed 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -621,6 +621,32 @@ xfs_lookup(
>  	return error;
>  }
>  
> +/* Return an exclusive ILOCK'd in-core inode. */
> +static int
> +xfs_ialloc_iget(
> +	struct xfs_mount	*mp,
> +	struct xfs_trans	*tp,
> +	xfs_ino_t		ino,
> +	struct xfs_inode	**ipp)
> +{
> +	return xfs_iget(mp, tp, ino, XFS_IGET_CREATE, XFS_ILOCK_EXCL, ipp);
> +}
> +
> +/* Propagate platform-specific inode properties into the new child. */
> +static void
> +xfs_ialloc_platform_init(
> +	struct xfs_trans		*tp,
> +	const struct xfs_ialloc_args	*args,
> +	struct xfs_inode		*ip)
> +{
> +	struct timespec64		tv;
> +
> +	tv = current_time(VFS_I(ip));
> +	VFS_I(ip)->i_mtime = tv;
> +	VFS_I(ip)->i_atime = tv;
> +	VFS_I(ip)->i_ctime = tv;

Doesn't set ip->i_d.di_crtime, so ....

> @@ -755,10 +781,11 @@ xfs_ialloc(
>  		inode_set_iversion(inode, 1);
>  		ip->i_d.di_flags2 = 0;
>  		ip->i_d.di_cowextsize = 0;
> -		ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
> -		ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
> +		ip->i_d.di_crtime.t_sec = 0;
> +		ip->i_d.di_crtime.t_nsec = 0;
>  	}
>  
> +	xfs_ialloc_platform_init(tp, args, ip);

This breaks create time functionality.

Don't we have a statx() test in fstests that checks that the
create time of inodes is set correctly?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc
  2019-01-17 22:26   ` Dave Chinner
@ 2019-01-17 23:02     ` Darrick J. Wong
  0 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-17 23:02 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-xfs

On Fri, Jan 18, 2019 at 09:26:14AM +1100, Dave Chinner wrote:
> On Mon, Dec 31, 2018 at 06:19:38PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Move the kernel-specific parts of xfs_ialloc into a separate function in
> > preparation for hoisting xfs_ialloc to libxfs.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/xfs_inode.c |   45 ++++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 36 insertions(+), 9 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> > index 403eb8fa2f1f..b635d43caeed 100644
> > --- a/fs/xfs/xfs_inode.c
> > +++ b/fs/xfs/xfs_inode.c
> > @@ -621,6 +621,32 @@ xfs_lookup(
> >  	return error;
> >  }
> >  
> > +/* Return an exclusive ILOCK'd in-core inode. */
> > +static int
> > +xfs_ialloc_iget(
> > +	struct xfs_mount	*mp,
> > +	struct xfs_trans	*tp,
> > +	xfs_ino_t		ino,
> > +	struct xfs_inode	**ipp)
> > +{
> > +	return xfs_iget(mp, tp, ino, XFS_IGET_CREATE, XFS_ILOCK_EXCL, ipp);
> > +}
> > +
> > +/* Propagate platform-specific inode properties into the new child. */
> > +static void
> > +xfs_ialloc_platform_init(
> > +	struct xfs_trans		*tp,
> > +	const struct xfs_ialloc_args	*args,
> > +	struct xfs_inode		*ip)
> > +{
> > +	struct timespec64		tv;
> > +
> > +	tv = current_time(VFS_I(ip));
> > +	VFS_I(ip)->i_mtime = tv;
> > +	VFS_I(ip)->i_atime = tv;
> > +	VFS_I(ip)->i_ctime = tv;
> 
> Doesn't set ip->i_d.di_crtime, so ....
> 
> > @@ -755,10 +781,11 @@ xfs_ialloc(
> >  		inode_set_iversion(inode, 1);
> >  		ip->i_d.di_flags2 = 0;
> >  		ip->i_d.di_cowextsize = 0;
> > -		ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
> > -		ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
> > +		ip->i_d.di_crtime.t_sec = 0;
> > +		ip->i_d.di_crtime.t_nsec = 0;
> >  	}
> >  
> > +	xfs_ialloc_platform_init(tp, args, ip);
> 
> This breaks create time functionality.

Yikes!

> Don't we have a statx() test in fstests that checks that the
> create time of inodes is set correctly?

Clearly not... <frown>

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc
  2019-01-17 14:22   ` Christoph Hellwig
@ 2019-01-17 23:28     ` Darrick J. Wong
  2019-01-18  7:17       ` Christoph Hellwig
  0 siblings, 1 reply; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-17 23:28 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Thu, Jan 17, 2019 at 06:22:21AM -0800, Christoph Hellwig wrote:
> On Mon, Dec 31, 2018 at 06:19:38PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Move the kernel-specific parts of xfs_ialloc into a separate function in
> > preparation for hoisting xfs_ialloc to libxfs.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/xfs_inode.c |   45 ++++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 36 insertions(+), 9 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> > index 403eb8fa2f1f..b635d43caeed 100644
> > --- a/fs/xfs/xfs_inode.c
> > +++ b/fs/xfs/xfs_inode.c
> > @@ -621,6 +621,32 @@ xfs_lookup(
> >  	return error;
> >  }
> >  
> > +/* Return an exclusive ILOCK'd in-core inode. */
> > +static int
> > +xfs_ialloc_iget(
> > +	struct xfs_mount	*mp,
> > +	struct xfs_trans	*tp,
> > +	xfs_ino_t		ino,
> > +	struct xfs_inode	**ipp)
> > +{
> > +	return xfs_iget(mp, tp, ino, XFS_IGET_CREATE, XFS_ILOCK_EXCL, ipp);
> > +}
> 
> This is just too ugly and pointless.  Can you explain (or even better
> show code) why we really need this for xfsprogs?

To avoid having a useless lock_flags argument to xfs_iget in userspace?
It wasn't so long ago that Eric removed it, but I don't mind adding it
back to avoid this kind of ugliness.

> > +	xfs_ialloc_platform_init(tp, args, ip);
> 
> And that platform naming also is rather horrible.

Hm.  Looking at it now, both platform_init() functions basically just
set ctime/mtime/atime/crtime, which means that I really just need to add
a XFS_ICHGTIME_CREATE flag to xfs_trans_ichgtime and replace all the
platform_init stuff with that.

The mkfs protofile platform_init also has some code to set other
attributes from a struct fsxattr, but I think I can just set them once
the inode allocation function returns to the protofile code.

--D

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 22/22] xfs: create library function to reset root inodes
  2019-01-17 14:25   ` Christoph Hellwig
@ 2019-01-17 23:29     ` Darrick J. Wong
  0 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-17 23:29 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Thu, Jan 17, 2019 at 06:25:25AM -0800, Christoph Hellwig wrote:
> On Mon, Dec 31, 2018 at 06:21:33PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Create a library function to reset a root inode, which xfs_repair will
> > take advantage of in xfsprogs.
> 
> And which will just add dead code to the kernel?

Fair enough, it can live in xfs_repair.

--D

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc
  2019-01-17 23:28     ` Darrick J. Wong
@ 2019-01-18  7:17       ` Christoph Hellwig
  2019-01-24  2:04         ` Darrick J. Wong
  0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2019-01-18  7:17 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs

On Thu, Jan 17, 2019 at 03:28:08PM -0800, Darrick J. Wong wrote:
> > This is just too ugly and pointless.  Can you explain (or even better
> > show code) why we really need this for xfsprogs?
> 
> To avoid having a useless lock_flags argument to xfs_iget in userspace?
> It wasn't so long ago that Eric removed it, but I don't mind adding it
> back to avoid this kind of ugliness.

I'd rather have prototypes match in userspace then working around
mismatches in the kernel.

> The mkfs protofile platform_init also has some code to set other
> attributes from a struct fsxattr, but I think I can just set them once
> the inode allocation function returns to the protofile code.

And we are killing that anyway, right?

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 05/22] xfs: pack inode allocation parameters into a separate structure
  2019-01-17 14:21   ` Christoph Hellwig
@ 2019-01-23  4:11     ` Darrick J. Wong
  0 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-23  4:11 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs, Eric Sandeen

On Thu, Jan 17, 2019 at 06:21:26AM -0800, Christoph Hellwig wrote:
> On Mon, Dec 31, 2018 at 06:19:32PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Instead of open-coding new inode parameters through xfs_dir_ialloc and
> > xfs_Ialloc, put everything into a structure and pass that instead.  This
> > will make it easier to share code with xfsprogs while maintaining the
> > ability for xfsprogs to supply extra new inode parameters.
> 
> I find these ad-hoc arg structures rather cumberssome to be honest.
> 
> > +/* Initial ids, link count, device number, and mode of a new inode. */
> > +struct xfs_ialloc_args {
> > +	struct xfs_inode		*pip;	/* parent inode or null */
> > +
> > +	uint32_t			uid;
> > +	uint32_t			gid;
> > +	prid_t				prid;
> > +
> > +	xfs_nlink_t			nlink;
> > +	dev_t				rdev;
> > +
> > +	umode_t				mode;
> 
> And except for the parent inode this is a significant subsystet of
> stat data.  Can't we just pass a struct kstat, and a mask of attributes
> from the kstat we want to set instead?  That is use a calling convention
> similar to ->setattr?

We could, but we still want to be able to pass in the parent inode and
the initial project id, so we still need struct xfs_ialloc_args.  Using
struct kstat instead of just picking up the fields xfs cares about will
waste stack space for things we're /never/ going to need, like blksize,
attributes* ino, dev, size, btime, and blocks.  That also requires us to
maintain enough of a port of struct kstat to userspace in order to share
uid, gid, nlink, rdev, and mode.

I don't think it's worth the trouble, but I want Eric's input before I
go doing things that have repercussions for xfsprogs. :)

--D

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc
  2019-01-18  7:17       ` Christoph Hellwig
@ 2019-01-24  2:04         ` Darrick J. Wong
  0 siblings, 0 replies; 40+ messages in thread
From: Darrick J. Wong @ 2019-01-24  2:04 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Thu, Jan 17, 2019 at 11:17:08PM -0800, Christoph Hellwig wrote:
> On Thu, Jan 17, 2019 at 03:28:08PM -0800, Darrick J. Wong wrote:
> > > This is just too ugly and pointless.  Can you explain (or even better
> > > show code) why we really need this for xfsprogs?
> > 
> > To avoid having a useless lock_flags argument to xfs_iget in userspace?
> > It wasn't so long ago that Eric removed it, but I don't mind adding it
> > back to avoid this kind of ugliness.
> 
> I'd rather have prototypes match in userspace then working around
> mismatches in the kernel.

The other thing about the userspace iget is that you can pass in a
custom set of inode fork verifiers.  xfs_repair uses this to deal with
broken '..' pointers by leaving them unfixed until phase 6, at which
point it knows enough to reset the pointer, but then wants to be able to
iget the directory to fix it.

We could work around that with a libxfs_iget_fork_ops variant so that
xfs_iget remains the same between kernel and userspace.  How about that?

> > The mkfs protofile platform_init also has some code to set other
> > attributes from a struct fsxattr, but I think I can just set them once
> > the inode allocation function returns to the protofile code.
> 
> And we are killing that anyway, right?

Yeah.  I think?  Eric? :)

--D

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2019-01-24  2:04 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-01  2:19 [PATCH 00/22] xfs: hoist inode operations to libxfs Darrick J. Wong
2019-01-01  2:19 ` [PATCH 01/22] xfs: hoist extent size helpers " Darrick J. Wong
2019-01-17 14:18   ` Christoph Hellwig
2019-01-17 19:11     ` Darrick J. Wong
2019-01-01  2:19 ` [PATCH 02/22] xfs: hoist inode flag conversion functions Darrick J. Wong
2019-01-01  2:19 ` [PATCH 03/22] xfs: convert projid get/set functions Darrick J. Wong
2019-01-17 14:19   ` Christoph Hellwig
2019-01-17 19:16     ` Darrick J. Wong
2019-01-01  2:19 ` [PATCH 04/22] xfs: hoist project id " Darrick J. Wong
2019-01-01  2:19 ` [PATCH 05/22] xfs: pack inode allocation parameters into a separate structure Darrick J. Wong
2019-01-17 14:21   ` Christoph Hellwig
2019-01-23  4:11     ` Darrick J. Wong
2019-01-01  2:19 ` [PATCH 06/22] xfs: refactor kernel-specific parts of xfs_ialloc Darrick J. Wong
2019-01-17 14:22   ` Christoph Hellwig
2019-01-17 23:28     ` Darrick J. Wong
2019-01-18  7:17       ` Christoph Hellwig
2019-01-24  2:04         ` Darrick J. Wong
2019-01-17 22:26   ` Dave Chinner
2019-01-17 23:02     ` Darrick J. Wong
2019-01-01  2:19 ` [PATCH 07/22] xfs: decouple platform-specific inode allocation functions Darrick J. Wong
2019-01-17 14:22   ` Christoph Hellwig
2019-01-01  2:19 ` [PATCH 08/22] xfs: split inode allocation and initialization Darrick J. Wong
2019-01-01  2:19 ` [PATCH 09/22] xfs: hoist inode allocation function Darrick J. Wong
2019-01-01  2:20 ` [PATCH 10/22] xfs: push xfs_ialloc_args creation out of xfs_dir_ialloc Darrick J. Wong
2019-01-01  2:20 ` [PATCH 11/22] xfs: refactor special inode roll " Darrick J. Wong
2019-01-17 14:24   ` Christoph Hellwig
2019-01-17 20:04     ` Darrick J. Wong
2019-01-01  2:20 ` [PATCH 12/22] xfs: move xfs_dir_ialloc to libxfs Darrick J. Wong
2019-01-01  2:20 ` [PATCH 13/22] xfs: hoist xfs_iunlink " Darrick J. Wong
2019-01-01  2:20 ` [PATCH 14/22] xfs: hoist xfs_{bump,drop}link " Darrick J. Wong
2019-01-01  2:20 ` [PATCH 15/22] xfs: create libxfs helper to link a new inode into a directory Darrick J. Wong
2019-01-01  2:20 ` [PATCH 16/22] xfs: create libxfs helper to link an existing " Darrick J. Wong
2019-01-01  2:21 ` [PATCH 17/22] xfs: hoist inode free function to libxfs Darrick J. Wong
2019-01-01  2:21 ` [PATCH 18/22] xfs: create libxfs helper to remove an existing inode/name from a directory Darrick J. Wong
2019-01-01  2:21 ` [PATCH 19/22] xfs: create libxfs helper to exchange two directory entries Darrick J. Wong
2019-01-01  2:21 ` [PATCH 20/22] xfs: create libxfs helper to rename " Darrick J. Wong
2019-01-01  2:21 ` [PATCH 21/22] xfs: get rid of cross_rename Darrick J. Wong
2019-01-01  2:21 ` [PATCH 22/22] xfs: create library function to reset root inodes Darrick J. Wong
2019-01-17 14:25   ` Christoph Hellwig
2019-01-17 23:29     ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.