Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v4 0/2] remove BUG_ON()s in btrfs_close_one_device()
@ 2019-11-26  8:40 Johannes Thumshirn
  2019-11-26  8:40 ` [PATCH v4 1/2] btrfs: decrement number of open devices after closing the device not before Johannes Thumshirn
  2019-11-26  8:40 ` [PATCH v4 2/2] btrfs: reset device back to allocation state when removing Johannes Thumshirn
  0 siblings, 2 replies; 8+ messages in thread
From: Johannes Thumshirn @ 2019-11-26  8:40 UTC (permalink / raw)
  To: David Sterba
  Cc: Nikolay Borisov, Qu Wenruo, Linux BTRFS Mailinglist, Johannes Thumshirn

This series attempts to remove the BUG_ON()s in btrfs_close_one_device().
Therefore some reorganization of btrfs_close_one_device() was needed, to
avoid the memory allocation.

This series has passed fstests without any deviation from the baseline.

Changes to v3:
- Clear BTRFS_DEV_STATE_WRITEABLE after calling btrfs_close_bdev() so
  btrfs_close_bdev() can call sync_blockdev() and invalidate_bdev() (Nikolay)

Changes to v2:
- Completly different approach to the origianl patchset, instead of handling
  eventual allocation failures.
- Dropped already merged patches for ' btrfs_fs_devices::rotating' and
  'btrfs_fs_devices::seeding'
- Kept the 1st patch of the old series, as it's a nice cleanup

Changes to v1:
- Fixed the decremt of btrfs_fs_devices::seeding.
- In addition to this, I've added two patches changing btrfs_fs_devices::seeding
  and btrfs_fs_devices::rotating to bool, as they are in fact used as booleans.

Johannes Thumshirn (2):
  btrfs: decrement number of open devices after closing the device not
    before
  btrfs: reset device back to allocation state when removing

 fs/btrfs/volumes.c | 41 ++++++++++++++++++-----------------------
 1 file changed, 18 insertions(+), 23 deletions(-)

-- 
2.16.4


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

* [PATCH v4 1/2] btrfs: decrement number of open devices after closing the device not before
  2019-11-26  8:40 [PATCH v4 0/2] remove BUG_ON()s in btrfs_close_one_device() Johannes Thumshirn
@ 2019-11-26  8:40 ` Johannes Thumshirn
  2019-11-26  8:40 ` [PATCH v4 2/2] btrfs: reset device back to allocation state when removing Johannes Thumshirn
  1 sibling, 0 replies; 8+ messages in thread
From: Johannes Thumshirn @ 2019-11-26  8:40 UTC (permalink / raw)
  To: David Sterba
  Cc: Nikolay Borisov, Qu Wenruo, Linux BTRFS Mailinglist, Johannes Thumshirn

In btrfs_close_one_device we're decrementing the number of open devices
before we're calling btrfs_close_bdev().

As there is no intermediate exit between these points in this function it
is technically OK to do so, but it makes the code a bit harder to understand.

Move both operations closer together and move the decrement step after
btrfs_close_bdev().

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/volumes.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d8e5560db285..2398b071bcf6 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1067,9 +1067,6 @@ static void btrfs_close_one_device(struct btrfs_device *device)
 	struct btrfs_device *new_device;
 	struct rcu_string *name;
 
