From: Christoph Hellwig <hch@infradead.org>
To: Alex Elder <aelder@sgi.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 1/4] xfs: add online discard support
Date: Fri, 20 May 2011 06:24:31 -0400 [thread overview]
Message-ID: <20110520102430.GA18199@infradead.org> (raw)
In-Reply-To: <1305842024.2825.86.camel@doink>
On Thu, May 19, 2011 at 04:53:44PM -0500, Alex Elder wrote:
> The first is, why not support it for non-delaylog?
Because:
a) performance is going to suck even more horribly with the
amount of trim commands needed, with no chance of actually
fixing it
b) the async discard code in patch 3 not easily applyable to
the non-delaylog case, we'd need to keep two parallel codebases,
one of them guaranteed to be untested.
> Second, why is it a two phase operation (marking an
> extent for discard, then doing all the discards at
> once)? Is it just so you can do the discards without
> holding the perag lock?
Because we must prevent the allocation code from reusing an extent
that is undergoing a discard right now to prevent corruption, thus
we need to mark it as do not touch first.
> > xfs_trans_committed_bulk(ctx->cil->xc_log->l_ailp, ctx->lv_chain,
> > ctx->start_lsn, abort);
> >
> > xfs_alloc_busy_sort(&ctx->busy_extents);
>
> I still think sorting the list belongs inside xfs_alloc_busy_clear().
> I see that list_sort() is not necessarily trivial for an already
> sorted list though...
It's a bad idea to do the sort twice for no good reason, and adding
another parameter to further overload xfs_alloc_busy_clear behaviour
doesn't seem smart either.
> if (error == EOPNOTSUPP) {
> /*
> * Report this once per mount point somehow?
> * If so, turn off the mount option?
> */
> break;
We've been through this discussion again lately with dm and ext4
folks, and the conclusion is that EOPNOTSUPP is perfectly fine to happen
here.
> > + * performing the discard a chance to mark the extent unbusy
> > + * and retry.
> > + */
> > + if (busyp->flags & XFS_ALLOC_BUSY_DISCARDED) {
> > + spin_unlock(&pag->pagb_lock);
> > + delay(1);
>
> I hate seeing calls to delay() although sometimes
> it's the right thing to do... I don't have a feel
> for how long a discard is likely to take so I don't
> know whether waiting here instead would be worth
> the effort.
It's not nice, but if the block layer gets fixed and we do asynchronous
discards it simply goes away.
> If this option is to only be available for delaylog, it should
> say so here (and maybe report that it's being ignored if it's
> supplied with nodelaylog at mount time).
ok.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2011-05-20 10:24 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-04 18:55 [PATCH 0/4] online discard support V3 Christoph Hellwig
2011-05-04 18:55 ` [PATCH 1/4] xfs: add online discard support Christoph Hellwig
2011-05-19 21:53 ` Alex Elder
2011-05-20 10:24 ` Christoph Hellwig [this message]
2011-05-20 11:43 ` Lukas Czerner
2011-05-20 13:57 ` Alex Elder
2011-05-20 13:40 ` Alex Elder
2011-05-20 13:45 ` [PATCH 1/4 v2] " Christoph Hellwig
2011-05-20 15:42 ` Alex Elder
2011-05-04 18:55 ` [PATCH 2/4] xfs: do not discard alloc btree blocks Christoph Hellwig
2011-05-19 21:54 ` Alex Elder
2011-05-04 18:55 ` [PATCH 3/4] xfs: add a reference count to the CIL context Christoph Hellwig
2011-05-19 21:54 ` Alex Elder
2011-05-20 10:25 ` Christoph Hellwig
2011-05-04 18:55 ` [PATCH 4/4] xfs: make discard operations asynchronous 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=20110520102430.GA18199@infradead.org \
--to=hch@infradead.org \
--cc=aelder@sgi.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.