* XFS hole punch races
@ 2016-03-22 15:57 Jan Kara
2016-05-02 23:44 ` Greg KH
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jan Kara @ 2016-03-22 15:57 UTC (permalink / raw)
To: stable; +Cc: xfs
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
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: XFS hole punch races 2016-03-22 15:57 XFS hole punch races Jan Kara @ 2016-05-02 23:44 ` Greg KH 2016-05-15 22:23 ` Ben Hutchings 2016-06-04 17:11 ` Ben Hutchings 2 siblings, 0 replies; 8+ messages in thread From: Greg KH @ 2016-05-02 23:44 UTC (permalink / raw) To: Jan Kara; +Cc: stable, xfs On Tue, Mar 22, 2016 at 04:57:40PM +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 None of these apply to 3.14-stable, so I'm not going to worry about them :) thanks, greg k-h _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: XFS hole punch races 2016-03-22 15:57 XFS hole punch races Jan Kara 2016-05-02 23:44 ` Greg KH @ 2016-05-15 22:23 ` Ben Hutchings 2016-06-04 17:11 ` Ben Hutchings 2 siblings, 0 replies; 8+ messages in thread From: Ben Hutchings @ 2016-05-15 22:23 UTC (permalink / raw) To: Jan Kara, stable; +Cc: xfs [-- Attachment #1.1: Type: text/plain, Size: 1195 bytes --] 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 Thanks. For 3.2 I needed to apply these first: 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 653c60b633a9 xfs: introduce mmap/truncate lock and I left out 723cac484733 "xfs: lock out page faults from extent swap operations" as it doesn't seem applicable. Ben. -- Ben Hutchings For every action, there is an equal and opposite criticism. - Harrison [-- 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: XFS hole punch races 2016-03-22 15:57 XFS hole punch races Jan Kara 2016-05-02 23:44 ` Greg KH 2016-05-15 22:23 ` Ben Hutchings @ 2016-06-04 17:11 ` Ben Hutchings 2016-06-04 23:28 ` Dave Chinner 2 siblings, 1 reply; 8+ messages in thread From: Ben Hutchings @ 2016-06-04 17:11 UTC (permalink / raw) To: Jan Kara, stable; +Cc: xfs [-- Attachment #1.1: Type: text/plain, Size: 1325 bytes --] 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. Ben. -- Ben Hutchings The most exhausting thing in life is being insincere. - Anne Morrow Lindberg [-- 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: XFS hole punch races 2016-06-04 17:11 ` Ben Hutchings @ 2016-06-04 23:28 ` Dave Chinner 2016-06-05 1:19 ` Ben Hutchings 0 siblings, 1 reply; 8+ messages in thread From: Dave Chinner @ 2016-06-04 23:28 UTC (permalink / raw) To: Ben Hutchings; +Cc: Jan Kara, stable, xfs 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. 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".... Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: XFS hole punch races 2016-06-04 23:28 ` Dave Chinner @ 2016-06-05 1:19 ` Ben Hutchings 2016-06-05 2:16 ` Dave Chinner 0 siblings, 1 reply; 8+ messages in thread From: Ben Hutchings @ 2016-06-05 1:19 UTC (permalink / raw) To: Dave Chinner; +Cc: Jan Kara, stable, xfs [-- 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: XFS hole punch races 2016-06-05 1:19 ` Ben Hutchings @ 2016-06-05 2:16 ` Dave Chinner 2016-06-05 5:16 ` Willy Tarreau 0 siblings, 1 reply; 8+ messages in thread From: Dave Chinner @ 2016-06-05 2:16 UTC (permalink / raw) To: Ben Hutchings; +Cc: Jan Kara, stable, xfs On Sun, Jun 05, 2016 at 02:19:32AM +0100, Ben Hutchings wrote: > 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. Sorry, Ben, I didn't mean to imply you hadn't done your due diligence properly. It's more a case of lots of things around these patches also changed, and from that perspective the changes are effective a random selection of changes spread across several years of development. It's subtle things, like changes to how IO completion is processed (especially for AIO), etc that the backported code might depend on for correct behaviour but aren't in the older kernels. These sorts of subtle problems are typically only discovered by users with uncommon applications and/or load.... > > 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. Probably for the best - these mmap/holepunch races had been in the code since XFS was first ported, so it's not like it's a new problem. :/ Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: XFS hole punch races 2016-06-05 2:16 ` Dave Chinner @ 2016-06-05 5:16 ` Willy Tarreau 0 siblings, 0 replies; 8+ messages in thread From: Willy Tarreau @ 2016-06-05 5:16 UTC (permalink / raw) To: Dave Chinner; +Cc: Jan Kara, Ben Hutchings, stable, xfs Dave, On Sun, Jun 05, 2016 at 12:16:54PM +1000, Dave Chinner wrote: > On Sun, Jun 05, 2016 at 02:19:32AM +0100, Ben Hutchings wrote: > > On Sun, 2016-06-05 at 09:28 +1000, Dave Chinner wrote: > > > 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. > > Sorry, Ben, I didn't mean to imply you hadn't done your due diligence > properly. It's more a case of lots of things around these patches > also changed, and from that perspective the changes are effective a > random selection of changes spread across several years of > development. > > It's subtle things, like changes to how IO completion is processed > (especially for AIO), etc that the backported code might depend on > for correct behaviour but aren't in the older kernels. These sorts > of subtle problems are typically only discovered by users with > uncommon applications and/or load.... Does this mean that as a rule of thumb we'd rather avoid backporting XFS fixes unless they seem really obvious (or at all) ? Thanks, Willy _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-06-05 5:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-22 15:57 XFS hole punch races Jan Kara 2016-05-02 23:44 ` Greg KH 2016-05-15 22:23 ` Ben Hutchings 2016-06-04 17:11 ` Ben Hutchings 2016-06-04 23:28 ` Dave Chinner 2016-06-05 1:19 ` Ben Hutchings 2016-06-05 2:16 ` Dave Chinner 2016-06-05 5:16 ` Willy Tarreau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox