Re: [PATCH 12/19] xfs: refactor xfs_bmap_add_extent_delay_real

[Brian Foster <bfoster@redhat.com>]

On Thu, Sep 21, 2017 at 03:27:35PM +0200, Christoph Hellwig wrote:
> On Tue, Sep 19, 2017 at 12:35:25PM -0400, Brian Foster wrote:
> > > @@ -2064,12 +2082,12 @@ xfs_bmap_add_extent_delay_real(
> > >  			if (error)
> > >  				goto done;
> > >  		}
> > > -		temp = xfs_bmap_worst_indlen(bma->ip, temp);
> > > -		temp2 = xfs_bmap_worst_indlen(bma->ip, temp2);
> > > -		diff = (int)(temp + temp2 -
> > > -			     (startblockval(PREV.br_startblock) -
> > > -			      (bma->cur ?
> > > -			       bma->cur->bc_private.b.allocated : 0)));
> > > +
> > > +		da_new = startblockval(PREV.br_blockcount) +
> > > +			 startblockval(RIGHT.br_blockcount);
> > 
> > s/br_blockcount/br_startblock/ :)
> 
> Yes.  And I've officially lost all faith in xfstests ever even testing
> this case in xfs_bmap_add_extent_delay_real at all.
> 
> I think it should be really easily testable by creating a large
> delalloc reservation and then fsyncing out the middle of it.
> Except of course we don't have a range fsync, and even then the
> writeback code might cluster it.  I might have to come up with
> a special kernel module to even reproduce this reliably..

What about creating a DEBUG mode option and/or an error injection tag
that allows for random partial extent allocation rather than the current
behavior of attempting to allocate the whole delalloc extent? E.g.,
could we "randomly" trim the ends of the delalloc extent looked up at
writeback time such that what we alloc it is smaller than the full
delalloc extent, but still large enough to satisfy the writeback? The
ideal result would be some combination of full allocs, some that trigger
either the left or right filling cases and some that trigger the
!LEFT_FILLING && !RIGHT_FILLING case. A bit hacky, but perhaps a test
could then use dio reads or something to induce ranged writebacks.

Brian