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

Christoph Hellwig hch at infradead.org
Wed Mar 23 07:31:35 CDT 2011


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




More information about the xfs mailing list