On Thu, Jan 12, 2012 at 11:22:10AM -0500, Christoph Hellwig wrote:
> With all the complications that we got compared to the initial version,
> namely multiple hole extents, dirty unwritten extent detection and
> so on I think it's time to stop using xfs_bmapi_read against Dave's
> initial suggestion, and switch to using xfs_bmap_search_extents
> The rationale for that is that
> a) using xfs_bmapi_read makes hole detection more complex, given
> that it has to fill potentially multiple xfs_bmbt_irec structures
> instead of skipping over them
> b) reading two extents at a time means we have to duplicate all the
> detection code.
c) having a cursor here means that Jeff can always get the job done
with a single btree search, which could be an important
optimisation for heavily preallocated workloads.
> if we use xfs_bmap_search_extents we need a bit of boilerplate code,
> but xfs_seek_data becomes really simple - we just loop over
> xfs_bmap_search_extents until we either find an extent or EOF.
> If we find an extent and it's unwritten we might have to probe for
> dirty areas from one single point, or just skip it but the code is
> still simple. xfs_seek_hole is just as simple - if
> xfs_bmap_search_extents fits the condition for a hole as written
> down in xfs_bmapi_read we've found it, if not we might again have
> to do the unwritten extent probing, but just from a single place
> instead of duplicating it twice.
I agree that this is a good idea. I would like to reiterate my
suggestion that Jeff go for the 'simple' implementation (assume
unwritten extents contain data) before going about scanning unwritten
extents for holes/data.