xfs
[Top] [All Lists]

Re: [PATCH 6/6] xfs: make discard operations asynchronous

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 6/6] xfs: make discard operations asynchronous
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 28 Mar 2011 08:44:05 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20110323004323.GH15270@dastard>
References: <20110322195550.260682574@xxxxxxxxxxxxxxxxxxxxxx> <20110322200138.216042448@xxxxxxxxxxxxxxxxxxxxxx> <20110323004323.GH15270@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Mar 23, 2011 at 11:43:23AM +1100, Dave Chinner wrote:
> > +   struct xfs_cil_ctx      *ctx = bio->bi_private;
> > +
> > +   if (err && err != -EOPNOTSUPP) {
> > +           xfs_info(ctx->cil->xc_log->l_mp,
> > +                    "I/O error during discard\n");
> > +   }
> 
> Same comment about the bno/len in the error message as the previous
> patch.

We don't have the len information available anymore at this point,
but I've added the startblock.

> > +   max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
> > +   if (q->limits.discard_granularity) {
> > +           unsigned int disc_sects = q->limits.discard_granularity >> 9;
> > +
> > +           max_discard_sectors &= ~(disc_sects - 1);
> > +   }
> 
> This is asking for a helper function....

Yes, but that helper needs to be shared with the block layer discard
code, so I'll send it through Jens' tree later.

> > +   if (keep_alive)
> > +           xfs_cil_discard_done(ctx);
> > +   else
> > +           kmem_free(ctx);
> 
> You could probably just call xfs_cil_discard_done(ctx) here as the
> busy extent list will be empty in the !keep_alive case and so it
> will simply do the kmem_free(ctx) call there.

This means we clear the busy list a little later in the non-discard
case, but it cleans up the code nicely.  I've added it to the series
as a separate patch.

<Prev in Thread] Current Thread [Next in Thread>