From: Dave Chinner <david@fromorbit.com>
To: Alex Elder <aelder@sgi.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 2/2] xfs: don't truncate prealloc from frequently accessed inodes
Date: Fri, 15 Oct 2010 08:28:41 +1100 [thread overview]
Message-ID: <20101014212841.GE4681@dastard> (raw)
In-Reply-To: <1287076970.2362.521.camel@doink>
On Thu, Oct 14, 2010 at 12:22:50PM -0500, Alex Elder wrote:
> On Mon, 2010-10-04 at 21:13 +1100, Dave Chinner wrote:
> > diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> > index b7bdc43..0c8eeba 100644
> > --- a/fs/xfs/xfs_vnodeops.c
> > +++ b/fs/xfs/xfs_vnodeops.c
>
> OK, this comment is unrelated to your exact change. But just above
> the next hunk there's a big nasty condition, which appears to
> be *almost* duplicated in xfs_inactive() (twice!). It would be
> very nice if, while you're at modifying this nearby code, you
> could encapsulate that condition in a macro that has a meaningful
> name.
I've looked at doing this factoring before, but the conditions are
subtly different and hence cannot be factored into a single macro.
The xfs_inactive case can be cleaned up (i've got old patches around
that do half the job, IIRC), but the tests in the two functions are
not the same.
> > @@ -979,14 +979,27 @@ xfs_release(
> > * chance to drop them once the last reference to
> > * the inode is dropped, so we'll never leak blocks
> > * permanently.
>
> I'm curious what the effect is if we simply don't do the truncate
> *except* when the inode becomes inactive. It means we hang onto
> the stuff for a while longer, and maybe it makes things messier
> in the event of a crash.
It doesn't change a thing in the event of a crash - speculative
preallocation is entirely in-memory state.
> Can you tell me why we do the truncate
> here as well as in xfs_inactive() (or what the problem is of
> *not* doing it here)?
the truncation in ->release is not guaranteed to succeed - it uses
trylock semantics to avoid blocking unnecessarily. It can do this
because we know that when xfs_inactive() is called, the truncation
will happen for real.
--
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-10-14 21:27 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-04 10:13 [RFC, PATCH 0/2] xfs: dynamic speculative preallocation for delalloc Dave Chinner
2010-10-04 10:13 ` [PATCH 1/2] xfs: dynamic speculative EOF preallocation Dave Chinner
2010-10-14 17:22 ` Alex Elder
2010-10-14 21:33 ` Dave Chinner
2010-10-15 6:51 ` allocsize mount option, was: " Michael Monnerie
2010-10-15 11:59 ` Dave Chinner
2010-10-04 10:13 ` [PATCH 2/2] xfs: don't truncate prealloc from frequently accessed inodes Dave Chinner
2010-10-14 17:22 ` Alex Elder
2010-10-14 21:28 ` Dave Chinner [this message]
2010-10-14 17:22 ` [RFC, PATCH 0/2] xfs: dynamic speculative preallocation for delalloc Alex Elder
2010-10-14 21:16 ` Dave Chinner
2010-10-14 21:50 ` Ivan.Novick
2010-10-15 7:14 ` Michael Monnerie
2010-10-15 11:45 ` Dave Chinner
2010-10-17 14:31 ` Michael Monnerie
2010-10-17 23:49 ` Dave Chinner
2010-10-18 6:39 ` Michael Monnerie
-- strict thread matches above, loose matches on Subject: below --
2010-11-29 0:43 [PATCH 0/2] xfs: dynamic speculative allocation beyond EOF V3 Dave Chinner
2010-11-29 0:43 ` [PATCH 2/2] xfs: don't truncate prealloc from frequently accessed inodes Dave Chinner
2010-11-29 9:42 ` Andi Kleen
2010-11-29 9:42 ` Andi Kleen
2010-11-30 1:00 ` Dave Chinner
2010-11-30 1:00 ` Dave Chinner
2010-11-30 17:03 ` Christoph Hellwig
2010-11-30 22:00 ` Dave Chinner
2010-12-13 1:25 [PATCH 0/2] xfs: dynamic speculative allocation beyond EOF V4 Dave Chinner
2010-12-13 1:25 ` [PATCH 2/2] xfs: don't truncate prealloc from frequently accessed inodes Dave Chinner
2010-12-16 15:46 ` 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=20101014212841.GE4681@dastard \
--to=david@fromorbit.com \
--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.