-	if (device->bdev)
-		fs_devices->open_devices--;
-
 	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
 	    device->devid != BTRFS_DEV_REPLACE_DEVID) {
 		list_del_init(&device->dev_alloc_list);
@@ -1080,6 +1077,8 @@ static void btrfs_close_one_device(struct btrfs_device *device)
 		fs_devices->missing_devices--;
 
 	btrfs_close_bdev(device);
+	if (device->bdev)
+		fs_devices->open_devices--;
 
 	new_device = btrfs_alloc_device(NULL, &device->devid,
 					device->uuid);
-- 
2.16.4


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

* [PATCH v4 2/2] btrfs: reset device back to allocation state when removing
  2019-11-26  8:40 [PATCH v4 0/2] remove BUG_ON()s in btrfs_close_one_device() Johannes Thumshirn
  2019-11-26  8:40 ` [PATCH v4 1/2] btrfs: decrement number of open devices after closing the device not before Johannes Thumshirn
@ 2019-11-26  8:40 ` Johannes Thumshirn
  2019-11-26 10:21   ` Nikolay Borisov
                     ` (2 more replies)
  1 sibling, 3 replies; 8+ messages in thread
From: Johannes Thumshirn @ 2019-11-26  8:40 UTC (permalink / raw)
  To: David Sterba
  Cc: Nikolay Borisov, Qu Wenruo, Linux BTRFS Mailinglist, Johannes Thumshirn

When closing a device, btrfs_close_one_device() first allocates a new
device, copies the device to close's name, replaces it in the dev_list
with the copy and then finally frees it.

This involves two memory allocation, which can potentially fail. As this
code path is tricky to unwind, the allocation failures where handled by
BUG_ON()s.

But this copying isn't strictly needed, all that is needed is resetting
the device in question to it's state it had after the allocation.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

---
Changes to v3:
- Clear DEV_STATE_WRITABLE _after_ btrfs_close_bdev() (Nik)
---
 fs/btrfs/volumes.c | 38 +++++++++++++++++---------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 2398b071bcf6..efabffa54a45 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1064,8 +1064,6 @@ static void btrfs_close_bdev(struct btrfs_device *device)
 static void btrfs_close_one_device(struct btrfs_device *device)
 {
 	struct btrfs_fs_devices *fs_devices = device->fs_devices;
-	struct btrfs_device *new_device;
-	struct rcu_string *name;
 
 	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
 	    device->devid != BTRFS_DEV_REPLACE_DEVID) {
@@ -1073,29 +1071,27 @@ static void btrfs_close_one_device(struct btrfs_device *device)
 		fs_devices->rw_devices--;
 	}
 
-	if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
+	if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state)) {
 		fs_devices->missing_devices--;
+		clear_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state);
+	}
 
+	clear_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
 	btrfs_close_bdev(device);
-	if (device->bdev)
+	if (device->bdev) {
 		fs_devices->open_devices--;
-
-	new_device = btrfs_alloc_device(NULL, &device->devid,
-					device->uuid);
-	BUG_ON(IS_ERR(new_device)); /* -ENOMEM */
-
-	/* Safe because we are under uuid_mutex */
-	if (device->name) {
-		name = rcu_string_strdup(device->name->str, GFP_NOFS);
-		BUG_ON(!name); /* -ENOMEM */
-		rcu_assign_pointer(new_device->name, name);
-	}
-
-	list_replace_rcu(&device->dev_list, &new_device->dev_list);
-	new_device->fs_devices = device->fs_devices;
-
-	synchronize_rcu();
-	btrfs_free_device(device);
+		device->bdev = NULL;
+	}
+	clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
+
+	/* Verify the device is back in a pristine state  */
+	ASSERT(!test_bit(BTRFS_DEV_STATE_FLUSH_SENT, &device->dev_state));
+	ASSERT(!test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state));
+	ASSERT(list_empty(&device->dev_alloc_list));
+	ASSERT(list_empty(&device->post_commit_list));
+	ASSERT(atomic_read(&device->reada_in_flight) == 0);
+	ASSERT(atomic_read(&device->dev_stats_ccnt) == 0);
+	ASSERT(RB_EMPTY_ROOT(&device->alloc_state.state));
 }
 
 static int close_fs_devices(struct btrfs_fs_devices *fs_devices)
-- 
2.16.4


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

* Re: [PATCH v4 2/2] btrfs: reset device back to allocation state when removing
  2019-11-26  8:40 ` [PATCH v4 2/2] btrfs: reset device back to allocation state when removing Johannes Thumshirn
@ 2019-11-26 10:21   ` Nikolay Borisov
  2019-11-26 17:17   ` David Sterba
  2019-11-27 17:07   ` David Sterba
  2 siblings, 0 replies; 8+ messages in thread
From: Nikolay Borisov @ 2019-11-26 10:21 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Qu Wenruo, Linux BTRFS Mailinglist



On 26.11.19 г. 10:40 ч., Johannes Thumshirn wrote:
> When closing a device, btrfs_close_one_device() first allocates a new
> device, copies the device to close's name, replaces it in the dev_list
> with the copy and then finally frees it.
> 
> This involves two memory allocation, which can potentially fail. As this
> code path is tricky to unwind, the allocation failures where handled by
> BUG_ON()s.
> 
> But this copying isn't strictly needed, all that is needed is resetting
> the device in question to it's state it had after the allocation.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

