linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] btrfs-progs: check/lowmem: Reset path in repair mode to avoid incorrect item from being passed to lowmem check.
@ 2019-05-17 14:00 Qu Wenruo
  2019-06-05 16:08 ` David Sterba
  0 siblings, 1 reply; 3+ messages in thread
From: Qu Wenruo @ 2019-05-17 14:00 UTC (permalink / raw)
  To: linux-btrfs

In lowmem mode, we check fs roots and free space cache by iterating
each root item and inode item, using btrfs_next_item() and a path
pointing to the root tree.

However in repair mode, check_fs_root() can modify the fs root, thus
CoWs the tree root, and the old path in check_fs

It could lead to strange behavior, e.g. after repairing a fs tree, the
path can point to a fs tree.
Since no ROOT_ITEM exists in fs tree, all remaining trees are skipped in
repair mode.

This bug exists from the early time of lowmem mode repair, and is only
exposed by recent free space inode check code. (Fs tree inodes are
passed to free space inode check, causing false alerts and repair
failure).

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-lowmem.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 6d7ae2bc0549..808d6be8db30 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -5184,6 +5184,28 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
 			err |= ret;
 		}
 next:
+		/*
+		 * In repair mode, our path is no longer reliable as CoW can
+		 * happen.
+		 * We need to reset our path.
+		 */
+		if (repair) {
+			btrfs_release_path(&path);
+			ret = btrfs_search_slot(NULL, tree_root, &key, &path,
+						0, 0);
+			if (ret < 0) {
+				if (!err)
+					err = ret;
+				goto out;
+			}
+			if (ret > 0) {
+				/* Key not found, but already at next item */
+				if (path.slots[0] <
+				    btrfs_header_nritems(path.nodes[0]))
+					continue;
+				/* falls through to next leaf */
+			}
+		}
 		ret = btrfs_next_item(tree_root, &path);
 		if (ret > 0)
 			goto out;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] btrfs-progs: check/lowmem: Reset path in repair mode to avoid incorrect item from being passed to lowmem check.
  2019-05-17 14:00 [PATCH] btrfs-progs: check/lowmem: Reset path in repair mode to avoid incorrect item from being passed to lowmem check Qu Wenruo
@ 2019-06-05 16:08 ` David Sterba
  2019-06-06  3:52   ` Qu Wenruo
  0 siblings, 1 reply; 3+ messages in thread
From: David Sterba @ 2019-06-05 16:08 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Fri, May 17, 2019 at 10:00:03PM +0800, Qu Wenruo wrote:
> In lowmem mode, we check fs roots and free space cache by iterating
> each root item and inode item, using btrfs_next_item() and a path
> pointing to the root tree.
> 
> However in repair mode, check_fs_root() can modify the fs root, thus
> CoWs the tree root, and the old path in check_fs
> 
> It could lead to strange behavior, e.g. after repairing a fs tree, the
> path can point to a fs tree.
> Since no ROOT_ITEM exists in fs tree, all remaining trees are skipped in
> repair mode.
> 
> This bug exists from the early time of lowmem mode repair, and is only
> exposed by recent free space inode check code. (Fs tree inodes are
> passed to free space inode check, causing false alerts and repair
> failure).
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

I'll add it to devel, however the lowmem mode of test-check does not
work now, so can't really test it.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] btrfs-progs: check/lowmem: Reset path in repair mode to avoid incorrect item from being passed to lowmem check.
  2019-06-05 16:08 ` David Sterba
@ 2019-06-06  3:52   ` Qu Wenruo
  0 siblings, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2019-06-06  3:52 UTC (permalink / raw)
  To: dsterba, Qu Wenruo, linux-btrfs


[-- Attachment #1.1: Type: text/plain, Size: 1223 bytes --]



On 2019/6/6 上午12:08, David Sterba wrote:
> On Fri, May 17, 2019 at 10:00:03PM +0800, Qu Wenruo wrote:
>> In lowmem mode, we check fs roots and free space cache by iterating
>> each root item and inode item, using btrfs_next_item() and a path
>> pointing to the root tree.
>>
>> However in repair mode, check_fs_root() can modify the fs root, thus
>> CoWs the tree root, and the old path in check_fs
>>
>> It could lead to strange behavior, e.g. after repairing a fs tree, the
>> path can point to a fs tree.
>> Since no ROOT_ITEM exists in fs tree, all remaining trees are skipped in
>> repair mode.
>>
>> This bug exists from the early time of lowmem mode repair, and is only
>> exposed by recent free space inode check code. (Fs tree inodes are
>> passed to free space inode check, causing false alerts and repair
>> failure).
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> I'll add it to devel, however the lowmem mode of test-check does not
> work now, so can't really test it.
> 
With this patch applies, test-fsck runs well for lowmem mode, and that's
what this patch should do.

I just tried devel branch it works as expected.

Or did you hit some new bug?

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-06-06  3:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-17 14:00 [PATCH] btrfs-progs: check/lowmem: Reset path in repair mode to avoid incorrect item from being passed to lowmem check Qu Wenruo
2019-06-05 16:08 ` David Sterba
2019-06-06  3:52   ` Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).