From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] xfs: add discard support (at transaction commit)
Date: Mon, 10 May 2010 07:59:44 +1000 [thread overview]
Message-ID: <20100509215944.GF25419@dastard> (raw)
In-Reply-To: <20100509175048.GA1435@infradead.org>
On Sun, May 09, 2010 at 01:50:48PM -0400, Christoph Hellwig wrote:
> Now that we have reliably tracking of deleted extents in a transaction
> we can easily implement "online" discard support which calls
> blkdev_issue_discard once a transaction commits. We simply have to
> walk the list of busy extents after transaction commit, but before deleting
> it from the rbtree tracking these busy extents.
>
> This does not replace by background discard support patch which is probably
> better for thin provisioned arrays - I will updated it to apply ontop of
> this patch when I'm ready to re-submit it.
I think this can be made to work, but I don't really like it that
much, especially the barrier flush part. Is there any particular
reason we need to issue discards at this level apart from "other
filesystems are doing it" rather than doing it lazily in a
non-performance critical piece of code?
Regardless of this, some questions about the patch come to mind:
1. is it safe to block the xfslogd in the block layer in,
say, get_request()? i.e. should we be issuing IO from an IO
completion handler? That raises red flags in my head...
2. issuing discards will block xfslogd and potentially stall
the log if there are lots of discards to issue.
3. DISCARD_FL_BARRIER appears to be used to allow async
issuing of the discard to ensure any followup write has the
discard processed first. What happens if the device does not
support barriers or barriers are turned off?
To me it appears that a lack of barriers could result in a
write being reordered in front of the discard. e.g.
delalloc results in btree block freed, marked busy. New
delalloc occurs, allocates block, marked sync, forces log,
issues async discard, completes transaction and then writes
data async. Which operation does the drive see and complete
first - the discard or the data write?
4. A barrier IO on every discard? In a word: Ouch.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2010-05-09 21:57 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-09 17:50 [PATCH] xfs: add discard support (at transaction commit) Christoph Hellwig
2010-05-09 21:59 ` Dave Chinner [this message]
2010-07-26 9:29 ` Christoph Hellwig
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=20100509215944.GF25419@dastard \
--to=david@fromorbit.com \
--cc=hch@infradead.org \
--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