From: Christoph Hellwig <hch@lst.de>
To: stable@vger.kernel.org
Cc: linux-xfs@vger.kernel.org, Brian Foster <bfoster@redhat.com>,
"Darrick J . Wong" <darrick.wong@oracle.com>
Subject: [PATCH 35/47] xfs: remove unnecessary dirty bli format check for ordered bufs
Date: Sun, 17 Sep 2017 14:07:00 -0700 [thread overview]
Message-ID: <20170917210712.10804-36-hch@lst.de> (raw)
In-Reply-To: <20170917210712.10804-1-hch@lst.de>
From: Brian Foster <bfoster@redhat.com>
commit 6453c65d3576bc3e602abb5add15f112755c08ca upstream.
xfs_buf_item_unlock() historically checked the dirty state of the
buffer by manually checking the buffer log formats for dirty
segments. The introduction of ordered buffers invalidated this check
because ordered buffers have dirty bli's but no dirty (logged)
segments. The check was updated to accommodate ordered buffers by
looking at the bli state first and considering the blf only if the
bli is clean.
This logic is safe but unnecessary. There is no valid case where the
bli is clean yet the blf has dirty segments. The bli is set dirty
whenever the blf is logged (via xfs_trans_log_buf()) and the blf is
cleared in the only place BLI_DIRTY is cleared (xfs_trans_binval()).
Remove the conditional blf dirty checks and replace with an assert
that should catch any discrepencies between bli and blf dirty
states. Refactor the old blf dirty check into a helper function to
be used by the assert.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_buf_item.c | 62 ++++++++++++++++++++++++++-------------------------
fs/xfs/xfs_buf_item.h | 1 +
2 files changed, 33 insertions(+), 30 deletions(-)
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index cdae0ad5e0a5..ff076d11804a 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -575,26 +575,18 @@ xfs_buf_item_unlock(
{
struct xfs_buf_log_item *bip = BUF_ITEM(lip);
struct xfs_buf *bp = bip->bli_buf;
- bool clean;
- bool aborted;
- int flags;
+ bool aborted = !!(lip->li_flags & XFS_LI_ABORTED);
+ bool hold = !!(bip->bli_flags & XFS_BLI_HOLD);
+ bool dirty = !!(bip->bli_flags & XFS_BLI_DIRTY);
+ bool ordered = !!(bip->bli_flags & XFS_BLI_ORDERED);
/* Clear the buffer's association with this transaction. */
bp->b_transp = NULL;
/*
- * If this is a transaction abort, don't return early. Instead, allow
- * the brelse to happen. Normally it would be done for stale
- * (cancelled) buffers at unpin time, but we'll never go through the
- * pin/unpin cycle if we abort inside commit.
+ * The per-transaction state has been copied above so clear it from the
+ * bli.
*/
- aborted = (lip->li_flags & XFS_LI_ABORTED) ? true : false;
- /*
- * Before possibly freeing the buf item, copy the per-transaction state
- * so we can reference it safely later after clearing it from the
- * buffer log item.
- */
- flags = bip->bli_flags;
bip->bli_flags &= ~(XFS_BLI_LOGGED | XFS_BLI_HOLD | XFS_BLI_ORDERED);
/*
@@ -602,7 +594,7 @@ xfs_buf_item_unlock(
* unlock the buffer and free the buf item when the buffer is unpinned
* for the last time.
*/
- if (flags & XFS_BLI_STALE) {
+ if (bip->bli_flags & XFS_BLI_STALE) {
trace_xfs_buf_item_unlock_stale(bip);
ASSERT(bip->__bli_format.blf_flags & XFS_BLF_CANCEL);
if (!aborted) {
@@ -620,20 +612,11 @@ xfs_buf_item_unlock(
* regardless of whether it is dirty or not. A dirty abort implies a
* shutdown, anyway.
*
- * Ordered buffers are dirty but may have no recorded changes, so ensure
- * we only release clean items here.
+ * The bli dirty state should match whether the blf has logged segments
+ * except for ordered buffers, where only the bli should be dirty.
*/
- clean = (flags & XFS_BLI_DIRTY) ? false : true;
- if (clean) {
- int i;
- for (i = 0; i < bip->bli_format_count; i++) {
- if (!xfs_bitmap_empty(bip->bli_formats[i].blf_data_map,
- bip->bli_formats[i].blf_map_size)) {
- clean = false;
- break;
- }
- }
- }
+ ASSERT((!ordered && dirty == xfs_buf_item_dirty_format(bip)) ||
+ (ordered && dirty && !xfs_buf_item_dirty_format(bip)));
/*
* Clean buffers, by definition, cannot be in the AIL. However, aborted
@@ -652,11 +635,11 @@ xfs_buf_item_unlock(
ASSERT(XFS_FORCED_SHUTDOWN(lip->li_mountp));
xfs_trans_ail_remove(lip, SHUTDOWN_LOG_IO_ERROR);
xfs_buf_item_relse(bp);
- } else if (clean)
+ } else if (!dirty)
xfs_buf_item_relse(bp);
}
- if (!(flags & XFS_BLI_HOLD))
+ if (!hold)
xfs_buf_relse(bp);
}
@@ -945,6 +928,25 @@ xfs_buf_item_log(
}
+/*
+ * Return true if the buffer has any ranges logged/dirtied by a transaction,
+ * false otherwise.
+ */
+bool
+xfs_buf_item_dirty_format(
+ struct xfs_buf_log_item *bip)
+{
+ int i;
+
+ for (i = 0; i < bip->bli_format_count; i++) {
+ if (!xfs_bitmap_empty(bip->bli_formats[i].blf_data_map,
+ bip->bli_formats[i].blf_map_size))
+ return true;
+ }
+
+ return false;
+}
+
STATIC void
xfs_buf_item_free(
xfs_buf_log_item_t *bip)
diff --git a/fs/xfs/xfs_buf_item.h b/fs/xfs/xfs_buf_item.h
index e0e744aefaa8..9690ce62c9a7 100644
--- a/fs/xfs/xfs_buf_item.h
+++ b/fs/xfs/xfs_buf_item.h
@@ -64,6 +64,7 @@ typedef struct xfs_buf_log_item {
int xfs_buf_item_init(struct xfs_buf *, struct xfs_mount *);
void xfs_buf_item_relse(struct xfs_buf *);
void xfs_buf_item_log(xfs_buf_log_item_t *, uint, uint);
+bool xfs_buf_item_dirty_format(struct xfs_buf_log_item *);
void xfs_buf_attach_iodone(struct xfs_buf *,
void(*)(struct xfs_buf *, xfs_log_item_t *),
xfs_log_item_t *);
--
2.14.1
next prev parent reply other threads:[~2017-09-17 21:07 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-17 21:06 4.9-stable updates for XFS Christoph Hellwig
2017-09-17 21:06 ` [PATCH 01/47] xfs: Move handling of missing page into one place in xfs_find_get_desired_pgoff() Christoph Hellwig
2017-09-17 21:06 ` [PATCH 02/47] xfs: fix spurious spin_is_locked() assert failures on non-smp kernels Christoph Hellwig
2017-09-17 21:06 ` [PATCH 03/47] xfs: push buffer of flush locked dquot to avoid quotacheck deadlock Christoph Hellwig
2017-09-17 21:06 ` [PATCH 04/47] xfs: try to avoid blowing out the transaction reservation when bunmaping a shared extent Christoph Hellwig
2017-09-17 21:06 ` [PATCH 05/47] xfs: release bli from transaction properly on fs shutdown Christoph Hellwig
2017-09-17 21:06 ` [PATCH 06/47] xfs: remove bli from AIL before release on transaction abort Christoph Hellwig
2017-09-17 21:06 ` [PATCH 07/47] xfs: don't allow bmap on rt files Christoph Hellwig
2017-09-17 21:06 ` [PATCH 08/47] xfs: free uncommitted transactions during log recovery Christoph Hellwig
2017-09-17 21:06 ` [PATCH 09/47] xfs: free cowblocks and retry on buffered write ENOSPC Christoph Hellwig
2017-09-17 21:06 ` [PATCH 10/47] xfs: don't crash on unexpected holes in dir/attr btrees Christoph Hellwig
2017-09-17 21:06 ` [PATCH 11/47] xfs: check _btree_check_block value Christoph Hellwig
2017-09-17 21:06 ` [PATCH 12/47] xfs: set firstfsb to NULLFSBLOCK before feeding it to _bmapi_write Christoph Hellwig
2017-09-17 21:06 ` [PATCH 13/47] xfs: check _alloc_read_agf buffer pointer before using Christoph Hellwig
2017-09-17 21:06 ` [PATCH 14/47] xfs: fix quotacheck dquot id overflow infinite loop Christoph Hellwig
2017-09-17 21:06 ` [PATCH 15/47] xfs: fix multi-AG deadlock in xfs_bunmapi Christoph Hellwig
2017-09-17 21:06 ` [PATCH 16/47] xfs: Fix per-inode DAX flag inheritance Christoph Hellwig
2017-09-17 21:06 ` [PATCH 17/47] xfs: fix inobt inode allocation search optimization Christoph Hellwig
2017-09-17 21:06 ` [PATCH 18/47] xfs: clear MS_ACTIVE after finishing log recovery Christoph Hellwig
2017-09-17 21:06 ` [PATCH 19/47] xfs: don't leak quotacheck dquots when cow recovery Christoph Hellwig
2017-09-17 21:06 ` [PATCH 20/47] iomap: fix integer truncation issues in the zeroing and dirtying helpers Christoph Hellwig
2017-09-17 21:06 ` [PATCH 21/47] xfs: write unmount record for ro mounts Christoph Hellwig
2017-09-17 21:06 ` [PATCH 22/47] xfs: toggle readonly state around xfs_log_mount_finish Christoph Hellwig
2017-09-17 21:06 ` [PATCH 23/47] xfs: remove xfs_trans_ail_delete_bulk Christoph Hellwig
2017-09-17 21:06 ` [PATCH 24/47] xfs: Add infrastructure needed for error propagation during buffer IO failure Christoph Hellwig
2017-09-17 21:06 ` [PATCH 25/47] xfs: Properly retry failed inode items in case of error during buffer writeback Christoph Hellwig
2017-09-17 21:06 ` [PATCH 26/47] xfs: fix recovery failure when log record header wraps log end Christoph Hellwig
2017-09-17 21:06 ` [PATCH 27/47] xfs: always verify the log tail during recovery Christoph Hellwig
2017-09-17 21:06 ` [PATCH 28/47] xfs: fix log recovery corruption error due to tail overwrite Christoph Hellwig
2017-09-17 21:06 ` [PATCH 29/47] xfs: handle -EFSCORRUPTED during head/tail verification Christoph Hellwig
2017-09-17 21:06 ` [PATCH 30/47] xfs: add log recovery tracepoint for head/tail Christoph Hellwig
2017-09-17 21:06 ` [PATCH 31/47] xfs: stop searching for free slots in an inode chunk when there are none Christoph Hellwig
2017-09-17 21:06 ` [PATCH 32/47] xfs: evict all inodes involved with log redo item Christoph Hellwig
2017-09-17 21:06 ` [PATCH 33/47] xfs: check for race with xfs_reclaim_inode() in xfs_ifree_cluster() Christoph Hellwig
2017-09-17 21:06 ` [PATCH 34/47] xfs: open-code xfs_buf_item_dirty() Christoph Hellwig
2017-09-17 21:07 ` Christoph Hellwig [this message]
2017-09-17 21:07 ` [PATCH 36/47] xfs: ordered buffer log items are never formatted Christoph Hellwig
2017-09-17 21:07 ` [PATCH 37/47] xfs: refactor buffer logging into buffer dirtying helper Christoph Hellwig
2017-09-17 21:07 ` [PATCH 38/47] xfs: don't log dirty ranges for ordered buffers Christoph Hellwig
2017-09-17 21:07 ` [PATCH 39/47] xfs: skip bmbt block ino validation during owner change Christoph Hellwig
2017-09-17 21:07 ` [PATCH 40/47] xfs: move bmbt owner change to last step of extent swap Christoph Hellwig
2017-09-17 21:07 ` [PATCH 41/47] xfs: disallow marking previously dirty buffers as ordered Christoph Hellwig
2017-09-17 21:07 ` [PATCH 42/47] xfs: relog dirty buffers during swapext bmbt owner change Christoph Hellwig
2017-09-17 21:07 ` [PATCH 43/47] xfs: disable per-inode DAX flag Christoph Hellwig
2017-09-17 21:07 ` [PATCH 44/47] xfs: fix incorrect log_flushed on fsync Christoph Hellwig
2017-09-17 21:07 ` [PATCH 45/47] xfs: don't set v3 xflags for v2 inodes Christoph Hellwig
2017-09-17 21:07 ` [PATCH 46/47] xfs: open code end_buffer_async_write in xfs_finish_page_writeback Christoph Hellwig
2017-09-17 21:07 ` [PATCH 47/47] xfs: use kmem_free to free return value of kmem_zalloc Christoph Hellwig
2017-09-18 8:22 ` 4.9-stable updates for XFS Greg KH
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=20170917210712.10804-36-hch@lst.de \
--to=hch@lst.de \
--cc=bfoster@redhat.com \
--cc=darrick.wong@oracle.com \
--cc=linux-xfs@vger.kernel.org \
--cc=stable@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).