Re: [PATCH v4] ext4: fix indirect punch hole corruption

[Omar Sandoval]

On Tue, Feb 10, 2015 at 08:59:23PM -0600, Chris J Arges wrote:
> On 02/10/2015 03:44 PM, Omar Sandoval wrote:
> > Commit 4f579ae7de56 (ext4: fix punch hole on files with indirect
> > mapping) rewrote FALLOC_FL_PUNCH_HOLE for ext4 files with indirect
> > mapping. However, there are bugs in several corner cases. This fixes 5
> > distinct bugs:
> > 
> > 1. When there is at least one entire level of indirection between the
> > start and end of the punch range and the end of the punch range is the
> > first block of its level, we can't return early; we have to free the
> > intervening levels.
> > 
> > 2. When the end is at a higher level of indirection than the start and
> > ext4_find_shared returns a top branch for the end, we still need to free
> > the rest of the shared branch it returns; we can't decrement partial2.
> > 
> > 3. When a punch happens within one level of indirection, we need to
> > converge on an indirect block that contains the start and end. However,
> > because the branches returned from ext4_find_shared do not necessarily
> > start at the same level (e.g., the partial2 chain will be shallower if
> > the last block occurs at the beginning of an indirect group), the walk
> > of the two chains can end up "missing" each other and freeing a bunch of
> > extra blocks in the process. This mismatch can be handled by first
> > making sure that the chains are at the same level, then walking them
> > together until they converge.
> > 
> > 4. When the punch happens within one level of indirection and
> > ext4_find_shared returns a top branch for the start, we must free it,
> > but only if the end does not occur within that branch.
> > 
> > 5. When the punch happens within one level of indirection and
> > ext4_find_shared returns a top branch for the end, then we shouldn't
> > free the block referenced by the end of the returned chain (this mirrors
> > the different levels case).
> > 
> > Signed-off-by: Omar Sandoval <osandov@xxxxxxxxxxx>
> > ---
> > Okay, two more bugfixes folded in, all described in the commit message.
> > I'm finally no longer seeing xfstest generic/270 cause corruptions, even
> > after running it overnight, so hopefully this is it. Chris, would you
> > mind trying this out?
> >
> 
> Omar,
> I've completed 80 iterations of this patch so far without failure!
> Normally failures have occurred between 2-15 runs. Great job, and thanks
> for your persistence in fixing this issue!
> 
> Tested-by: Chris J Arges <chris.j.arges@xxxxxxxxxxxxx>
> 

Awesome, I was starting to run out of ideas ;) Thanks for all of your
testing.

LukÃÅ, would you like to take a look at this?

Also, Ted and Andreas, would you prefer this all in one patch, or should
I split out each individual fix into its own patch?

Thanks!
-- 
Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/