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] xfs: dummy transactions should not dirty VFS state
Date: Tue, 3 Aug 2010 09:50:49 -0400 [thread overview]
Message-ID: <20100803135049.GA32679@infradead.org> (raw)
In-Reply-To: <20100803120728.GA26402@dastard>
On Tue, Aug 03, 2010 at 10:07:28PM +1000, Dave Chinner wrote:
> Yes, I noticed those things. Especially as I modified the wrong
> one in the first place and realised both need fixing and the
> duplication of code seems completely unnecessary. We should have
> only one copy of this code, not two copies that do slightly
> different things.
Yes, having one copy is much better.
> > For one
> > xfs_commit_dummy_trans doesn't actually commit a synchronous transaction
> > (or rather forces out the log) unless SYNC_WAIT is set,
>
> I don't think that we really _need_ a non-blocking version - waiting
> for a single sync transaction in xfssyncd once every 36s is hardly
> going to kill performance.
Sounds fair, but it needs documentation in the changelog, and possibly
in the source code as well.
> > in addition
> > to that xfs_fs_log_dummy uses _xfs_trans_alloc, which doesn't get
> > blocked by the filesystem freezing.
>
> Everything will be clean on a frozen filesystem, so all the current
> code does is block the xfssyncd until the filesytem is
> unfrozen. Given that we can still read everything on the frozen
> filesystem, inode caches can still grow and hence we still need to
> run regular reclaiming. If the xfssyncd is blocked then only memory
> pressure can free up inodes.
That's a reason not to wait. But given the bugs we had in this area
I'd rather not blindly start the transaction here.
Instead we could check s_frozen manually to no bother even doing
the calls to write the dummy record, plus maybe an assert so that it
trips up for debug builds.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
prev parent reply other threads:[~2010-08-03 13:50 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-03 9:10 [PATCH] xfs: dummy transactions should not dirty VFS state Dave Chinner
2010-08-03 9:55 ` Christoph Hellwig
2010-08-03 12:07 ` Dave Chinner
2010-08-03 13:50 ` Christoph Hellwig [this message]
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=20100803135049.GA32679@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 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.