Thread (59 messages) 59 messages, 6 authors, 2021-03-05

Re: [PATCH 5/8] xfs: CIL checkpoint flushes caches unconditionally

From: Christoph Hellwig <hch@infradead.org>
Date: 2021-02-25 08:45:16

This looks ok, but please make add two trivial checks that the device
actually supports/needs flushes.  All that magic of allocating a bio


On Tue, Feb 23, 2021 at 02:34:39PM +1100, Dave Chinner wrote:
quoted hunk ↗ jump to hunk
 	new_ctx = kmem_zalloc(sizeof(*new_ctx), KM_NOFS);
 	new_ctx->ticket = xlog_cil_ticket_alloc(log);
@@ -719,10 +720,24 @@ xlog_cil_push_work(
 	spin_unlock(&cil->xc_push_lock);
 
 	/*
-	 * pull all the log vectors off the items in the CIL, and
-	 * remove the items from the CIL. We don't need the CIL lock
-	 * here because it's only needed on the transaction commit
-	 * side which is currently locked out by the flush lock.
+	 * The CIL is stable at this point - nothing new will be added to it
+	 * because we hold the flush lock exclusively. Hence we can now issue
+	 * a cache flush to ensure all the completed metadata in the journal we
+	 * are about to overwrite is on stable storage.
+	 *
+	 * This avoids the need to have the iclogs issue REQ_PREFLUSH based
+	 * cache flushes to provide this ordering guarantee, and hence for CIL
+	 * checkpoints that require hundreds or thousands of log writes no
+	 * longer need to issue device cache flushes to provide metadata
+	 * writeback ordering.
+	 */
+	xfs_flush_bdev_async(log->l_mp->m_ddev_targp->bt_bdev, &bdev_flush);
This still causes a bio allocation, also even if the device does not need
flush.  Please also use bio_init on a bio passed into xfs_flush_bdev_async to
avoid that, and make the whole code conditional to only run if we actually need
to flush caches.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help