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@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
