xfs
[Top] [All Lists]

Re: [PATCH 5/6] xfs: add online discard support

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 5/6] xfs: add online discard support
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 23 Mar 2011 08:31:35 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20110323003003.GG15270@dastard>
References: <20110322195550.260682574@xxxxxxxxxxxxxxxxxxxxxx> <20110322200138.024991786@xxxxxxxxxxxxxxxxxxxxxx> <20110323003003.GG15270@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
> > +   if ((mp->m_flags & XFS_MOUNT_DISCARD) == 0)
> > +           return 0;
> 
> I'd move this check to the callers, otherwise we are going to be
> doing lots of function calls in a relatively performance sensitive
> loop just to run a single check when discard is not enabled...

Ok.  As you noticed it's done in the next patch, and they will probably
be merged into one before the final submission.  But for now I'll
move it to make the patch more obvious.

> > +   error = -blkdev_issue_discard(mp->m_ddev_targp->bt_bdev, bno, len,
> > +                                 GFP_NOFS, 0);
> > +   if (error && error != EOPNOTSUPP)
> > +           xfs_info(mp, "discard failed, error %d", error);
> 
> This would be more informative if it also printed the bno and len of
> the discard that failed. A couple of tracepoints here (e.g.
> discard_extent_issued, discard_extent_failed) could also be useful
> for tracking discard operations.

Yeah, I was planning to add a lot more tracepoint later on, both
for the busy extent handling and discards.

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