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: > 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.
