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.