From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from plane.gmane.org ([80.91.229.3]:35157 "EHLO plane.gmane.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751405AbcFRQeR (ORCPT ); Sat, 18 Jun 2016 12:34:17 -0400 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1bEJCJ-0007NJ-NC for linux-btrfs@vger.kernel.org; Sat, 18 Jun 2016 18:34:15 +0200 Received: from pd953e4e5.dip0.t-ipconnect.de ([217.83.228.229]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sat, 18 Jun 2016 18:34:15 +0200 Received: from holger by pd953e4e5.dip0.t-ipconnect.de with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sat, 18 Jun 2016 18:34:15 +0200 To: linux-btrfs@vger.kernel.org From: Holger =?iso-8859-1?q?Hoffst=E4tte?= Subject: Re: [PATCH 2/2] btrfs: wait for bdev put Date: Sat, 18 Jun 2016 16:34:01 +0000 (UTC) Message-ID: References: <1e7bc117-aaa3-c77f-b933-cd0c37b5ce68@oracle.com> <1465901726-15490-1-git-send-email-anand.jain@oracle.com> <1465901726-15490-2-git-send-email-anand.jain@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Sender: linux-btrfs-owner@vger.kernel.org List-ID: On Tue, 14 Jun 2016 18:55:26 +0800, Anand Jain wrote: > Further to the previous commit > bc178622d40d87e75abc131007342429c9b03351 > btrfs: use rcu_barrier() to wait for bdev puts at unmount > > Since free_device() spinoff __free_device() the rcu_barrier() for > call_rcu(&device->rcu, free_device); > didn't help. > > This patch reverts changes by > bc178622d40d87e75abc131007342429c9b03351 > and implement a method to wait on the __free_device() by using > a new bdev_closing member in struct btrfs_device. > > Signed-off-by: Anand Jain > [rework: bc178622d40d87e75abc131007342429c9b03351] > --- > fs/btrfs/volumes.c | 44 ++++++++++++++++++++++++++++++++++++++------ > fs/btrfs/volumes.h | 1 + > 2 files changed, 39 insertions(+), 6 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index a4e8d48acd4b..404ce1daebb1 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -27,6 +27,7 @@ > #include > #include > #include > +#include > #include > #include "ctree.h" > #include "extent_map.h" > @@ -254,6 +255,17 @@ static struct btrfs_device *__alloc_device(void) > return dev; > } > > +static int is_device_closing(struct list_head *head) > +{ > + struct btrfs_device *dev; > + > + list_for_each_entry(dev, head, dev_list) { > + if (dev->bdev_closing) > + return 1; > + } > + return 0; > +} > + > static noinline struct btrfs_device *__find_device(struct list_head *head, > u64 devid, u8 *uuid) > { > @@ -832,12 +844,22 @@ again: > static void __free_device(struct work_struct *work) > { > struct btrfs_device *device; > + struct btrfs_device *new_device_addr; > > device = container_of(work, struct btrfs_device, rcu_work); > > if (device->bdev) > blkdev_put(device->bdev, device->mode); > > + /* > + * If we are coming here from btrfs_close_one_device() > + * then it allocates a new device structure for the same > + * devid, so find device again with the devid > + */ > + new_device_addr = __find_device(&device->fs_devices->devices, > + device->devid, NULL); > + > + new_device_addr->bdev_closing = 0; > rcu_string_free(device->name); > kfree(device); > } > @@ -884,6 +906,12 @@ static void btrfs_close_one_device(struct btrfs_device *device) > list_replace_rcu(&device->dev_list, &new_device->dev_list); > new_device->fs_devices = device->fs_devices; > > + /* > + * So to wait for kworkers to finish all blkdev_puts, > + * so device is really free when umount is done. > + */ > + new_device->bdev_closing = 1; > + > call_rcu(&device->rcu, free_device); > } > > @@ -912,6 +940,7 @@ int btrfs_close_devices(struct btrfs_fs_devices *fs_devices) > { > struct btrfs_fs_devices *seed_devices = NULL; > int ret; > + int retry_cnt = 5; > > mutex_lock(&uuid_mutex); > ret = __btrfs_close_devices(fs_devices); > @@ -927,12 +956,15 @@ 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(); > + > + while (is_device_closing(&fs_devices->devices) && > + --retry_cnt) { > + mdelay(1000); //1 sec > + } > + > + if (!(retry_cnt > 0)) > + printk(KERN_WARNING "BTRFS: %pU bdev_put didn't complete, giving up\n", > + fs_devices->fsid); > return ret; > } > > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 0ac90f8d85bd..945e49f5e17d 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -150,6 +150,7 @@ struct btrfs_device { > /* Counter to record the change of device stats */ > atomic_t dev_stats_ccnt; > atomic_t dev_stat_values[BTRFS_DEV_STAT_VALUES_MAX]; > + int bdev_closing; > }; > > /* > -- > 2.7.0 I gave this a try and somehow it seems to make unmounting worse: it now always takes ~5s (max retry time) and I see the warning every time. Without the patch unmounting a single volume (disk) is much faster (1-2s), without problems. Any ideas? cheers, Holger