Re: [PATCH 5/9] xfs: split out iclog writing from xfs_trans_commit()

[Dave Chinner <david@fromorbit.com>]

On Sat, Mar 06, 2010 at 06:08:33AM -0500, Christoph Hellwig wrote:
> On Sat, Mar 06, 2010 at 12:51:20PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Split the the part of xfs_trans_commit() that deals with writing the
> > transaction into the iclog into a separate function. This isolates the
> > physical commit process from the logical commit operation and makes
> > it easier to insert different transaction commit paths without affecting
> > the existing algorithm adversely.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> 
> Looks good, but some comments below:
> 
> > +        if (nvec > XFS_TRANS_LOGVEC_COUNT)
> > +                kmem_free(log_vector);
> 
> These two lines are indented with whitespaces instead of tabs.

Oops. Looks like I need to update my whitespace damage highlighting
rule...

> > +shut_us_down:
> > +	shutdown = XFS_FORCED_SHUTDOWN(mp) ? EIO : 0;
> > +	if (!(tp->t_flags & XFS_TRANS_DIRTY) || shutdown) {
> > +		xfs_trans_unreserve_and_mod_sb(tp);
> > +		/*
> 
> This whole area in _xfs_trans_commit is still a complete mess.

Yes, it is I didn't touch that part of the function, even though the
diff moves the whole thing about. Actually, the diff makes the
change look for more complex than it really is because it doesn't
keep it as three separate chunks of code that got moved....

> What about throwing the patch below in once you're rewriting it anyway?

I'll add it to the series and see how it goes.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs