Btrfs devel - Mar 2013 - [PATCH] use rcu_barrier() to wait for bdev puts at unmount

Doing this would reliably fail with -EBUSY for me:

# mount /dev/sdb2 /mnt/scratch; umount /mnt/scratch; mkfs.btrfs -f /dev/sdb2
...
unable to open /dev/sdb2: Device or resource busy

because mkfs.btrfs tries to open the device O_EXCL, and somebody still has it.

Using systemtap to track bdev gets & puts shows a kworker thread doing a
blkdev put after mkfs attempts a get; this is left over from the unmount.

Adding an rcu_barrier() to btrfs_close_devices() causes unmount to wait
until all blkdev_put()s are done, and the device is truly free once
unmount completes.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

p.s. I debated putting it into close_ctree(); I don''t know if
there'' anything
else to wait for.  Thoughts?

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5cbb7f4..258316a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -680,6 +680,7 @@ int btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
 		__btrfs_close_devices(fs_devices);
 		free_fs_devices(fs_devices);
 	}
+	rcu_barrier();
 	return ret;
 }
 

--
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

On Fri, Mar 08, 2013 at 10:23:01PM -0700, Eric Sandeen
wrote:
> Doing this would reliably fail with -EBUSY for me:
> 
> # mount /dev/sdb2 /mnt/scratch; umount /mnt/scratch; mkfs.btrfs -f
/dev/sdb2
> ...
> unable to open /dev/sdb2: Device or resource busy
> 
> because mkfs.btrfs tries to open the device O_EXCL, and somebody still has
it.
> 
> Using systemtap to track bdev gets & puts shows a kworker thread doing
a
> blkdev put after mkfs attempts a get; this is left over from the unmount.
> 
> Adding an rcu_barrier() to btrfs_close_devices() causes unmount to wait
> until all blkdev_put()s are done, and the device is truly free once
> unmount completes.
Thanks for tracking this down Eric.  Is this kworker triggered by btrfs
or is this something we should be doing for the other filesystems too?

I''d move it down to close_ctree, but I don''t really have a
good reason.

-chris
--
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

On 3/9/13 6:27 AM, Chris Mason wrote:
> On Fri, Mar 08, 2013 at 10:23:01PM -0700, Eric Sandeen wrote:
>> Doing this would reliably fail with -EBUSY for me:
>>
>> # mount /dev/sdb2 /mnt/scratch; umount /mnt/scratch; mkfs.btrfs -f
/dev/sdb2
>> ...
>> unable to open /dev/sdb2: Device or resource busy
>>
>> because mkfs.btrfs tries to open the device O_EXCL, and somebody still
has it.
>>
>> Using systemtap to track bdev gets & puts shows a kworker thread
doing a
>> blkdev put after mkfs attempts a get; this is left over from the
unmount.
>>
>> Adding an rcu_barrier() to btrfs_close_devices() causes unmount to wait
>> until all blkdev_put()s are done, and the device is truly free once
>> unmount completes.
> 
> Thanks for tracking this down Eric.  
Sure thing, sorry it took so long.
> Is this kworker triggered by btrfs
> or is this something we should be doing for the other filesystems too?
It''s all btrfs ;)

btrfs_close_devices
	__btrfs_close_devices
		call_rcu(&device->rcu, free_device);
			free_device
				INIT_WORK(&device->rcu_work, __free_device);
				schedule_work(&device->rcu_work);


The behavior came from:

commit 1f78160ce1b1b8e657e2248118c4d91f881763f0
Author: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
Date:   Wed Apr 20 10:09:16 2011 +0000

    Btrfs: using rcu lock in the reader side of devices list

Anyway, I can send V2 in close_ctree if you like. Thinking about it more
though, btrfs_close_devices is closer to the action, so now I think
I''d leave it there. :)

I probably should have put a comment in to say what the heck it''s for,
too.  Feel free to fix on merge or I can send another patch.

