[Top] [All Lists]

Re: [PATCH 4/8] xfs: log file size updates at I/O completion time

To: Ben Myers <bpm@xxxxxxx>
Subject: Re: [PATCH 4/8] xfs: log file size updates at I/O completion time
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 12 Mar 2012 09:26:17 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20120305200018.GN7762@xxxxxxx>
References: <20120229095347.009884687@xxxxxxxxxxxxxxxxxxxxxx> <20120229095508.812609910@xxxxxxxxxxxxxxxxxxxxxx> <20120305200018.GN7762@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Mar 05, 2012 at 02:00:18PM -0600, Ben Myers wrote:
> > -   if (iohead)
> > +   if (iohead) {
> > +           /*
> > +            * Reserve log space if we might write beyond the on-disk
> > +            * inode size.
> > +            */
> > +           if (ioend->io_type != IO_UNWRITTEN &&
> > +               xfs_ioend_is_append(ioend)) {
>                       ^^^
> I suggest that xfs_ioend_is_append should look at every ioend in the
> chain in order to determine if an append is possible, not just the
> first.  Note that xfs_submit_ioend_bio above is called for each ioend in
> the chain.  You'd only see this on a system with a larger page size than
> filesystem block size. 

It doesn't look at the first, it looks at the last one - I initially
thought we might need to do it for all, but Dave convinced me otherwise.

I wish I'd still remember why exactly and should have written that down
in a comment though.  I'll try to get back to it once I had a bit more

> In the situation where we are converting an unwritten extent we cancel
> the preallocated transaction and call xfs_iomap_write_unwritten where
> the inode core is logged with the updated size.  We were already
> allocating an ioend here, so when you said 'To make this possible we
> have to preallocate an ioend that allows deferring it here', did you
> really mean to say that we're preallocating the transaction?  Maybe
> there are just to many 'its' in the comment or I'm just dense.

I'll replace the comment with something that makes sense.

<Prev in Thread] Current Thread [Next in Thread>