public-mirrors/linux
1
0
Fork 0
mirror of git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2025-08-05 16:54:27 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

btrfs: fix inode lookup error handling during log replay

Browse source 
When replaying log trees we use read_one_inode() to get an inode, which is
just a wrapper around btrfs_iget_logging(), which in turn is a wrapper for
btrfs_iget(). But read_one_inode() always returns NULL for any error
that btrfs_iget_logging() / btrfs_iget() may return and this is a problem
because:

1) In many callers of read_one_inode() we convert the NULL into -EIO,
   which is not accurate since btrfs_iget() may return -ENOMEM and -ENOENT
   for example, besides -EIO and other errors. So during log replay we
   may end up reporting a false -EIO, which is confusing since we may
   not have had any IO error at all;

2) When replaying directory deletes, at replay_dir_deletes(), we assume
   the NULL returned from read_one_inode() means that the inode doesn't
   exist and then proceed as if no error had happened. This is wrong
   because unless btrfs_iget() returned ERR_PTR(-ENOENT), we had an
   actual error and the target inode may exist in the target subvolume
   root - this may later result in the log replay code failing at a
   later stage (if we are "lucky") or succeed but leaving some
   inconsistency in the filesystem.

So fix this by not ignoring errors from btrfs_iget_logging() and as
a consequence remove the read_one_inode() wrapper and just use
btrfs_iget_logging() directly. Also since btrfs_iget_logging() is
supposed to be called only against subvolume roots, just like
read_one_inode() which had a comment about it, add an assertion to
btrfs_iget_logging() to check that the target root corresponds to a
subvolume root.

Fixes: 5d4f98a28c ("Btrfs: Mixed back reference  (FORWARD ROLLING FORMAT CHANGE)")
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
This commit is contained in:
Filipe Manana 2025-06-18 15:58:31 +01:00 • committed by David Sterba
parent 54a7081ed1
commit 5f61b96159
1 changed files with 62 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

125
fs/btrfs/tree-log.c
View file