Overall looks good one minor nit which can be fixed at commit time (or
ignored).

Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> 
> ---
> Changes to v3:
> - Clear DEV_STATE_WRITABLE _after_ btrfs_close_bdev() (Nik)
> ---
>  fs/btrfs/volumes.c | 38 +++++++++++++++++---------------------
>  1 file changed, 17 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 2398b071bcf6..efabffa54a45 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1064,8 +1064,6 @@ static void btrfs_close_bdev(struct btrfs_device *device)
>  static void btrfs_close_one_device(struct btrfs_device *device)
>  {
>  	struct btrfs_fs_devices *fs_devices = device->fs_devices;
> -	struct btrfs_device *new_device;
> -	struct rcu_string *name;
>  
>  	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
>  	    device->devid != BTRFS_DEV_REPLACE_DEVID) {
> @@ -1073,29 +1071,27 @@ static void btrfs_close_one_device(struct btrfs_device *device)
>  		fs_devices->rw_devices--;
>  	}
>  
> -	if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
> +	if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state)) {
>  		fs_devices->missing_devices--;
> +		clear_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state);
> +	}
nit: you can use test_and_clear_bit

>  
> +	clear_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
>  	btrfs_close_bdev(device);
> -	if (device->bdev)
> +	if (device->bdev) {
>  		fs_devices->open_devices--;
> -
> -	new_device = btrfs_alloc_device(NULL, &device->devid,
> -					device->uuid);
> -	BUG_ON(IS_ERR(new_device)); /* -ENOMEM */
> -
> -	/* Safe because we are under uuid_mutex */
> -	if (device->name) {
> -		name = rcu_string_strdup(device->name->str, GFP_NOFS);
> -		BUG_ON(!name); /* -ENOMEM */
> -		rcu_assign_pointer(new_device->name, name);
> -	}
> -
> -	list_replace_rcu(&device->dev_list, &new_device->dev_list);
> -	new_device->fs_devices = device->fs_devices;
> -
> -	synchronize_rcu();
> -	btrfs_free_device(device);
> +		device->bdev = NULL;
> +	}
> +	clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
> +
> +	/* Verify the device is back in a pristine state  */
> +	ASSERT(!test_bit(BTRFS_DEV_STATE_FLUSH_SENT, &device->dev_state));
> +	ASSERT(!test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state));
> +	ASSERT(list_empty(&device->dev_alloc_list));
> +	ASSERT(list_empty(&device->post_commit_list));
> +	ASSERT(atomic_read(&device->reada_in_flight) == 0);
> +	ASSERT(atomic_read(&device->dev_stats_ccnt) == 0);
> +	ASSERT(RB_EMPTY_ROOT(&device->alloc_state.state));
>  }
>  
>  static int close_fs_devices(struct btrfs_fs_devices *fs_devices)
> 

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

* Re: [PATCH v4 2/2] btrfs: reset device back to allocation state when removing
  2019-11-26  8:40 ` [PATCH v4 2/2] btrfs: reset device back to allocation state when removing Johannes Thumshirn
  2019-11-26 10:21   ` Nikolay Borisov
