| Message ID | 5bcfc94b-ecb9-468b-78f3-ae5de53f62a0@suse.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On 07/25/17 08:47, Nikolay Borisov wrote: > I'd like to request the aforementioned patch to be included into stable > kernels. I've attached the 4.4 backport, the patch got introduced in > 4.10 so for 4.9 it will likely be able to apply cleanly, for 4.4 it > needed a bit of adjustment. It has been in 4.9-stable since January, see: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/fs/xfs?h=linux-4.9.y&id=3978c5bb004312fd267aed7279fe64b119e126b0 -h -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 25.07.2017 13:43, Holger Hoffstätte wrote: > On 07/25/17 08:47, Nikolay Borisov wrote: >> I'd like to request the aforementioned patch to be included into stable >> kernels. I've attached the 4.4 backport, the patch got introduced in >> 4.10 so for 4.9 it will likely be able to apply cleanly, for 4.4 it >> needed a bit of adjustment. > > It has been in 4.9-stable since January, see: > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/fs/xfs?h=linux-4.9.y&id=3978c5bb004312fd267aed7279fe64b119e126b0 > Strange, it's missing a stable tag > -h > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 25, 2017 at 01:44:30PM +0300, Nikolay Borisov wrote: > > > On 25.07.2017 13:43, Holger Hoffstätte wrote: > > On 07/25/17 08:47, Nikolay Borisov wrote: > >> I'd like to request the aforementioned patch to be included into stable > >> kernels. I've attached the 4.4 backport, the patch got introduced in > >> 4.10 so for 4.9 it will likely be able to apply cleanly, for 4.4 it > >> needed a bit of adjustment. > > > > It has been in 4.9-stable since January, see: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/fs/xfs?h=linux-4.9.y&id=3978c5bb004312fd267aed7279fe64b119e126b0 > > > > Strange, it's missing a stable tag What does that have to do with anything? Lots of patches get asked to be included that don't have stable tags, just like you just did :) Also, please always include the original git commit id of the patch you wish to have included with your request so that I don't have to turn around and ask for it... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 25, 2017 at 06:42:21AM -0700, Greg KH wrote: > On Tue, Jul 25, 2017 at 01:44:30PM +0300, Nikolay Borisov wrote: > > > > > > On 25.07.2017 13:43, Holger Hoffstätte wrote: > > > On 07/25/17 08:47, Nikolay Borisov wrote: > > >> I'd like to request the aforementioned patch to be included into stable > > >> kernels. I've attached the 4.4 backport, the patch got introduced in > > >> 4.10 so for 4.9 it will likely be able to apply cleanly, for 4.4 it > > >> needed a bit of adjustment. > > > > > > It has been in 4.9-stable since January, see: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/fs/xfs?h=linux-4.9.y&id=3978c5bb004312fd267aed7279fe64b119e126b0 > > > > > > > Strange, it's missing a stable tag > > What does that have to do with anything? Lots of patches get asked to > be included that don't have stable tags, just like you just did :) > > Also, please always include the original git commit id of the patch you > wish to have included with your request so that I don't have to turn > around and ask for it... Ah nevermind, it was in the subject, I haven't had my morning coffee yet... -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 25, 2017 at 06:43:34AM -0700, Greg KH wrote: > On Tue, Jul 25, 2017 at 06:42:21AM -0700, Greg KH wrote: > > On Tue, Jul 25, 2017 at 01:44:30PM +0300, Nikolay Borisov wrote: > > > > > > > > > On 25.07.2017 13:43, Holger Hoffstätte wrote: > > > > On 07/25/17 08:47, Nikolay Borisov wrote: > > > >> I'd like to request the aforementioned patch to be included into stable > > > >> kernels. I've attached the 4.4 backport, the patch got introduced in > > > >> 4.10 so for 4.9 it will likely be able to apply cleanly, for 4.4 it > > > >> needed a bit of adjustment. > > > > > > > > It has been in 4.9-stable since January, see: > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/fs/xfs?h=linux-4.9.y&id=3978c5bb004312fd267aed7279fe64b119e126b0 > > > > > > > > > > Strange, it's missing a stable tag > > > > What does that have to do with anything? Lots of patches get asked to > > be included that don't have stable tags, just like you just did :) > > > > Also, please always include the original git commit id of the patch you > > wish to have included with your request so that I don't have to turn > > around and ask for it... > > Ah nevermind, it was in the subject, I haven't had my morning coffee > yet... Oh, and I need an ack from the xfs maintainers to be able to take this, there must be some reason they didn't backport it like they did for a number of other xfs patches... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 25, 2017 at 06:47:45AM -0700, Greg KH wrote: > Oh, and I need an ack from the xfs maintainers to be able to take this, > there must be some reason they didn't backport it like they did for a > number of other xfs patches... In general we'd like to see xfstests runs for all stable backports, because file system code is hairy. So for any xfs stable backports please send the list of patches you want backported, and tell us that you did indeed run xfstests. No proof is required, we'd like to trust our contributors. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 25.07.2017 16:50, Christoph Hellwig wrote: > On Tue, Jul 25, 2017 at 06:47:45AM -0700, Greg KH wrote: >> Oh, and I need an ack from the xfs maintainers to be able to take this, >> there must be some reason they didn't backport it like they did for a >> number of other xfs patches... > > In general we'd like to see xfstests runs for all stable backports, > because file system code is hairy. > > So for any xfs stable backports please send the list of patches you > want backported, and tell us that you did indeed run xfstests. No > proof is required, we'd like to trust our contributors. > Before including this in our enterprise distribution I did run xfstest with and without the patch and there was no difference in the result. Hence I sent the patch. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 25, 2017 at 05:02:54PM +0300, Nikolay Borisov wrote: > > > On 25.07.2017 16:50, Christoph Hellwig wrote: > > On Tue, Jul 25, 2017 at 06:47:45AM -0700, Greg KH wrote: > >> Oh, and I need an ack from the xfs maintainers to be able to take this, > >> there must be some reason they didn't backport it like they did for a > >> number of other xfs patches... > > > > In general we'd like to see xfstests runs for all stable backports, > > because file system code is hairy. > > > > So for any xfs stable backports please send the list of patches you > > want backported, and tell us that you did indeed run xfstests. No > > proof is required, we'd like to trust our contributors. > > > > Before including this in our enterprise distribution I did run xfstest > with and without the patch and there was no difference in the result. > Hence I sent the patch. Looks fine to me... Acked-by: Darrick J. Wong <darrick.wong@oracle.com> --D > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Brian Foster <bfoster@redhat.com>
Date: Tue, 8 Nov 2016 12:54:14 +1100
Subject: xfs: don't BUG() on mixed direct and mapped I/O
We've had reports of generic/095 causing XFS to BUG() in
__xfs_get_blocks() due to the existence of delalloc blocks on a
direct I/O read. generic/095 issues a mix of various types of I/O,
including direct and memory mapped I/O to a single file. This is
clearly not supported behavior and is known to lead to such
problems. E.g., the lack of exclusion between the direct I/O and
write fault paths means that a write fault can allocate delalloc
blocks in a region of a file that was previously a hole after the
direct read has attempted to flush/inval the file range, but before
it actually reads the block mapping. In turn, the direct read
discovers a delalloc extent and cannot proceed.
While the appropriate solution here is to not mix direct and memory
mapped I/O to the same regions of the same file, the current
BUG_ON() behavior is probably overkill as it can crash the entire
system. Instead, localize the failure to the I/O in question by
returning an error for a direct I/O that cannot be handled safely
due to delalloc blocks. Be careful to allow the case of a direct
write to post-eof delalloc blocks. This can occur due to speculative
preallocation and is safe as post-eof blocks are not accompanied by
dirty pages in pagecache (conversely, preallocation within eof must
have been zeroed, and thus dirtied, before the inode size could have
been increased beyond said blocks).
Finally, provide an additional warning if a direct I/O write occurs
while the file is memory mapped. This may not catch all problematic
scenarios, but provides a hint that some known-to-be-problematic I/O
methods are in use.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/xfs/xfs_aops.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index d4752078b471..11fa73d8be78 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1439,6 +1439,26 @@ __xfs_get_blocks(
if (error)
goto out_unlock;
+ /*
+ * The only time we can ever safely find delalloc blocks on direct I/O
+ * is a dio write to post-eof speculative preallocation. All other
+ * scenarios are indicative of a problem or misuse (such as mixing
+ * direct and mapped I/O).
+ *
+ * The file may be unmapped by the time we get here so we cannot
+ * reliably fail the I/O based on mapping. Instead, fail the I/O if this
+ * is a read or a write within eof. Otherwise, carry on but warn as a
+ * precuation if the file happens to be mapped.
+ */
+ if (direct && imap.br_startblock == DELAYSTARTBLOCK) {
+ if (!create || offset < i_size_read(VFS_I(ip))) {
+ WARN_ON_ONCE(1);
+ error = -EIO;
+ goto out_unlock;
+ }
+ WARN_ON_ONCE(mapping_mapped(VFS_I(ip)->i_mapping));
+ }
+
/* for DAX, we convert unwritten extents directly */
if (create &&
(!nimaps ||
@@ -1538,7 +1558,6 @@ __xfs_get_blocks(
set_buffer_new(bh_result);
if (imap.br_startblock == DELAYSTARTBLOCK) {
- BUG_ON(direct);
if (create) {
set_buffer_uptodate(bh_result);
set_buffer_mapped(bh_result);
--
2.7.4