Thanks,
-Eric

> I''d move it down to close_ctree, but I don''t really have
a good reason.
> 
> -chris
> 
--
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

On Sat, Mar 09, 2013 at 07:17:04AM -0700, Eric Sandeen
wrote:
> On 3/9/13 6:27 AM, Chris Mason wrote:
> > On Fri, Mar 08, 2013 at 10:23:01PM -0700, Eric Sandeen wrote:
> >> Doing this would reliably fail with -EBUSY for me:
> >>
> >> # mount /dev/sdb2 /mnt/scratch; umount /mnt/scratch; mkfs.btrfs -f
/dev/sdb2
> >> ...
> >> unable to open /dev/sdb2: Device or resource busy
> >>
> >> because mkfs.btrfs tries to open the device O_EXCL, and somebody
still has it.
> >>
> >> Using systemtap to track bdev gets & puts shows a kworker
thread doing a
> >> blkdev put after mkfs attempts a get; this is left over from the
unmount.
> >>
> >> Adding an rcu_barrier() to btrfs_close_devices() causes unmount to
wait
> >> until all blkdev_put()s are done, and the device is truly free
once
> >> unmount completes.
> > 
> > Thanks for tracking this down Eric.  
> 
> Sure thing, sorry it took so long.
> 
> > Is this kworker triggered by btrfs
> > or is this something we should be doing for the other filesystems too?
> 
> It''s all btrfs ;)
> 
> btrfs_close_devices
> 	__btrfs_close_devices
> 		call_rcu(&device->rcu, free_device);
> 			free_device
> 				INIT_WORK(&device->rcu_work, __free_device);
> 				schedule_work(&device->rcu_work);
> 
Great, that makes a ton more sense.  I''m a little confused on why
we''re
seeing it so much more now than in the past.
> 
> The behavior came from:
> 
> commit 1f78160ce1b1b8e657e2248118c4d91f881763f0
> Author: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> Date:   Wed Apr 20 10:09:16 2011 +0000
> 
>     Btrfs: using rcu lock in the reader side of devices list
> 
> Anyway, I can send V2 in close_ctree if you like. Thinking about it more
> though, btrfs_close_devices is closer to the action, so now I think
> I''d leave it there. :)
> 
> I probably should have put a comment in to say what the heck it''s
for,
> too.  Feel free to fix on merge or I can send another patch.
Ok, please add the comment and a cc to stable.  Thanks again.

-chris

--
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

Doing this would reliably fail with -EBUSY for me:

# mount /dev/sdb2 /mnt/scratch; umount /mnt/scratch; mkfs.btrfs -f /dev/sdb2
...
unable to open /dev/sdb2: Device or resource busy

because mkfs.btrfs tries to open the device O_EXCL, and somebody still has it.

Using systemtap to track bdev gets & puts shows a kworker thread doing a
blkdev put after mkfs attempts a get; this is left over from the unmount
path:

btrfs_close_devices
	__btrfs_close_devices
		call_rcu(&device->rcu, free_device);
			free_device
				INIT_WORK(&device->rcu_work, __free_device);
				schedule_work(&device->rcu_work);

so unmount might complete before __free_device fires & does its blkdev_put.

Adding an rcu_barrier() to btrfs_close_devices() causes unmount to wait
until all blkdev_put()s are done, and the device is truly free once
unmount completes.

Cc: stable@vger.kernel.org
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: expand commit msg, add code comment, cc: stable

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5cbb7f4..92a8bfc 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -680,6 +680,12 @@ int btrfs_close_devices(struct btrfs_fs_devices
*fs_devices)
 		__btrfs_close_devices(fs_devices);
 		free_fs_devices(fs_devices);
 	}
+	/*
+	 * Wait for rcu kworkers under __btrfs_close_devices
+	 * to finish all blkdev_puts so device is really
+	 * free when umount is done.
+	 */
+	rcu_barrier();
 	return ret;
 }