@ 2019-11-26 17:17   ` David Sterba
  2019-11-27 15:26     ` David Sterba
  2019-11-27 17:07   ` David Sterba
  2 siblings, 1 reply; 8+ messages in thread
From: David Sterba @ 2019-11-26 17:17 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: David Sterba, Nikolay Borisov, Qu Wenruo, Linux BTRFS Mailinglist

On Tue, Nov 26, 2019 at 09:40:06AM +0100, Johannes Thumshirn wrote:
> When closing a device, btrfs_close_one_device() first allocates a new
> device, copies the device to close's name, replaces it in the dev_list
> with the copy and then finally frees it.
> 
> This involves two memory allocation, which can potentially fail. As this
> code path is tricky to unwind, the allocation failures where handled by
> BUG_ON()s.
> 
> But this copying isn't strictly needed, all that is needed is resetting
> the device in question to it's state it had after the allocation.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> 
> ---
> Changes to v3:
> - Clear DEV_STATE_WRITABLE _after_ btrfs_close_bdev() (Nik)
> ---
>  fs/btrfs/volumes.c | 38 +++++++++++++++++---------------------
>  1 file changed, 17 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 2398b071bcf6..efabffa54a45 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1064,8 +1064,6 @@ static void btrfs_close_bdev(struct btrfs_device *device)
>  static void btrfs_close_one_device(struct btrfs_device *device)
>  {
>  	struct btrfs_fs_devices *fs_devices = device->fs_devices;
> -	struct btrfs_device *new_device;
> -	struct rcu_string *name;
>  
>  	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
>  	    device->devid != BTRFS_DEV_REPLACE_DEVID) {
> @@ -1073,29 +1071,27 @@ static void btrfs_close_one_device(struct btrfs_device *device)
>  		fs_devices->rw_devices--;
>  	}
>  
> -	if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
> +	if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state)) {
>  		fs_devices->missing_devices--;
> +		clear_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state);
> +	}
>  
> +	clear_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
>  	btrfs_close_bdev(device);
> -	if (device->bdev)
> +	if (device->bdev) {
>  		fs_devices->open_devices--;
> -
> -	new_device = btrfs_alloc_device(NULL, &device->devid,
> -					device->uuid);
> -	BUG_ON(IS_ERR(new_device)); /* -ENOMEM */
> -
> -	/* Safe because we are under uuid_mutex */
> -	if (device->name) {
> -		name = rcu_string_strdup(device->name->str, GFP_NOFS);
> -		BUG_ON(!name); /* -ENOMEM */
> -		rcu_assign_pointer(new_device->name, name);
> -	}
> -
> -	list_replace_rcu(&device->dev_list, &new_device->dev_list);
> -	new_device->fs_devices = device->fs_devices;
> -
> -	synchronize_rcu();

The changelong should say something about RCU as this is being changed
here.

RCU was for the dev_list and and device name, with some read-only list
traversal that can run in parallel (like FS_INFO or DEV_INFO ioctls).
This is safe because the device list hook is not removed and the name is
not changed.

> -	btrfs_free_device(device);
> +		device->bdev = NULL;
> +	}
> +	clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
> +
> +	/* Verify the device is back in a pristine state  */
> +	ASSERT(!test_bit(BTRFS_DEV_STATE_FLUSH_SENT, &device->dev_state));
> +	ASSERT(!test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state));
> +	ASSERT(list_empty(&device->dev_alloc_list));
> +	ASSERT(list_empty(&device->post_commit_list));
> +	ASSERT(atomic_read(&device->reada_in_flight) == 0);
> +	ASSERT(atomic_read(&device->dev_stats_ccnt) == 0);
> +	ASSERT(RB_EMPTY_ROOT(&device->alloc_state.state));

I went through members of the device struct, lots of them are set once
so don't change. last_flush_error is set and read during commit,

Besides the dev_state bits handled above, I think tre rest should be
here too, ie.  BTRFS_DEV_STATE_IN_FS_METADATA and
BTRFS_DEV_STATE_MISSING (though this might be ok to keep as-is).

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

* Re: [PATCH v4 2/2] btrfs: reset device back to allocation state when removing
  2019-11-26 17:17   ` David Sterba
@ 2019-11-27 15:26     ` David Sterba
  0 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2019-11-27 15:26 UTC (permalink / raw)
  To: dsterba, Johannes Thumshirn, David Sterba, Nikolay Borisov,
	Qu Wenruo, Linux BTRFS Mailinglist

On Tue, Nov 26, 2019 at 06:17:03PM +0100, David Sterba wrote:
> > +	/* Verify the device is back in a pristine state  */
> > +	ASSERT(!test_bit(BTRFS_DEV_STATE_FLUSH_SENT, &device->dev_state));
> > +	ASSERT(!test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state));
> > +	ASSERT(list_empty(&device->dev_alloc_list));
> > +	ASSERT(list_empty(&device->post_commit_list));
> > +	ASSERT(atomic_read(&device->reada_in_flight) == 0);
> > +	ASSERT(atomic_read(&device->dev_stats_ccnt) == 0);
> > +	ASSERT(RB_EMPTY_ROOT(&device->alloc_state.state));
> 
> I went through members of the device struct, lots of them are set once
> so don't change. last_flush_error is set and read during commit,
> 
> Besides the dev_state bits handled above, I think tre rest should be
> here too, ie.  BTRFS_DEV_STATE_IN_FS_METADATA and
> BTRFS_DEV_STATE_MISSING (though this might be ok to keep as-is).

So BTRFS_DEV_STATE_MISSING should stay, the state is changed through the
scanning.

BTRFS_DEV_STATE_IN_FS_METADATA should be asserted for 'not-set', this is
normally set_bit at mount time so the last use of devices with the bit
set should set it back to zero.

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

* Re: [PATCH v4 2/2] btrfs: reset device back to allocation state when removing
  2019-11-26  8:40 ` [PATCH v4 2/2] btrfs: reset device back to allocation state when removing Johannes Thumshirn
  2019-11-26 10:21   ` Nikolay Borisov
  2019-11-26 17:17   ` David Sterba
