[PATCH 0/7] xfsprogs: libxfs spring cleaning take 3

[PATCH 0/7] xfsprogs: libxfs spring cleaning take 3

[Eric Sandeen]

Ok, i'll be damned if I have a nice handle on libxfs namespace still,
so just drop the 2 patches that touched it in any way, shape, or form.
Also drop the libxfs_getsb() arg patches, since Darrick would like me
to merge that from kernelspace later.

So this series just does some cosmetic fixups, mostly with an eye
towards sharing xfs_trans_inode.c with the kernel, which does finally
happen in patch 7.

ALl the previous RVBs are intact, only changes are libxfs namespace
swizzling, but I'll leave time for a NAK if anyone wants to.

Thanks,
-Eric

[PATCH 1/7] libxfs: Remove XACT_DEBUG #ifdefs

[Eric Sandeen]

Remove XACT_DEBUG #ifdefs to reduce more cosmetic differences
between userspace & kernelspace libxfs.  Add in some corresponding
(stubbed-out) tracepoint calls.

If these are felt to be particularly useful, the tracepoint calls
could be fleshed out to provide similar information.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 include/xfs_trace.h | 13 ++++++++
 libxfs/trans.c      | 85 ++++++++++++++---------------------------------------
 2 files changed, 35 insertions(+), 63 deletions(-)

diff --git a/include/xfs_trace.h b/include/xfs_trace.h
index 793ac56..4372004 100644
--- a/include/xfs_trace.h
+++ b/include/xfs_trace.h
@@ -161,6 +161,19 @@
 #define trace_xfs_perag_get_tag(a,b,c,d) ((c) = (c))
 #define trace_xfs_perag_put(a,b,c,d)	((c) = (c))
 
+#define trace_xfs_trans_alloc(a,b)		((void) 0)
+#define trace_xfs_trans_cancel(a,b)		((void) 0)
+#define trace_xfs_trans_brelse(a)		((void) 0)
+#define trace_xfs_trans_binval(a)		((void) 0)
+#define trace_xfs_trans_bjoin(a)		((void) 0)
+#define trace_xfs_trans_bhold(a)		((void) 0)
+#define trace_xfs_trans_get_buf(a)		((void) 0)
+#define trace_xfs_trans_getsb_recur(a)		((void) 0)
+#define trace_xfs_trans_getsb(a)		((void) 0)
+#define trace_xfs_trans_read_buf_recur(a)	((void) 0)
+#define trace_xfs_trans_read_buf(a)		((void) 0)
+#define trace_xfs_trans_commit(a,b)		((void) 0)
+
 #define trace_xfs_defer_cancel(a,b)		((void) 0)
 #define trace_xfs_defer_pending_commit(a,b)	((void) 0)
 #define trace_xfs_defer_pending_abort(a,b)	((void) 0)
diff --git a/libxfs/trans.c b/libxfs/trans.c
index cb15552..f50a9b2 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -18,6 +18,7 @@
 #include "xfs_trans.h"
 #include "xfs_sb.h"
 #include "xfs_defer.h"
+#include "xfs_trace.h"
 
 static void xfs_trans_free_items(struct xfs_trans *tp);
 STATIC struct xfs_trans *xfs_trans_dup(struct xfs_trans *tp);
@@ -269,9 +270,9 @@ libxfs_trans_alloc(
 		xfs_trans_cancel(tp);
 		return error;
 	}
-#ifdef XACT_DEBUG
-	fprintf(stderr, "allocated new transaction %p\n", tp);
-#endif
+
+	trace_xfs_trans_alloc(tp, _RET_IP_);
+
 	*tpp = tp;
 	return 0;
 }
@@ -317,23 +318,16 @@ void
 libxfs_trans_cancel(
 	struct xfs_trans	*tp)
 {
-#ifdef XACT_DEBUG
-	struct xfs_trans	*otp = tp;
-#endif
+	trace_xfs_trans_cancel(tp, _RET_IP_);
+
 	if (tp == NULL)
-		goto out;
+		return;
 
 	if (tp->t_flags & XFS_TRANS_PERM_LOG_RES)
 		xfs_defer_cancel(tp);
 
 	xfs_trans_free_items(tp);
 	xfs_trans_free(tp);
-
-out:
-#ifdef XACT_DEBUG
-	fprintf(stderr, "## cancelled transaction %p\n", otp);
-#endif
-	return;
 }
 
 void
@@ -353,10 +347,6 @@ libxfs_trans_ijoin(
 	iip->ili_lock_flags = lock_flags;
 
 	xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
-
-#ifdef XACT_DEBUG
-	fprintf(stderr, "ijoin'd inode %llu, transaction %p\n", ip->i_ino, tp);
-#endif
 }
 
 void
