On Tue, Feb 10, 2015 at 07:37:20PM -0800, Omar Sandoval wrote: > 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 Hi, I figure things are busy because of the merge window, but I wanted to check on the status of this patch. I also have some regression tests for xfstests ready to go. Thanks! -- Omar -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
