[PATCH] xfs: dummy transactions should not dirty VFS state
Christoph Hellwig
hch at infradead.org
Tue Aug 3 08:50:49 CDT 2010
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.
More information about the xfs
mailing list