@@ -388,9 +378,6 @@ xfs_trans_log_inode(
 	uint			flags)
 {
 	ASSERT(ip->i_itemp != NULL);
-#ifdef XACT_DEBUG
-	fprintf(stderr, "dirtied inode %llu, transaction %p\n", ip->i_ino, tp);
-#endif
 
 	tp->t_flags |= XFS_TRANS_DIRTY;
 	set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
@@ -434,9 +421,6 @@ libxfs_trans_dirty_buf(
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 
-#ifdef XACT_DEBUG
-	fprintf(stderr, "dirtied buffer %p, transaction %p\n", bp, tp);
-#endif
 	tp->t_flags |= XFS_TRANS_DIRTY;
 	set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
 }
@@ -501,15 +485,14 @@ libxfs_trans_brelse(
 	xfs_buf_t		*bp)
 {
 	xfs_buf_log_item_t	*bip;
-#ifdef XACT_DEBUG
-	fprintf(stderr, "released buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	if (tp == NULL) {
 		ASSERT(bp->b_transp == NULL);
 		libxfs_putbuf(bp);
 		return;
 	}
+
+	trace_xfs_trans_brelse(bip);
 	ASSERT(bp->b_transp == tp);
 	bip = bp->b_log_item;
 	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
@@ -536,13 +519,12 @@ libxfs_trans_binval(
 	xfs_buf_t		*bp)
 {
 	xfs_buf_log_item_t	*bip = bp->b_log_item;
-#ifdef XACT_DEBUG
-	fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 
+	trace_xfs_trans_binval(bip);
+
 	if (bip->bli_flags & XFS_BLI_STALE)
 		return;
 	XFS_BUF_UNDELAYWRITE(bp);
@@ -563,14 +545,12 @@ libxfs_trans_bjoin(
 	xfs_buf_log_item_t	*bip;
 
 	ASSERT(bp->b_transp == NULL);
-#ifdef XACT_DEBUG
-	fprintf(stderr, "bjoin'd buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	xfs_buf_item_init(bp, tp->t_mountp);
 	bip = bp->b_log_item;
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
 	bp->b_transp = tp;
+	trace_xfs_trans_bjoin(bp->b_log_item);
 }
 
 void
@@ -582,11 +562,9 @@ libxfs_trans_bhold(
 
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
-#ifdef XACT_DEBUG
-	fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	bip->bli_flags |= XFS_BLI_HOLD;
+	trace_xfs_trans_bhold(bip);
 }
 
 xfs_buf_t *
@@ -615,13 +593,11 @@ libxfs_trans_get_buf_map(
 	bp = libxfs_getbuf_map(btp, map, nmaps, 0);
 	if (bp == NULL)
 		return NULL;
-#ifdef XACT_DEBUG
-	fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	libxfs_trans_bjoin(tp, bp);
 	bip = bp->b_log_item;
 	bip->bli_recur = 0;
+	trace_xfs_trans_get_buf(bp->b_log_item);
 	return bp;
 }
 
@@ -645,17 +621,16 @@ libxfs_trans_getsb(
 		bip = bp->b_log_item;
 		ASSERT(bip != NULL);
 		bip->bli_recur++;
+		trace_xfs_trans_getsb_recur(bip);
 		return bp;
 	}
 
 	bp = libxfs_getsb(mp, flags);
-#ifdef XACT_DEBUG
-	fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	libxfs_trans_bjoin(tp, bp);
 	bip = bp->b_log_item;
 	bip->bli_recur = 0;
+	trace_xfs_trans_getsb(bp->b_log_item);
 	return bp;
 }
 
@@ -692,6 +667,7 @@ libxfs_trans_read_buf_map(
 		ASSERT(bp->b_log_item != NULL);
 		bip = bp->b_log_item;
 		bip->bli_recur++;
+		trace_xfs_trans_read_buf_recur(bip);
 		goto done;
 	}
 
@@ -702,14 +678,11 @@ libxfs_trans_read_buf_map(
 	if (bp->b_error)
 		goto out_relse;
 
-#ifdef XACT_DEBUG
-	fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
-#endif
-
 	xfs_trans_bjoin(tp, bp);
 	bip = bp->b_log_item;
 	bip->bli_recur = 0;
 done:
+	trace_xfs_trans_read_buf(bp->b_log_item);
 	*bpp = bp;
 	return 0;
 out_relse:
@@ -825,10 +798,6 @@ inode_item_done(
 	}
 
 	libxfs_writebuf(bp, 0);
-#ifdef XACT_DEBUG
-	fprintf(stderr, "flushing dirty inode %llu, buffer %p\n",
-			ip->i_ino, bp);
-#endif
 free:
 	xfs_inode_item_put(iip);
 }
@@ -846,13 +815,8 @@ buf_item_done(
 	bp->b_transp = NULL;			/* remove xact ptr */
 
 	hold = (bip->bli_flags & XFS_BLI_HOLD);
-	if (bip->bli_flags & XFS_BLI_DIRTY) {
-#ifdef XACT_DEBUG
-		fprintf(stderr, "flushing/staling buffer %p (hold=%d)\n",
-			bp, hold);
-#endif
+	if (bip->bli_flags & XFS_BLI_DIRTY)
 		libxfs_writebuf_int(bp, 0);
-	}
 
 	bip->bli_flags &= ~XFS_BLI_HOLD;
 	xfs_buf_item_put(bip);
@@ -938,6 +902,8 @@ __xfs_trans_commit(
 	struct xfs_sb		*sbp;
 	int			error = 0;
 
+	trace_xfs_trans_commit(tp, _RET_IP_);
+
 	if (tp == NULL)
 		return 0;
 
@@ -953,12 +919,8 @@ __xfs_trans_commit(
 			goto out_unreserve;
 	}
 
-	if (!(tp->t_flags & XFS_TRANS_DIRTY)) {
-#ifdef XACT_DEBUG
-		fprintf(stderr, "committed clean transaction %p\n", tp);
-#endif
+	if (!(tp->t_flags & XFS_TRANS_DIRTY))
 		goto out_unreserve;
-	}
 
 	if (tp->t_flags & XFS_TRANS_SB_DIRTY) {
 		sbp = &(tp->t_mountp->m_sb);
@@ -973,9 +935,6 @@ __xfs_trans_commit(
 		xfs_log_sb(tp);
 	}
 
-#ifdef XACT_DEBUG
-	fprintf(stderr, "committing dirty transaction %p\n", tp);
-#endif
 	trans_committed(tp);
 
 	/* That's it for the transaction structure.  Free it. */
-- 
1.8.3.1

Re: [PATCH 1/7] libxfs: Remove XACT_DEBUG #ifdefs

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/7] libxfs: rename bli_format to avoid confusion with bli_formats

[Eric Sandeen]

Rename the bli_format structure to __bli_format to avoid
accidently confusing them with the bli_formats pointer.

(nb: userspace currently has no bli_formats pointer)

Source kernel commit: b94381737e9c4d014a4003e8ece9ba88670a2dd4

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 include/xfs_trans.h | 2 +-
 libxfs/logitem.c    | 6 +++---
 libxfs/trans.c      | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 10b7453..118fa0b 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -39,7 +39,7 @@ typedef struct xfs_buf_log_item {
 	struct xfs_buf		*bli_buf;	/* real buffer pointer */
 	unsigned int		bli_flags;	/* misc flags */
 	unsigned int		bli_recur;	/* recursion count */
-	xfs_buf_log_format_t	bli_format;	/* in-log header */
+	xfs_buf_log_format_t	__bli_format;	/* in-log header */
 } xfs_buf_log_item_t;
 
 #define XFS_BLI_DIRTY			(1<<0)
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index 4da9bc1..e862ab4 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -107,9 +107,9 @@ xfs_buf_item_init(
 	bip->bli_item.li_mountp = mp;
 	INIT_LIST_HEAD(&bip->bli_item.li_trans);
 	bip->bli_buf = bp;
-	bip->bli_format.blf_type = XFS_LI_BUF;
-	bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
-	bip->bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
+	bip->__bli_format.blf_type = XFS_LI_BUF;
+	bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
+	bip->__bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
 	bp->b_log_item = bip;
 }
 
diff --git a/libxfs/trans.c b/libxfs/trans.c
index f50a9b2..763daa0 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -531,8 +531,8 @@ libxfs_trans_binval(
 	xfs_buf_stale(bp);
 	bip->bli_flags |= XFS_BLI_STALE;
 	bip->bli_flags &= ~XFS_BLI_DIRTY;
-	bip->bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
-	bip->bli_format.blf_flags |= XFS_BLF_CANCEL;
+	bip->__bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
+	bip->__bli_format.blf_flags |= XFS_BLF_CANCEL;
 	set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
 	tp->t_flags |= XFS_TRANS_DIRTY;
 }
-- 
1.8.3.1

Re: [PATCH 2/7] libxfs: rename bli_format to avoid confusion with bli_formats

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 3/7] libxfs: factor common xfs_trans_bjoin code

[Eric Sandeen]

From: Christoph Hellwig <hch@lst.de>

Most of xfs_trans_bjoin is duplicated in xfs_trans_get_buf_map,
xfs_trans_getsb and xfs_trans_read_buf_map.  Add a new
_xfs_trans_bjoin which can be called by all three functions.

Source kernel commit: d7e84f413726876c0ec66bbf90770f69841f7663

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Alex Elder <aelder@sgi.com>
[sandeen: merge to userspace]
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/trans.c | 53 +++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 14 deletions(-)

diff --git a/libxfs/trans.c b/libxfs/trans.c
index 763daa0..dc924fa 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -537,19 +537,50 @@ libxfs_trans_binval(
 	tp->t_flags |= XFS_TRANS_DIRTY;
 }
 
-void
-libxfs_trans_bjoin(
-	xfs_trans_t		*tp,
-	xfs_buf_t		*bp)
+/*
+ * Add the locked buffer to the transaction.
+ *
+ * The buffer must be locked, and it cannot be associated with any
+ * transaction.
+ *
+ * If the buffer does not yet have a buf log item associated with it,
+ * then allocate one for it.  Then add the buf item to the transaction.
+ */
+STATIC void
+_libxfs_trans_bjoin(
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp,
+	int			reset_recur)
 {
-	xfs_buf_log_item_t	*bip;
+	struct xfs_buf_log_item	*bip;
 
 	ASSERT(bp->b_transp == NULL);
 
+        /*
+	 * The xfs_buf_log_item pointer is stored in b_log_item.  If
+	 * it doesn't have one yet, then allocate one and initialize it.
+	 * The checks to see if one is there are in xfs_buf_item_init().
+	 */
 	xfs_buf_item_init(bp, tp->t_mountp);
 	bip = bp->b_log_item;
+	if (reset_recur)
+		bip->bli_recur = 0;
+
+	/*
+	 * Attach the item to the transaction so we can find it in
+	 * xfs_trans_get_buf() and friends.
+	 */
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
 	bp->b_transp = tp;
+
+}
+
+void
+libxfs_trans_bjoin(
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp)
+{
+	_libxfs_trans_bjoin(tp, bp, 0);
 	trace_xfs_trans_bjoin(bp->b_log_item);
 }
 
@@ -594,9 +625,7 @@ libxfs_trans_get_buf_map(
 	if (bp == NULL)
 		return NULL;
 
-	libxfs_trans_bjoin(tp, bp);
-	bip = bp->b_log_item;
-	bip->bli_recur = 0;
+	_libxfs_trans_bjoin(tp, bp, 1);
 	trace_xfs_trans_get_buf(bp->b_log_item);
 	return bp;
 }
@@ -627,9 +656,7 @@ libxfs_trans_getsb(
 
 	bp = libxfs_getsb(mp, flags);
 
-	libxfs_trans_bjoin(tp, bp);
-	bip = bp->b_log_item;
-	bip->bli_recur = 0;
+	_libxfs_trans_bjoin(tp, bp, 1);
 	trace_xfs_trans_getsb(bp->b_log_item);
 	return bp;
 }
@@ -678,9 +705,7 @@ libxfs_trans_read_buf_map(
 	if (bp->b_error)
 		goto out_relse;
 
-	xfs_trans_bjoin(tp, bp);
-	bip = bp->b_log_item;
-	bip->bli_recur = 0;
+	_libxfs_trans_bjoin(tp, bp, 1);
 done:
 	trace_xfs_trans_read_buf(bp->b_log_item);
 	*bpp = bp;
-- 
1.8.3.1

Re: [PATCH 3/7] libxfs: factor common xfs_trans_bjoin code

[Christoph Hellwig]

Looks just as good in userspace as it did in the kernel :)

[PATCH 4/7] libxfs: fix argument to xfs_trans_add_item

[Eric Sandeen]

The hack of casting an inode_log_item or buf_log_item to a
xfs_log_item_t is pretty gross; yes it's the first member in the
structure, but yuk.  Pass in the correct structure member.

This was fixed in the kernel with commit e98c414f9
("xfs: simplify log item descriptor tracking")

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 libxfs/trans.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libxfs/trans.c b/libxfs/trans.c
index dc924fa..c7a1d52 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -346,7 +346,7 @@ libxfs_trans_ijoin(
 	ASSERT(iip->ili_lock_flags == 0);
 	iip->ili_lock_flags = lock_flags;
 
-	xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
+	xfs_trans_add_item(tp, &iip->ili_item);
 }
 
 void
@@ -570,7 +570,7 @@ _libxfs_trans_bjoin(
 	 * Attach the item to the transaction so we can find it in
 	 * xfs_trans_get_buf() and friends.
 	 */
-	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
+	xfs_trans_add_item(tp, &bip->bli_item);
 	bp->b_transp = tp;
 
 }
-- 
1.8.3.1

Re: [PATCH 4/7] libxfs: fix argument to xfs_trans_add_item

[Christoph Hellwig]

On Mon, May 20, 2019 at 10:47:04PM -0500, Eric Sandeen wrote:
> The hack of casting an inode_log_item or buf_log_item to a
> xfs_log_item_t is pretty gross; yes it's the first member in the
> structure, but yuk.  Pass in the correct structure member.
> 
> This was fixed in the kernel with commit e98c414f9
> ("xfs: simplify log item descriptor tracking")
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 5/7] xfs: factor log item initialisation

[Eric Sandeen]

From: Dave Chinner <dchinner@redhat.com>

Each log item type does manual initialisation of the log item.
Delayed logging introduces new fields that need initialisation, so
factor all the open coded initialisation into a common function
first.

Source kernel commit: 43f5efc5b59db1b66e39fe9fdfc4ba6a27152afa

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
[sandeen: merge to userspace]
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/libxfs_priv.h |  1 +
 libxfs/logitem.c     |  8 ++------
 libxfs/util.c        | 12 ++++++++++++
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index d668a15..a8c0f0b 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -564,6 +564,7 @@ int libxfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
 
 
 bool xfs_log_check_lsn(struct xfs_mount *, xfs_lsn_t);
+void xfs_log_item_init(struct xfs_mount *, struct xfs_log_item *, int);
 #define xfs_log_in_recovery(mp)	(false)
 
 /* xfs_icache.c */
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index e862ab4..14c62f6 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -103,9 +103,7 @@ xfs_buf_item_init(
 	fprintf(stderr, "adding buf item %p for not-logged buffer %p\n",
 		bip, bp);
 #endif
-	bip->bli_item.li_type = XFS_LI_BUF;
-	bip->bli_item.li_mountp = mp;
-	INIT_LIST_HEAD(&bip->bli_item.li_trans);
+	xfs_log_item_init(mp, &bip->bli_item, XFS_LI_BUF);
 	bip->bli_buf = bp;
 	bip->__bli_format.blf_type = XFS_LI_BUF;
 	bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
@@ -149,8 +147,6 @@ xfs_inode_item_init(
 		ip->i_ino, iip);
 #endif
 
-	iip->ili_item.li_type = XFS_LI_INODE;
-	iip->ili_item.li_mountp = mp;
-	INIT_LIST_HEAD(&iip->ili_item.li_trans);
+	xfs_log_item_init(mp, &iip->ili_item, XFS_LI_INODE);
 	iip->ili_inode = ip;
 }
diff --git a/libxfs/util.c b/libxfs/util.c
index 8c9954f..0496dcb 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -692,6 +692,18 @@ xfs_log_check_lsn(
 	return true;
 }
 
+void
+xfs_log_item_init(
+	struct xfs_mount	*mp,
+	struct xfs_log_item	*item,
+	int			type)
+{
+	item->li_mountp = mp; 
+	item->li_type = type;
+        
+	INIT_LIST_HEAD(&item->li_trans);
+}   
+
 static struct xfs_buftarg *
 xfs_find_bdev_for_inode(
 	struct xfs_inode	*ip)
-- 
1.8.3.1

Re: [PATCH 5/7] xfs: factor log item initialisation

[Christoph Hellwig]

Looks good.

[PATCH 6/7] libxfs: create current_time helper and sync xfs_trans_ichgtime

[Eric Sandeen]

Make xfs_trans_ichgtime() almost match kernelspace by creating a
new current_time() helper to match the kernel utility.

This reduces still more cosmetic change.  We may want to sync the
creation flag over to the kernel even though it's not used today.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 include/xfs_inode.h | 14 ++++++++++++++
 libxfs/util.c       | 28 ++++++++++++++++++++--------
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index 88b58ac..9565adc 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -17,6 +17,16 @@ struct xfs_inode_log_item;
 struct xfs_dir_ops;
 
 /*
+ * These are not actually used, they are only for userspace build
+ * compatibility in code that looks at i_state
+ */
+#define I_DIRTY_TIME		0
+#define I_DIRTY_TIME_EXPIRED	0
+
+#define IS_I_VERSION(inode)			(0)
+#define inode_maybe_inc_iversion(inode,flags)	(0)
+
+/*
  * Inode interface. This fakes up a "VFS inode" to make the xfs_inode appear
  * similar to the kernel which now is used tohold certain parts of the on-disk
  * metadata.
@@ -25,6 +35,7 @@ struct inode {
 	mode_t		i_mode;
 	uint32_t	i_nlink;
 	xfs_dev_t	i_rdev;		/* This actually holds xfs_dev_t */
+	unsigned long	i_state;	/* Not actually used in userspace */
 	uint32_t	i_generation;
 	uint64_t	i_version;
 	struct timespec	i_atime;
@@ -149,6 +160,9 @@ extern void	libxfs_trans_ichgtime(struct xfs_trans *,
 				struct xfs_inode *, int);
 extern int	libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
 
+#define timespec64 timespec
+extern struct timespec64 current_time(struct inode *inode);
+
 /* Inode Cache Interfaces */
 extern bool	libxfs_inode_verify_forks(struct xfs_inode *ip);
 extern int	libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
diff --git a/libxfs/util.c b/libxfs/util.c
index 0496dcb..2ba9dc2 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -136,11 +136,21 @@ xfs_log_calc_unit_res(
 	return unit_bytes;
 }
 
+struct timespec64
+current_time(struct inode *inode)
+{
+	struct timespec64	tv;
+	struct timeval		stv;
+
+	gettimeofday(&stv, (struct timezone *)0);
+	tv.tv_sec = stv.tv_sec;
+	tv.tv_nsec = stv.tv_usec * 1000;
+
+	return tv;
+}
+
 /*
  * Change the requested timestamp in the given inode.
- *
- * This was once shared with the kernel, but has diverged to the point
- * where it's no longer worth the hassle of maintaining common code.
  */
 void
 libxfs_trans_ichgtime(
@@ -148,12 +158,14 @@ libxfs_trans_ichgtime(
 	struct xfs_inode	*ip,
 	int			flags)
 {
-	struct timespec tv;
-	struct timeval	stv;
+	struct inode		*inode = VFS_I(ip);
+	struct timespec64	tv;
+
+	ASSERT(tp);
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+
+	tv = current_time(inode);
 
-	gettimeofday(&stv, (struct timezone *)0);
-	tv.tv_sec = stv.tv_sec;
-	tv.tv_nsec = stv.tv_usec * 1000;
 	if (flags & XFS_ICHGTIME_MOD)
 		VFS_I(ip)->i_mtime = tv;
 	if (flags & XFS_ICHGTIME_CHG)
-- 
1.8.3.1

Re: [PATCH 6/7] libxfs: create current_time helper and sync xfs_trans_ichgtime

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 7/7] libxfs: share kernel's xfs_trans_inode.c

[Eric Sandeen]

Now that the majority of cosmetic changes and compat shims
are in place, we can directly share kernelspace's
xfs_trans_inode.c with just a couple more small tweaks.
In addition to the file move,

* ili_fsync_fields is added to xfs_inode_log_item (but not used)
* test_and_set_bit() helper is created

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 include/xfs_trans.h  |  3 ++-
 libxfs/Makefile      |  1 +
 libxfs/libxfs_priv.h |  9 ++++++++
 libxfs/trans.c       | 65 ----------------------------------------------------
 libxfs/util.c        | 27 ----------------------
 5 files changed, 12 insertions(+), 93 deletions(-)

diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 118fa0b..60e1dbd 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -30,8 +30,9 @@ typedef struct xfs_inode_log_item {
 	xfs_log_item_t		ili_item;		/* common portion */
 	struct xfs_inode	*ili_inode;		/* inode pointer */
 	unsigned short		ili_lock_flags;		/* lock flags */
-	unsigned int		ili_fields;		/* fields to be logged */
 	unsigned int		ili_last_fields;	/* fields when flushed*/
+	unsigned int		ili_fields;		/* fields to be logged */
+	unsigned int		ili_fsync_fields;	/* ignored by userspace */
 } xfs_inode_log_item_t;
 
 typedef struct xfs_buf_log_item {
diff --git a/libxfs/Makefile b/libxfs/Makefile
index 160498d..8c681e0 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -93,6 +93,7 @@ CFILES = cache.c \
 	xfs_rtbitmap.c \
 	xfs_sb.c \
 	xfs_symlink_remote.c \
+	xfs_trans_inode.c \
 	xfs_trans_resv.c \
 	xfs_types.c
 
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index a8c0f0b..c75b92d 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -608,6 +608,15 @@ static inline int test_bit(int nr, const volatile unsigned long *addr)
 	return *p & mask;
 }
 
+/* Sets and returns original value of the bit */
+static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
+{
+	if (test_bit(nr, addr))
+		return 1;
+	set_bit(nr, addr);
+	return 0;
+}
+
 /* Keep static checkers quiet about nonstatic functions by exporting */
 int xfs_inode_hasattr(struct xfs_inode *ip);
 int xfs_attr_get_ilocked(struct xfs_inode *ip, struct xfs_da_args *args);
diff --git a/libxfs/trans.c b/libxfs/trans.c
index c7a1d52..5c56b4f 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -331,25 +331,6 @@ libxfs_trans_cancel(
 }
 
 void
-libxfs_trans_ijoin(
-	xfs_trans_t		*tp,
-	xfs_inode_t		*ip,
-	uint			lock_flags)
-{
-	xfs_inode_log_item_t	*iip;
-
-	if (ip->i_itemp == NULL)
-		xfs_inode_item_init(ip, ip->i_mount);
-	iip = ip->i_itemp;
-	ASSERT(iip->ili_inode != NULL);
-
-	ASSERT(iip->ili_lock_flags == 0);
-	iip->ili_lock_flags = lock_flags;
-
-	xfs_trans_add_item(tp, &iip->ili_item);
-}
-
-void
 libxfs_trans_inode_alloc_buf(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
@@ -363,52 +344,6 @@ libxfs_trans_inode_alloc_buf(
 }
 
 /*
- * This is called to mark the fields indicated in fieldmask as needing
- * to be logged when the transaction is committed.  The inode must
- * already be associated with the given transaction.
- *
- * The values for fieldmask are defined in xfs_log_format.h.  We always
- * log all of the core inode if any of it has changed, and we always log
- * all of the inline data/extents/b-tree root if any of them has changed.
- */
-void
-xfs_trans_log_inode(
-	xfs_trans_t		*tp,
-	xfs_inode_t		*ip,
-	uint			flags)
-{
-	ASSERT(ip->i_itemp != NULL);
-
-	tp->t_flags |= XFS_TRANS_DIRTY;
-	set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
-
-	/*
-	 * Always OR in the bits from the ili_last_fields field.
-	 * This is to coordinate with the xfs_iflush() and xfs_iflush_done()
-	 * routines in the eventual clearing of the ilf_fields bits.
-	 * See the big comment in xfs_iflush() for an explanation of
-	 * this coordination mechanism.
-	 */
-	flags |= ip->i_itemp->ili_last_fields;
-	ip->i_itemp->ili_fields |= flags;
-}
-
-int
-libxfs_trans_roll_inode(
-	struct xfs_trans	**tpp,
-	struct xfs_inode	*ip)
-{
-	int			error;
-
-	xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
-	error = xfs_trans_roll(tpp);
-	if (!error)
-		xfs_trans_ijoin(*tpp, ip, 0);
-	return error;
-}
-
-
-/*
  * Mark a buffer dirty in the transaction.
  */
 void
diff --git a/libxfs/util.c b/libxfs/util.c
index 2ba9dc2..171a172 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -149,33 +149,6 @@ current_time(struct inode *inode)
 	return tv;
 }
 
-/*
- * Change the requested timestamp in the given inode.
- */
-void
-libxfs_trans_ichgtime(
-	struct xfs_trans	*tp,
-	struct xfs_inode	*ip,
-	int			flags)
-{
-	struct inode		*inode = VFS_I(ip);
-	struct timespec64	tv;
-
-	ASSERT(tp);
-	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-
-	tv = current_time(inode);
-
-	if (flags & XFS_ICHGTIME_MOD)
-		VFS_I(ip)->i_mtime = tv;
-	if (flags & XFS_ICHGTIME_CHG)
-		VFS_I(ip)->i_ctime = tv;
-	if (flags & XFS_ICHGTIME_CREATE) {
-		ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
-		ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
-	}
-}
-
 STATIC uint16_t
 xfs_flags2diflags(
 	struct xfs_inode	*ip,
-- 
1.8.3.1

[PATCH 7/7 V2] libxfs: share kernel's xfs_trans_inode.c

[Eric Sandeen]

Now that the majority of cosmetic changes and compat shims
are in place, we can directly share kernelspace's
xfs_trans_inode.c with just a couple more small tweaks.
In addition to the file move,

* ili_fsync_fields is added to xfs_inode_log_item (but not used)
* test_and_set_bit() helper is created

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---

V2: good grief, get the new file in the patch

 include/xfs_trans.h      |    3 
 libxfs/Makefile          |    1 
 libxfs/libxfs_priv.h     |    9 ++
 libxfs/trans.c           |   65 -------------------
 libxfs/util.c            |   27 --------
 libxfs/xfs_trans_inode.c |  155 +++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 167 insertions(+), 93 deletions(-)

diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 118fa0b..60e1dbd 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -30,8 +30,9 @@ typedef struct xfs_inode_log_item {
 	xfs_log_item_t		ili_item;		/* common portion */
 	struct xfs_inode	*ili_inode;		/* inode pointer */
 	unsigned short		ili_lock_flags;		/* lock flags */
-	unsigned int		ili_fields;		/* fields to be logged */
 	unsigned int		ili_last_fields;	/* fields when flushed*/
+	unsigned int		ili_fields;		/* fields to be logged */
+	unsigned int		ili_fsync_fields;	/* ignored by userspace */
 } xfs_inode_log_item_t;
 
 typedef struct xfs_buf_log_item {
diff --git a/libxfs/Makefile b/libxfs/Makefile
index 160498d..8c681e0 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -93,6 +93,7 @@ CFILES = cache.c \
 	xfs_rtbitmap.c \
 	xfs_sb.c \
 	xfs_symlink_remote.c \
+	xfs_trans_inode.c \
 	xfs_trans_resv.c \
 	xfs_types.c
 
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index a8c0f0b..c75b92d 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -608,6 +608,15 @@ static inline int test_bit(int nr, const volatile unsigned long *addr)
 	return *p & mask;
 }
 
+/* Sets and returns original value of the bit */
+static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
+{
+	if (test_bit(nr, addr))
+		return 1;
+	set_bit(nr, addr);
+	return 0;
+}
+
 /* Keep static checkers quiet about nonstatic functions by exporting */
 int xfs_inode_hasattr(struct xfs_inode *ip);
 int xfs_attr_get_ilocked(struct xfs_inode *ip, struct xfs_da_args *args);
diff --git a/libxfs/trans.c b/libxfs/trans.c
index c7a1d52..5c56b4f 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -331,25 +331,6 @@ libxfs_trans_cancel(
 }
 
 void
-libxfs_trans_ijoin(
-	xfs_trans_t		*tp,
-	xfs_inode_t		*ip,
-	uint			lock_flags)
-{
-	xfs_inode_log_item_t	*iip;
-
-	if (ip->i_itemp == NULL)
-		xfs_inode_item_init(ip, ip->i_mount);
-	iip = ip->i_itemp;
-	ASSERT(iip->ili_inode != NULL);
-
-	ASSERT(iip->ili_lock_flags == 0);
-	iip->ili_lock_flags = lock_flags;
-
-	xfs_trans_add_item(tp, &iip->ili_item);
-}
-
-void
 libxfs_trans_inode_alloc_buf(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
@@ -363,52 +344,6 @@ libxfs_trans_inode_alloc_buf(
 }
 
 /*
- * This is called to mark the fields indicated in fieldmask as needing
- * to be logged when the transaction is committed.  The inode must
- * already be associated with the given transaction.
- *
- * The values for fieldmask are defined in xfs_log_format.h.  We always
- * log all of the core inode if any of it has changed, and we always log
- * all of the inline data/extents/b-tree root if any of them has changed.
- */
-void
-xfs_trans_log_inode(
-	xfs_trans_t		*tp,
-	xfs_inode_t		*ip,
-	uint			flags)
-{
-	ASSERT(ip->i_itemp != NULL);
-
-	tp->t_flags |= XFS_TRANS_DIRTY;
-	set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
-
-	/*
-	 * Always OR in the bits from the ili_last_fields field.
-	 * This is to coordinate with the xfs_iflush() and xfs_iflush_done()
-	 * routines in the eventual clearing of the ilf_fields bits.
-	 * See the big comment in xfs_iflush() for an explanation of
-	 * this coordination mechanism.
-	 */
-	flags |= ip->i_itemp->ili_last_fields;
-	ip->i_itemp->ili_fields |= flags;
-}
-
-int
-libxfs_trans_roll_inode(
-	struct xfs_trans	**tpp,
-	struct xfs_inode	*ip)
-{
-	int			error;
-
-	xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
-	error = xfs_trans_roll(tpp);
-	if (!error)
-		xfs_trans_ijoin(*tpp, ip, 0);
-	return error;
-}
-
-
-/*
  * Mark a buffer dirty in the transaction.
  */
 void
diff --git a/libxfs/util.c b/libxfs/util.c
index 2ba9dc2..171a172 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -149,33 +149,6 @@ current_time(struct inode *inode)
 	return tv;
 }
 
-/*
- * Change the requested timestamp in the given inode.
- */
-void
-libxfs_trans_ichgtime(
-	struct xfs_trans	*tp,
-	struct xfs_inode	*ip,
-	int			flags)
-{
-	struct inode		*inode = VFS_I(ip);
-	struct timespec64	tv;
-
-	ASSERT(tp);
-	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-
-	tv = current_time(inode);
-
-	if (flags & XFS_ICHGTIME_MOD)
-		VFS_I(ip)->i_mtime = tv;
-	if (flags & XFS_ICHGTIME_CHG)
-		VFS_I(ip)->i_ctime = tv;
-	if (flags & XFS_ICHGTIME_CREATE) {
-		ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
-		ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
-	}
-}
-
 STATIC uint16_t
 xfs_flags2diflags(
 	struct xfs_inode	*ip,
diff --git a/libxfs/xfs_trans_inode.c b/libxfs/xfs_trans_inode.c
new file mode 100644
index 0000000..87e6335
--- /dev/null
+++ b/libxfs/xfs_trans_inode.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000,2005 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ */
+#include "libxfs_priv.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_mount.h"
+#include "xfs_inode.h"
+#include "xfs_trans.h"
+#include "xfs_trace.h"
+
+/*
+ * Add a locked inode to the transaction.
+ *
+ * The inode must be locked, and it cannot be associated with any transaction.
+ * If lock_flags is non-zero the inode will be unlocked on transaction commit.
+ */
+void
+xfs_trans_ijoin(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip,
+	uint			lock_flags)
+{
+	xfs_inode_log_item_t	*iip;
+
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+	if (ip->i_itemp == NULL)
+		xfs_inode_item_init(ip, ip->i_mount);
+	iip = ip->i_itemp;
+
+	ASSERT(iip->ili_lock_flags == 0);
+	iip->ili_lock_flags = lock_flags;
+
+	/*
+	 * Get a log_item_desc to point at the new item.
+	 */
+	xfs_trans_add_item(tp, &iip->ili_item);
+}
+
+/*
+ * Transactional inode timestamp update. Requires the inode to be locked and
+ * joined to the transaction supplied. Relies on the transaction subsystem to
+ * track dirty state and update/writeback the inode accordingly.
+ */
+void
+xfs_trans_ichgtime(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip,
+	int			flags)
+{
+	struct inode		*inode = VFS_I(ip);
+	struct timespec64 tv;
+
+	ASSERT(tp);
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+
+	tv = current_time(inode);
+
+	if (flags & XFS_ICHGTIME_MOD)
+		inode->i_mtime = tv;
+	if (flags & XFS_ICHGTIME_CHG)
+		inode->i_ctime = tv;
+	if (flags & XFS_ICHGTIME_CREATE) {
+		ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
+		ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
+	}
+}
+
+/*
+ * This is called to mark the fields indicated in fieldmask as needing
+ * to be logged when the transaction is committed.  The inode must
+ * already be associated with the given transaction.
+ *
+ * The values for fieldmask are defined in xfs_inode_item.h.  We always
+ * log all of the core inode if any of it has changed, and we always log
+ * all of the inline data/extents/b-tree root if any of them has changed.
+ */
+void
+xfs_trans_log_inode(
+	xfs_trans_t	*tp,
+	xfs_inode_t	*ip,
+	uint		flags)
+{
+	struct inode	*inode = VFS_I(ip);
+
+	ASSERT(ip->i_itemp != NULL);
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+
+	/*
+	 * Don't bother with i_lock for the I_DIRTY_TIME check here, as races
+	 * don't matter - we either will need an extra transaction in 24 hours
+	 * to log the timestamps, or will clear already cleared fields in the
+	 * worst case.
+	 */
+	if (inode->i_state & (I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED)) {
+		spin_lock(&inode->i_lock);
+		inode->i_state &= ~(I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED);
+		spin_unlock(&inode->i_lock);
+	}
+
+	/*
+	 * Record the specific change for fdatasync optimisation. This
+	 * allows fdatasync to skip log forces for inodes that are only
+	 * timestamp dirty. We do this before the change count so that
+	 * the core being logged in this case does not impact on fdatasync
+	 * behaviour.
+	 */
+	ip->i_itemp->ili_fsync_fields |= flags;
+
+	/*
+	 * First time we log the inode in a transaction, bump the inode change
+	 * counter if it is configured for this to occur. While we have the
+	 * inode locked exclusively for metadata modification, we can usually
+	 * avoid setting XFS_ILOG_CORE if no one has queried the value since
+	 * the last time it was incremented. If we have XFS_ILOG_CORE already
+	 * set however, then go ahead and bump the i_version counter
+	 * unconditionally.
+	 */
+	if (!test_and_set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags) &&
+	    IS_I_VERSION(VFS_I(ip))) {
+		if (inode_maybe_inc_iversion(VFS_I(ip), flags & XFS_ILOG_CORE))
+			flags |= XFS_ILOG_CORE;
+	}
+
+	tp->t_flags |= XFS_TRANS_DIRTY;
+
+	/*
+	 * Always OR in the bits from the ili_last_fields field.
+	 * This is to coordinate with the xfs_iflush() and xfs_iflush_done()
+	 * routines in the eventual clearing of the ili_fields bits.
+	 * See the big comment in xfs_iflush() for an explanation of
+	 * this coordination mechanism.
+	 */
+	flags |= ip->i_itemp->ili_last_fields;
+	ip->i_itemp->ili_fields |= flags;
+}
+
+int
+xfs_trans_roll_inode(
+	struct xfs_trans	**tpp,
+	struct xfs_inode	*ip)
+{
+	int			error;
+
+	xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
+	error = xfs_trans_roll(tpp);
+	if (!error)
+		xfs_trans_ijoin(*tpp, ip, 0);
+	return error;
+}

Re: [PATCH 7/7 V2] libxfs: share kernel's xfs_trans_inode.c

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>