@ -143,6 +143,9 @@ static struct btrfs_inode *btrfs_iget_logging(u64 objectid, struct btrfs_root *r
unsigned int nofs_flag; unsigned int nofs_flag;
struct btrfs_inode *inode; struct btrfs_inode *inode;
/* Only meant to be called for subvolume roots and not for log roots. */
ASSERT(is_fstree(btrfs_root_id(root)));
/* /*
* We're holding a transaction handle whether we are logging or * We're holding a transaction handle whether we are logging or
* replaying a log tree, so we must make sure NOFS semantics apply * replaying a log tree, so we must make sure NOFS semantics apply
@ -604,21 +607,6 @@ static int read_alloc_one_name(struct extent_buffer *eb, void *start, int len,
return 0; return 0;
} }
/*
* simple helper to read an inode off the disk from a given root
* This can only be called for subvolume roots and not for the log
*/
static noinline struct btrfs_inode *read_one_inode(struct btrfs_root *root,
u64 objectid)
{
struct btrfs_inode *inode;
inode = btrfs_iget_logging(objectid, root);
if (IS_ERR(inode))
return NULL;
return inode;
}
/* replays a single extent in 'eb' at 'slot' with 'key' into the /* replays a single extent in 'eb' at 'slot' with 'key' into the
* subvolume 'root'. path is released on entry and should be released * subvolume 'root'. path is released on entry and should be released
* on exit. * on exit.
@ -674,9 +662,9 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
return -EUCLEAN; return -EUCLEAN;
} }
inode = read_one_inode(root, key->objectid); inode = btrfs_iget_logging(key->objectid, root);
if (!inode) if (IS_ERR(inode))
return -EIO; return PTR_ERR(inode);
/* /*
* first check to see if we already have this extent in the * first check to see if we already have this extent in the
@ -948,9 +936,10 @@ static noinline int drop_one_dir_item(struct btrfs_trans_handle *trans,
btrfs_release_path(path); btrfs_release_path(path);
inode = read_one_inode(root, location.objectid); inode = btrfs_iget_logging(location.objectid, root);
if (!inode) { if (IS_ERR(inode)) {
ret = -EIO; ret = PTR_ERR(inode);
inode = NULL;
goto out; goto out;
} }
@ -1169,10 +1158,10 @@ again:
kfree(victim_name.name); kfree(victim_name.name);
return ret; return ret;
} else if (!ret) { } else if (!ret) {
ret = -ENOENT; victim_parent = btrfs_iget_logging(parent_objectid, root);
victim_parent = read_one_inode(root, if (IS_ERR(victim_parent)) {
parent_objectid); ret = PTR_ERR(victim_parent);
if (victim_parent) { } else {
inc_nlink(&inode->vfs_inode); inc_nlink(&inode->vfs_inode);
btrfs_release_path(path); btrfs_release_path(path);
@ -1317,9 +1306,9 @@ again:
struct btrfs_inode *dir; struct btrfs_inode *dir;
btrfs_release_path(path); btrfs_release_path(path);
dir = read_one_inode(root, parent_id); dir = btrfs_iget_logging(parent_id, root);
if (!dir) { if (IS_ERR(dir)) {
ret = -ENOENT; ret = PTR_ERR(dir);
kfree(name.name); kfree(name.name);
goto out; goto out;
} }
@ -1391,15 +1380,17 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
* copy the back ref in. The link count fixup code will take * copy the back ref in. The link count fixup code will take
* care of the rest * care of the rest
*/ */
dir = read_one_inode(root, parent_objectid); dir = btrfs_iget_logging(parent_objectid, root);
if (!dir) { if (IS_ERR(dir)) {
ret = -ENOENT; ret = PTR_ERR(dir);
dir = NULL;
goto out; goto out;
} }
inode = read_one_inode(root, inode_objectid); inode = btrfs_iget_logging(inode_objectid, root);
if (!inode) { if (IS_ERR(inode)) {
ret = -EIO; ret = PTR_ERR(inode);
inode = NULL;
goto out; goto out;
} }
@ -1411,12 +1402,14 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
* parent object can change from one array * parent object can change from one array
* item to another. * item to another.
*/ */
if (!dir)
dir = read_one_inode(root, parent_objectid);
if (!dir) { if (!dir) {
ret = -ENOENT; dir = btrfs_iget_logging(parent_objectid, root);
if (IS_ERR(dir)) {
ret = PTR_ERR(dir);
dir = NULL;
goto out; goto out;
} }
}
} else { } else {
ret = ref_get_fields(eb, ref_ptr, &name, &ref_index); ret = ref_get_fields(eb, ref_ptr, &name, &ref_index);
} }
@ -1684,9 +1677,9 @@ static noinline int fixup_inode_link_counts(struct btrfs_trans_handle *trans,
break; break;
btrfs_release_path(path); btrfs_release_path(path);
inode = read_one_inode(root, key.offset); inode = btrfs_iget_logging(key.offset, root);
if (!inode) { if (IS_ERR(inode)) {
ret = -EIO; ret = PTR_ERR(inode);
break; break;
} }
@ -1722,9 +1715,9 @@ static noinline int link_to_fixup_dir(struct btrfs_trans_handle *trans,
struct btrfs_inode *inode; struct btrfs_inode *inode;
struct inode *vfs_inode; struct inode *vfs_inode;
inode = read_one_inode(root, objectid); inode = btrfs_iget_logging(objectid, root);
if (!inode) if (IS_ERR(inode))
return -EIO; return PTR_ERR(inode);
vfs_inode = &inode->vfs_inode; vfs_inode = &inode->vfs_inode;
key.objectid = BTRFS_TREE_LOG_FIXUP_OBJECTID; key.objectid = BTRFS_TREE_LOG_FIXUP_OBJECTID;
@ -1763,14 +1756,14 @@ static noinline int insert_one_name(struct btrfs_trans_handle *trans,
struct btrfs_inode *dir; struct btrfs_inode *dir;
int ret; int ret;
inode = read_one_inode(root, location->objectid); inode = btrfs_iget_logging(location->objectid, root);
if (!inode) if (IS_ERR(inode))
return -ENOENT; return PTR_ERR(inode);
dir = read_one_inode(root, dirid); dir = btrfs_iget_logging(dirid, root);
if (!dir) { if (IS_ERR(dir)) {
iput(&inode->vfs_inode); iput(&inode->vfs_inode);
return -EIO; return PTR_ERR(dir);
} }
ret = btrfs_add_link(trans, dir, inode, name, 1, index); ret = btrfs_add_link(trans, dir, inode, name, 1, index);
@ -1847,9 +1840,9 @@ static noinline int replay_one_name(struct btrfs_trans_handle *trans,
bool update_size = true; bool update_size = true;
bool name_added = false; bool name_added = false;
dir = read_one_inode(root, key->objectid); dir = btrfs_iget_logging(key->objectid, root);
if (!dir) if (IS_ERR(dir))
return -EIO; return PTR_ERR(dir);
ret = read_alloc_one_name(eb, di + 1, btrfs_dir_name_len(eb, di), &name); ret = read_alloc_one_name(eb, di + 1, btrfs_dir_name_len(eb, di), &name);
if (ret) if (ret)
@ -2149,9 +2142,10 @@ static noinline int check_item_in_log(struct btrfs_trans_handle *trans,
btrfs_dir_item_key_to_cpu(eb, di, &location); btrfs_dir_item_key_to_cpu(eb, di, &location);
btrfs_release_path(path); btrfs_release_path(path);
btrfs_release_path(log_path); btrfs_release_path(log_path);
inode = read_one_inode(root, location.objectid); inode = btrfs_iget_logging(location.objectid, root);
if (!inode) { if (IS_ERR(inode)) {
ret = -EIO; ret = PTR_ERR(inode);
inode = NULL;
goto out; goto out;
} }
@ -2303,14 +2297,17 @@ static noinline int replay_dir_deletes(struct btrfs_trans_handle *trans,
if (!log_path) if (!log_path)
return -ENOMEM; return -ENOMEM;
dir = read_one_inode(root, dirid); dir = btrfs_iget_logging(dirid, root);
/* it isn't an error if the inode isn't there, that can happen /*
* because we replay the deletes before we copy in the inode item * It isn't an error if the inode isn't there, that can happen because
* from the log * we replay the deletes before we copy in the inode item from the log.
*/ */
if (!dir) { if (IS_ERR(dir)) {
btrfs_free_path(log_path); btrfs_free_path(log_path);
return 0; ret = PTR_ERR(dir);
if (ret == -ENOENT)
ret = 0;
return ret;
} }
range_start = 0; range_start = 0;
@ -2469,9 +2466,9 @@ static int replay_one_buffer(struct btrfs_root *log, struct extent_buffer *eb,
struct btrfs_inode *inode; struct btrfs_inode *inode;
u64 from; u64 from;
inode = read_one_inode(root, key.objectid); inode = btrfs_iget_logging(key.objectid, root);
if (!inode) { if (IS_ERR(inode)) {
ret = -EIO; ret = PTR_ERR(inode);
break; break;
} }
from = ALIGN(i_size_read(&inode->vfs_inode), from = ALIGN(i_size_read(&inode->vfs_inode),