@ 2019-11-27 17:07   ` David Sterba
  2019-11-28  9:29     ` Johannes Thumshirn
  2 siblings, 1 reply; 8+ messages in thread
From: David Sterba @ 2019-11-27 17:07 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: David Sterba, Nikolay Borisov, Qu Wenruo, Linux BTRFS Mailinglist

On Tue, Nov 26, 2019 at 09:40:06AM +0100, Johannes Thumshirn wrote:
> +	ASSERT(atomic_read(&device->dev_stats_ccnt) == 0);

btrfs/020               [16:59:58][ 3477.975072] run fstests btrfs/020 at 2019-11-27 16:59:58
[ 3478.974314] kworker/dying (5607) used greatest stack depth: 10792 bytes left
[ 3479.206988] BTRFS: device fsid 818a4909-467d-4599-979c-3b258fb4fc41 devid 1 transid 5 /dev/loop0 scanned by mkfs.btrfs (27347)
[ 3479.234212] BTRFS: device fsid 818a4909-467d-4599-979c-3b258fb4fc41 devid 2 transid 5 /dev/loop1 scanned by mkfs.btrfs (27347)
[ 3479.343996] BTRFS info (device loop0): disk space caching is enabled
[ 3479.349590] BTRFS info (device loop0): has skinny extents
[ 3479.352721] BTRFS info (device loop0): flagging fs with big metadata feature
[ 3479.360793] BTRFS info (device loop0): enabling ssd optimizations
[ 3479.614065] assertion failed: atomic_read(&device->dev_stats_ccnt) == 0, in fs/btrfs/volumes.c:1093
[ 3479.622041] ------------[ cut here ]------------
[ 3479.625272] kernel BUG at fs/btrfs/ctree.h:3118!
[ 3479.628555] invalid opcode: 0000 [#1] SMP
[ 3479.631350] CPU: 1 PID: 27375 Comm: umount Not tainted 5.4.0-rc8-default+ #882
[ 3479.633838] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58-rebuilt.opensuse.org 04/01/2014
[ 3479.637681] RIP: 0010:assfail.constprop.0+0x18/0x26 [btrfs]
[ 3479.646004] RSP: 0018:ffff9f4444a6fd98 EFLAGS: 00010246
[ 3479.647947] RAX: 0000000000000057 RBX: ffff9e0e6bb64c00 RCX: 0000000000000006
[ 3479.650343] RDX: 0000000000000000 RSI: ffff9e0e71352408 RDI: ffff9e0e71351bc0
[ 3479.652838] RBP: ffff9e0e6bb64c60 R08: 0000032a29a25ac1 R09: 0000000000000000
[ 3479.654766] R10: 0000000000000000 R11: 0000000000000000 R12: ffff9e0e746e0200
[ 3479.656866] R13: ffff9e0e746e0200 R14: ffff9e0e746e0318 R15: ffff9e0e6bb61000
[ 3479.658852] FS:  00007fabe5d8a800(0000) GS:ffff9e0e7d800000(0000) knlGS:0000000000000000
[ 3479.661542] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3479.663642] CR2: 00007ffeb8101f68 CR3: 000000004e979002 CR4: 0000000000160ee0
[ 3479.666132] Call Trace:
[ 3479.667333]  close_fs_devices.cold+0x66/0x77 [btrfs]
[ 3479.669143]  btrfs_close_devices+0x22/0x90 [btrfs]
[ 3479.670733]  close_ctree+0x2b9/0x32c [btrfs]
[ 3479.672296]  generic_shutdown_super+0x69/0x100
[ 3479.673662]  kill_anon_super+0x14/0x30
[ 3479.675045]  btrfs_kill_super+0x12/0xa0 [btrfs]
[ 3479.676550]  deactivate_locked_super+0x2c/0x70
[ 3479.678056]  cleanup_mnt+0x100/0x160
[ 3479.679245]  task_work_run+0x90/0xc0
[ 3479.680582]  exit_to_usermode_loop+0x96/0xa0
[ 3479.681945]  do_syscall_64+0x19d/0x1f0
[ 3479.683243]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 3479.684908] RIP: 0033:0x7fabe5fce4e7
[ 3479.691687] RSP: 002b:00007ffeb81037c8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
[ 3479.694285] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00007fabe5fce4e7
[ 3479.696603] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 000056236d27cb80
[ 3479.698989] RBP: 000056236d27c970 R08: 0000000000000000 R09: 00007ffeb8102540
[ 3479.701220] R10: 000056236d27cba0 R11: 0000000000000246 R12: 000056236d27cb80
[ 3479.703416] R13: 0000000000000000 R14: 000056236d27ca68 R15: 0000000000000000
[ 3479.705272] Modules linked in: xxhash_generic btrfs libcrc32c crc32c_intel xor zstd_decompress zstd_compress xxhash lzo_compress lzo_decompress raid6_pq loop
[ 3479.709315] ---[ end trace 6a6ab278b1509a6f ]---
[ 3479.710971] RIP: 0010:assfail.constprop.0+0x18/0x26 [btrfs]
[ 3479.718120] RSP: 0018:ffff9f4444a6fd98 EFLAGS: 00010246
[ 3479.719761] RAX: 0000000000000057 RBX: ffff9e0e6bb64c00 RCX: 0000000000000006
[ 3479.721828] RDX: 0000000000000000 RSI: ffff9e0e71352408 RDI: ffff9e0e71351bc0
[ 3479.723586] RBP: ffff9e0e6bb64c60 R08: 0000032a29a25ac1 R09: 0000000000000000
[ 3479.725770] R10: 0000000000000000 R11: 0000000000000000 R12: ffff9e0e746e0200
[ 3479.727906] R13: ffff9e0e746e0200 R14: ffff9e0e746e0318 R15: ffff9e0e6bb61000
[ 3479.739121] FS:  00007fabe5d8a800(0000) GS:ffff9e0e7d800000(0000) knlGS:0000000000000000
[ 3479.741653] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3479.743411] CR2: 00007ffeb8101f68 CR3: 000000004e979002 CR4: 0000000000160ee0

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

