public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: Christoph Hellwig <hch@infradead.org>, xfs@oss.sgi.com
Subject: Re: [PATCH 5/6] xfs: add online discard support
Date: Wed, 23 Mar 2011 08:31:35 -0400	[thread overview]
Message-ID: <20110323123135.GA19111@infradead.org> (raw)
In-Reply-To: <20110323003003.GG15270@dastard>

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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2011-03-23 12:28 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-22 19:55 [PATCH 0/6] [PATCH 0/6] more efficient busy extent handling and discard support Christoph Hellwig
2011-03-22 19:55 ` [PATCH 1/6] xfs: optimize AGFL refills Christoph Hellwig
2011-03-22 22:30   ` Alex Elder
2011-03-23 12:16     ` Christoph Hellwig
2011-03-25 21:03       ` Alex Elder
2011-03-28 12:07         ` Christoph Hellwig
2011-03-22 23:30   ` Dave Chinner
2011-03-23 12:16     ` Christoph Hellwig
2011-03-22 19:55 ` [PATCH 2/6] xfs: do not immediately reuse busy extent ranges Christoph Hellwig
2011-03-22 22:30   ` Alex Elder
2011-03-23 12:17     ` Christoph Hellwig
2011-03-25 21:03   ` Alex Elder
2011-03-28 12:07     ` Christoph Hellwig
2011-03-22 19:55 ` [PATCH 3/6] xfs: exact busy extent tracking Christoph Hellwig
2011-03-22 23:47   ` Dave Chinner
2011-03-23 12:24     ` Christoph Hellwig
2011-03-25 21:04   ` Alex Elder
2011-03-28 12:10     ` Christoph Hellwig
2011-03-22 19:55 ` [PATCH 4/6] xfs: allow reusing busy extents where safe Christoph Hellwig
2011-03-23  0:20   ` Dave Chinner
2011-03-23 12:26     ` Christoph Hellwig
2011-03-25 21:04   ` Alex Elder
2011-03-22 19:55 ` [PATCH 5/6] xfs: add online discard support Christoph Hellwig
2011-03-23  0:30   ` Dave Chinner
2011-03-23 12:31     ` Christoph Hellwig [this message]
2011-03-25 21:04   ` Alex Elder
2011-03-28 12:35     ` Christoph Hellwig
2011-03-22 19:55 ` [PATCH 6/6] xfs: make discard operations asynchronous Christoph Hellwig
2011-03-23  0:43   ` Dave Chinner
2011-03-28 12:44     ` Christoph Hellwig
2011-03-25 21:04   ` Alex Elder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110323123135.GA19111@infradead.org \
    --to=hch@infradead.org \
    --cc=david@fromorbit.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox