From: Anand Jain <anand.jain@oracle.com>
To: Josef Bacik <josef@toxicpanda.com>
Cc: kernel-team@fb.com, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2 1/7] btrfs: do not call close_fs_devices in btrfs_rm_device
Date: Wed, 1 Sep 2021 16:13:23 +0800 [thread overview]
Message-ID: <649d8f42-49db-69b4-45df-d0d502d7db7a@oracle.com> (raw)
In-Reply-To: <27fa361288b46bcc0f4b1225f7c76c96ce6dbe5f.1627419595.git.josef@toxicpanda.com>
On 28/07/2021 05:01, Josef Bacik wrote:
> There's a subtle case where if we're removing the seed device from a
> file system we need to free its private copy of the fs_devices. However
> we do not need to call close_fs_devices(), because at this point there
> are no devices left to close as we've closed the last one. The only
> thing that close_fs_devices() does is decrement ->opened, which should
> be 1. We want to avoid calling close_fs_devices() here because it has a
> lockdep_assert_held(&uuid_mutex), and we are going to stop holding the
> uuid_mutex in this path.
>
> So add an assert for the ->opened counter and simply decrement it like
> we should, and then clean up like normal. Also add a comment explaining
> what we're doing here as I initially removed this code erroneously.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
> fs/btrfs/volumes.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 86846d6e58d0..5217b93172b4 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2200,9 +2200,17 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
> synchronize_rcu();
> btrfs_free_device(device);
>
> + /*
> + * This can happen if cur_devices is the private seed devices list. We
> + * cannot call close_fs_devices() here because it expects the uuid_mutex
> + * to be held, but in fact we don't need that for the private
> + * seed_devices, we can simply decrement cur_devices->opened and then
> + * remove it from our list and free the fs_devices.
> + */
> if (cur_devices->open_devices == 0) {
We should in fact use cur_devices->num_devices == 0 here.
Sent a patch [1] to fix it.
[1]
https://patchwork.kernel.org/project/linux-btrfs/patch/d9c89b1740a876b3851fcf358f22809aa7f1ad2a.1630478246.git.anand.jain@oracle.com/
> + ASSERT(cur_devices->opened == 1);
We don't need to assert(). free_fs_devices() has a warning for it.
WARN_ON(fs_devices->opened);
> list_del_init(&cur_devices->seed_list);
> - close_fs_devices(cur_devices);
> + cur_devices->opened--;
> free_fs_devices(cur_devices);
> }
With the above two fixed.
Reviewed-by: Anand Jain <anand.jain@oracle.com>
Thanks, Anand
next prev parent reply other threads:[~2021-09-01 8:13 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-27 21:01 [PATCH v2 0/7] Josef Bacik
2021-07-27 21:01 ` [PATCH v2 1/7] btrfs: do not call close_fs_devices in btrfs_rm_device Josef Bacik
2021-09-01 8:13 ` Anand Jain [this message]
2021-07-27 21:01 ` [PATCH v2 2/7] btrfs: do not take the uuid_mutex " Josef Bacik
2021-09-01 12:01 ` Anand Jain
2021-09-01 17:08 ` David Sterba
2021-09-01 17:10 ` Josef Bacik
2021-09-01 19:49 ` Anand Jain
2021-09-02 12:58 ` David Sterba
2021-09-02 14:10 ` Josef Bacik
2021-09-17 14:33 ` David Sterba
2021-09-20 7:45 ` Anand Jain
2021-09-20 8:26 ` David Sterba
2021-09-20 9:41 ` Anand Jain
2021-09-23 4:33 ` Anand Jain
2021-09-21 11:59 ` Filipe Manana
2021-09-21 12:17 ` Filipe Manana
2021-09-22 15:33 ` Filipe Manana
2021-09-23 4:15 ` Anand Jain
2021-09-23 3:58 ` [PATCH] btrfs: drop lockdep assert in close_fs_devices() Anand Jain
2021-09-23 4:04 ` Anand Jain
2021-07-27 21:01 ` [PATCH v2 3/7] btrfs: do not read super look for a device path Josef Bacik
2021-08-25 2:00 ` Anand Jain
2021-09-27 15:32 ` Josef Bacik
2021-09-28 11:50 ` Anand Jain
2021-07-27 21:01 ` [PATCH v2 4/7] btrfs: update the bdev time directly when closing Josef Bacik
2021-08-25 0:35 ` Anand Jain
2021-09-02 12:16 ` David Sterba
2021-07-27 21:01 ` [PATCH v2 5/7] btrfs: delay blkdev_put until after the device remove Josef Bacik
2021-08-25 1:00 ` Anand Jain
2021-09-02 12:16 ` David Sterba
2021-07-27 21:01 ` [PATCH v2 6/7] btrfs: unify common code for the v1 and v2 versions of " Josef Bacik
2021-08-25 1:19 ` Anand Jain
2021-09-01 14:05 ` Nikolay Borisov
2021-07-27 21:01 ` [PATCH v2 7/7] btrfs: do not take the device_list_mutex in clone_fs_devices Josef Bacik
2021-08-24 22:08 ` Anand Jain
2021-09-01 13:35 ` Nikolay Borisov
2021-09-02 12:59 ` David Sterba
2021-09-17 15:06 ` [PATCH v2 0/7] David Sterba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=649d8f42-49db-69b4-45df-d0d502d7db7a@oracle.com \
--to=anand.jain@oracle.com \
--cc=josef@toxicpanda.com \
--cc=kernel-team@fb.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).