On Mon, Mar 08, 2021 at 02:24:07PM -0800, Darrick J. Wong wrote:
> On Mon, Mar 08, 2021 at 03:18:09PM +0530, Chandan Babu R wrote:
> > On 05 Mar 2021 at 10:41, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > >
> > > The new checkpoint caceh flush mechanism requires us to issue an
> > > unconditional cache flush before we start a new checkpoint. We don't
> > > want to block for this if we can help it, and we have a fair chunk
> > > of CPU work to do between starting the checkpoint and issuing the
> > > first journal IO.
> > >
> > > Hence it makes sense to amortise the latency cost of the cache flush
> > > by issuing it asynchronously and then waiting for it only when we
> > > need to issue the first IO in the transaction.
> > >
> > > TO do this, we need async cache flush primitives to submit the cache
> > > flush bio and to wait on it. THe block layer has no such primitives
> > > for filesystems, so roll our own for the moment.
> > >
> > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > > ---
> > > fs/xfs/xfs_bio_io.c | 36 ++++++++++++++++++++++++++++++++++++
> > > fs/xfs/xfs_linux.h | 2 ++
> > > 2 files changed, 38 insertions(+)
> > >
> > > diff --git a/fs/xfs/xfs_bio_io.c b/fs/xfs/xfs_bio_io.c
> > > index 17f36db2f792..668f8bd27b4a 100644
> > > --- a/fs/xfs/xfs_bio_io.c
> > > +++ b/fs/xfs/xfs_bio_io.c
> > > @@ -9,6 +9,42 @@ static inline unsigned int bio_max_vecs(unsigned int count)
> > > return bio_max_segs(howmany(count, PAGE_SIZE));
> > > }
> > >
> > > +void
> > > +xfs_flush_bdev_async_endio(
> > > + struct bio *bio)
> > > +{
> > > + if (bio->bi_private)
> > > + complete(bio->bi_private);
> > > +}
> > > +
> > > +/*
> > > + * Submit a request for an async cache flush to run. If the request queue does
> > > + * not require flush operations, just skip it altogether. If the caller needsi
> > > + * to wait for the flush completion at a later point in time, they must supply a
> > > + * valid completion. This will be signalled when the flush completes. The
> > > + * caller never sees the bio that is issued here.
> > > + */
> > > +void
> > > +xfs_flush_bdev_async(
> > > + struct bio *bio,
> > > + struct block_device *bdev,
> > > + struct completion *done)
> > > +{
> > > + struct request_queue *q = bdev->bd_disk->queue;
> > > +
> > > + if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) {
> > > + complete(done);
> >
> > complete() should be invoked only when "done" has a non-NULL value.
>
> The only caller always provides a completion.
>
IMO, if the mechanism (i.e. the helper) accommodates a NULL parameter,
the underlying completion callback should as well..
Brian
> --D
>
> > > + return;
> > > + }
> >
> > --
> > chandan
>
