All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 02/10] xfs: catch log items multiply joined to a transaction
Date: Wed,  2 May 2018 18:01:49 +1000	[thread overview]
Message-ID: <20180502080157.11386-3-david@fromorbit.com> (raw)
In-Reply-To: <20180502080157.11386-1-david@fromorbit.com>

From: Dave Chinner <dchinner@redhat.com>

I've comes across a series of subtle bugs in development code that
are a result of inodes being joined to the same transaction more
than once. This means the transaction has multiple log item
descriptors pointing to the same log item, and so processes them
multiple times during transaction commit processing. Further, the
log item can only store a single log item descriptor back pointer,
so this means all but one of the log item descriptors pointing
to the log item can't be found from the log item.

Add a log item flag to say the item has been joined to a transaction
and assert that it's not set when adding a log item to a
transaction. Clear it when the log item is disconnected from the log
item descriptor. This will tell us if there are other log items that
transactions are referencing multiple times.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_trans.c | 6 ++++--
 fs/xfs/xfs_trans.h | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index c6a2a3cb9df5..33c40788cffa 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -741,6 +741,7 @@ xfs_trans_add_item(
 
 	ASSERT(lip->li_mountp == tp->t_mountp);
 	ASSERT(lip->li_ailp == tp->t_mountp->m_ail);
+	ASSERT(!test_bit(XFS_LI_TRANS, &lip->li_flags));
 
 	lidp = kmem_zone_zalloc(xfs_log_item_desc_zone, KM_SLEEP | KM_NOFS);
 
@@ -749,6 +750,7 @@ xfs_trans_add_item(
 	list_add_tail(&lidp->lid_trans, &tp->t_items);
 
 	lip->li_desc = lidp;
+	set_bit(XFS_LI_TRANS, &lip->li_flags);
 }
 
 STATIC void
@@ -766,6 +768,7 @@ void
 xfs_trans_del_item(
 	struct xfs_log_item	*lip)
 {
+	clear_bit(XFS_LI_TRANS, &lip->li_flags);
 	xfs_trans_free_item_desc(lip->li_desc);
 	lip->li_desc = NULL;
 }
@@ -785,7 +788,7 @@ xfs_trans_free_items(
 	list_for_each_entry_safe(lidp, next, &tp->t_items, lid_trans) {
 		struct xfs_log_item	*lip = lidp->lid_item;
 
-		lip->li_desc = NULL;
+		xfs_trans_del_item(lip);
 
 		if (commit_lsn != NULLCOMMITLSN)
 			lip->li_ops->iop_committing(lip, commit_lsn);
@@ -793,7 +796,6 @@ xfs_trans_free_items(
 			set_bit(XFS_LI_ABORTED, &lip->li_flags);
 		lip->li_ops->iop_unlock(lip);
 
-		xfs_trans_free_item_desc(lidp);
 	}
 }
 
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 51145ddf7e5b..af52107adffc 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -72,11 +72,13 @@ typedef struct xfs_log_item {
 #define	XFS_LI_IN_AIL	0
 #define	XFS_LI_ABORTED	1
 #define	XFS_LI_FAILED	2
+#define	XFS_LI_TRANS	3	/* attached to an active transaction */
 
 #define XFS_LI_FLAGS \
 	{ (1 << XFS_LI_IN_AIL),		"IN_AIL" }, \
 	{ (1 << XFS_LI_ABORTED),	"ABORTED" }, \
-	{ (1 << XFS_LI_FAILED),		"FAILED" }
+	{ (1 << XFS_LI_FAILED),		"FAILED" }, \
+	{ (1 << XFS_LI_TRANS),		"TRANS" }
 
 struct xfs_item_ops {
 	void (*iop_size)(xfs_log_item_t *, int *, int *);
-- 
2.17.0


  parent reply	other threads:[~2018-05-02  8:02 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-02  8:01 [PATCH 0/10] xfs: log item and transaction cleanups Dave Chinner
2018-05-02  8:01 ` [PATCH 01/10] xfs: log item flags are racy Dave Chinner
2018-05-07 12:16   ` Brian Foster
2018-05-07 14:45   ` Christoph Hellwig
2018-05-02  8:01 ` Dave Chinner [this message]
2018-05-07 12:16   ` [PATCH 02/10] xfs: catch log items multiply joined to a transaction Brian Foster
2018-05-07 14:48   ` Christoph Hellwig
2018-05-08  0:06     ` Dave Chinner
2018-05-02  8:01 ` [PATCH 03/10] xfs: add tracing to high level transaction operations Dave Chinner
2018-05-07 12:17   ` Brian Foster
2018-05-07 14:48   ` Christoph Hellwig
2018-05-02  8:01 ` [PATCH 04/10] xfs: adder caller IP to xfs_defer* tracepoints Dave Chinner
2018-05-07 12:17   ` Brian Foster
2018-05-07 14:49   ` Christoph Hellwig
2018-05-02  8:01 ` [PATCH 05/10] xfs: don't assert fail with AIL lock held Dave Chinner
2018-05-07 12:18   ` Brian Foster
2018-05-07 14:50     ` Christoph Hellwig
2018-05-07 23:59       ` Dave Chinner
2018-05-02  8:01 ` [PATCH 06/10] xfs: fix double ijoin in xfs_inactive_symlink_rmt() Dave Chinner
2018-05-07 14:51   ` Christoph Hellwig
2018-05-02  8:01 ` [PATCH 07/10] xfs: fix double ijoin in xfs_reflink_cancel_cow_range Dave Chinner
2018-05-07 14:51   ` Christoph Hellwig
2018-05-02  8:01 ` [PATCH 08/10] xfs: fix double ijoin in xfs_reflink_clear_inode_flag() Dave Chinner
2018-05-07 14:52   ` Christoph Hellwig
2018-05-02  8:01 ` [PATCH 09/10] xfs: add some more debug checks to buffer log item reuse Dave Chinner
2018-05-07 14:52   ` Christoph Hellwig
2018-05-02  8:01 ` [PATCH 10/10] xfs: get rid of the log item descriptor Dave Chinner
2018-05-02 20:05   ` Darrick J. Wong
2018-05-02 21:53     ` Dave Chinner
2018-05-03 23:47       ` Darrick J. Wong
2018-05-07 14:55   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180502080157.11386-3-david@fromorbit.com \
    --to=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.