d7b6404116
A recent multithreaded write data corruption has been uncovered in
the iomap write code. The core of the problem is partial folio
writes can be flushed to disk while a new racing write can map it
and fill the rest of the page:
writeback new write
allocate blocks
blocks are unwritten
submit IO
.....
map blocks
iomap indicates UNWRITTEN range
loop {
lock folio
copyin data
.....
IO completes
runs unwritten extent conv
blocks are marked written
<iomap now stale>
get next folio
}
Now add memory pressure such that memory reclaim evicts the
partially written folio that has already been written to disk.
When the new write finally gets to the last partial page of the new
write, it does not find it in cache, so it instantiates a new page,
sees the iomap is unwritten, and zeros the part of the page that
it does not have data from. This overwrites the data on disk that
was originally written.
The full description of the corruption mechanism can be found here:
https://lore.kernel.org/linux-xfs/20220817093627.GZ3600936@dread.disaster.area/
To solve this problem, we need to check whether the iomap is still
valid after we lock each folio during the write. We have to do it
after we lock the page so that we don't end up with state changes
occurring while we wait for the folio to be locked.
Hence we need a mechanism to be able to check that the cached iomap
is still valid (similar to what we already do in buffered
writeback), and we need a way for ->begin_write to back out and
tell the high level iomap iterator that we need to remap the
remaining write range.
The iomap needs to grow some storage for the validity cookie that
the filesystem provides to travel with the iomap. XFS, in
particular, also needs to know some more information about what the
iomap maps (attribute extents rather than file data extents) to for
the validity cookie to cover all the types of iomaps we might need
to validate.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
98 lines
3.1 KiB
C
98 lines
3.1 KiB
C
// SPDX-License-Identifier: GPL-2.0
|
|
/*
|
|
* Copyright (C) 2010 Red Hat, Inc.
|
|
* Copyright (c) 2016-2021 Christoph Hellwig.
|
|
*/
|
|
#include <linux/fs.h>
|
|
#include <linux/iomap.h>
|
|
#include "trace.h"
|
|
|
|
/*
|
|
* Advance to the next range we need to map.
|
|
*
|
|
* If the iomap is marked IOMAP_F_STALE, it means the existing map was not fully
|
|
* processed - it was aborted because the extent the iomap spanned may have been
|
|
* changed during the operation. In this case, the iteration behaviour is to
|
|
* remap the unprocessed range of the iter, and that means we may need to remap
|
|
* even when we've made no progress (i.e. iter->processed = 0). Hence the
|
|
* "finished iterating" case needs to distinguish between
|
|
* (processed = 0) meaning we are done and (processed = 0 && stale) meaning we
|
|
* need to remap the entire remaining range.
|
|
*/
|
|
static inline int iomap_iter_advance(struct iomap_iter *iter)
|
|
{
|
|
bool stale = iter->iomap.flags & IOMAP_F_STALE;
|
|
|
|
/* handle the previous iteration (if any) */
|
|
if (iter->iomap.length) {
|
|
if (iter->processed < 0)
|
|
return iter->processed;
|
|
if (!iter->processed && !stale)
|
|
return 0;
|
|
if (WARN_ON_ONCE(iter->processed > iomap_length(iter)))
|
|
return -EIO;
|
|
iter->pos += iter->processed;
|
|
iter->len -= iter->processed;
|
|
if (!iter->len)
|
|
return 0;
|
|
}
|
|
|
|
/* clear the state for the next iteration */
|
|
iter->processed = 0;
|
|
memset(&iter->iomap, 0, sizeof(iter->iomap));
|
|
memset(&iter->srcmap, 0, sizeof(iter->srcmap));
|
|
return 1;
|
|
}
|
|
|
|
static inline void iomap_iter_done(struct iomap_iter *iter)
|
|
{
|
|
WARN_ON_ONCE(iter->iomap.offset > iter->pos);
|
|
WARN_ON_ONCE(iter->iomap.length == 0);
|
|
WARN_ON_ONCE(iter->iomap.offset + iter->iomap.length <= iter->pos);
|
|
WARN_ON_ONCE(iter->iomap.flags & IOMAP_F_STALE);
|
|
|
|
trace_iomap_iter_dstmap(iter->inode, &iter->iomap);
|
|
if (iter->srcmap.type != IOMAP_HOLE)
|
|
trace_iomap_iter_srcmap(iter->inode, &iter->srcmap);
|
|
}
|
|
|
|
/**
|
|
* iomap_iter - iterate over a ranges in a file
|
|
* @iter: iteration structue
|
|
* @ops: iomap ops provided by the file system
|
|
*
|
|
* Iterate over filesystem-provided space mappings for the provided file range.
|
|
*
|
|
* This function handles cleanup of resources acquired for iteration when the
|
|
* filesystem indicates there are no more space mappings, which means that this
|
|
* function must be called in a loop that continues as long it returns a
|
|
* positive value. If 0 or a negative value is returned, the caller must not
|
|
* return to the loop body. Within a loop body, there are two ways to break out
|
|
* of the loop body: leave @iter.processed unchanged, or set it to a negative
|
|
* errno.
|
|
*/
|
|
int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
|
|
{
|
|
int ret;
|
|
|
|
if (iter->iomap.length && ops->iomap_end) {
|
|
ret = ops->iomap_end(iter->inode, iter->pos, iomap_length(iter),
|
|
iter->processed > 0 ? iter->processed : 0,
|
|
iter->flags, &iter->iomap);
|
|
if (ret < 0 && !iter->processed)
|
|
return ret;
|
|
}
|
|
|
|
trace_iomap_iter(iter, ops, _RET_IP_);
|
|
ret = iomap_iter_advance(iter);
|
|
if (ret <= 0)
|
|
return ret;
|
|
|
|
ret = ops->iomap_begin(iter->inode, iter->pos, iter->len, iter->flags,
|
|
&iter->iomap, &iter->srcmap);
|
|
if (ret < 0)
|
|
return ret;
|
|
iomap_iter_done(iter);
|
|
return 1;
|
|
}
|