From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
Josef Bacik <jbacik@fb.com>, Amir Goldstein <amir73il@gmail.com>,
"Darrick J. Wong" <darrick.wong@oracle.com>
Subject: [PATCH 4.4 06/24] xfs: fix incorrect log_flushed on fsync
Date: Tue, 12 Jun 2018 18:51:50 +0200 [thread overview]
Message-ID: <20180612164816.861511958@linuxfoundation.org> (raw)
In-Reply-To: <20180612164816.587001852@linuxfoundation.org>
4.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Amir Goldstein <amir73il@gmail.com>
commit 47c7d0b19502583120c3f396c7559e7a77288a68 upstream.
When calling into _xfs_log_force{,_lsn}() with a pointer
to log_flushed variable, log_flushed will be set to 1 if:
1. xlog_sync() is called to flush the active log buffer
AND/OR
2. xlog_wait() is called to wait on a syncing log buffers
xfs_file_fsync() checks the value of log_flushed after
_xfs_log_force_lsn() call to optimize away an explicit
PREFLUSH request to the data block device after writing
out all the file's pages to disk.
This optimization is incorrect in the following sequence of events:
Task A Task B
-------------------------------------------------------
xfs_file_fsync()
_xfs_log_force_lsn()
xlog_sync()
[submit PREFLUSH]
xfs_file_fsync()
file_write_and_wait_range()
[submit WRITE X]
[endio WRITE X]
_xfs_log_force_lsn()
xlog_wait()
[endio PREFLUSH]
The write X is not guarantied to be on persistent storage
when PREFLUSH request in completed, because write A was submitted
after the PREFLUSH request, but xfs_file_fsync() of task A will
be notified of log_flushed=1 and will skip explicit flush.
If the system crashes after fsync of task A, write X may not be
present on disk after reboot.
This bug was discovered and demonstrated using Josef Bacik's
dm-log-writes target, which can be used to record block io operations
and then replay a subset of these operations onto the target device.
The test goes something like this:
- Use fsx to execute ops of a file and record ops on log device
- Every now and then fsync the file, store md5 of file and mark
the location in the log
- Then replay log onto device for each mark, mount fs and compare
md5 of file to stored value
Cc: Christoph Hellwig <hch@lst.de>
Cc: Josef Bacik <jbacik@fb.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/xfs/xfs_log.c | 7 -------
1 file changed, 7 deletions(-)
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -3323,8 +3323,6 @@ maybe_sleep:
*/
if (iclog->ic_state & XLOG_STATE_IOERROR)
return -EIO;
- if (log_flushed)
- *log_flushed = 1;
} else {
no_sleep:
@@ -3432,8 +3430,6 @@ try_again:
xlog_wait(&iclog->ic_prev->ic_write_wait,
&log->l_icloglock);
- if (log_flushed)
- *log_flushed = 1;
already_slept = 1;
goto try_again;
}
@@ -3467,9 +3463,6 @@ try_again:
*/
if (iclog->ic_state & XLOG_STATE_IOERROR)
return -EIO;
-
- if (log_flushed)
- *log_flushed = 1;
} else { /* just return */
spin_unlock(&log->l_icloglock);
}
next prev parent reply other threads:[~2018-06-12 16:53 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-12 16:51 [PATCH 4.4 00/24] 4.4.137-stable review Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 01/24] tpm: do not suspend/resume if power stays on Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 02/24] tpm: self test failure should not cause suspend to fail Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 03/24] mmap: introduce sane default mmap limits Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 04/24] mmap: relax file size limit for regular files Greg Kroah-Hartman
2018-06-12 16:51 ` Greg Kroah-Hartman [this message]
2018-06-12 16:51 ` [PATCH 4.4 07/24] drm: set FMODE_UNSIGNED_OFFSET for drm files Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 08/24] brcmfmac: Fix check for ISO3166 code Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 09/24] bnx2x: use the right constant Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 10/24] dccp: dont free ccid2_hc_tx_sock struct in dccp_disconnect() Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 11/24] enic: set DMA mask to 47 bit Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 12/24] ip6mr: only set ip6mr_table from setsockopt when ip6mr_new_table succeeds Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 13/24] ipv4: remove warning in ip_recv_error Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 14/24] isdn: eicon: fix a missing-check bug Greg Kroah-Hartman
2018-06-12 16:51 ` [PATCH 4.4 15/24] netdev-FAQ: clarify DaveMs position for stable backports Greg Kroah-Hartman
2018-06-12 16:52 ` [PATCH 4.4 16/24] net/packet: refine check for priv area size Greg Kroah-Hartman
2018-06-12 16:52 ` [PATCH 4.4 18/24] packet: fix reserve calculation Greg Kroah-Hartman
2018-06-12 16:52 ` [PATCH 4.4 19/24] qed: Fix mask for physical address in ILT entry Greg Kroah-Hartman
2018-06-12 16:52 ` [PATCH 4.4 20/24] net/mlx4: Fix irq-unsafe spinlock usage Greg Kroah-Hartman
2018-06-12 16:52 ` [PATCH 4.4 21/24] team: use netdev_features_t instead of u32 Greg Kroah-Hartman
2018-06-12 16:52 ` [PATCH 4.4 22/24] rtnetlink: validate attributes in do_setlink() Greg Kroah-Hartman
2018-06-12 16:52 ` [PATCH 4.4 23/24] net: phy: broadcom: Fix bcm_write_exp() Greg Kroah-Hartman
2018-06-12 16:52 ` [PATCH 4.4 24/24] net: metrics: add proper netlink validation Greg Kroah-Hartman
2018-06-19 13:15 ` Ben Hutchings
2018-06-12 18:17 ` [PATCH 4.4 00/24] 4.4.137-stable review Nathan Chancellor
2018-06-12 18:49 ` Greg Kroah-Hartman
2018-06-12 20:59 ` Shuah Khan
2018-06-13 13:48 ` Guenter Roeck
2018-06-13 20:47 ` Rafael Tinoco
2018-06-13 21:00 ` Greg Kroah-Hartman
2018-06-13 21:08 ` Rafael David Tinoco
2018-06-14 1:48 ` Rafael Tinoco
2018-06-14 6:34 ` Greg Kroah-Hartman
2018-06-14 8:54 ` Naresh Kamboju
2018-06-14 9:01 ` Greg Kroah-Hartman
2018-06-14 9:49 ` [LTP] " Jan Stancek
2018-06-14 10:21 ` Greg Kroah-Hartman
2018-06-14 10:36 ` Jan Stancek
2018-06-14 10:54 ` Rafael Tinoco
2018-06-14 11:36 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180612164816.861511958@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=amir73il@gmail.com \
--cc=darrick.wong@oracle.com \
--cc=hch@lst.de \
--cc=jbacik@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).