Date: Tue, 10 Feb 2015 19:37:20 -0800
From: Omar Sandoval <osandov@osandov.com>
To: Chris J Arges <chris.j.arges@canonical.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>, Andreas Dilger <adilger.kernel@dilger.ca>,
        =?utf-8?B?THVrw6HFoQ==?= Czerner <lczerner@redhat.com>,
        linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] ext4: fix indirect punch hole corruption
Message-ID: <20150211033720.GA20820@mew>
References: <20150209212805.GA10892@mew.cs.washington.edu>
 <bdb70d657b83644c6ee6eaaaf4f95460f937f05c.1423602557.git.osandov@osandov.com>
 <54DAC58B.8030600@canonical.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <54DAC58B.8030600@canonical.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3023
Lines: 67

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@osandov.com>
> > ---
> > 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@canonical.com>
> 

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/