Re: XFS hole punch races

From: Ben Hutchings <ben@decadent.org.uk>
To: Dave Chinner <david@fromorbit.com>
Cc: Jan Kara <jack@suse.cz>, stable@vger.kernel.org, xfs@oss.sgi.com
Subject: Re: XFS hole punch races
Date: Sun, 05 Jun 2016 02:19:32 +0100	[thread overview]
Message-ID: <1465089572.2847.180.camel@decadent.org.uk> (raw)
In-Reply-To: <20160604232800.GW26977@dastard>

[-- Attachment #1.1: Type: text/plain, Size: 2937 bytes --]

On Sun, 2016-06-05 at 09:28 +1000, Dave Chinner wrote:
> On Sat, Jun 04, 2016 at 06:11:10PM +0100, Ben Hutchings wrote:
> > On Tue, 2016-03-22 at 16:57 +0100, Jan Kara wrote:
> > 
> > Hi,
> > 
> > similarly to ext4 also XFS had races between hole punching and page faults
> > which could result in data corruption. The fixes were merged in 4.1-rc1 but
> > it might make sense to backport them to older stable releases given the
> > nature of the issue.
> > 
> > Relevant fixes are:
> > 
> > de0e8c20ba3a65b0f15040aabbefdc1999876e6b
> > 075a924d45cc69c75a35f20b4912b85aa98b180a
> > e8e9ad42c1f1e1bfbe0e8c32c8cac02e9ebfb7ef
> > 0f9160b444e4de33b65dfcd3b901358a3129461a
> > 723cac48473358939759885a18e8df113ea96138
> > ec56b1f1fdc69599963574ce94cc5693d535dd64
> > 
> > 
> > You missed the first in that sequence:
> > 
> > 653c60b633a9 xfs: introduce mmap/truncate lock
> > 
> > For 3.16, I've queued up all those fixes with one further prerequisite:
> > 
> > 812176832169 xfs: fix swapext ilock deadlock
> > 
> > For 3.2, I've queued up all but 723cac484733, with these additional
> > prerequisites:
> > 
> > f38996f57687 xfs: reduce ilock hold times in xfs_setattr_size
> > bc4010ecb8f4 xfs: use iolock on XFS_IOC_ALLOCSP calls
> > 76ca4c238cf5 xfs: always take the iolock around xfs_setattr_size
> > 5f8aca8b43f4 xfs: always hold the iolock when calling xfs_change_file_space
> > I realise I'll need to check for regressions with xfstests.
> 
> Hi Ben,
> 
> You do realise that this sort of backport effectively makes the
> stable kernels unsupportable by the upstream XFS developers? You're
> taking random changes from the upstream kernel until the kernel
> compiles, and then mostly hoping that it works.

I'm applying slightly more intelligence than that, but of course I'm
not an XFS developer.

> It's trivially easy to break truncate by screwing up the locking and
> IO barriers that it depends on, and this set of patches make quite a
> lot of different changes that have an unknown set of dependencies
> for correct behaviour. xfstests won't find all those problems; at
> best it will tell us that there won't be obvious problems.
> 
> Stable kernels are supposed to be stable, and backports like this
> are riskier than the original changes in the upstream kernels. if a
> user is hitting a mmap/hole-punch race on an XFS filesytem (I can't
> remember any reports to the upstream list for any kernel) on a 3.2
> kernel, then correct answer is "upgrade to a newer upstream kernel",
> not "do a risky backport that exposes the entire user base to the
> potential of new data corruption bugs"....

OK, then I'll drop them for 3.2.

Ben.

-- 
Ben Hutchings
Everything should be made as simple as possible, but not simpler.
                                                           - Albert
Einstein

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs