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
sleep.
> 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.
|