[PATCH RESEND v4 0/4] device_list_add() peparation to add reappearing missing device

[PATCH RESEND v4 0/4] device_list_add() peparation to add reappearing missing device

[Anand Jain]

(Apply on top of my patchset
   [PATCH v4 0/6] preparatory work to add device forget
 for conflict free apply. They don't actually depend on
 each other though).

v3->v4:
 @3/4: Just return device instead of PTR_ERR(ERR_PTR(device));

v2->v3:
 Fix device_list_add() fn description which was still referring to the
 previous return values.

v1->v2:
 Drop patch 5/5 for uuid_mutex optimize. That was wrong. Thanks Josef.
 In patch 3/5 make btrfs_device * as return.

Cleanup of device_list_add(), mainly in preparation to handle
reappearing missing device which its next reroll will be sent
separately.

Anand Jain (4):
  btrfs: move pr_info into device_list_add
  btrfs: set the total_devices in device_list_add()
  btrfs: get device pointer from device_list_add()
  btrfs: drop devid as device_list_add() arg

 fs/btrfs/volumes.c | 63 +++++++++++++++++++++++-------------------------------
 1 file changed, 27 insertions(+), 36 deletions(-)

-- 
2.7.0

[PATCH 1/4] btrfs: move pr_info into device_list_add

[Anand Jain]

Commit 60999ca4b403 ("btrfs: make device scan less noisy")
adds return value 1 to device_list_add(), so that parent function can
call pr_info only when new device is added. Move the pr_info() part
into device_list_add() so that this function can be kept simple.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Josef Bacik <jbacik@fb.com>
---
 fs/btrfs/volumes.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 68674da7f5fc..0b145276ff46 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -726,8 +726,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
  * Add new device to list of registered devices
  *
  * Returns:
