diff mbox

xfs: strengthen log intent item refcount checking

Message ID 20170421200043.GL23371@birch.djwong.org (mailing list archive)
State Accepted
Headers show

Commit Message

Darrick J. Wong April 21, 2017, 8 p.m. UTC
Use ASSERTs on the log intent item refcounts so that we fail noisily if
anyone tries to double-free the item.  This will help us diagnose
potential problems with the deferred-ops mechanism via generic/388.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_bmap_item.c     |    1 +
 fs/xfs/xfs_extfree_item.c  |    1 +
 fs/xfs/xfs_refcount_item.c |    1 +
 fs/xfs/xfs_rmap_item.c     |    1 +
 4 files changed, 4 insertions(+)

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Christoph Hellwig April 23, 2017, 8:23 a.m. UTC | #1
On Fri, Apr 21, 2017 at 01:00:43PM -0700, Darrick J. Wong wrote:
> Use ASSERTs on the log intent item refcounts so that we fail noisily if
> anyone tries to double-free the item.  This will help us diagnose
> potential problems with the deferred-ops mechanism via generic/388.

Any specific problems you're looking at?

Otherwise looks fine to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick J. Wong April 23, 2017, 5:39 p.m. UTC | #2
On Sun, Apr 23, 2017 at 01:23:15AM -0700, Christoph Hellwig wrote:
> On Fri, Apr 21, 2017 at 01:00:43PM -0700, Darrick J. Wong wrote:
> > Use ASSERTs on the log intent item refcounts so that we fail noisily if
> > anyone tries to double-free the item.  This will help us diagnose
> > potential problems with the deferred-ops mechanism via generic/388.
> 
> Any specific problems you're looking at?

Not at the moment -- this was a debug patch I'd written up the last
there were refcounting problems with xfs_defer, and the refcount_t
discussions prompted me to send this in as a stopgap until we figure
out what we're doing w.r.t. refcount_t.

--D

> 
> Otherwise looks fine to me:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 055ab8f..41262dc 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -217,6 +217,7 @@  void
 xfs_bui_release(
 	struct xfs_bui_log_item	*buip)
 {
+	ASSERT(atomic_read(&buip->bui_refcount) > 0);
 	if (atomic_dec_and_test(&buip->bui_refcount)) {
 		xfs_trans_ail_remove(&buip->bui_item, SHUTDOWN_LOG_IO_ERROR);
 		xfs_bui_item_free(buip);
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index d7bc149..44f8c54 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -290,6 +290,7 @@  void
 xfs_efi_release(
 	struct xfs_efi_log_item	*efip)
 {
+	ASSERT(atomic_read(&efip->efi_refcount) > 0);
 	if (atomic_dec_and_test(&efip->efi_refcount)) {
 		xfs_trans_ail_remove(&efip->efi_item, SHUTDOWN_LOG_IO_ERROR);
 		xfs_efi_item_free(efip);
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index 6e4c744..96fe209 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -221,6 +221,7 @@  void
 xfs_cui_release(
 	struct xfs_cui_log_item	*cuip)
 {
+	ASSERT(atomic_read(&cuip->cui_refcount) > 0);
 	if (atomic_dec_and_test(&cuip->cui_refcount)) {
 		xfs_trans_ail_remove(&cuip->cui_item, SHUTDOWN_LOG_IO_ERROR);
 		xfs_cui_item_free(cuip);
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index 73c8278..f3b139c 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -243,6 +243,7 @@  void
 xfs_rui_release(
 	struct xfs_rui_log_item	*ruip)
 {
+	ASSERT(atomic_read(&ruip->rui_refcount) > 0);
 	if (atomic_dec_and_test(&ruip->rui_refcount)) {
 		xfs_trans_ail_remove(&ruip->rui_item, SHUTDOWN_LOG_IO_ERROR);
 		xfs_rui_item_free(ruip);