[Top] [All Lists]

Re: [PATCH 3/3] xfs: always log file size updates

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 3/3] xfs: always log file size updates
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 25 Aug 2011 01:23:11 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20110825010421.GL3162@dastard>
References: <20110824055924.139283426@xxxxxxxxxxxxxxxxxxxxxx> <20110824060150.245096567@xxxxxxxxxxxxxxxxxxxxxx> <20110825010421.GL3162@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Aug 25, 2011 at 11:04:21AM +1000, Dave Chinner wrote:
> Now that I've thought about this a little longer, the only concern I
> have about this is that xfs_trans_reserve() can block for long
> periods of time and require IO completion to make progress. This is
> one of the reasons we push unwritten extent conversion off into it's
> workqueue, so that it doesn't block non-conversion IO completions.
> Given that once we have a reservation we can hold it for as long as
> we want, maybe we should check before IO submission whether the
> completion is beyond the current EOF and allocate and reserve the
> transaction space before IO submission then attach it to the ioend.
> All that leaves on IO completion is this:
>       if (xfs_ioend_is_append(ioend)) {
>               lock inode
>               update size
>               join inode to transaction
>               log inode core
>               commit transaction
>       } else
>               cancel transaction
> That way we minimise the amount blocking we potentially do in IO
> completion.

Where would we do the reservation?  The last time we see user context
is ->writepage and I'm not sure we want this.

Maybe I should wait with this until I can put in my non-reservation
fast-path for simple inode core updates to avoid this for the common

We'll have to fix the missing i_size updates for backports anyway.  My
other plan was to add back i_update_size, mirroring i_update_core
and check it from all the relevant places so that we make sure we
pick it up for any inode flush.

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