xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 3/3] xfs: always log file size updates
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 25 Aug 2011 15:41:40 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20110825052311.GC5617@xxxxxxxxxxxxx>
References: <20110824055924.139283426@xxxxxxxxxxxxxxxxxxxxxx> <20110824060150.245096567@xxxxxxxxxxxxxxxxxxxxxx> <20110825010421.GL3162@dastard> <20110825052311.GC5617@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Aug 25, 2011 at 01:23:11AM -0400, Christoph Hellwig wrote:
> 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.

Once the ioend is built in .writepage, just before IO submission. We
do transaction reservations in that path for allocation, so it woul
dbe fine to do it this way for size updates, too.

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

That's probably the least risky short-term solution right now, I
think.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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