* Re: [PATCH v4 2/2] btrfs: reset device back to allocation state when removing
  2019-11-27 17:07   ` David Sterba
@ 2019-11-28  9:29     ` Johannes Thumshirn
  0 siblings, 0 replies; 8+ messages in thread
From: Johannes Thumshirn @ 2019-11-28  9:29 UTC (permalink / raw)
  To: dsterba, David Sterba, Nikolay Borisov, Qu Wenruo,
	Linux BTRFS Mailinglist

On 27/11/2019 18:07, David Sterba wrote:
> On Tue, Nov 26, 2019 at 09:40:06AM +0100, Johannes Thumshirn wrote:
>> +	ASSERT(atomic_read(&device->dev_stats_ccnt) == 0);
> 
> btrfs/020               [16:59:58][ 3477.975072] run fstests btrfs/020 at 2019-11-27 16:59:58
> [ 3478.974314] kworker/dying (5607) used greatest stack depth: 10792 bytes left
> [ 3479.206988] BTRFS: device fsid 818a4909-467d-4599-979c-3b258fb4fc41 devid 1 transid 5 /dev/loop0 scanned by mkfs.btrfs (27347)
> [ 3479.234212] BTRFS: device fsid 818a4909-467d-4599-979c-3b258fb4fc41 devid 2 transid 5 /dev/loop1 scanned by mkfs.btrfs (27347)
> [ 3479.343996] BTRFS info (device loop0): disk space caching is enabled
> [ 3479.349590] BTRFS info (device loop0): has skinny extents
> [ 3479.352721] BTRFS info (device loop0): flagging fs with big metadata feature
> [ 3479.360793] BTRFS info (device loop0): enabling ssd optimizations
> [ 3479.614065] assertion failed: atomic_read(&device->dev_stats_ccnt) == 0, in fs/btrfs/volumes.c:1093
> [ 3479.622041] ------------[ cut here ]------------
> [ 3479.625272] kernel BUG at fs/btrfs/ctree.h:3118!

My test setup was missing loopback device support, fixed that.


-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26  8:40 [PATCH v4 0/2] remove BUG_ON()s in btrfs_close_one_device() Johannes Thumshirn
2019-11-26  8:40 ` [PATCH v4 1/2] btrfs: decrement number of open devices after closing the device not before Johannes Thumshirn
2019-11-26  8:40 ` [PATCH v4 2/2] btrfs: reset device back to allocation state when removing Johannes Thumshirn
2019-11-26 10:21   ` Nikolay Borisov
2019-11-26 17:17   ` David Sterba
2019-11-27 15:26     ` David Sterba
2019-11-27 17:07   ` David Sterba
2019-11-28  9:29     ` Johannes Thumshirn

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org
	public-inbox-index linux-btrfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git