From: Stefan Behrens <sbehrens@giantdisaster.de>
To: Josef Bacik <jbacik@fusionio.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] Btrfs: check to see if root_list is empty before adding it to dead roots
Date: Fri, 26 Jul 2013 11:38:47 +0200 [thread overview]
Message-ID: <51F243A7.1060907@giantdisaster.de> (raw)
In-Reply-To: <1374779620-28788-1-git-send-email-jbacik@fusionio.com>
On Thu, 25 Jul 2013 15:13:40 -0400, Josef Bacik wrote:
> A user reported a panic when running with autodefrag and deleting snapshots.
> This is because we could end up trying to add the root to the dead roots list
> twice. To fix this check to see if we are empty before adding ourselves to the
> dead roots list. Thanks,
>
> Signed-off-by: Josef Bacik <jbacik@fusionio.com>
Tested-by: Stefan Behrens <sbehrens@giantdisaster.de>
The patch eliminates the crash. The question still is whether the double addition to the list is an indication for a problem and this patch just hides that there is a problem.
> ---
> fs/btrfs/transaction.c | 8 ++++----
> fs/btrfs/transaction.h | 2 +-
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 61a5c2c..18f7e71 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -983,12 +983,12 @@ static noinline int commit_cowonly_roots(struct btrfs_trans_handle *trans,
> * a dirty root struct and adds it into the list of dead roots that need to
> * be deleted
> */
> -int btrfs_add_dead_root(struct btrfs_root *root)
> +void btrfs_add_dead_root(struct btrfs_root *root)
> {
> spin_lock(&root->fs_info->trans_lock);
> - list_add_tail(&root->root_list, &root->fs_info->dead_roots);
> + if (list_empty(&root->root_list))
> + list_add_tail(&root->root_list, &root->fs_info->dead_roots);
> spin_unlock(&root->fs_info->trans_lock);
> - return 0;
> }
>
> /*
> @@ -1925,7 +1925,7 @@ int btrfs_clean_one_deleted_snapshot(struct btrfs_root *root)
> }
> root = list_first_entry(&fs_info->dead_roots,
> struct btrfs_root, root_list);
> - list_del(&root->root_list);
> + list_del_init(&root->root_list);
> spin_unlock(&fs_info->trans_lock);
>
> pr_debug("btrfs: cleaner removing %llu\n",
> diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
> index 005b037..defbc42 100644
> --- a/fs/btrfs/transaction.h
> +++ b/fs/btrfs/transaction.h
> @@ -143,7 +143,7 @@ int btrfs_wait_for_commit(struct btrfs_root *root, u64 transid);
> int btrfs_write_and_wait_transaction(struct btrfs_trans_handle *trans,
> struct btrfs_root *root);
>
> -int btrfs_add_dead_root(struct btrfs_root *root);
> +void btrfs_add_dead_root(struct btrfs_root *root);
> int btrfs_defrag_root(struct btrfs_root *root);
> int btrfs_clean_one_deleted_snapshot(struct btrfs_root *root);
> int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>
next prev parent reply other threads:[~2013-07-26 9:38 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-25 19:13 [PATCH] Btrfs: check to see if root_list is empty before adding it to dead roots Josef Bacik
2013-07-25 21:31 ` Zach Brown
2013-07-26 9:38 ` Stefan Behrens [this message]
2013-07-26 13:09 ` Josef Bacik
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=51F243A7.1060907@giantdisaster.de \
--to=sbehrens@giantdisaster.de \
--cc=jbacik@fusionio.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).