[Top] [All Lists]

Re: [PATCH 4/5] xfs: log file size updates as part of unwritten extent c

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 4/5] xfs: log file size updates as part of unwritten extent conversion
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 17 Nov 2011 02:51:50 -0500
Cc: Ben Myers <bpm@xxxxxxx>, Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20111116235853.GD7046@dastard>
References: <20111115201407.038216766@xxxxxxxxxxxxxxxxxxxxxx> <20111115201427.104680606@xxxxxxxxxxxxxxxxxxxxxx> <20111116210347.GH29840@xxxxxxx> <20111116235853.GD7046@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Nov 17, 2011 at 10:58:53AM +1100, Dave Chinner wrote:
> However, I'm not sure this is even relevant - if we've got multiple
> extents to write to and then convert in a single high level IO, they
> would have been mapped into separate IOs, bios and ioends. That's
> definitely the case for buffered IO, and AFAICT, the same for direct
> IO (Christoph - can you confirm this?).

For direct I/O we do a single call to xfs_iomap_write_unwritten for
the whole write system call, which might span multiple extents.

> Chrisptoph, if this "only one extent per ioend" condition is really
> true, then is there any need for this loop at all? i.e. do we ever
> do a partial extent conversion for a completed IO and therefore have
> to do a second transaction to convert the rest of the extent?

It might be a good idea to move the looping code directly into the
direct I/O completion path. Or replace the current direct I/O end_io
handler mess with something that actually makes sense.

For now I'd leave the code as-is but with the fix pointed out by Ben,
and look into a test case reproducing that bug.  I'll look into
figuring out something saner for the direct I/O case later.

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