- * 1   - first time device is seen
- * 0   - device already known
+ * 0   - device already known or newly added
  * < 0 - error
  */
 static noinline int device_list_add(const char *path,
@@ -737,7 +736,6 @@ static noinline int device_list_add(const char *path,
 	struct btrfs_device *device;
 	struct btrfs_fs_devices *fs_devices;
 	struct rcu_string *name;
-	int ret = 0;
 	u64 found_transid = btrfs_super_generation(disk_super);
 
 	fs_devices = find_fsid(disk_super->fsid);
@@ -777,9 +775,16 @@ static noinline int device_list_add(const char *path,
 		fs_devices->num_devices++;
 		mutex_unlock(&fs_devices->device_list_mutex);
 
-		ret = 1;
 		device->fs_devices = fs_devices;
 		btrfs_free_stale_devices(path, device);
+
+		if (disk_super->label[0])
+			pr_info("BTRFS: device label %s devid %llu transid %llu %s\n",
+				disk_super->label, devid, found_transid, path);
+		else
+			pr_info("BTRFS: device fsid %pU devid %llu transid %llu %s\n",
+				disk_super->fsid, devid, found_transid, path);
+
 	} else if (!device->name || strcmp(device->name->str, path)) {
 		/*
 		 * When FS is already mounted.
@@ -840,7 +845,7 @@ static noinline int device_list_add(const char *path,
 
 	*fs_devices_ret = fs_devices;
 
-	return ret;
+	return 0;
 }
 
 static struct btrfs_fs_devices *clone_fs_devices(struct btrfs_fs_devices *orig)
@@ -1179,7 +1184,6 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	struct page *page;
 	int ret;
 	u64 devid;
-	u64 transid;
 	u64 total_devices;
 	u64 bytenr;
 
@@ -1202,25 +1206,14 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	}
 
 	devid = btrfs_stack_device_id(&disk_super->dev_item);
-	transid = btrfs_super_generation(disk_super);
 	total_devices = btrfs_super_num_devices(disk_super);
 
 	mutex_lock(&uuid_mutex);
 	ret = device_list_add(path, disk_super, devid, fs_devices_ret);
-	if (ret >= 0 && fs_devices_ret)
+	if (!ret && fs_devices_ret)
 		(*fs_devices_ret)->total_devices = total_devices;
 	mutex_unlock(&uuid_mutex);
 
-	if (ret > 0) {
-		if (disk_super->label[0])
-			pr_info("BTRFS: device label %s ", disk_super->label);
-		else
-			pr_info("BTRFS: device fsid %pU ", disk_super->fsid);
-
-		pr_cont("devid %llu transid %llu %s\n", devid, transid, path);
-		ret = 0;
-	}
-
 	btrfs_release_disk_super(page);
 
 error_bdev_put:
-- 
2.7.0

[PATCH 2/4] btrfs: set the total_devices in device_list_add()

[Anand Jain]

There is no other parent for device_list_add() except for
btrfs_scan_one_device(), which would set btrfs_fs_devices::total_devices
if device_list_add is successful and this can be done with in
device_list_add() itself.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Josef Bacik <jbacik@fb.com>
---
 fs/btrfs/volumes.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0b145276ff46..66e5dada2d74 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -843,6 +843,8 @@ static noinline int device_list_add(const char *path,
 	if (!fs_devices->opened)
 		device->generation = found_transid;
 
+	fs_devices->total_devices = btrfs_super_num_devices(disk_super);
+
 	*fs_devices_ret = fs_devices;
 
 	return 0;
@@ -1184,7 +1186,6 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	struct page *page;
 	int ret;
 	u64 devid;
-	u64 total_devices;
 	u64 bytenr;
 
 	/*
@@ -1206,12 +1207,9 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	}
 
 	devid = btrfs_stack_device_id(&disk_super->dev_item);
-	total_devices = btrfs_super_num_devices(disk_super);
 
 	mutex_lock(&uuid_mutex);
 	ret = device_list_add(path, disk_super, devid, fs_devices_ret);
-	if (!ret && fs_devices_ret)
-		(*fs_devices_ret)->total_devices = total_devices;
 	mutex_unlock(&uuid_mutex);
 
 	btrfs_release_disk_super(page);
-- 
2.7.0

[PATCH 3/4] btrfs: get device pointer from device_list_add()

[Anand Jain]

Instead of pointer to btrfs_fs_devices as an arg in device_list_add()
better to get pointer to btrfs_device as return value, then we have
both, pointer to btrfs_device and btrfs_fs_devices. btrfs_device is
needed to handle reappearing missing device.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 66e5dada2d74..d93ee0b91ad9 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -726,12 +726,11 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
  * Add new device to list of registered devices
  *
  * Returns:
- * 0   - device already known or newly added
- * < 0 - error
+ * device pointer which was just added or updated when successful
+ * error pointer when failed
  */
-static noinline int device_list_add(const char *path,
-			   struct btrfs_super_block *disk_super,
-			   u64 devid, struct btrfs_fs_devices **fs_devices_ret)
+static noinline struct btrfs_device *device_list_add(const char *path,
+			   struct btrfs_super_block *disk_super, u64 devid)
 {
 	struct btrfs_device *device;
 	struct btrfs_fs_devices *fs_devices;
@@ -742,7 +741,7 @@ static noinline int device_list_add(const char *path,
 	if (!fs_devices) {
 		fs_devices = alloc_fs_devices(disk_super->fsid);
 		if (IS_ERR(fs_devices))
-			return PTR_ERR(fs_devices);
+			return ERR_PTR(PTR_ERR(fs_devices));
 
 		list_add(&fs_devices->list, &fs_uuids);
 
@@ -754,19 +753,19 @@ static noinline int device_list_add(const char *path,
 
 	if (!device) {
 		if (fs_devices->opened)
-			return -EBUSY;
+			return ERR_PTR(-EBUSY);
 
 		device = btrfs_alloc_device(NULL, &devid,
 					    disk_super->dev_item.uuid);
 		if (IS_ERR(device)) {
 			/* we can safely leave the fs_devices entry around */
-			return PTR_ERR(device);
+			return device;
 		}
 
 		name = rcu_string_strdup(path, GFP_NOFS);
 		if (!name) {
 			free_device(device);
-			return -ENOMEM;
+			return ERR_PTR(-ENOMEM);
 		}
 		rcu_assign_pointer(device->name, name);
 
@@ -820,12 +819,12 @@ static noinline int device_list_add(const char *path,
 			 * with larger generation number or the last-in if
 			 * generation are equal.
 			 */
-			return -EEXIST;
+			return ERR_PTR(-EEXIST);
 		}
 
 		name = rcu_string_strdup(path, GFP_NOFS);
 		if (!name)
-			return -ENOMEM;
+			return ERR_PTR(-ENOMEM);
 		rcu_string_free(device->name);
 		rcu_assign_pointer(device->name, name);
 		if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state)) {
@@ -845,9 +844,7 @@ static noinline int device_list_add(const char *path,
 
 	fs_devices->total_devices = btrfs_super_num_devices(disk_super);
 
-	*fs_devices_ret = fs_devices;
-
-	return 0;
+	return device;
 }
 
 static struct btrfs_fs_devices *clone_fs_devices(struct btrfs_fs_devices *orig)
@@ -1182,9 +1179,10 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 			  struct btrfs_fs_devices **fs_devices_ret)
 {
 	struct btrfs_super_block *disk_super;
+	struct btrfs_device *device;
 	struct block_device *bdev;
 	struct page *page;
-	int ret;
+	int ret = 0;
 	u64 devid;
 	u64 bytenr;
 
@@ -1209,8 +1207,12 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	devid = btrfs_stack_device_id(&disk_super->dev_item);
 
 	mutex_lock(&uuid_mutex);
-	ret = device_list_add(path, disk_super, devid, fs_devices_ret);
+	device = device_list_add(path, disk_super, devid);
 	mutex_unlock(&uuid_mutex);
+	if (IS_ERR(device))
+		ret = PTR_ERR(device);
+
+	*fs_devices_ret = device->fs_devices;
 
 	btrfs_release_disk_super(page);
 
-- 
2.7.0

[PATCH 4/4] btrfs: drop devid as device_list_add() arg

[Anand Jain]

As struct btrfs_disk_super is being passed, so it can get devid
the same way its parent does.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Josef Bacik <jbacik@fb.com>
---
 fs/btrfs/volumes.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d93ee0b91ad9..e947e47f8fff 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -730,12 +730,13 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
  * error pointer when failed
  */
 static noinline struct btrfs_device *device_list_add(const char *path,
-			   struct btrfs_super_block *disk_super, u64 devid)
+			   struct btrfs_super_block *disk_super)
 {
 	struct btrfs_device *device;
 	struct btrfs_fs_devices *fs_devices;
 	struct rcu_string *name;
 	u64 found_transid = btrfs_super_generation(disk_super);
+	u64 devid = btrfs_stack_device_id(&disk_super->dev_item);
 
 	fs_devices = find_fsid(disk_super->fsid);
 	if (!fs_devices) {
@@ -1183,7 +1184,6 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	struct block_device *bdev;
 	struct page *page;
 	int ret = 0;
-	u64 devid;
 	u64 bytenr;
 
 	/*
@@ -1204,10 +1204,8 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 		goto error_bdev_put;
 	}
 
-	devid = btrfs_stack_device_id(&disk_super->dev_item);
-
 	mutex_lock(&uuid_mutex);
-	device = device_list_add(path, disk_super, devid);
+	device = device_list_add(path, disk_super);
 	mutex_unlock(&uuid_mutex);
 	if (IS_ERR(device))
 		ret = PTR_ERR(device);
-- 
2.7.0

Re: [PATCH RESEND v4 0/4] device_list_add() peparation to add reappearing missing device

[David Sterba]

On Thu, Jan 18, 2018 at 10:02:32PM +0800, Anand Jain wrote:
> (Apply on top of my patchset
>    [PATCH v4 0/6] preparatory work to add device forget
>  for conflict free apply. They don't actually depend on
>  each other though).

> Cleanup of device_list_add(), mainly in preparation to handle
> reappearing missing device which its next reroll will be sent
> separately.

I'm adding the two patchsets to the 4.16 queue but will push the updated
branch after the current tests finish and I also test the updated branch
as well.

Re: [PATCH RESEND v4 0/4] device_list_add() peparation to add reappearing missing device

[David Sterba]

On Thu, Jan 18, 2018 at 06:47:17PM +0100, David Sterba wrote:
> On Thu, Jan 18, 2018 at 10:02:32PM +0800, Anand Jain wrote:
> > (Apply on top of my patchset
> >    [PATCH v4 0/6] preparatory work to add device forget
> >  for conflict free apply. They don't actually depend on
> >  each other though).
> 
> > Cleanup of device_list_add(), mainly in preparation to handle
> > reappearing missing device which its next reroll will be sent
> > separately.
> 
> I'm adding the two patchsets to the 4.16 queue but will push the updated
> branch after the current tests finish and I also test the updated branch
> as well.

So this did not survive the first fstests run, I'm going to move the patchset
to the 4.17 dev queue.

[ 2912.493351] run fstests btrfs/064 at 2018-01-19 20:55:50
[ 2914.218654] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 1 transid 5 /dev/sdb6
[ 2914.261560] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 2 transid 5 /dev/sdc5
[ 2914.296819] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 3 transid 5 /dev/sdb7
[ 2914.348140] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 4 transid 5 /dev/sdc6
[ 2914.389368] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 5 transid 5 /dev/sdb8
[ 2914.425378] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 6 transid 5 /dev/sdc7
[ 2914.443497] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 7 transid 5 /dev/sdb9
[ 2914.488145] BTRFS info (device sdb9): disk space caching is enabled
[ 2914.494744] BTRFS info (device sdb9): has skinny extents
[ 2914.500328] BTRFS info (device sdb9): flagging fs with big metadata feature
[ 2914.514809] BTRFS info (device sdb9): enabling ssd optimizations
[ 2914.522114] BTRFS info (device sdb9): creating UUID tree
[ 2914.716867] BTRFS info (device sdb9): dev_replace from /dev/sdc5 (devid 2) to /dev/sdc8 started
[ 2914.852699] BTRFS info (device sdb9): dev_replace from /dev/sdc5 (devid 2) to /dev/sdc8 finished
[ 2915.028666] BTRFS info (device sdb9): dev_replace from /dev/sdb7 (devid 3) to /dev/sdc5 started
[ 2915.110374] BTRFS info (device sdb9): dev_replace from /dev/sdb7 (devid 3) to /dev/sdc5 finished
[ 2915.309674] BTRFS info (device sdb9): dev_replace from /dev/sdc6 (devid 4) to /dev/sdb7 started
[ 2915.340819] BTRFS info (device sdb9): dev_replace from /dev/sdc6 (devid 4) to /dev/sdb7 finished
[ 2915.350220] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
[ 2915.358350] IP: btrfs_scan_one_device+0x127/0x180 [btrfs]
[ 2915.358353] PGD 0 P4D 0 
[ 2915.358366] Oops: 0000 [#1] PREEMPT SMP
[ 2915.358493] CPU: 2 PID: 1076 Comm: systemd-udevd Tainted: G        W        4.15.0-rc8-1.ge195904-vanilla+ #128
[ 2915.358495] Hardware name: empty empty/S3993, BIOS PAQEX0-3 02/24/2008
[ 2915.358534] RIP: 0010:btrfs_scan_one_device+0x127/0x180 [btrfs]
[ 2915.358537] RSP: 0018:ffffb35a4524be30 EFLAGS: 00010206
[ 2915.358541] RAX: fffffffffffffff0 RBX: 0000000000000081 RCX: 000000000000000f
[ 2915.358544] RDX: ffff96a791c7e10b RSI: 0000000000000001 RDI: ffff96a79f734200
[ 2915.358546] RBP: ffff96a7a2ea6000 R08: 000000000000002b R09: 0000000000000000
[ 2915.358548] R10: 0000000000000000 R11: 0000000000000004 R12: 00000000fffffff0
[ 2915.358550] R13: ffffb35a4524be60 R14: fffff07d48471f80 R15: 000055ee5775ad74
[ 2915.358554] FS:  00007f736b3648c0(0000) GS:ffff96a7a6a00000(0000) knlGS:0000000000000000
[ 2915.358556] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2915.358558] CR2: 0000000000000010 CR3: 000000021201e000 CR4: 00000000000006e0
[ 2915.358560] Call Trace:
[ 2915.358600]  btrfs_control_ioctl+0xad/0xe0 [btrfs]
[ 2915.358610]  ? trace_hardirqs_on_caller+0xf2/0x1a0
[ 2915.358618]  do_vfs_ioctl+0x90/0x6b0
[ 2915.358625]  ? __audit_syscall_entry+0xb5/0x110
[ 2915.358632]  ? syscall_trace_enter+0x1ae/0x360
[ 2915.358638]  ? return_from_SYSCALL_64+0x10/0x75
[ 2915.358643]  SyS_ioctl+0x74/0x80
[ 2915.358647]  ? do_syscall_64+0x1e/0x1a0
[ 2915.358653]  do_syscall_64+0x64/0x1a0
[ 2915.358659]  entry_SYSCALL64_slow_path+0x25/0x25
[ 2915.358663] RIP: 0033:0x7f736a1f3227
[ 2915.358665] RSP: 002b:00007fff9dcad618 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 2915.358669] RAX: ffffffffffffffda RBX: 00007fff9dcad630 RCX: 00007f736a1f3227
[ 2915.358670] RDX: 00007fff9dcad630 RSI: 0000000090009427 RDI: 000000000000000f
[ 2915.358672] RBP: 000000000000000f R08: 376264732f766564 R09: 0000000000000003
[ 2915.358674] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[ 2915.358676] R13: 000055ee59813e90 R14: 0000000000000000 R15: 000055ee5775ad74
[ 2915.358810] RIP: btrfs_scan_one_device+0x127/0x180 [btrfs] RSP: ffffb35a4524be30
[ 2915.358812] CR2: 0000000000000010
[ 2915.358970] ---[ end trace 900a4fff1ad9ece2 ]---
[ 2915.441581] BTRFS info (device sdb9): dev_replace from /dev/sdb8 (devid 5) to /dev/sdc6 started

Re: [PATCH RESEND v4 0/4] device_list_add() peparation to add reappearing missing device

[Anand Jain]

On 01/20/2018 07:27 AM, David Sterba wrote:
> On Thu, Jan 18, 2018 at 06:47:17PM +0100, David Sterba wrote:
>> On Thu, Jan 18, 2018 at 10:02:32PM +0800, Anand Jain wrote:
>>> (Apply on top of my patchset
>>>     [PATCH v4 0/6] preparatory work to add device forget
>>>   for conflict free apply. They don't actually depend on
>>>   each other though).
>>
>>> Cleanup of device_list_add(), mainly in preparation to handle
>>> reappearing missing device which its next reroll will be sent
>>> separately.
>>
>> I'm adding the two patchsets to the 4.16 queue but will push the updated
>> branch after the current tests finish and I also test the updated branch
>> as well.
> 
> So this did not survive the first fstests run, I'm going to move the patchset
> to the 4.17 dev queue.
> 
> [ 2912.493351] run fstests btrfs/064 at 2018-01-19 20:55:50
> [ 2914.218654] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 1 transid 5 /dev/sdb6
> [ 2914.261560] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 2 transid 5 /dev/sdc5
> [ 2914.296819] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 3 transid 5 /dev/sdb7
> [ 2914.348140] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 4 transid 5 /dev/sdc6
> [ 2914.389368] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 5 transid 5 /dev/sdb8
> [ 2914.425378] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 6 transid 5 /dev/sdc7
> [ 2914.443497] BTRFS: device fsid ee7e811a-fdb3-42e9-8a81-5ed8e1a4282b devid 7 transid 5 /dev/sdb9
> [ 2914.488145] BTRFS info (device sdb9): disk space caching is enabled
> [ 2914.494744] BTRFS info (device sdb9): has skinny extents
> [ 2914.500328] BTRFS info (device sdb9): flagging fs with big metadata feature
> [ 2914.514809] BTRFS info (device sdb9): enabling ssd optimizations
> [ 2914.522114] BTRFS info (device sdb9): creating UUID tree
> [ 2914.716867] BTRFS info (device sdb9): dev_replace from /dev/sdc5 (devid 2) to /dev/sdc8 started
> [ 2914.852699] BTRFS info (device sdb9): dev_replace from /dev/sdc5 (devid 2) to /dev/sdc8 finished
> [ 2915.028666] BTRFS info (device sdb9): dev_replace from /dev/sdb7 (devid 3) to /dev/sdc5 started
> [ 2915.110374] BTRFS info (device sdb9): dev_replace from /dev/sdb7 (devid 3) to /dev/sdc5 finished
> [ 2915.309674] BTRFS info (device sdb9): dev_replace from /dev/sdc6 (devid 4) to /dev/sdb7 started
> [ 2915.340819] BTRFS info (device sdb9): dev_replace from /dev/sdc6 (devid 4) to /dev/sdb7 finished
> [ 2915.350220] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
> [ 2915.358350] IP: btrfs_scan_one_device+0x127/0x180 [btrfs]
> [ 2915.358353] PGD 0 P4D 0
> [ 2915.358366] Oops: 0000 [#1] PREEMPT SMP
> [ 2915.358493] CPU: 2 PID: 1076 Comm: systemd-udevd Tainted: G        W        4.15.0-rc8-1.ge195904-vanilla+ #128
> [ 2915.358495] Hardware name: empty empty/S3993, BIOS PAQEX0-3 02/24/2008
> [ 2915.358534] RIP: 0010:btrfs_scan_one_device+0x127/0x180 [btrfs]

  I couldn't reproduce with btrfs/064 which ran for several iterations.
  But a script [1] could trigger the problem.

  [1]
---
  mkfs.btrfs -fq -draid1 -mraid1 /dev/sdb /dev/sdc
  modprobe -r btrfs
  mount -o degraded /dev/sdb /btrfs
  btrfs repl start -Bf 2 /dev/sdd /btrfs
  umount /btrfs
  modprobe -r btrfs
  btrfs dev scan
  btrfs dev scan /dev/sdc
---

  Problem was mainly due to the patch 3/4, which tried to access the
  return pointer even for the failed condition. The fix is to bring the
  device point access under the else part as show below [2]. I have
  included this fix in V5. Which is tested with btrfs xfstests.
  Pls could you consider v5 for 4.16 ?

[2]
-----
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 462bae3627e3..a86c3a14ec89 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1214,8 +1214,8 @@ int btrfs_scan_one_device(const char *path, 
fmode_t flags, void *holder,
         mutex_unlock(&uuid_mutex);
         if (IS_ERR(device))
                 ret = PTR_ERR(device);
-
-       *fs_devices_ret = device->fs_devices;
+       else
+               *fs_devices_ret = device->fs_devices;

         btrfs_release_disk_super(page);
------


Thanks, Anand

> [ 2915.358537] RSP: 0018:ffffb35a4524be30 EFLAGS: 00010206
> [ 2915.358541] RAX: fffffffffffffff0 RBX: 0000000000000081 RCX: 000000000000000f
> [ 2915.358544] RDX: ffff96a791c7e10b RSI: 0000000000000001 RDI: ffff96a79f734200
> [ 2915.358546] RBP: ffff96a7a2ea6000 R08: 000000000000002b R09: 0000000000000000
> [ 2915.358548] R10: 0000000000000000 R11: 0000000000000004 R12: 00000000fffffff0
> [ 2915.358550] R13: ffffb35a4524be60 R14: fffff07d48471f80 R15: 000055ee5775ad74
> [ 2915.358554] FS:  00007f736b3648c0(0000) GS:ffff96a7a6a00000(0000) knlGS:0000000000000000
> [ 2915.358556] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 2915.358558] CR2: 0000000000000010 CR3: 000000021201e000 CR4: 00000000000006e0
> [ 2915.358560] Call Trace:
> [ 2915.358600]  btrfs_control_ioctl+0xad/0xe0 [btrfs]
> [ 2915.358610]  ? trace_hardirqs_on_caller+0xf2/0x1a0
> [ 2915.358618]  do_vfs_ioctl+0x90/0x6b0
> [ 2915.358625]  ? __audit_syscall_entry+0xb5/0x110
> [ 2915.358632]  ? syscall_trace_enter+0x1ae/0x360
> [ 2915.358638]  ? return_from_SYSCALL_64+0x10/0x75
> [ 2915.358643]  SyS_ioctl+0x74/0x80
> [ 2915.358647]  ? do_syscall_64+0x1e/0x1a0
> [ 2915.358653]  do_syscall_64+0x64/0x1a0
> [ 2915.358659]  entry_SYSCALL64_slow_path+0x25/0x25
> [ 2915.358663] RIP: 0033:0x7f736a1f3227
> [ 2915.358665] RSP: 002b:00007fff9dcad618 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [ 2915.358669] RAX: ffffffffffffffda RBX: 00007fff9dcad630 RCX: 00007f736a1f3227
> [ 2915.358670] RDX: 00007fff9dcad630 RSI: 0000000090009427 RDI: 000000000000000f
> [ 2915.358672] RBP: 000000000000000f R08: 376264732f766564 R09: 0000000000000003
> [ 2915.358674] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> [ 2915.358676] R13: 000055ee59813e90 R14: 0000000000000000 R15: 000055ee5775ad74
> [ 2915.358810] RIP: btrfs_scan_one_device+0x127/0x180 [btrfs] RSP: ffffb35a4524be30
> [ 2915.358812] CR2: 0000000000000010
> [ 2915.358970] ---[ end trace 900a4fff1ad9ece2 ]---
> [ 2915.441581] BTRFS info (device sdb9): dev_replace from /dev/sdb8 (devid 5) to /dev/sdc6 started



> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH RESEND v4 0/4] device_list_add() peparation to add reappearing missing device

[David Sterba]

On Mon, Jan 22, 2018 at 09:31:47PM +0800, Anand Jain wrote:
>   Problem was mainly due to the patch 3/4, which tried to access the
>   return pointer even for the failed condition. The fix is to bring the
>   device point access under the else part as show below [2]. I have
>   included this fix in V5. Which is tested with btrfs xfstests.
>   Pls could you consider v5 for 4.16 ?

Hm ok, thre's still some time to test it. One more fstests report that
appeared before and also with the v5:

btrfs/007 4s ...        [16:38:09] [16:38:12] [failed, exit status 1] - output mismatch (see /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/007.out.bad)
    --- tests/btrfs/007.out     2017-09-20 14:24:58.334716658 +0200 
    +++ /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/007.out.bad       2018-01-22 16:38:12.883931593 +0100
    @@ -1,4 +1,5 @@
     QA output created by 007
     *** test send / receive
    -*** done
    +failed: '/root/test/mmtests/work/sources/xfstests-git-installed/src/fssum -r /tmp/tmp.eZcr17wqNn/incr.fssum /root/test/mmtests/scratch_mnt/incr'
    +(see /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/007.full for details)
     *** unmount
    ...
    (Run 'diff -u tests/btrfs/007.out /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/007.out.bad'  to see the entire diff)

Re: [PATCH RESEND v4 0/4] device_list_add() peparation to add reappearing missing device

[Anand Jain]

On 01/22/2018 11:26 PM, David Sterba wrote:
> On Mon, Jan 22, 2018 at 09:31:47PM +0800, Anand Jain wrote:
>>    Problem was mainly due to the patch 3/4, which tried to access the
>>    return pointer even for the failed condition. The fix is to bring the
>>    device point access under the else part as show below [2]. I have
>>    included this fix in V5. Which is tested with btrfs xfstests.
>>    Pls could you consider v5 for 4.16 ?
> 
> Hm ok, thre's still some time to test it. One more fstests report that
> appeared before and also with the v5:

  I will try to nail it down. It passes on bare metal and a VM here.
  btrfs-progs: I am using your latest master at
     (git://github.com/kdave/btrfs-progs.git).


> btrfs/007 4s ...        [16:38:09] [16:38:12] [failed, exit status 1] - output mismatch (see /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/007.out.bad)
>      --- tests/btrfs/007.out     2017-09-20 14:24:58.334716658 +0200
>      +++ /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/007.out.bad       2018-01-22 16:38:12.883931593 +0100
>      @@ -1,4 +1,5 @@
>       QA output created by 007
>       *** test send / receive
>      -*** done
>      +failed: '/root/test/mmtests/work/sources/xfstests-git-installed/src/fssum -r /tmp/tmp.eZcr17wqNn/incr.fssum /root/test/mmtests/scratch_mnt/incr'

  Looks like fssum on the reverse copied file failed.

>      +(see /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/007.full for details)

  Can you pls send me this ?

Thanks, Anand

>       *** unmount
>      ...
>      (Run 'diff -u tests/btrfs/007.out /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/007.out.bad'  to see the entire diff)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH RESEND v4 0/4] device_list_add() peparation to add reappearing missing device

[Gu, Jinxiang]

[-- Attachment #1: Type: text/plain, Size: 2890 bytes --]

Hi,

I reproduced this using kernel v4.17-rc1.
It is not always happens.( occurred times/test times: 1/20)

> -----Original Message-----
> From: linux-btrfs-owner@vger.kernel.org [mailto:linux-btrfs-owner@vger.kernel.org] On Behalf Of Anand Jain
> Sent: Tuesday, January 23, 2018 5:53 AM
> To: dsterba@suse.cz; linux-btrfs@vger.kernel.org
> Subject: Re: [PATCH RESEND v4 0/4] device_list_add() peparation to add reappearing missing device
> 
> 
> 
> On 01/22/2018 11:26 PM, David Sterba wrote:
> > On Mon, Jan 22, 2018 at 09:31:47PM +0800, Anand Jain wrote:
> >>    Problem was mainly due to the patch 3/4, which tried to access the
> >>    return pointer even for the failed condition. The fix is to bring the
> >>    device point access under the else part as show below [2]. I have
> >>    included this fix in V5. Which is tested with btrfs xfstests.
> >>    Pls could you consider v5 for 4.16 ?
> >
> > Hm ok, thre's still some time to test it. One more fstests report that
> > appeared before and also with the v5:
> 
>   I will try to nail it down. It passes on bare metal and a VM here.
>   btrfs-progs: I am using your latest master at
>      (git://github.com/kdave/btrfs-progs.git).
> 
> 
> > btrfs/007 4s ...        [16:38:09] [16:38:12] [failed, exit status 1] - output mismatch (see
> /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/007.out.bad)
> >      --- tests/btrfs/007.out     2017-09-20 14:24:58.334716658 +0200
> >      +++ /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/007.out.bad       2018-01-22 16:38:12.883931593
> +0100
> >      @@ -1,4 +1,5 @@
> >       QA output created by 007
> >       *** test send / receive
> >      -*** done
> >      +failed: '/root/test/mmtests/work/sources/xfstests-git-installed/src/fssum -r /tmp/tmp.eZcr17wqNn/incr.fssum
> /root/test/mmtests/scratch_mnt/incr'
> 
>   Looks like fssum on the reverse copied file failed.
> 
> >      +(see
> > /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/
> > 007.full for details)
> 
>   Can you pls send me this ?
Please see the attachment.

And I confirmed btrfs/007 using kernel v4.16-rc1.
It also occurred sometimes
> 
> Thanks, Anand
> 
> >       *** unmount
> >      ...
> >      (Run 'diff -u tests/btrfs/007.out
> > /root/test/mmtests/work/sources/xfstests-git-installed/results//btrfs/
> > 007.out.bad'  to see the entire diff)
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
> > in the body of a message to majordomo@vger.kernel.org More majordomo
> > info at  http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More
> majordomo info at  http://vger.kernel.org/majordomo-info.html
> 




[-- Attachment #2: 007.full --]
[-- Type: application/octet-stream, Size: 2408 bytes --]

*** mkfs -dsize=2097152000

btrfs-progs v4.16
See http://btrfs.wiki.kernel.org for more information.

Label:              (null)
UUID:               2b8b8abe-0706-42e0-8d2b-76a22473ec71
Node size:          16384
Sector size:        4096
Filesystem size:    1.95GiB
Block group profiles:
  Data:             single            8.00MiB
  Metadata:         DUP             100.00MiB
  System:           DUP               8.00MiB
SSD detected:       no
Incompat features:  extref, skinny-metadata
Number of devices:  1
Devices:
   ID        SIZE  PATH
    1     1.95GiB  /dev/vdd1

# ./ltp/fsstress -d /mnt/scratch -n 200 -x /usr/local/bin/btrfs subvolume snapshot -r /mnt/scratch /mnt/scratch/base
seed = 1524346151
Create a readonly snapshot of '/mnt/scratch' in '/mnt/scratch/base'
# /usr/local/bin/btrfs subvolume snapshot -r /mnt/scratch /mnt/scratch/incr
Create a readonly snapshot of '/mnt/scratch' in '/mnt/scratch/incr'
# /usr/local/bin/btrfs send /mnt/scratch/base > /tmp/tmp.SkEUIXw683/base.snap
At subvol /mnt/scratch/base
# /usr/local/bin/btrfs send -p /mnt/scratch/base		/mnt/scratch/incr > /tmp/tmp.SkEUIXw683/incr.snap
At subvol /mnt/scratch/incr
# /home/gujx/xfstests-dev/src/fssum -A -f -w /tmp/tmp.SkEUIXw683/base.fssum /mnt/scratch/base
# /home/gujx/xfstests-dev/src/fssum -A -f -w /tmp/tmp.SkEUIXw683/incr.fssum -x /mnt/scratch/incr/base /mnt/scratch/incr
*** mkfs -dsize=2097152000

btrfs-progs v4.16
See http://btrfs.wiki.kernel.org for more information.

Label:              (null)
UUID:               03ea1d49-b093-4535-ae14-dab448a2e6ae
Node size:          16384
Sector size:        4096
Filesystem size:    1.95GiB
Block group profiles:
  Data:             single            8.00MiB
  Metadata:         DUP             100.00MiB
  System:           DUP               8.00MiB
SSD detected:       no
Incompat features:  extref, skinny-metadata
Number of devices:  1
Devices:
   ID        SIZE  PATH
    1     1.95GiB  /dev/vdd1

# /usr/local/bin/btrfs receive /mnt/scratch
At subvol base
# /home/gujx/xfstests-dev/src/fssum -r /tmp/tmp.SkEUIXw683/base.fssum /mnt/scratch/base
OK
# /usr/local/bin/btrfs receive /mnt/scratch
At snapshot incr
# /home/gujx/xfstests-dev/src/fssum -r /tmp/tmp.SkEUIXw683/incr.fssum /mnt/scratch/incr
metadata and data mismatch in /p0/f0
data mismatch in /p0/
FAIL
failed: '/home/gujx/xfstests-dev/src/fssum -r /tmp/tmp.SkEUIXw683/incr.fssum /mnt/scratch/incr'

Re: [PATCH RESEND v4 0/4] device_list_add() peparation to add reappearing missing device

[Anand Jain]

On 04/20/2018 02:22 PM, Gu, Jinxiang wrote:
> Hi,
> 
> I reproduced this using kernel v4.17-rc1.
> It is not always happens.( occurred times/test times: 1/20)

  Though it was reported here, its not related to this patch set.
  Instead its about the send receive.
  Pls post it as a separate thread so it can be followed up properly.

Thanks, Anand