Re: Uninitialized extent races

[Jan Kara <jack@suse.cz>]

On Mon 24-12-12 19:17:45, Zheng Liu wrote:
> On Fri, Dec 21, 2012 at 01:02:43PM -0500, Theodore Ts'o wrote:
> > On Fri, Dec 21, 2012 at 05:19:29PM +0100, Jan Kara wrote:
> > >   No, I'm speaking about merging currently uninitialized extents. I.e.
> > > suppose someone does the following on a filesystem with dioread_nolock so
> > > that writeback happens via unwritten extents:
> > >   fd = open("file", O_RDWR);
> > >   pwrite(fd, buf, 4096, 0);
> > > 					flusher thread starts writing
> > > 					we create uninitialized extent for
> > > 					  range 0-4096
> > >   fallocate(fd, 0, 4096, 4096);
> > >     - we merge extents and now have just 1 uninitialized extent for range
> > >       0-8192
> > > 					ext4_convert_unwritten_extents() now
> > > 					  has to split the extent to finish
> > > 					  the IO.
> > 
> > Ah, I see.  Disabling the the merging that might take place as a
> > result of the fallocate.  Yes, I agree that's a completely sane thing
> > to do.
> > 
> > The alternate approach would be to add a flag in the extent status
> > tree indicating that an unwritten conversion is pending, but that
> > would add more complexity.
> 
> Sorry for delay reply.  Indeed we could add a flag in extent status tree
> to indicate an pending unwritten extent, and I believe that it can bring
> us some benefits.  But I wonder whether this case often happens.  Do we
> have some real workloads?
  It doesn't happen often but it *can* happen. Thus you have to implement
a code which handles the case. I don't think bit in extent status tree is
really necessary. Just disabling merging of uninitialized extents is
simple. If we see there are some real workloads which have problems with
it, we can resort to a more complex solution using extent tree...

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR