2024-06-25 14:52:28 +00:00
|
|
|
// SPDX-License-Identifier: GPL-2.0
|
|
|
|
|
|
|
|
#include <linux/fsverity.h>
|
|
|
|
#include <linux/iomap.h>
|
|
|
|
#include "ctree.h"
|
|
|
|
#include "delalloc-space.h"
|
|
|
|
#include "direct-io.h"
|
|
|
|
#include "extent-tree.h"
|
|
|
|
#include "file.h"
|
|
|
|
#include "fs.h"
|
|
|
|
#include "transaction.h"
|
|
|
|
#include "volumes.h"
|
|
|
|
|
|
|
|
struct btrfs_dio_data {
|
|
|
|
ssize_t submitted;
|
|
|
|
struct extent_changeset *data_reserved;
|
|
|
|
struct btrfs_ordered_extent *ordered;
|
|
|
|
bool data_space_reserved;
|
|
|
|
bool nocow_done;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct btrfs_dio_private {
|
|
|
|
/* Range of I/O */
|
|
|
|
u64 file_offset;
|
|
|
|
u32 bytes;
|
|
|
|
|
|
|
|
/* This must be last */
|
|
|
|
struct btrfs_bio bbio;
|
|
|
|
};
|
|
|
|
|
|
|
|
static struct bio_set btrfs_dio_bioset;
|
|
|
|
|
|
|
|
static int lock_extent_direct(struct inode *inode, u64 lockstart, u64 lockend,
|
|
|
|
struct extent_state **cached_state,
|
|
|
|
unsigned int iomap_flags)
|
|
|
|
{
|
|
|
|
const bool writing = (iomap_flags & IOMAP_WRITE);
|
|
|
|
const bool nowait = (iomap_flags & IOMAP_NOWAIT);
|
|
|
|
struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
|
|
|
|
struct btrfs_ordered_extent *ordered;
|
|
|
|
int ret = 0;
|
|
|
|
|
btrfs: take the dio extent lock during O_DIRECT operations
Currently we hold the extent lock for the entire duration of a read.
This isn't really necessary in the buffered case, we're protected by the
page lock, however it's necessary for O_DIRECT.
For O_DIRECT reads, if we only locked the extent for the part where we
get the extent, we could potentially race with an O_DIRECT write in the
same region. This isn't really a problem, unless the read is delayed so
much that the write does the COW, unpins the old extent, and some other
application re-allocates the extent before the read is actually able to
be submitted. At that point at best we'd have a checksum mismatch, but
at worse we could read data that doesn't belong to us.
To address this potential race we need to make sure we don't have
overlapping, concurrent direct io reads and writes.
To accomplish this use the new EXTENT_DIO_LOCKED bit in the direct IO
case in the same spot as the current extent lock. The writes will take
this while they're creating the ordered extent, which is also used to
make sure concurrent buffered reads or concurrent direct reads are not
allowed to occur, and drop it after the ordered extent is taken. For
reads it will act as the current read behavior for the EXTENT_LOCKED
bit, we set it when we're starting the read, we clear it in the end_io
to allow other direct writes to continue.
This still has the drawback of disallowing concurrent overlapping direct
reads from occurring, but that exists with the current extent locking.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-08-19 20:50:01 +00:00
|
|
|
/* Direct lock must be taken before the extent lock. */
|
|
|
|
if (nowait) {
|
|
|
|
if (!try_lock_dio_extent(io_tree, lockstart, lockend, cached_state))
|
|
|
|
return -EAGAIN;
|
|
|
|
} else {
|
|
|
|
lock_dio_extent(io_tree, lockstart, lockend, cached_state);
|
|
|
|
}
|
|
|
|
|
2024-06-25 14:52:28 +00:00
|
|
|
while (1) {
|
|
|
|
if (nowait) {
|
|
|
|
if (!try_lock_extent(io_tree, lockstart, lockend,
|
btrfs: take the dio extent lock during O_DIRECT operations
Currently we hold the extent lock for the entire duration of a read.
This isn't really necessary in the buffered case, we're protected by the
page lock, however it's necessary for O_DIRECT.
For O_DIRECT reads, if we only locked the extent for the part where we
get the extent, we could potentially race with an O_DIRECT write in the
same region. This isn't really a problem, unless the read is delayed so
much that the write does the COW, unpins the old extent, and some other
application re-allocates the extent before the read is actually able to
be submitted. At that point at best we'd have a checksum mismatch, but
at worse we could read data that doesn't belong to us.
To address this potential race we need to make sure we don't have
overlapping, concurrent direct io reads and writes.
To accomplish this use the new EXTENT_DIO_LOCKED bit in the direct IO
case in the same spot as the current extent lock. The writes will take
this while they're creating the ordered extent, which is also used to
make sure concurrent buffered reads or concurrent direct reads are not
allowed to occur, and drop it after the ordered extent is taken. For
reads it will act as the current read behavior for the EXTENT_LOCKED
bit, we set it when we're starting the read, we clear it in the end_io
to allow other direct writes to continue.
This still has the drawback of disallowing concurrent overlapping direct
reads from occurring, but that exists with the current extent locking.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-08-19 20:50:01 +00:00
|
|
|
cached_state)) {
|
|
|
|
ret = -EAGAIN;
|
|
|
|
break;
|
|
|
|
}
|
2024-06-25 14:52:28 +00:00
|
|
|
} else {
|
|
|
|
lock_extent(io_tree, lockstart, lockend, cached_state);
|
|
|
|
}
|
|
|
|
/*
|
|
|
|
* We're concerned with the entire range that we're going to be
|
|
|
|
* doing DIO to, so we need to make sure there's no ordered
|
|
|
|
* extents in this range.
|
|
|
|
*/
|
|
|
|
ordered = btrfs_lookup_ordered_range(BTRFS_I(inode), lockstart,
|
|
|
|
lockend - lockstart + 1);
|
|
|
|
|
|
|
|
/*
|
|
|
|
* We need to make sure there are no buffered pages in this
|
|
|
|
* range either, we could have raced between the invalidate in
|
|
|
|
* generic_file_direct_write and locking the extent. The
|
|
|
|
* invalidate needs to happen so that reads after a write do not
|
|
|
|
* get stale data.
|
|
|
|
*/
|
|
|
|
if (!ordered &&
|
|
|
|
(!writing || !filemap_range_has_page(inode->i_mapping,
|
|
|
|
lockstart, lockend)))
|
|
|
|
break;
|
|
|
|
|
|
|
|
unlock_extent(io_tree, lockstart, lockend, cached_state);
|
|
|
|
|
|
|
|
if (ordered) {
|
|
|
|
if (nowait) {
|
|
|
|
btrfs_put_ordered_extent(ordered);
|
|
|
|
ret = -EAGAIN;
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
/*
|
|
|
|
* If we are doing a DIO read and the ordered extent we
|
|
|
|
* found is for a buffered write, we can not wait for it
|
|
|
|
* to complete and retry, because if we do so we can
|
|
|
|
* deadlock with concurrent buffered writes on page
|
|
|
|
* locks. This happens only if our DIO read covers more
|
|
|
|
* than one extent map, if at this point has already
|
|
|
|
* created an ordered extent for a previous extent map
|
|
|
|
* and locked its range in the inode's io tree, and a
|
|
|
|
* concurrent write against that previous extent map's
|
|
|
|
* range and this range started (we unlock the ranges
|
|
|
|
* in the io tree only when the bios complete and
|
|
|
|
* buffered writes always lock pages before attempting
|
|
|
|
* to lock range in the io tree).
|
|
|
|
*/
|
|
|
|
if (writing ||
|
|
|
|
test_bit(BTRFS_ORDERED_DIRECT, &ordered->flags))
|
|
|
|
btrfs_start_ordered_extent(ordered);
|
|
|
|
else
|
|
|
|
ret = nowait ? -EAGAIN : -ENOTBLK;
|
|
|
|
btrfs_put_ordered_extent(ordered);
|
|
|
|
} else {
|
|
|
|
/*
|
|
|
|
* We could trigger writeback for this range (and wait
|
|
|
|
* for it to complete) and then invalidate the pages for
|
|
|
|
* this range (through invalidate_inode_pages2_range()),
|
|
|
|
* but that can lead us to a deadlock with a concurrent
|
|
|
|
* call to readahead (a buffered read or a defrag call
|
|
|
|
* triggered a readahead) on a page lock due to an
|
|
|
|
* ordered dio extent we created before but did not have
|
|
|
|
* yet a corresponding bio submitted (whence it can not
|
|
|
|
* complete), which makes readahead wait for that
|
|
|
|
* ordered extent to complete while holding a lock on
|
|
|
|
* that page.
|
|
|
|
*/
|
|
|
|
ret = nowait ? -EAGAIN : -ENOTBLK;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (ret)
|
|
|
|
break;
|
|
|
|
|
|
|
|
cond_resched();
|
|
|
|
}
|
|
|
|
|
btrfs: take the dio extent lock during O_DIRECT operations
Currently we hold the extent lock for the entire duration of a read.
This isn't really necessary in the buffered case, we're protected by the
page lock, however it's necessary for O_DIRECT.
For O_DIRECT reads, if we only locked the extent for the part where we
get the extent, we could potentially race with an O_DIRECT write in the
same region. This isn't really a problem, unless the read is delayed so
much that the write does the COW, unpins the old extent, and some other
application re-allocates the extent before the read is actually able to
be submitted. At that point at best we'd have a checksum mismatch, but
at worse we could read data that doesn't belong to us.
To address this potential race we need to make sure we don't have
overlapping, concurrent direct io reads and writes.
To accomplish this use the new EXTENT_DIO_LOCKED bit in the direct IO
case in the same spot as the current extent lock. The writes will take
this while they're creating the ordered extent, which is also used to
make sure concurrent buffered reads or concurrent direct reads are not
allowed to occur, and drop it after the ordered extent is taken. For
reads it will act as the current read behavior for the EXTENT_LOCKED
bit, we set it when we're starting the read, we clear it in the end_io
to allow other direct writes to continue.
This still has the drawback of disallowing concurrent overlapping direct
reads from occurring, but that exists with the current extent locking.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-08-19 20:50:01 +00:00
|
|
|
if (ret)
|
|
|
|
unlock_dio_extent(io_tree, lockstart, lockend, cached_state);
|
2024-06-25 14:52:28 +00:00
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
|
|
|
static struct extent_map *btrfs_create_dio_extent(struct btrfs_inode *inode,
|
|
|
|
struct btrfs_dio_data *dio_data,
|
|
|
|
const u64 start,
|
|
|
|
const struct btrfs_file_extent *file_extent,
|
|
|
|
const int type)
|
|
|
|
{
|
|
|
|
struct extent_map *em = NULL;
|
|
|
|
struct btrfs_ordered_extent *ordered;
|
|
|
|
|
|
|
|
if (type != BTRFS_ORDERED_NOCOW) {
|
|
|
|
em = btrfs_create_io_em(inode, start, file_extent, type);
|
|
|
|
if (IS_ERR(em))
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
|
|
|
ordered = btrfs_alloc_ordered_extent(inode, start, file_extent,
|
|
|
|
(1 << type) |
|
|
|
|
(1 << BTRFS_ORDERED_DIRECT));
|
|
|
|
if (IS_ERR(ordered)) {
|
|
|
|
if (em) {
|
|
|
|
free_extent_map(em);
|
|
|
|
btrfs_drop_extent_map_range(inode, start,
|
|
|
|
start + file_extent->num_bytes - 1, false);
|
|
|
|
}
|
|
|
|
em = ERR_CAST(ordered);
|
|
|
|
} else {
|
|
|
|
ASSERT(!dio_data->ordered);
|
|
|
|
dio_data->ordered = ordered;
|
|
|
|
}
|
|
|
|
out:
|
|
|
|
|
|
|
|
return em;
|
|
|
|
}
|
|
|
|
|
|
|
|
static struct extent_map *btrfs_new_extent_direct(struct btrfs_inode *inode,
|
|
|
|
struct btrfs_dio_data *dio_data,
|
|
|
|
u64 start, u64 len)
|
|
|
|
{
|
|
|
|
struct btrfs_root *root = inode->root;
|
|
|
|
struct btrfs_fs_info *fs_info = root->fs_info;
|
|
|
|
struct btrfs_file_extent file_extent;
|
|
|
|
struct extent_map *em;
|
|
|
|
struct btrfs_key ins;
|
|
|
|
u64 alloc_hint;
|
|
|
|
int ret;
|
|
|
|
|
|
|
|
alloc_hint = btrfs_get_extent_allocation_hint(inode, start, len);
|
|
|
|
again:
|
|
|
|
ret = btrfs_reserve_extent(root, len, len, fs_info->sectorsize,
|
|
|
|
0, alloc_hint, &ins, 1, 1);
|
|
|
|
if (ret == -EAGAIN) {
|
|
|
|
ASSERT(btrfs_is_zoned(fs_info));
|
|
|
|
wait_on_bit_io(&inode->root->fs_info->flags, BTRFS_FS_NEED_ZONE_FINISH,
|
|
|
|
TASK_UNINTERRUPTIBLE);
|
|
|
|
goto again;
|
|
|
|
}
|
|
|
|
if (ret)
|
|
|
|
return ERR_PTR(ret);
|
|
|
|
|
|
|
|
file_extent.disk_bytenr = ins.objectid;
|
|
|
|
file_extent.disk_num_bytes = ins.offset;
|
|
|
|
file_extent.num_bytes = ins.offset;
|
|
|
|
file_extent.ram_bytes = ins.offset;
|
|
|
|
file_extent.offset = 0;
|
|
|
|
file_extent.compression = BTRFS_COMPRESS_NONE;
|
|
|
|
em = btrfs_create_dio_extent(inode, dio_data, start, &file_extent,
|
|
|
|
BTRFS_ORDERED_REGULAR);
|
|
|
|
btrfs_dec_block_group_reservations(fs_info, ins.objectid);
|
|
|
|
if (IS_ERR(em))
|
|
|
|
btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset,
|
|
|
|
1);
|
|
|
|
|
|
|
|
return em;
|
|
|
|
}
|
|
|
|
|
|
|
|
static int btrfs_get_blocks_direct_write(struct extent_map **map,
|
|
|
|
struct inode *inode,
|
|
|
|
struct btrfs_dio_data *dio_data,
|
|
|
|
u64 start, u64 *lenp,
|
|
|
|
unsigned int iomap_flags)
|
|
|
|
{
|
|
|
|
const bool nowait = (iomap_flags & IOMAP_NOWAIT);
|
|
|
|
struct btrfs_fs_info *fs_info = inode_to_fs_info(inode);
|
|
|
|
struct btrfs_file_extent file_extent;
|
|
|
|
struct extent_map *em = *map;
|
|
|
|
int type;
|
|
|
|
u64 block_start;
|
|
|
|
struct btrfs_block_group *bg;
|
|
|
|
bool can_nocow = false;
|
|
|
|
bool space_reserved = false;
|
|
|
|
u64 len = *lenp;
|
|
|
|
u64 prev_len;
|
|
|
|
int ret = 0;
|
|
|
|
|
|
|
|
/*
|
|
|
|
* We don't allocate a new extent in the following cases
|
|
|
|
*
|
|
|
|
* 1) The inode is marked as NODATACOW. In this case we'll just use the
|
|
|
|
* existing extent.
|
|
|
|
* 2) The extent is marked as PREALLOC. We're good to go here and can
|
|
|
|
* just use the extent.
|
|
|
|
*
|
|
|
|
*/
|
|
|
|
if ((em->flags & EXTENT_FLAG_PREALLOC) ||
|
|
|
|
((BTRFS_I(inode)->flags & BTRFS_INODE_NODATACOW) &&
|
|
|
|
em->disk_bytenr != EXTENT_MAP_HOLE)) {
|
|
|
|
if (em->flags & EXTENT_FLAG_PREALLOC)
|
|
|
|
type = BTRFS_ORDERED_PREALLOC;
|
|
|
|
else
|
|
|
|
type = BTRFS_ORDERED_NOCOW;
|
|
|
|
len = min(len, em->len - (start - em->start));
|
|
|
|
block_start = extent_map_block_start(em) + (start - em->start);
|
|
|
|
|
|
|
|
if (can_nocow_extent(inode, start, &len,
|
|
|
|
&file_extent, false, false) == 1) {
|
|
|
|
bg = btrfs_inc_nocow_writers(fs_info, block_start);
|
|
|
|
if (bg)
|
|
|
|
can_nocow = true;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
prev_len = len;
|
|
|
|
if (can_nocow) {
|
|
|
|
struct extent_map *em2;
|
|
|
|
|
|
|
|
/* We can NOCOW, so only need to reserve metadata space. */
|
|
|
|
ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode), len, len,
|
|
|
|
nowait);
|
|
|
|
if (ret < 0) {
|
|
|
|
/* Our caller expects us to free the input extent map. */
|
|
|
|
free_extent_map(em);
|
|
|
|
*map = NULL;
|
|
|
|
btrfs_dec_nocow_writers(bg);
|
|
|
|
if (nowait && (ret == -ENOSPC || ret == -EDQUOT))
|
|
|
|
ret = -EAGAIN;
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
space_reserved = true;
|
|
|
|
|
|
|
|
em2 = btrfs_create_dio_extent(BTRFS_I(inode), dio_data, start,
|
|
|
|
&file_extent, type);
|
|
|
|
btrfs_dec_nocow_writers(bg);
|
|
|
|
if (type == BTRFS_ORDERED_PREALLOC) {
|
|
|
|
free_extent_map(em);
|
|
|
|
*map = em2;
|
|
|
|
em = em2;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (IS_ERR(em2)) {
|
|
|
|
ret = PTR_ERR(em2);
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
|
|
|
dio_data->nocow_done = true;
|
|
|
|
} else {
|
|
|
|
/* Our caller expects us to free the input extent map. */
|
|
|
|
free_extent_map(em);
|
|
|
|
*map = NULL;
|
|
|
|
|
|
|
|
if (nowait) {
|
|
|
|
ret = -EAGAIN;
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* If we could not allocate data space before locking the file
|
|
|
|
* range and we can't do a NOCOW write, then we have to fail.
|
|
|
|
*/
|
|
|
|
if (!dio_data->data_space_reserved) {
|
|
|
|
ret = -ENOSPC;
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* We have to COW and we have already reserved data space before,
|
|
|
|
* so now we reserve only metadata.
|
|
|
|
*/
|
|
|
|
ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode), len, len,
|
|
|
|
false);
|
|
|
|
if (ret < 0)
|
|
|
|
goto out;
|
|
|
|
space_reserved = true;
|
|
|
|
|
|
|
|
em = btrfs_new_extent_direct(BTRFS_I(inode), dio_data, start, len);
|
|
|
|
if (IS_ERR(em)) {
|
|
|
|
ret = PTR_ERR(em);
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
*map = em;
|
|
|
|
len = min(len, em->len - (start - em->start));
|
|
|
|
if (len < prev_len)
|
|
|
|
btrfs_delalloc_release_metadata(BTRFS_I(inode),
|
|
|
|
prev_len - len, true);
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* We have created our ordered extent, so we can now release our reservation
|
|
|
|
* for an outstanding extent.
|
|
|
|
*/
|
|
|
|
btrfs_delalloc_release_extents(BTRFS_I(inode), prev_len);
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Need to update the i_size under the extent lock so buffered
|
|
|
|
* readers will get the updated i_size when we unlock.
|
|
|
|
*/
|
|
|
|
if (start + len > i_size_read(inode))
|
|
|
|
i_size_write(inode, start + len);
|
|
|
|
out:
|
|
|
|
if (ret && space_reserved) {
|
|
|
|
btrfs_delalloc_release_extents(BTRFS_I(inode), len);
|
|
|
|
btrfs_delalloc_release_metadata(BTRFS_I(inode), len, true);
|
|
|
|
}
|
|
|
|
*lenp = len;
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
|
|
|
static int btrfs_dio_iomap_begin(struct inode *inode, loff_t start,
|
|
|
|
loff_t length, unsigned int flags, struct iomap *iomap,
|
|
|
|
struct iomap *srcmap)
|
|
|
|
{
|
|
|
|
struct iomap_iter *iter = container_of(iomap, struct iomap_iter, iomap);
|
|
|
|
struct btrfs_fs_info *fs_info = inode_to_fs_info(inode);
|
|
|
|
struct extent_map *em;
|
|
|
|
struct extent_state *cached_state = NULL;
|
|
|
|
struct btrfs_dio_data *dio_data = iter->private;
|
|
|
|
u64 lockstart, lockend;
|
|
|
|
const bool write = !!(flags & IOMAP_WRITE);
|
|
|
|
int ret = 0;
|
|
|
|
u64 len = length;
|
|
|
|
const u64 data_alloc_len = length;
|
btrfs: do not hold the extent lock for entire read
Historically we've held the extent lock throughout the entire read.
There's been a few reasons for this, but it's mostly just caused us
problems. For example, this prevents us from allowing page faults
during direct io reads, because we could deadlock. This has forced us
to only allow 4k reads at a time for io_uring NOWAIT requests because we
have no idea if we'll be forced to page fault and thus have to do a
whole lot of work.
On the buffered side we are protected by the page lock, as long as we're
reading things like buffered writes, punch hole, and even direct IO to a
certain degree will get hung up on the page lock while the page is in
flight.
On the direct side we have the dio extent lock, which acts much like the
way the extent lock worked previously to this patch, however just for
direct reads. This protects direct reads from concurrent direct writes,
while we're protected from buffered writes via the inode lock.
Now that we're protected in all cases, narrow the extent lock to the
part where we're getting the extent map to submit the reads, no longer
holding the extent lock for the entire read operation. Push the extent
lock down into do_readpage() so that we're only grabbing it when looking
up the extent map. This portion was contributed by Goldwyn.
Co-developed-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Reviewed-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-08-16 19:16:24 +00:00
|
|
|
u32 unlock_bits = EXTENT_LOCKED;
|
2024-06-25 14:52:28 +00:00
|
|
|
|
|
|
|
/*
|
|
|
|
* We could potentially fault if we have a buffer > PAGE_SIZE, and if
|
|
|
|
* we're NOWAIT we may submit a bio for a partial range and return
|
|
|
|
* EIOCBQUEUED, which would result in an errant short read.
|
|
|
|
*
|
|
|
|
* The best way to handle this would be to allow for partial completions
|
|
|
|
* of iocb's, so we could submit the partial bio, return and fault in
|
|
|
|
* the rest of the pages, and then submit the io for the rest of the
|
|
|
|
* range. However we don't have that currently, so simply return
|
|
|
|
* -EAGAIN at this point so that the normal path is used.
|
|
|
|
*/
|
|
|
|
if (!write && (flags & IOMAP_NOWAIT) && length > PAGE_SIZE)
|
|
|
|
return -EAGAIN;
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Cap the size of reads to that usually seen in buffered I/O as we need
|
|
|
|
* to allocate a contiguous array for the checksums.
|
|
|
|
*/
|
|
|
|
if (!write)
|
|
|
|
len = min_t(u64, len, fs_info->sectorsize * BTRFS_MAX_BIO_SECTORS);
|
|
|
|
|
|
|
|
lockstart = start;
|
|
|
|
lockend = start + len - 1;
|
|
|
|
|
|
|
|
/*
|
|
|
|
* iomap_dio_rw() only does filemap_write_and_wait_range(), which isn't
|
|
|
|
* enough if we've written compressed pages to this area, so we need to
|
|
|
|
* flush the dirty pages again to make absolutely sure that any
|
|
|
|
* outstanding dirty pages are on disk - the first flush only starts
|
|
|
|
* compression on the data, while keeping the pages locked, so by the
|
|
|
|
* time the second flush returns we know bios for the compressed pages
|
|
|
|
* were submitted and finished, and the pages no longer under writeback.
|
|
|
|
*
|
|
|
|
* If we have a NOWAIT request and we have any pages in the range that
|
|
|
|
* are locked, likely due to compression still in progress, we don't want
|
|
|
|
* to block on page locks. We also don't want to block on pages marked as
|
|
|
|
* dirty or under writeback (same as for the non-compression case).
|
|
|
|
* iomap_dio_rw() did the same check, but after that and before we got
|
|
|
|
* here, mmap'ed writes may have happened or buffered reads started
|
|
|
|
* (readpage() and readahead(), which lock pages), as we haven't locked
|
|
|
|
* the file range yet.
|
|
|
|
*/
|
|
|
|
if (test_bit(BTRFS_INODE_HAS_ASYNC_EXTENT,
|
|
|
|
&BTRFS_I(inode)->runtime_flags)) {
|
|
|
|
if (flags & IOMAP_NOWAIT) {
|
|
|
|
if (filemap_range_needs_writeback(inode->i_mapping,
|
|
|
|
lockstart, lockend))
|
|
|
|
return -EAGAIN;
|
|
|
|
} else {
|
|
|
|
ret = filemap_fdatawrite_range(inode->i_mapping, start,
|
|
|
|
start + length - 1);
|
|
|
|
if (ret)
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
memset(dio_data, 0, sizeof(*dio_data));
|
|
|
|
|
|
|
|
/*
|
|
|
|
* We always try to allocate data space and must do it before locking
|
|
|
|
* the file range, to avoid deadlocks with concurrent writes to the same
|
|
|
|
* range if the range has several extents and the writes don't expand the
|
|
|
|
* current i_size (the inode lock is taken in shared mode). If we fail to
|
|
|
|
* allocate data space here we continue and later, after locking the
|
|
|
|
* file range, we fail with ENOSPC only if we figure out we can not do a
|
|
|
|
* NOCOW write.
|
|
|
|
*/
|
|
|
|
if (write && !(flags & IOMAP_NOWAIT)) {
|
|
|
|
ret = btrfs_check_data_free_space(BTRFS_I(inode),
|
|
|
|
&dio_data->data_reserved,
|
|
|
|
start, data_alloc_len, false);
|
|
|
|
if (!ret)
|
|
|
|
dio_data->data_space_reserved = true;
|
|
|
|
else if (ret && !(BTRFS_I(inode)->flags &
|
|
|
|
(BTRFS_INODE_NODATACOW | BTRFS_INODE_PREALLOC)))
|
|
|
|
goto err;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* If this errors out it's because we couldn't invalidate pagecache for
|
|
|
|
* this range and we need to fallback to buffered IO, or we are doing a
|
|
|
|
* NOWAIT read/write and we need to block.
|
|
|
|
*/
|
|
|
|
ret = lock_extent_direct(inode, lockstart, lockend, &cached_state, flags);
|
|
|
|
if (ret < 0)
|
|
|
|
goto err;
|
|
|
|
|
|
|
|
em = btrfs_get_extent(BTRFS_I(inode), NULL, start, len);
|
|
|
|
if (IS_ERR(em)) {
|
|
|
|
ret = PTR_ERR(em);
|
|
|
|
goto unlock_err;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Ok for INLINE and COMPRESSED extents we need to fallback on buffered
|
|
|
|
* io. INLINE is special, and we could probably kludge it in here, but
|
|
|
|
* it's still buffered so for safety lets just fall back to the generic
|
|
|
|
* buffered path.
|
|
|
|
*
|
|
|
|
* For COMPRESSED we _have_ to read the entire extent in so we can
|
|
|
|
* decompress it, so there will be buffering required no matter what we
|
|
|
|
* do, so go ahead and fallback to buffered.
|
|
|
|
*
|
|
|
|
* We return -ENOTBLK because that's what makes DIO go ahead and go back
|
|
|
|
* to buffered IO. Don't blame me, this is the price we pay for using
|
|
|
|
* the generic code.
|
|
|
|
*/
|
|
|
|
if (extent_map_is_compressed(em) || em->disk_bytenr == EXTENT_MAP_INLINE) {
|
|
|
|
free_extent_map(em);
|
|
|
|
/*
|
|
|
|
* If we are in a NOWAIT context, return -EAGAIN in order to
|
|
|
|
* fallback to buffered IO. This is not only because we can
|
|
|
|
* block with buffered IO (no support for NOWAIT semantics at
|
|
|
|
* the moment) but also to avoid returning short reads to user
|
|
|
|
* space - this happens if we were able to read some data from
|
|
|
|
* previous non-compressed extents and then when we fallback to
|
|
|
|
* buffered IO, at btrfs_file_read_iter() by calling
|
|
|
|
* filemap_read(), we fail to fault in pages for the read buffer,
|
|
|
|
* in which case filemap_read() returns a short read (the number
|
|
|
|
* of bytes previously read is > 0, so it does not return -EFAULT).
|
|
|
|
*/
|
|
|
|
ret = (flags & IOMAP_NOWAIT) ? -EAGAIN : -ENOTBLK;
|
|
|
|
goto unlock_err;
|
|
|
|
}
|
|
|
|
|
|
|
|
len = min(len, em->len - (start - em->start));
|
|
|
|
|
|
|
|
/*
|
|
|
|
* If we have a NOWAIT request and the range contains multiple extents
|
|
|
|
* (or a mix of extents and holes), then we return -EAGAIN to make the
|
|
|
|
* caller fallback to a context where it can do a blocking (without
|
|
|
|
* NOWAIT) request. This way we avoid doing partial IO and returning
|
|
|
|
* success to the caller, which is not optimal for writes and for reads
|
|
|
|
* it can result in unexpected behaviour for an application.
|
|
|
|
*
|
|
|
|
* When doing a read, because we use IOMAP_DIO_PARTIAL when calling
|
|
|
|
* iomap_dio_rw(), we can end up returning less data then what the caller
|
|
|
|
* asked for, resulting in an unexpected, and incorrect, short read.
|
|
|
|
* That is, the caller asked to read N bytes and we return less than that,
|
|
|
|
* which is wrong unless we are crossing EOF. This happens if we get a
|
|
|
|
* page fault error when trying to fault in pages for the buffer that is
|
|
|
|
* associated to the struct iov_iter passed to iomap_dio_rw(), and we
|
|
|
|
* have previously submitted bios for other extents in the range, in
|
|
|
|
* which case iomap_dio_rw() may return us EIOCBQUEUED if not all of
|
|
|
|
* those bios have completed by the time we get the page fault error,
|
|
|
|
* which we return back to our caller - we should only return EIOCBQUEUED
|
|
|
|
* after we have submitted bios for all the extents in the range.
|
|
|
|
*/
|
|
|
|
if ((flags & IOMAP_NOWAIT) && len < length) {
|
|
|
|
free_extent_map(em);
|
|
|
|
ret = -EAGAIN;
|
|
|
|
goto unlock_err;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (write) {
|
|
|
|
ret = btrfs_get_blocks_direct_write(&em, inode, dio_data,
|
|
|
|
start, &len, flags);
|
|
|
|
if (ret < 0)
|
|
|
|
goto unlock_err;
|
|
|
|
/* Recalc len in case the new em is smaller than requested */
|
|
|
|
len = min(len, em->len - (start - em->start));
|
|
|
|
if (dio_data->data_space_reserved) {
|
|
|
|
u64 release_offset;
|
|
|
|
u64 release_len = 0;
|
|
|
|
|
|
|
|
if (dio_data->nocow_done) {
|
|
|
|
release_offset = start;
|
|
|
|
release_len = data_alloc_len;
|
|
|
|
} else if (len < data_alloc_len) {
|
|
|
|
release_offset = start + len;
|
|
|
|
release_len = data_alloc_len - len;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (release_len > 0)
|
|
|
|
btrfs_free_reserved_data_space(BTRFS_I(inode),
|
|
|
|
dio_data->data_reserved,
|
|
|
|
release_offset,
|
|
|
|
release_len);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Translate extent map information to iomap.
|
|
|
|
* We trim the extents (and move the addr) even though iomap code does
|
|
|
|
* that, since we have locked only the parts we are performing I/O in.
|
|
|
|
*/
|
|
|
|
if ((em->disk_bytenr == EXTENT_MAP_HOLE) ||
|
|
|
|
((em->flags & EXTENT_FLAG_PREALLOC) && !write)) {
|
|
|
|
iomap->addr = IOMAP_NULL_ADDR;
|
|
|
|
iomap->type = IOMAP_HOLE;
|
|
|
|
} else {
|
|
|
|
iomap->addr = extent_map_block_start(em) + (start - em->start);
|
|
|
|
iomap->type = IOMAP_MAPPED;
|
|
|
|
}
|
|
|
|
iomap->offset = start;
|
|
|
|
iomap->bdev = fs_info->fs_devices->latest_dev->bdev;
|
|
|
|
iomap->length = len;
|
|
|
|
free_extent_map(em);
|
|
|
|
|
btrfs: do not hold the extent lock for entire read
Historically we've held the extent lock throughout the entire read.
There's been a few reasons for this, but it's mostly just caused us
problems. For example, this prevents us from allowing page faults
during direct io reads, because we could deadlock. This has forced us
to only allow 4k reads at a time for io_uring NOWAIT requests because we
have no idea if we'll be forced to page fault and thus have to do a
whole lot of work.
On the buffered side we are protected by the page lock, as long as we're
reading things like buffered writes, punch hole, and even direct IO to a
certain degree will get hung up on the page lock while the page is in
flight.
On the direct side we have the dio extent lock, which acts much like the
way the extent lock worked previously to this patch, however just for
direct reads. This protects direct reads from concurrent direct writes,
while we're protected from buffered writes via the inode lock.
Now that we're protected in all cases, narrow the extent lock to the
part where we're getting the extent map to submit the reads, no longer
holding the extent lock for the entire read operation. Push the extent
lock down into do_readpage() so that we're only grabbing it when looking
up the extent map. This portion was contributed by Goldwyn.
Co-developed-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Reviewed-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-08-16 19:16:24 +00:00
|
|
|
/*
|
|
|
|
* Reads will hold the EXTENT_DIO_LOCKED bit until the io is completed,
|
|
|
|
* writes only hold it for this part. We hold the extent lock until
|
|
|
|
* we're completely done with the extent map to make sure it remains
|
|
|
|
* valid.
|
|
|
|
*/
|
|
|
|
if (write)
|
|
|
|
unlock_bits |= EXTENT_DIO_LOCKED;
|
|
|
|
|
|
|
|
clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart, lockend,
|
|
|
|
unlock_bits, &cached_state);
|
|
|
|
|
|
|
|
/* We didn't use everything, unlock the dio extent for the remainder. */
|
|
|
|
if (!write && (start + len) < lockend)
|
|
|
|
unlock_dio_extent(&BTRFS_I(inode)->io_tree, start + len,
|
|
|
|
lockend, NULL);
|
|
|
|
|
2024-06-25 14:52:28 +00:00
|
|
|
return 0;
|
|
|
|
|
|
|
|
unlock_err:
|
btrfs: take the dio extent lock during O_DIRECT operations
Currently we hold the extent lock for the entire duration of a read.
This isn't really necessary in the buffered case, we're protected by the
page lock, however it's necessary for O_DIRECT.
For O_DIRECT reads, if we only locked the extent for the part where we
get the extent, we could potentially race with an O_DIRECT write in the
same region. This isn't really a problem, unless the read is delayed so
much that the write does the COW, unpins the old extent, and some other
application re-allocates the extent before the read is actually able to
be submitted. At that point at best we'd have a checksum mismatch, but
at worse we could read data that doesn't belong to us.
To address this potential race we need to make sure we don't have
overlapping, concurrent direct io reads and writes.
To accomplish this use the new EXTENT_DIO_LOCKED bit in the direct IO
case in the same spot as the current extent lock. The writes will take
this while they're creating the ordered extent, which is also used to
make sure concurrent buffered reads or concurrent direct reads are not
allowed to occur, and drop it after the ordered extent is taken. For
reads it will act as the current read behavior for the EXTENT_LOCKED
bit, we set it when we're starting the read, we clear it in the end_io
to allow other direct writes to continue.
This still has the drawback of disallowing concurrent overlapping direct
reads from occurring, but that exists with the current extent locking.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-08-19 20:50:01 +00:00
|
|
|
/*
|
|
|
|
* Don't use EXTENT_LOCK_BITS here in case we extend it later and forget
|
|
|
|
* to update this, be explicit that we expect EXTENT_LOCKED and
|
|
|
|
* EXTENT_DIO_LOCKED to be set here, and so that's what we're clearing.
|
|
|
|
*/
|
|
|
|
clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart, lockend,
|
|
|
|
EXTENT_LOCKED | EXTENT_DIO_LOCKED, &cached_state);
|
2024-06-25 14:52:28 +00:00
|
|
|
err:
|
|
|
|
if (dio_data->data_space_reserved) {
|
|
|
|
btrfs_free_reserved_data_space(BTRFS_I(inode),
|
|
|
|
dio_data->data_reserved,
|
|
|
|
start, data_alloc_len);
|
|
|
|
extent_changeset_free(dio_data->data_reserved);
|
|
|
|
}
|
|
|
|
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
|
|
|
static int btrfs_dio_iomap_end(struct inode *inode, loff_t pos, loff_t length,
|
|
|
|
ssize_t written, unsigned int flags, struct iomap *iomap)
|
|
|
|
{
|
|
|
|
struct iomap_iter *iter = container_of(iomap, struct iomap_iter, iomap);
|
|
|
|
struct btrfs_dio_data *dio_data = iter->private;
|
|
|
|
size_t submitted = dio_data->submitted;
|
|
|
|
const bool write = !!(flags & IOMAP_WRITE);
|
|
|
|
int ret = 0;
|
|
|
|
|
|
|
|
if (!write && (iomap->type == IOMAP_HOLE)) {
|
|
|
|
/* If reading from a hole, unlock and return */
|
btrfs: do not hold the extent lock for entire read
Historically we've held the extent lock throughout the entire read.
There's been a few reasons for this, but it's mostly just caused us
problems. For example, this prevents us from allowing page faults
during direct io reads, because we could deadlock. This has forced us
to only allow 4k reads at a time for io_uring NOWAIT requests because we
have no idea if we'll be forced to page fault and thus have to do a
whole lot of work.
On the buffered side we are protected by the page lock, as long as we're
reading things like buffered writes, punch hole, and even direct IO to a
certain degree will get hung up on the page lock while the page is in
flight.
On the direct side we have the dio extent lock, which acts much like the
way the extent lock worked previously to this patch, however just for
direct reads. This protects direct reads from concurrent direct writes,
while we're protected from buffered writes via the inode lock.
Now that we're protected in all cases, narrow the extent lock to the
part where we're getting the extent map to submit the reads, no longer
holding the extent lock for the entire read operation. Push the extent
lock down into do_readpage() so that we're only grabbing it when looking
up the extent map. This portion was contributed by Goldwyn.
Co-developed-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Reviewed-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-08-16 19:16:24 +00:00
|
|
|
unlock_dio_extent(&BTRFS_I(inode)->io_tree, pos,
|
|
|
|
pos + length - 1, NULL);
|
2024-06-25 14:52:28 +00:00
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (submitted < length) {
|
|
|
|
pos += submitted;
|
|
|
|
length -= submitted;
|
|
|
|
if (write)
|
|
|
|
btrfs_finish_ordered_extent(dio_data->ordered, NULL,
|
|
|
|
pos, length, false);
|
|
|
|
else
|
btrfs: do not hold the extent lock for entire read
Historically we've held the extent lock throughout the entire read.
There's been a few reasons for this, but it's mostly just caused us
problems. For example, this prevents us from allowing page faults
during direct io reads, because we could deadlock. This has forced us
to only allow 4k reads at a time for io_uring NOWAIT requests because we
have no idea if we'll be forced to page fault and thus have to do a
whole lot of work.
On the buffered side we are protected by the page lock, as long as we're
reading things like buffered writes, punch hole, and even direct IO to a
certain degree will get hung up on the page lock while the page is in
flight.
On the direct side we have the dio extent lock, which acts much like the
way the extent lock worked previously to this patch, however just for
direct reads. This protects direct reads from concurrent direct writes,
while we're protected from buffered writes via the inode lock.
Now that we're protected in all cases, narrow the extent lock to the
part where we're getting the extent map to submit the reads, no longer
holding the extent lock for the entire read operation. Push the extent
lock down into do_readpage() so that we're only grabbing it when looking
up the extent map. This portion was contributed by Goldwyn.
Co-developed-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Reviewed-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-08-16 19:16:24 +00:00
|
|
|
unlock_dio_extent(&BTRFS_I(inode)->io_tree, pos,
|
|
|
|
pos + length - 1, NULL);
|
2024-06-25 14:52:28 +00:00
|
|
|
ret = -ENOTBLK;
|
|
|
|
}
|
|
|
|
if (write) {
|
|
|
|
btrfs_put_ordered_extent(dio_data->ordered);
|
|
|
|
dio_data->ordered = NULL;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (write)
|
|
|
|
extent_changeset_free(dio_data->data_reserved);
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
|
|
|
static void btrfs_dio_end_io(struct btrfs_bio *bbio)
|
|
|
|
{
|
|
|
|
struct btrfs_dio_private *dip =
|
|
|
|
container_of(bbio, struct btrfs_dio_private, bbio);
|
|
|
|
struct btrfs_inode *inode = bbio->inode;
|
|
|
|
struct bio *bio = &bbio->bio;
|
|
|
|
|
|
|
|
if (bio->bi_status) {
|
|
|
|
btrfs_warn(inode->root->fs_info,
|
|
|
|
"direct IO failed ino %llu op 0x%0x offset %#llx len %u err no %d",
|
|
|
|
btrfs_ino(inode), bio->bi_opf,
|
|
|
|
dip->file_offset, dip->bytes, bio->bi_status);
|
|
|
|
}
|
|
|
|
|
|
|
|
if (btrfs_op(bio) == BTRFS_MAP_WRITE) {
|
|
|
|
btrfs_finish_ordered_extent(bbio->ordered, NULL,
|
|
|
|
dip->file_offset, dip->bytes,
|
|
|
|
!bio->bi_status);
|
|
|
|
} else {
|
btrfs: do not hold the extent lock for entire read
Historically we've held the extent lock throughout the entire read.
There's been a few reasons for this, but it's mostly just caused us
problems. For example, this prevents us from allowing page faults
during direct io reads, because we could deadlock. This has forced us
to only allow 4k reads at a time for io_uring NOWAIT requests because we
have no idea if we'll be forced to page fault and thus have to do a
whole lot of work.
On the buffered side we are protected by the page lock, as long as we're
reading things like buffered writes, punch hole, and even direct IO to a
certain degree will get hung up on the page lock while the page is in
flight.
On the direct side we have the dio extent lock, which acts much like the
way the extent lock worked previously to this patch, however just for
direct reads. This protects direct reads from concurrent direct writes,
while we're protected from buffered writes via the inode lock.
Now that we're protected in all cases, narrow the extent lock to the
part where we're getting the extent map to submit the reads, no longer
holding the extent lock for the entire read operation. Push the extent
lock down into do_readpage() so that we're only grabbing it when looking
up the extent map. This portion was contributed by Goldwyn.
Co-developed-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Reviewed-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-08-16 19:16:24 +00:00
|
|
|
unlock_dio_extent(&inode->io_tree, dip->file_offset,
|
|
|
|
dip->file_offset + dip->bytes - 1, NULL);
|
2024-06-25 14:52:28 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
bbio->bio.bi_private = bbio->private;
|
|
|
|
iomap_dio_bio_end_io(bio);
|
|
|
|
}
|
|
|
|
|
|
|
|
static int btrfs_extract_ordered_extent(struct btrfs_bio *bbio,
|
|
|
|
struct btrfs_ordered_extent *ordered)
|
|
|
|
{
|
|
|
|
u64 start = (u64)bbio->bio.bi_iter.bi_sector << SECTOR_SHIFT;
|
|
|
|
u64 len = bbio->bio.bi_iter.bi_size;
|
|
|
|
struct btrfs_ordered_extent *new;
|
|
|
|
int ret;
|
|
|
|
|
|
|
|
/* Must always be called for the beginning of an ordered extent. */
|
|
|
|
if (WARN_ON_ONCE(start != ordered->disk_bytenr))
|
|
|
|
return -EINVAL;
|
|
|
|
|
|
|
|
/* No need to split if the ordered extent covers the entire bio. */
|
|
|
|
if (ordered->disk_num_bytes == len) {
|
|
|
|
refcount_inc(&ordered->refs);
|
|
|
|
bbio->ordered = ordered;
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Don't split the extent_map for NOCOW extents, as we're writing into
|
|
|
|
* a pre-existing one.
|
|
|
|
*/
|
|
|
|
if (!test_bit(BTRFS_ORDERED_NOCOW, &ordered->flags)) {
|
|
|
|
ret = split_extent_map(bbio->inode, bbio->file_offset,
|
|
|
|
ordered->num_bytes, len,
|
|
|
|
ordered->disk_bytenr);
|
|
|
|
if (ret)
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
|
|
|
new = btrfs_split_ordered_extent(ordered, len);
|
|
|
|
if (IS_ERR(new))
|
|
|
|
return PTR_ERR(new);
|
|
|
|
bbio->ordered = new;
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
static void btrfs_dio_submit_io(const struct iomap_iter *iter, struct bio *bio,
|
|
|
|
loff_t file_offset)
|
|
|
|
{
|
|
|
|
struct btrfs_bio *bbio = btrfs_bio(bio);
|
|
|
|
struct btrfs_dio_private *dip =
|
|
|
|
container_of(bbio, struct btrfs_dio_private, bbio);
|
|
|
|
struct btrfs_dio_data *dio_data = iter->private;
|
|
|
|
|
|
|
|
btrfs_bio_init(bbio, BTRFS_I(iter->inode)->root->fs_info,
|
|
|
|
btrfs_dio_end_io, bio->bi_private);
|
|
|
|
bbio->inode = BTRFS_I(iter->inode);
|
|
|
|
bbio->file_offset = file_offset;
|
|
|
|
|
|
|
|
dip->file_offset = file_offset;
|
|
|
|
dip->bytes = bio->bi_iter.bi_size;
|
|
|
|
|
|
|
|
dio_data->submitted += bio->bi_iter.bi_size;
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Check if we are doing a partial write. If we are, we need to split
|
|
|
|
* the ordered extent to match the submitted bio. Hang on to the
|
|
|
|
* remaining unfinishable ordered_extent in dio_data so that it can be
|
|
|
|
* cancelled in iomap_end to avoid a deadlock wherein faulting the
|
|
|
|
* remaining pages is blocked on the outstanding ordered extent.
|
|
|
|
*/
|
|
|
|
if (iter->flags & IOMAP_WRITE) {
|
|
|
|
int ret;
|
|
|
|
|
|
|
|
ret = btrfs_extract_ordered_extent(bbio, dio_data->ordered);
|
|
|
|
if (ret) {
|
|
|
|
btrfs_finish_ordered_extent(dio_data->ordered, NULL,
|
|
|
|
file_offset, dip->bytes,
|
|
|
|
!ret);
|
|
|
|
bio->bi_status = errno_to_blk_status(ret);
|
|
|
|
iomap_dio_bio_end_io(bio);
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2024-08-27 01:40:11 +00:00
|
|
|
btrfs_submit_bbio(bbio, 0);
|
2024-06-25 14:52:28 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
static const struct iomap_ops btrfs_dio_iomap_ops = {
|
|
|
|
.iomap_begin = btrfs_dio_iomap_begin,
|
|
|
|
.iomap_end = btrfs_dio_iomap_end,
|
|
|
|
};
|
|
|
|
|
|
|
|
static const struct iomap_dio_ops btrfs_dio_ops = {
|
|
|
|
.submit_io = btrfs_dio_submit_io,
|
|
|
|
.bio_set = &btrfs_dio_bioset,
|
|
|
|
};
|
|
|
|
|
|
|
|
static ssize_t btrfs_dio_read(struct kiocb *iocb, struct iov_iter *iter,
|
|
|
|
size_t done_before)
|
|
|
|
{
|
|
|
|
struct btrfs_dio_data data = { 0 };
|
|
|
|
|
|
|
|
return iomap_dio_rw(iocb, iter, &btrfs_dio_iomap_ops, &btrfs_dio_ops,
|
|
|
|
IOMAP_DIO_PARTIAL, &data, done_before);
|
|
|
|
}
|
|
|
|
|
|
|
|
static struct iomap_dio *btrfs_dio_write(struct kiocb *iocb, struct iov_iter *iter,
|
|
|
|
size_t done_before)
|
|
|
|
{
|
|
|
|
struct btrfs_dio_data data = { 0 };
|
|
|
|
|
|
|
|
return __iomap_dio_rw(iocb, iter, &btrfs_dio_iomap_ops, &btrfs_dio_ops,
|
|
|
|
IOMAP_DIO_PARTIAL, &data, done_before);
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t check_direct_IO(struct btrfs_fs_info *fs_info,
|
|
|
|
const struct iov_iter *iter, loff_t offset)
|
|
|
|
{
|
|
|
|
const u32 blocksize_mask = fs_info->sectorsize - 1;
|
|
|
|
|
|
|
|
if (offset & blocksize_mask)
|
|
|
|
return -EINVAL;
|
|
|
|
|
|
|
|
if (iov_iter_alignment(iter) & blocksize_mask)
|
|
|
|
return -EINVAL;
|
|
|
|
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
|
|
|
|
{
|
|
|
|
struct file *file = iocb->ki_filp;
|
|
|
|
struct inode *inode = file_inode(file);
|
|
|
|
struct btrfs_fs_info *fs_info = inode_to_fs_info(inode);
|
|
|
|
loff_t pos;
|
|
|
|
ssize_t written = 0;
|
|
|
|
ssize_t written_buffered;
|
|
|
|
size_t prev_left = 0;
|
|
|
|
loff_t endbyte;
|
|
|
|
ssize_t ret;
|
|
|
|
unsigned int ilock_flags = 0;
|
|
|
|
struct iomap_dio *dio;
|
|
|
|
|
|
|
|
if (iocb->ki_flags & IOCB_NOWAIT)
|
|
|
|
ilock_flags |= BTRFS_ILOCK_TRY;
|
|
|
|
|
|
|
|
/*
|
|
|
|
* If the write DIO is within EOF, use a shared lock and also only if
|
|
|
|
* security bits will likely not be dropped by file_remove_privs() called
|
|
|
|
* from btrfs_write_check(). Either will need to be rechecked after the
|
|
|
|
* lock was acquired.
|
|
|
|
*/
|
|
|
|
if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode) && IS_NOSEC(inode))
|
|
|
|
ilock_flags |= BTRFS_ILOCK_SHARED;
|
|
|
|
|
|
|
|
relock:
|
|
|
|
ret = btrfs_inode_lock(BTRFS_I(inode), ilock_flags);
|
|
|
|
if (ret < 0)
|
|
|
|
return ret;
|
|
|
|
|
|
|
|
/* Shared lock cannot be used with security bits set. */
|
|
|
|
if ((ilock_flags & BTRFS_ILOCK_SHARED) && !IS_NOSEC(inode)) {
|
|
|
|
btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
|
|
|
|
ilock_flags &= ~BTRFS_ILOCK_SHARED;
|
|
|
|
goto relock;
|
|
|
|
}
|
|
|
|
|
|
|
|
ret = generic_write_checks(iocb, from);
|
|
|
|
if (ret <= 0) {
|
|
|
|
btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
|
|
|
ret = btrfs_write_check(iocb, from, ret);
|
|
|
|
if (ret < 0) {
|
|
|
|
btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
|
|
|
pos = iocb->ki_pos;
|
|
|
|
/*
|
|
|
|
* Re-check since file size may have changed just before taking the
|
|
|
|
* lock or pos may have changed because of O_APPEND in generic_write_check()
|
|
|
|
*/
|
|
|
|
if ((ilock_flags & BTRFS_ILOCK_SHARED) &&
|
|
|
|
pos + iov_iter_count(from) > i_size_read(inode)) {
|
|
|
|
btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
|
|
|
|
ilock_flags &= ~BTRFS_ILOCK_SHARED;
|
|
|
|
goto relock;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (check_direct_IO(fs_info, from, pos)) {
|
|
|
|
btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
|
|
|
|
goto buffered;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* The iov_iter can be mapped to the same file range we are writing to.
|
|
|
|
* If that's the case, then we will deadlock in the iomap code, because
|
|
|
|
* it first calls our callback btrfs_dio_iomap_begin(), which will create
|
|
|
|
* an ordered extent, and after that it will fault in the pages that the
|
|
|
|
* iov_iter refers to. During the fault in we end up in the readahead
|
|
|
|
* pages code (starting at btrfs_readahead()), which will lock the range,
|
|
|
|
* find that ordered extent and then wait for it to complete (at
|
|
|
|
* btrfs_lock_and_flush_ordered_range()), resulting in a deadlock since
|
|
|
|
* obviously the ordered extent can never complete as we didn't submit
|
|
|
|
* yet the respective bio(s). This always happens when the buffer is
|
|
|
|
* memory mapped to the same file range, since the iomap DIO code always
|
|
|
|
* invalidates pages in the target file range (after starting and waiting
|
|
|
|
* for any writeback).
|
|
|
|
*
|
|
|
|
* So here we disable page faults in the iov_iter and then retry if we
|
|
|
|
* got -EFAULT, faulting in the pages before the retry.
|
|
|
|
*/
|
btrfs: fix corruption after buffer fault in during direct IO append write
During an append (O_APPEND write flag) direct IO write if the input buffer
was not previously faulted in, we can corrupt the file in a way that the
final size is unexpected and it includes an unexpected hole.
The problem happens like this:
1) We have an empty file, with size 0, for example;
2) We do an O_APPEND direct IO with a length of 4096 bytes and the input
buffer is not currently faulted in;
3) We enter btrfs_direct_write(), lock the inode and call
generic_write_checks(), which calls generic_write_checks_count(), and
that function sets the iocb position to 0 with the following code:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
4) We call btrfs_dio_write() and enter into iomap, which will end up
calling btrfs_dio_iomap_begin() and that calls
btrfs_get_blocks_direct_write(), where we update the i_size of the
inode to 4096 bytes;
5) After btrfs_dio_iomap_begin() returns, iomap will attempt to access
the page of the write input buffer (at iomap_dio_bio_iter(), with a
call to bio_iov_iter_get_pages()) and fail with -EFAULT, which gets
returned to btrfs at btrfs_direct_write() via btrfs_dio_write();
6) At btrfs_direct_write() we get the -EFAULT error, unlock the inode,
fault in the write buffer and then goto to the label 'relock';
7) We lock again the inode, do all the necessary checks again and call
again generic_write_checks(), which calls generic_write_checks_count()
again, and there we set the iocb's position to 4K, which is the current
i_size of the inode, with the following code pointed above:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
8) Then we go again to btrfs_dio_write() and enter iomap and the write
succeeds, but it wrote to the file range [4K, 8K), leaving a hole in
the [0, 4K) range and an i_size of 8K, which goes against the
expectations of having the data written to the range [0, 4K) and get an
i_size of 4K.
Fix this by not unlocking the inode before faulting in the input buffer,
in case we get -EFAULT or an incomplete write, and not jumping to the
'relock' label after faulting in the buffer - instead jump to a location
immediately before calling iomap, skipping all the write checks and
relocking. This solves this problem and it's fine even in case the input
buffer is memory mapped to the same file range, since only holding the
range locked in the inode's io tree can cause a deadlock, it's safe to
keep the inode lock (VFS lock), as was fixed and described in commit
51bd9563b678 ("btrfs: fix deadlock due to page faults during direct IO
reads and writes").
A sample reproducer provided by a reporter is the following:
$ cat test.c
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <fcntl.h>
#include <stdio.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <unistd.h>
int main(int argc, char *argv[])
{
if (argc < 2) {
fprintf(stderr, "Usage: %s <test file>\n", argv[0]);
return 1;
}
int fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC | O_DIRECT |
O_APPEND, 0644);
if (fd < 0) {
perror("creating test file");
return 1;
}
char *buf = mmap(NULL, 4096, PROT_READ,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ssize_t ret = write(fd, buf, 4096);
if (ret < 0) {
perror("pwritev2");
return 1;
}
struct stat stbuf;
ret = fstat(fd, &stbuf);
if (ret < 0) {
perror("stat");
return 1;
}
printf("size: %llu\n", (unsigned long long)stbuf.st_size);
return stbuf.st_size == 4096 ? 0 : 1;
}
A test case for fstests will be sent soon.
Reported-by: Hanna Czenczek <hreitz@redhat.com>
Link: https://lore.kernel.org/linux-btrfs/0b841d46-12fe-4e64-9abb-871d8d0de271@redhat.com/
Fixes: 8184620ae212 ("btrfs: fix lost file sync on direct IO write with nowait and dsync iocb")
CC: stable@vger.kernel.org # 6.1+
Tested-by: Hanna Czenczek <hreitz@redhat.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-07-26 10:12:52 +00:00
|
|
|
again:
|
2024-06-25 14:52:28 +00:00
|
|
|
from->nofault = true;
|
|
|
|
dio = btrfs_dio_write(iocb, from, written);
|
|
|
|
from->nofault = false;
|
|
|
|
|
btrfs: fix corruption after buffer fault in during direct IO append write
During an append (O_APPEND write flag) direct IO write if the input buffer
was not previously faulted in, we can corrupt the file in a way that the
final size is unexpected and it includes an unexpected hole.
The problem happens like this:
1) We have an empty file, with size 0, for example;
2) We do an O_APPEND direct IO with a length of 4096 bytes and the input
buffer is not currently faulted in;
3) We enter btrfs_direct_write(), lock the inode and call
generic_write_checks(), which calls generic_write_checks_count(), and
that function sets the iocb position to 0 with the following code:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
4) We call btrfs_dio_write() and enter into iomap, which will end up
calling btrfs_dio_iomap_begin() and that calls
btrfs_get_blocks_direct_write(), where we update the i_size of the
inode to 4096 bytes;
5) After btrfs_dio_iomap_begin() returns, iomap will attempt to access
the page of the write input buffer (at iomap_dio_bio_iter(), with a
call to bio_iov_iter_get_pages()) and fail with -EFAULT, which gets
returned to btrfs at btrfs_direct_write() via btrfs_dio_write();
6) At btrfs_direct_write() we get the -EFAULT error, unlock the inode,
fault in the write buffer and then goto to the label 'relock';
7) We lock again the inode, do all the necessary checks again and call
again generic_write_checks(), which calls generic_write_checks_count()
again, and there we set the iocb's position to 4K, which is the current
i_size of the inode, with the following code pointed above:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
8) Then we go again to btrfs_dio_write() and enter iomap and the write
succeeds, but it wrote to the file range [4K, 8K), leaving a hole in
the [0, 4K) range and an i_size of 8K, which goes against the
expectations of having the data written to the range [0, 4K) and get an
i_size of 4K.
Fix this by not unlocking the inode before faulting in the input buffer,
in case we get -EFAULT or an incomplete write, and not jumping to the
'relock' label after faulting in the buffer - instead jump to a location
immediately before calling iomap, skipping all the write checks and
relocking. This solves this problem and it's fine even in case the input
buffer is memory mapped to the same file range, since only holding the
range locked in the inode's io tree can cause a deadlock, it's safe to
keep the inode lock (VFS lock), as was fixed and described in commit
51bd9563b678 ("btrfs: fix deadlock due to page faults during direct IO
reads and writes").
A sample reproducer provided by a reporter is the following:
$ cat test.c
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <fcntl.h>
#include <stdio.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <unistd.h>
int main(int argc, char *argv[])
{
if (argc < 2) {
fprintf(stderr, "Usage: %s <test file>\n", argv[0]);
return 1;
}
int fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC | O_DIRECT |
O_APPEND, 0644);
if (fd < 0) {
perror("creating test file");
return 1;
}
char *buf = mmap(NULL, 4096, PROT_READ,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ssize_t ret = write(fd, buf, 4096);
if (ret < 0) {
perror("pwritev2");
return 1;
}
struct stat stbuf;
ret = fstat(fd, &stbuf);
if (ret < 0) {
perror("stat");
return 1;
}
printf("size: %llu\n", (unsigned long long)stbuf.st_size);
return stbuf.st_size == 4096 ? 0 : 1;
}
A test case for fstests will be sent soon.
Reported-by: Hanna Czenczek <hreitz@redhat.com>
Link: https://lore.kernel.org/linux-btrfs/0b841d46-12fe-4e64-9abb-871d8d0de271@redhat.com/
Fixes: 8184620ae212 ("btrfs: fix lost file sync on direct IO write with nowait and dsync iocb")
CC: stable@vger.kernel.org # 6.1+
Tested-by: Hanna Czenczek <hreitz@redhat.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-07-26 10:12:52 +00:00
|
|
|
if (IS_ERR_OR_NULL(dio)) {
|
2024-06-25 14:52:28 +00:00
|
|
|
ret = PTR_ERR_OR_ZERO(dio);
|
btrfs: fix corruption after buffer fault in during direct IO append write
During an append (O_APPEND write flag) direct IO write if the input buffer
was not previously faulted in, we can corrupt the file in a way that the
final size is unexpected and it includes an unexpected hole.
The problem happens like this:
1) We have an empty file, with size 0, for example;
2) We do an O_APPEND direct IO with a length of 4096 bytes and the input
buffer is not currently faulted in;
3) We enter btrfs_direct_write(), lock the inode and call
generic_write_checks(), which calls generic_write_checks_count(), and
that function sets the iocb position to 0 with the following code:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
4) We call btrfs_dio_write() and enter into iomap, which will end up
calling btrfs_dio_iomap_begin() and that calls
btrfs_get_blocks_direct_write(), where we update the i_size of the
inode to 4096 bytes;
5) After btrfs_dio_iomap_begin() returns, iomap will attempt to access
the page of the write input buffer (at iomap_dio_bio_iter(), with a
call to bio_iov_iter_get_pages()) and fail with -EFAULT, which gets
returned to btrfs at btrfs_direct_write() via btrfs_dio_write();
6) At btrfs_direct_write() we get the -EFAULT error, unlock the inode,
fault in the write buffer and then goto to the label 'relock';
7) We lock again the inode, do all the necessary checks again and call
again generic_write_checks(), which calls generic_write_checks_count()
again, and there we set the iocb's position to 4K, which is the current
i_size of the inode, with the following code pointed above:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
8) Then we go again to btrfs_dio_write() and enter iomap and the write
succeeds, but it wrote to the file range [4K, 8K), leaving a hole in
the [0, 4K) range and an i_size of 8K, which goes against the
expectations of having the data written to the range [0, 4K) and get an
i_size of 4K.
Fix this by not unlocking the inode before faulting in the input buffer,
in case we get -EFAULT or an incomplete write, and not jumping to the
'relock' label after faulting in the buffer - instead jump to a location
immediately before calling iomap, skipping all the write checks and
relocking. This solves this problem and it's fine even in case the input
buffer is memory mapped to the same file range, since only holding the
range locked in the inode's io tree can cause a deadlock, it's safe to
keep the inode lock (VFS lock), as was fixed and described in commit
51bd9563b678 ("btrfs: fix deadlock due to page faults during direct IO
reads and writes").
A sample reproducer provided by a reporter is the following:
$ cat test.c
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <fcntl.h>
#include <stdio.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <unistd.h>
int main(int argc, char *argv[])
{
if (argc < 2) {
fprintf(stderr, "Usage: %s <test file>\n", argv[0]);
return 1;
}
int fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC | O_DIRECT |
O_APPEND, 0644);
if (fd < 0) {
perror("creating test file");
return 1;
}
char *buf = mmap(NULL, 4096, PROT_READ,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ssize_t ret = write(fd, buf, 4096);
if (ret < 0) {
perror("pwritev2");
return 1;
}
struct stat stbuf;
ret = fstat(fd, &stbuf);
if (ret < 0) {
perror("stat");
return 1;
}
printf("size: %llu\n", (unsigned long long)stbuf.st_size);
return stbuf.st_size == 4096 ? 0 : 1;
}
A test case for fstests will be sent soon.
Reported-by: Hanna Czenczek <hreitz@redhat.com>
Link: https://lore.kernel.org/linux-btrfs/0b841d46-12fe-4e64-9abb-871d8d0de271@redhat.com/
Fixes: 8184620ae212 ("btrfs: fix lost file sync on direct IO write with nowait and dsync iocb")
CC: stable@vger.kernel.org # 6.1+
Tested-by: Hanna Czenczek <hreitz@redhat.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-07-26 10:12:52 +00:00
|
|
|
} else {
|
|
|
|
/*
|
|
|
|
* If we have a synchronous write, we must make sure the fsync
|
|
|
|
* triggered by the iomap_dio_complete() call below doesn't
|
|
|
|
* deadlock on the inode lock - we are already holding it and we
|
|
|
|
* can't call it after unlocking because we may need to complete
|
|
|
|
* partial writes due to the input buffer (or parts of it) not
|
|
|
|
* being already faulted in.
|
|
|
|
*/
|
btrfs: fix race between direct IO write and fsync when using same fd
If we have 2 threads that are using the same file descriptor and one of
them is doing direct IO writes while the other is doing fsync, we have a
race where we can end up either:
1) Attempt a fsync without holding the inode's lock, triggering an
assertion failures when assertions are enabled;
2) Do an invalid memory access from the fsync task because the file private
points to memory allocated on stack by the direct IO task and it may be
used by the fsync task after the stack was destroyed.
The race happens like this:
1) A user space program opens a file descriptor with O_DIRECT;
2) The program spawns 2 threads using libpthread for example;
3) One of the threads uses the file descriptor to do direct IO writes,
while the other calls fsync using the same file descriptor.
4) Call task A the thread doing direct IO writes and task B the thread
doing fsyncs;
5) Task A does a direct IO write, and at btrfs_direct_write() sets the
file's private to an on stack allocated private with the member
'fsync_skip_inode_lock' set to true;
6) Task B enters btrfs_sync_file() and sees that there's a private
structure associated to the file which has 'fsync_skip_inode_lock' set
to true, so it skips locking the inode's VFS lock;
7) Task A completes the direct IO write, and resets the file's private to
NULL since it had no prior private and our private was stack allocated.
Then it unlocks the inode's VFS lock;
8) Task B enters btrfs_get_ordered_extents_for_logging(), then the
assertion that checks the inode's VFS lock is held fails, since task B
never locked it and task A has already unlocked it.
The stack trace produced is the following:
assertion failed: inode_is_locked(&inode->vfs_inode), in fs/btrfs/ordered-data.c:983
------------[ cut here ]------------
kernel BUG at fs/btrfs/ordered-data.c:983!
Oops: invalid opcode: 0000 [#1] PREEMPT SMP PTI
CPU: 9 PID: 5072 Comm: worker Tainted: G U OE 6.10.5-1-default #1 openSUSE Tumbleweed 69f48d427608e1c09e60ea24c6c55e2ca1b049e8
Hardware name: Acer Predator PH315-52/Covini_CFS, BIOS V1.12 07/28/2020
RIP: 0010:btrfs_get_ordered_extents_for_logging.cold+0x1f/0x42 [btrfs]
Code: 50 d6 86 c0 e8 (...)
RSP: 0018:ffff9e4a03dcfc78 EFLAGS: 00010246
RAX: 0000000000000054 RBX: ffff9078a9868e98 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff907dce4a7800 RDI: ffff907dce4a7800
RBP: ffff907805518800 R08: 0000000000000000 R09: ffff9e4a03dcfb38
R10: ffff9e4a03dcfb30 R11: 0000000000000003 R12: ffff907684ae7800
R13: 0000000000000001 R14: ffff90774646b600 R15: 0000000000000000
FS: 00007f04b96006c0(0000) GS:ffff907dce480000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f32acbfc000 CR3: 00000001fd4fa005 CR4: 00000000003726f0
Call Trace:
<TASK>
? __die_body.cold+0x14/0x24
? die+0x2e/0x50
? do_trap+0xca/0x110
? do_error_trap+0x6a/0x90
? btrfs_get_ordered_extents_for_logging.cold+0x1f/0x42 [btrfs bb26272d49b4cdc847cf3f7faadd459b62caee9a]
? exc_invalid_op+0x50/0x70
? btrfs_get_ordered_extents_for_logging.cold+0x1f/0x42 [btrfs bb26272d49b4cdc847cf3f7faadd459b62caee9a]
? asm_exc_invalid_op+0x1a/0x20
? btrfs_get_ordered_extents_for_logging.cold+0x1f/0x42 [btrfs bb26272d49b4cdc847cf3f7faadd459b62caee9a]
? btrfs_get_ordered_extents_for_logging.cold+0x1f/0x42 [btrfs bb26272d49b4cdc847cf3f7faadd459b62caee9a]
btrfs_sync_file+0x21a/0x4d0 [btrfs bb26272d49b4cdc847cf3f7faadd459b62caee9a]
? __seccomp_filter+0x31d/0x4f0
__x64_sys_fdatasync+0x4f/0x90
do_syscall_64+0x82/0x160
? do_futex+0xcb/0x190
? __x64_sys_futex+0x10e/0x1d0
? switch_fpu_return+0x4f/0xd0
? syscall_exit_to_user_mode+0x72/0x220
? do_syscall_64+0x8e/0x160
? syscall_exit_to_user_mode+0x72/0x220
? do_syscall_64+0x8e/0x160
? syscall_exit_to_user_mode+0x72/0x220
? do_syscall_64+0x8e/0x160
? syscall_exit_to_user_mode+0x72/0x220
? do_syscall_64+0x8e/0x160
entry_SYSCALL_64_after_hwframe+0x76/0x7e
Another problem here is if task B grabs the private pointer and then uses
it after task A has finished, since the private was allocated in the stack
of task A, it results in some invalid memory access with a hard to predict
result.
This issue, triggering the assertion, was observed with QEMU workloads by
two users in the Link tags below.
Fix this by not relying on a file's private to pass information to fsync
that it should skip locking the inode and instead pass this information
through a special value stored in current->journal_info. This is safe
because in the relevant section of the direct IO write path we are not
holding a transaction handle, so current->journal_info is NULL.
The following C program triggers the issue:
$ cat repro.c
/* Get the O_DIRECT definition. */
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <stdint.h>
#include <fcntl.h>
#include <errno.h>
#include <string.h>
#include <pthread.h>
static int fd;
static ssize_t do_write(int fd, const void *buf, size_t count, off_t offset)
{
while (count > 0) {
ssize_t ret;
ret = pwrite(fd, buf, count, offset);
if (ret < 0) {
if (errno == EINTR)
continue;
return ret;
}
count -= ret;
buf += ret;
}
return 0;
}
static void *fsync_loop(void *arg)
{
while (1) {
int ret;
ret = fsync(fd);
if (ret != 0) {
perror("Fsync failed");
exit(6);
}
}
}
int main(int argc, char *argv[])
{
long pagesize;
void *write_buf;
pthread_t fsyncer;
int ret;
if (argc != 2) {
fprintf(stderr, "Use: %s <file path>\n", argv[0]);
return 1;
}
fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC | O_DIRECT, 0666);
if (fd == -1) {
perror("Failed to open/create file");
return 1;
}
pagesize = sysconf(_SC_PAGE_SIZE);
if (pagesize == -1) {
perror("Failed to get page size");
return 2;
}
ret = posix_memalign(&write_buf, pagesize, pagesize);
if (ret) {
perror("Failed to allocate buffer");
return 3;
}
ret = pthread_create(&fsyncer, NULL, fsync_loop, NULL);
if (ret != 0) {
fprintf(stderr, "Failed to create writer thread: %d\n", ret);
return 4;
}
while (1) {
ret = do_write(fd, write_buf, pagesize, 0);
if (ret != 0) {
perror("Write failed");
exit(5);
}
}
return 0;
}
$ mkfs.btrfs -f /dev/sdi
$ mount /dev/sdi /mnt/sdi
$ timeout 10 ./repro /mnt/sdi/foo
Usually the race is triggered within less than 1 second. A test case for
fstests will follow soon.
Reported-by: Paulo Dias <paulo.miguel.dias@gmail.com>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=219187
Reported-by: Andreas Jahn <jahn-andi@web.de>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=219199
Reported-by: syzbot+4704b3cc972bd76024f1@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-btrfs/00000000000044ff540620d7dee2@google.com/
Fixes: 939b656bc8ab ("btrfs: fix corruption after buffer fault in during direct IO append write")
CC: stable@vger.kernel.org # 5.15+
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-08-29 17:25:49 +00:00
|
|
|
ASSERT(current->journal_info == NULL);
|
|
|
|
current->journal_info = BTRFS_TRANS_DIO_WRITE_STUB;
|
2024-06-25 14:52:28 +00:00
|
|
|
ret = iomap_dio_complete(dio);
|
btrfs: fix race between direct IO write and fsync when using same fd
If we have 2 threads that are using the same file descriptor and one of
them is doing direct IO writes while the other is doing fsync, we have a
race where we can end up either:
1) Attempt a fsync without holding the inode's lock, triggering an
assertion failures when assertions are enabled;
2) Do an invalid memory access from the fsync task because the file private
points to memory allocated on stack by the direct IO task and it may be
used by the fsync task after the stack was destroyed.
The race happens like this:
1) A user space program opens a file descriptor with O_DIRECT;
2) The program spawns 2 threads using libpthread for example;
3) One of the threads uses the file descriptor to do direct IO writes,
while the other calls fsync using the same file descriptor.
4) Call task A the thread doing direct IO writes and task B the thread
doing fsyncs;
5) Task A does a direct IO write, and at btrfs_direct_write() sets the
file's private to an on stack allocated private with the member
'fsync_skip_inode_lock' set to true;
6) Task B enters btrfs_sync_file() and sees that there's a private
structure associated to the file which has 'fsync_skip_inode_lock' set
to true, so it skips locking the inode's VFS lock;
7) Task A completes the direct IO write, and resets the file's private to
NULL since it had no prior private and our private was stack allocated.
Then it unlocks the inode's VFS lock;
8) Task B enters btrfs_get_ordered_extents_for_logging(), then the
assertion that checks the inode's VFS lock is held fails, since task B
never locked it and task A has already unlocked it.
The stack trace produced is the following:
assertion failed: inode_is_locked(&inode->vfs_inode), in fs/btrfs/ordered-data.c:983
------------[ cut here ]------------
kernel BUG at fs/btrfs/ordered-data.c:983!
Oops: invalid opcode: 0000 [#1] PREEMPT SMP PTI
CPU: 9 PID: 5072 Comm: worker Tainted: G U OE 6.10.5-1-default #1 openSUSE Tumbleweed 69f48d427608e1c09e60ea24c6c55e2ca1b049e8
Hardware name: Acer Predator PH315-52/Covini_CFS, BIOS V1.12 07/28/2020
RIP: 0010:btrfs_get_ordered_extents_for_logging.cold+0x1f/0x42 [btrfs]
Code: 50 d6 86 c0 e8 (...)
RSP: 0018:ffff9e4a03dcfc78 EFLAGS: 00010246
RAX: 0000000000000054 RBX: ffff9078a9868e98 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff907dce4a7800 RDI: ffff907dce4a7800
RBP: ffff907805518800 R08: 0000000000000000 R09: ffff9e4a03dcfb38
R10: ffff9e4a03dcfb30 R11: 0000000000000003 R12: ffff907684ae7800
R13: 0000000000000001 R14: ffff90774646b600 R15: 0000000000000000
FS: 00007f04b96006c0(0000) GS:ffff907dce480000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f32acbfc000 CR3: 00000001fd4fa005 CR4: 00000000003726f0
Call Trace:
<TASK>
? __die_body.cold+0x14/0x24
? die+0x2e/0x50
? do_trap+0xca/0x110
? do_error_trap+0x6a/0x90
? btrfs_get_ordered_extents_for_logging.cold+0x1f/0x42 [btrfs bb26272d49b4cdc847cf3f7faadd459b62caee9a]
? exc_invalid_op+0x50/0x70
? btrfs_get_ordered_extents_for_logging.cold+0x1f/0x42 [btrfs bb26272d49b4cdc847cf3f7faadd459b62caee9a]
? asm_exc_invalid_op+0x1a/0x20
? btrfs_get_ordered_extents_for_logging.cold+0x1f/0x42 [btrfs bb26272d49b4cdc847cf3f7faadd459b62caee9a]
? btrfs_get_ordered_extents_for_logging.cold+0x1f/0x42 [btrfs bb26272d49b4cdc847cf3f7faadd459b62caee9a]
btrfs_sync_file+0x21a/0x4d0 [btrfs bb26272d49b4cdc847cf3f7faadd459b62caee9a]
? __seccomp_filter+0x31d/0x4f0
__x64_sys_fdatasync+0x4f/0x90
do_syscall_64+0x82/0x160
? do_futex+0xcb/0x190
? __x64_sys_futex+0x10e/0x1d0
? switch_fpu_return+0x4f/0xd0
? syscall_exit_to_user_mode+0x72/0x220
? do_syscall_64+0x8e/0x160
? syscall_exit_to_user_mode+0x72/0x220
? do_syscall_64+0x8e/0x160
? syscall_exit_to_user_mode+0x72/0x220
? do_syscall_64+0x8e/0x160
? syscall_exit_to_user_mode+0x72/0x220
? do_syscall_64+0x8e/0x160
entry_SYSCALL_64_after_hwframe+0x76/0x7e
Another problem here is if task B grabs the private pointer and then uses
it after task A has finished, since the private was allocated in the stack
of task A, it results in some invalid memory access with a hard to predict
result.
This issue, triggering the assertion, was observed with QEMU workloads by
two users in the Link tags below.
Fix this by not relying on a file's private to pass information to fsync
that it should skip locking the inode and instead pass this information
through a special value stored in current->journal_info. This is safe
because in the relevant section of the direct IO write path we are not
holding a transaction handle, so current->journal_info is NULL.
The following C program triggers the issue:
$ cat repro.c
/* Get the O_DIRECT definition. */
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <stdint.h>
#include <fcntl.h>
#include <errno.h>
#include <string.h>
#include <pthread.h>
static int fd;
static ssize_t do_write(int fd, const void *buf, size_t count, off_t offset)
{
while (count > 0) {
ssize_t ret;
ret = pwrite(fd, buf, count, offset);
if (ret < 0) {
if (errno == EINTR)
continue;
return ret;
}
count -= ret;
buf += ret;
}
return 0;
}
static void *fsync_loop(void *arg)
{
while (1) {
int ret;
ret = fsync(fd);
if (ret != 0) {
perror("Fsync failed");
exit(6);
}
}
}
int main(int argc, char *argv[])
{
long pagesize;
void *write_buf;
pthread_t fsyncer;
int ret;
if (argc != 2) {
fprintf(stderr, "Use: %s <file path>\n", argv[0]);
return 1;
}
fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC | O_DIRECT, 0666);
if (fd == -1) {
perror("Failed to open/create file");
return 1;
}
pagesize = sysconf(_SC_PAGE_SIZE);
if (pagesize == -1) {
perror("Failed to get page size");
return 2;
}
ret = posix_memalign(&write_buf, pagesize, pagesize);
if (ret) {
perror("Failed to allocate buffer");
return 3;
}
ret = pthread_create(&fsyncer, NULL, fsync_loop, NULL);
if (ret != 0) {
fprintf(stderr, "Failed to create writer thread: %d\n", ret);
return 4;
}
while (1) {
ret = do_write(fd, write_buf, pagesize, 0);
if (ret != 0) {
perror("Write failed");
exit(5);
}
}
return 0;
}
$ mkfs.btrfs -f /dev/sdi
$ mount /dev/sdi /mnt/sdi
$ timeout 10 ./repro /mnt/sdi/foo
Usually the race is triggered within less than 1 second. A test case for
fstests will follow soon.
Reported-by: Paulo Dias <paulo.miguel.dias@gmail.com>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=219187
Reported-by: Andreas Jahn <jahn-andi@web.de>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=219199
Reported-by: syzbot+4704b3cc972bd76024f1@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-btrfs/00000000000044ff540620d7dee2@google.com/
Fixes: 939b656bc8ab ("btrfs: fix corruption after buffer fault in during direct IO append write")
CC: stable@vger.kernel.org # 5.15+
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-08-29 17:25:49 +00:00
|
|
|
current->journal_info = NULL;
|
btrfs: fix corruption after buffer fault in during direct IO append write
During an append (O_APPEND write flag) direct IO write if the input buffer
was not previously faulted in, we can corrupt the file in a way that the
final size is unexpected and it includes an unexpected hole.
The problem happens like this:
1) We have an empty file, with size 0, for example;
2) We do an O_APPEND direct IO with a length of 4096 bytes and the input
buffer is not currently faulted in;
3) We enter btrfs_direct_write(), lock the inode and call
generic_write_checks(), which calls generic_write_checks_count(), and
that function sets the iocb position to 0 with the following code:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
4) We call btrfs_dio_write() and enter into iomap, which will end up
calling btrfs_dio_iomap_begin() and that calls
btrfs_get_blocks_direct_write(), where we update the i_size of the
inode to 4096 bytes;
5) After btrfs_dio_iomap_begin() returns, iomap will attempt to access
the page of the write input buffer (at iomap_dio_bio_iter(), with a
call to bio_iov_iter_get_pages()) and fail with -EFAULT, which gets
returned to btrfs at btrfs_direct_write() via btrfs_dio_write();
6) At btrfs_direct_write() we get the -EFAULT error, unlock the inode,
fault in the write buffer and then goto to the label 'relock';
7) We lock again the inode, do all the necessary checks again and call
again generic_write_checks(), which calls generic_write_checks_count()
again, and there we set the iocb's position to 4K, which is the current
i_size of the inode, with the following code pointed above:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
8) Then we go again to btrfs_dio_write() and enter iomap and the write
succeeds, but it wrote to the file range [4K, 8K), leaving a hole in
the [0, 4K) range and an i_size of 8K, which goes against the
expectations of having the data written to the range [0, 4K) and get an
i_size of 4K.
Fix this by not unlocking the inode before faulting in the input buffer,
in case we get -EFAULT or an incomplete write, and not jumping to the
'relock' label after faulting in the buffer - instead jump to a location
immediately before calling iomap, skipping all the write checks and
relocking. This solves this problem and it's fine even in case the input
buffer is memory mapped to the same file range, since only holding the
range locked in the inode's io tree can cause a deadlock, it's safe to
keep the inode lock (VFS lock), as was fixed and described in commit
51bd9563b678 ("btrfs: fix deadlock due to page faults during direct IO
reads and writes").
A sample reproducer provided by a reporter is the following:
$ cat test.c
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <fcntl.h>
#include <stdio.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <unistd.h>
int main(int argc, char *argv[])
{
if (argc < 2) {
fprintf(stderr, "Usage: %s <test file>\n", argv[0]);
return 1;
}
int fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC | O_DIRECT |
O_APPEND, 0644);
if (fd < 0) {
perror("creating test file");
return 1;
}
char *buf = mmap(NULL, 4096, PROT_READ,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ssize_t ret = write(fd, buf, 4096);
if (ret < 0) {
perror("pwritev2");
return 1;
}
struct stat stbuf;
ret = fstat(fd, &stbuf);
if (ret < 0) {
perror("stat");
return 1;
}
printf("size: %llu\n", (unsigned long long)stbuf.st_size);
return stbuf.st_size == 4096 ? 0 : 1;
}
A test case for fstests will be sent soon.
Reported-by: Hanna Czenczek <hreitz@redhat.com>
Link: https://lore.kernel.org/linux-btrfs/0b841d46-12fe-4e64-9abb-871d8d0de271@redhat.com/
Fixes: 8184620ae212 ("btrfs: fix lost file sync on direct IO write with nowait and dsync iocb")
CC: stable@vger.kernel.org # 6.1+
Tested-by: Hanna Czenczek <hreitz@redhat.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-07-26 10:12:52 +00:00
|
|
|
}
|
2024-06-25 14:52:28 +00:00
|
|
|
|
|
|
|
/* No increment (+=) because iomap returns a cumulative value. */
|
|
|
|
if (ret > 0)
|
|
|
|
written = ret;
|
|
|
|
|
|
|
|
if (iov_iter_count(from) > 0 && (ret == -EFAULT || ret > 0)) {
|
|
|
|
const size_t left = iov_iter_count(from);
|
|
|
|
/*
|
|
|
|
* We have more data left to write. Try to fault in as many as
|
|
|
|
* possible of the remainder pages and retry. We do this without
|
|
|
|
* releasing and locking again the inode, to prevent races with
|
|
|
|
* truncate.
|
|
|
|
*
|
|
|
|
* Also, in case the iov refers to pages in the file range of the
|
|
|
|
* file we want to write to (due to a mmap), we could enter an
|
|
|
|
* infinite loop if we retry after faulting the pages in, since
|
|
|
|
* iomap will invalidate any pages in the range early on, before
|
|
|
|
* it tries to fault in the pages of the iov. So we keep track of
|
|
|
|
* how much was left of iov in the previous EFAULT and fallback
|
|
|
|
* to buffered IO in case we haven't made any progress.
|
|
|
|
*/
|
|
|
|
if (left == prev_left) {
|
|
|
|
ret = -ENOTBLK;
|
|
|
|
} else {
|
|
|
|
fault_in_iov_iter_readable(from, left);
|
|
|
|
prev_left = left;
|
btrfs: fix corruption after buffer fault in during direct IO append write
During an append (O_APPEND write flag) direct IO write if the input buffer
was not previously faulted in, we can corrupt the file in a way that the
final size is unexpected and it includes an unexpected hole.
The problem happens like this:
1) We have an empty file, with size 0, for example;
2) We do an O_APPEND direct IO with a length of 4096 bytes and the input
buffer is not currently faulted in;
3) We enter btrfs_direct_write(), lock the inode and call
generic_write_checks(), which calls generic_write_checks_count(), and
that function sets the iocb position to 0 with the following code:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
4) We call btrfs_dio_write() and enter into iomap, which will end up
calling btrfs_dio_iomap_begin() and that calls
btrfs_get_blocks_direct_write(), where we update the i_size of the
inode to 4096 bytes;
5) After btrfs_dio_iomap_begin() returns, iomap will attempt to access
the page of the write input buffer (at iomap_dio_bio_iter(), with a
call to bio_iov_iter_get_pages()) and fail with -EFAULT, which gets
returned to btrfs at btrfs_direct_write() via btrfs_dio_write();
6) At btrfs_direct_write() we get the -EFAULT error, unlock the inode,
fault in the write buffer and then goto to the label 'relock';
7) We lock again the inode, do all the necessary checks again and call
again generic_write_checks(), which calls generic_write_checks_count()
again, and there we set the iocb's position to 4K, which is the current
i_size of the inode, with the following code pointed above:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
8) Then we go again to btrfs_dio_write() and enter iomap and the write
succeeds, but it wrote to the file range [4K, 8K), leaving a hole in
the [0, 4K) range and an i_size of 8K, which goes against the
expectations of having the data written to the range [0, 4K) and get an
i_size of 4K.
Fix this by not unlocking the inode before faulting in the input buffer,
in case we get -EFAULT or an incomplete write, and not jumping to the
'relock' label after faulting in the buffer - instead jump to a location
immediately before calling iomap, skipping all the write checks and
relocking. This solves this problem and it's fine even in case the input
buffer is memory mapped to the same file range, since only holding the
range locked in the inode's io tree can cause a deadlock, it's safe to
keep the inode lock (VFS lock), as was fixed and described in commit
51bd9563b678 ("btrfs: fix deadlock due to page faults during direct IO
reads and writes").
A sample reproducer provided by a reporter is the following:
$ cat test.c
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <fcntl.h>
#include <stdio.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <unistd.h>
int main(int argc, char *argv[])
{
if (argc < 2) {
fprintf(stderr, "Usage: %s <test file>\n", argv[0]);
return 1;
}
int fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC | O_DIRECT |
O_APPEND, 0644);
if (fd < 0) {
perror("creating test file");
return 1;
}
char *buf = mmap(NULL, 4096, PROT_READ,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ssize_t ret = write(fd, buf, 4096);
if (ret < 0) {
perror("pwritev2");
return 1;
}
struct stat stbuf;
ret = fstat(fd, &stbuf);
if (ret < 0) {
perror("stat");
return 1;
}
printf("size: %llu\n", (unsigned long long)stbuf.st_size);
return stbuf.st_size == 4096 ? 0 : 1;
}
A test case for fstests will be sent soon.
Reported-by: Hanna Czenczek <hreitz@redhat.com>
Link: https://lore.kernel.org/linux-btrfs/0b841d46-12fe-4e64-9abb-871d8d0de271@redhat.com/
Fixes: 8184620ae212 ("btrfs: fix lost file sync on direct IO write with nowait and dsync iocb")
CC: stable@vger.kernel.org # 6.1+
Tested-by: Hanna Czenczek <hreitz@redhat.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-07-26 10:12:52 +00:00
|
|
|
goto again;
|
2024-06-25 14:52:28 +00:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
btrfs: fix corruption after buffer fault in during direct IO append write
During an append (O_APPEND write flag) direct IO write if the input buffer
was not previously faulted in, we can corrupt the file in a way that the
final size is unexpected and it includes an unexpected hole.
The problem happens like this:
1) We have an empty file, with size 0, for example;
2) We do an O_APPEND direct IO with a length of 4096 bytes and the input
buffer is not currently faulted in;
3) We enter btrfs_direct_write(), lock the inode and call
generic_write_checks(), which calls generic_write_checks_count(), and
that function sets the iocb position to 0 with the following code:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
4) We call btrfs_dio_write() and enter into iomap, which will end up
calling btrfs_dio_iomap_begin() and that calls
btrfs_get_blocks_direct_write(), where we update the i_size of the
inode to 4096 bytes;
5) After btrfs_dio_iomap_begin() returns, iomap will attempt to access
the page of the write input buffer (at iomap_dio_bio_iter(), with a
call to bio_iov_iter_get_pages()) and fail with -EFAULT, which gets
returned to btrfs at btrfs_direct_write() via btrfs_dio_write();
6) At btrfs_direct_write() we get the -EFAULT error, unlock the inode,
fault in the write buffer and then goto to the label 'relock';
7) We lock again the inode, do all the necessary checks again and call
again generic_write_checks(), which calls generic_write_checks_count()
again, and there we set the iocb's position to 4K, which is the current
i_size of the inode, with the following code pointed above:
if (iocb->ki_flags & IOCB_APPEND)
iocb->ki_pos = i_size_read(inode);
8) Then we go again to btrfs_dio_write() and enter iomap and the write
succeeds, but it wrote to the file range [4K, 8K), leaving a hole in
the [0, 4K) range and an i_size of 8K, which goes against the
expectations of having the data written to the range [0, 4K) and get an
i_size of 4K.
Fix this by not unlocking the inode before faulting in the input buffer,
in case we get -EFAULT or an incomplete write, and not jumping to the
'relock' label after faulting in the buffer - instead jump to a location
immediately before calling iomap, skipping all the write checks and
relocking. This solves this problem and it's fine even in case the input
buffer is memory mapped to the same file range, since only holding the
range locked in the inode's io tree can cause a deadlock, it's safe to
keep the inode lock (VFS lock), as was fixed and described in commit
51bd9563b678 ("btrfs: fix deadlock due to page faults during direct IO
reads and writes").
A sample reproducer provided by a reporter is the following:
$ cat test.c
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <fcntl.h>
#include <stdio.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <unistd.h>
int main(int argc, char *argv[])
{
if (argc < 2) {
fprintf(stderr, "Usage: %s <test file>\n", argv[0]);
return 1;
}
int fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC | O_DIRECT |
O_APPEND, 0644);
if (fd < 0) {
perror("creating test file");
return 1;
}
char *buf = mmap(NULL, 4096, PROT_READ,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ssize_t ret = write(fd, buf, 4096);
if (ret < 0) {
perror("pwritev2");
return 1;
}
struct stat stbuf;
ret = fstat(fd, &stbuf);
if (ret < 0) {
perror("stat");
return 1;
}
printf("size: %llu\n", (unsigned long long)stbuf.st_size);
return stbuf.st_size == 4096 ? 0 : 1;
}
A test case for fstests will be sent soon.
Reported-by: Hanna Czenczek <hreitz@redhat.com>
Link: https://lore.kernel.org/linux-btrfs/0b841d46-12fe-4e64-9abb-871d8d0de271@redhat.com/
Fixes: 8184620ae212 ("btrfs: fix lost file sync on direct IO write with nowait and dsync iocb")
CC: stable@vger.kernel.org # 6.1+
Tested-by: Hanna Czenczek <hreitz@redhat.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
2024-07-26 10:12:52 +00:00
|
|
|
btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
|
|
|
|
|
2024-06-25 14:52:28 +00:00
|
|
|
/*
|
|
|
|
* If 'ret' is -ENOTBLK or we have not written all data, then it means
|
|
|
|
* we must fallback to buffered IO.
|
|
|
|
*/
|
|
|
|
if ((ret < 0 && ret != -ENOTBLK) || !iov_iter_count(from))
|
|
|
|
goto out;
|
|
|
|
|
|
|
|
buffered:
|
|
|
|
/*
|
|
|
|
* If we are in a NOWAIT context, then return -EAGAIN to signal the caller
|
|
|
|
* it must retry the operation in a context where blocking is acceptable,
|
|
|
|
* because even if we end up not blocking during the buffered IO attempt
|
|
|
|
* below, we will block when flushing and waiting for the IO.
|
|
|
|
*/
|
|
|
|
if (iocb->ki_flags & IOCB_NOWAIT) {
|
|
|
|
ret = -EAGAIN;
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
|
|
|
pos = iocb->ki_pos;
|
|
|
|
written_buffered = btrfs_buffered_write(iocb, from);
|
|
|
|
if (written_buffered < 0) {
|
|
|
|
ret = written_buffered;
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
/*
|
|
|
|
* Ensure all data is persisted. We want the next direct IO read to be
|
|
|
|
* able to read what was just written.
|
|
|
|
*/
|
|
|
|
endbyte = pos + written_buffered - 1;
|
|
|
|
ret = btrfs_fdatawrite_range(BTRFS_I(inode), pos, endbyte);
|
|
|
|
if (ret)
|
|
|
|
goto out;
|
|
|
|
ret = filemap_fdatawait_range(inode->i_mapping, pos, endbyte);
|
|
|
|
if (ret)
|
|
|
|
goto out;
|
|
|
|
written += written_buffered;
|
|
|
|
iocb->ki_pos = pos + written_buffered;
|
|
|
|
invalidate_mapping_pages(file->f_mapping, pos >> PAGE_SHIFT,
|
|
|
|
endbyte >> PAGE_SHIFT);
|
|
|
|
out:
|
|
|
|
return ret < 0 ? ret : written;
|
|
|
|
}
|
|
|
|
|
|
|
|
static int check_direct_read(struct btrfs_fs_info *fs_info,
|
|
|
|
const struct iov_iter *iter, loff_t offset)
|
|
|
|
{
|
|
|
|
int ret;
|
|
|
|
int i, seg;
|
|
|
|
|
|
|
|
ret = check_direct_IO(fs_info, iter, offset);
|
|
|
|
if (ret < 0)
|
|
|
|
return ret;
|
|
|
|
|
|
|
|
if (!iter_is_iovec(iter))
|
|
|
|
return 0;
|
|
|
|
|
|
|
|
for (seg = 0; seg < iter->nr_segs; seg++) {
|
|
|
|
for (i = seg + 1; i < iter->nr_segs; i++) {
|
|
|
|
const struct iovec *iov1 = iter_iov(iter) + seg;
|
|
|
|
const struct iovec *iov2 = iter_iov(iter) + i;
|
|
|
|
|
|
|
|
if (iov1->iov_base == iov2->iov_base)
|
|
|
|
return -EINVAL;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
ssize_t btrfs_direct_read(struct kiocb *iocb, struct iov_iter *to)
|
|
|
|
{
|
|
|
|
struct inode *inode = file_inode(iocb->ki_filp);
|
|
|
|
size_t prev_left = 0;
|
|
|
|
ssize_t read = 0;
|
|
|
|
ssize_t ret;
|
|
|
|
|
|
|
|
if (fsverity_active(inode))
|
|
|
|
return 0;
|
|
|
|
|
|
|
|
if (check_direct_read(inode_to_fs_info(inode), to, iocb->ki_pos))
|
|
|
|
return 0;
|
|
|
|
|
|
|
|
btrfs_inode_lock(BTRFS_I(inode), BTRFS_ILOCK_SHARED);
|
|
|
|
again:
|
|
|
|
/*
|
|
|
|
* This is similar to what we do for direct IO writes, see the comment
|
|
|
|
* at btrfs_direct_write(), but we also disable page faults in addition
|
|
|
|
* to disabling them only at the iov_iter level. This is because when
|
|
|
|
* reading from a hole or prealloc extent, iomap calls iov_iter_zero(),
|
|
|
|
* which can still trigger page fault ins despite having set ->nofault
|
|
|
|
* to true of our 'to' iov_iter.
|
|
|
|
*
|
|
|
|
* The difference to direct IO writes is that we deadlock when trying
|
|
|
|
* to lock the extent range in the inode's tree during he page reads
|
|
|
|
* triggered by the fault in (while for writes it is due to waiting for
|
|
|
|
* our own ordered extent). This is because for direct IO reads,
|
|
|
|
* btrfs_dio_iomap_begin() returns with the extent range locked, which
|
|
|
|
* is only unlocked in the endio callback (end_bio_extent_readpage()).
|
|
|
|
*/
|
|
|
|
pagefault_disable();
|
|
|
|
to->nofault = true;
|
|
|
|
ret = btrfs_dio_read(iocb, to, read);
|
|
|
|
to->nofault = false;
|
|
|
|
pagefault_enable();
|
|
|
|
|
|
|
|
/* No increment (+=) because iomap returns a cumulative value. */
|
|
|
|
if (ret > 0)
|
|
|
|
read = ret;
|
|
|
|
|
|
|
|
if (iov_iter_count(to) > 0 && (ret == -EFAULT || ret > 0)) {
|
|
|
|
const size_t left = iov_iter_count(to);
|
|
|
|
|
|
|
|
if (left == prev_left) {
|
|
|
|
/*
|
|
|
|
* We didn't make any progress since the last attempt,
|
|
|
|
* fallback to a buffered read for the remainder of the
|
|
|
|
* range. This is just to avoid any possibility of looping
|
|
|
|
* for too long.
|
|
|
|
*/
|
|
|
|
ret = read;
|
|
|
|
} else {
|
|
|
|
/*
|
|
|
|
* We made some progress since the last retry or this is
|
|
|
|
* the first time we are retrying. Fault in as many pages
|
|
|
|
* as possible and retry.
|
|
|
|
*/
|
|
|
|
fault_in_iov_iter_writeable(to, left);
|
|
|
|
prev_left = left;
|
|
|
|
goto again;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
btrfs_inode_unlock(BTRFS_I(inode), BTRFS_ILOCK_SHARED);
|
|
|
|
return ret < 0 ? ret : read;
|
|
|
|
}
|
|
|
|
|
|
|
|
int __init btrfs_init_dio(void)
|
|
|
|
{
|
|
|
|
if (bioset_init(&btrfs_dio_bioset, BIO_POOL_SIZE,
|
|
|
|
offsetof(struct btrfs_dio_private, bbio.bio),
|
|
|
|
BIOSET_NEED_BVECS))
|
|
|
|
return -ENOMEM;
|
|
|
|
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
void __cold btrfs_destroy_dio(void)
|
|
|
|
{
|
|
|
|
bioset_exit(&btrfs_dio_bioset);
|
|
|
|
}
|