From: Qu Wenruo <quwenruo@cn.fujitsu.com>
To: Miao Xie <miaoxie@huawei.com>, <dsterba@suse.cz>,
<linux-btrfs@vger.kernel.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Subject: Re: [PATCH v5 0/9] btrfs: Fix freeze/sysfs deadlock in better method.
Date: Mon, 2 Feb 2015 09:36:53 +0800 [thread overview]
Message-ID: <54CED4B5.3030702@cn.fujitsu.com> (raw)
In-Reply-To: <54CC3E59.2050204@huawei.com>
-------- Original Message --------
Subject: Re: [PATCH v5 0/9] btrfs: Fix freeze/sysfs deadlock in better
method.
From: Miao Xie <miaoxie@huawei.com>
To: <dsterba@suse.cz>, Qu Wenruo <quwenruo@cn.fujitsu.com>,
<linux-btrfs@vger.kernel.org>
Date: 2015年01月31日 10:30
> On Fri, 30 Jan 2015 20:17:49 +0100, David Sterba wrote:
>> On Fri, Jan 30, 2015 at 05:20:45PM +0800, Qu Wenruo wrote:
>>> [Use VFS protect for sysfs change]
>>> The 6th patch will introduce a new help function sb_want_write() to
>>> claim write permission to a superblock.
>>> With this, we are able to do write protection like mnt_want_write() but
>>> only needs to ensure that the superblock is writeable.
>>> This also keeps the same synchronized behavior using ioctl, which will
>>> block on frozen fs until it is unfrozen.
>> You know what I think abuot the commit inside sysfs, but it looks better
>> to me now with the sb_* protections so I give it a go.
>> --
>> 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
>>
> I worried about the following case
>
> # fsfreeze btrfs
> # echo "new label" > btrfs_sysfs
> It should be hangup
Yes, same as "btrfs pro set <mnt> label "
>
>
> On the other terminal
> # umount btrfs
Oh, that's a problem.
It can be umounted, but....
>
>
> Because the 2nd echo command didn't increase mount reference, so umount
> would not know someone still blocked on the fs, it would not go back and
> return EBUSY like someone access the fs by common fs interface, it would
> deactive fs directly and then blocked on sysfs removal.
The btrfs module can't be removed, but not blocked. Just return -EBUSY.
So at least it won't cause annoying block problem.
Although hard for sysadmin to find pinpoint the real block reason, it is
still resolvable, mount again and unfreeze
can resolve it.
But it never seems to be a reasonable behavior for me either.
Just as you mentioned, in sysfs, we only have sb, no vfsmount, so
without huge VFS change(*),
we could either accept the wrong method, or drop all write sysfs support
for transaction-invoking features.
*: Maybe we can add ref counts to sb, and umount syscall checks for the
sb ref counts if the mount point
is the last vfsmount of it. I'm not sure whether it is worthy but it
seems valid.
Thanks,
Qu
>
>
> Thanks
> Miao
prev parent reply other threads:[~2015-02-02 1:34 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-30 9:20 [PATCH v5 0/9] btrfs: Fix freeze/sysfs deadlock in better method Qu Wenruo
2015-01-30 9:20 ` [PATCH v5 1/9] Revert "btrfs: add support for processing pending changes" related commits Qu Wenruo
2015-01-30 19:14 ` David Sterba
2015-01-30 9:20 ` [PATCH v5 2/9] btrfs: Make btrfs_parse_options() parse mount option in a atomic way Qu Wenruo
2015-01-30 18:28 ` David Sterba
2015-01-30 9:20 ` [PATCH v5 3/9] btrfs: Introduce per-transaction mount_opt to keep mount option consistent during transaction Qu Wenruo
2015-01-30 19:12 ` David Sterba
2015-01-30 9:20 ` [PATCH v5 4/9] btrfs: Use btrfs_test_trans_opt() to handle SPACE_CACHE if it's under transaction protect Qu Wenruo
2015-01-30 19:12 ` David Sterba
2015-01-30 9:20 ` [PATCH v5 5/9] btrfs: Use btrfs_test_trans_opt() to handle INODE_CACHE " Qu Wenruo
2015-01-30 19:12 ` David Sterba
2015-01-30 9:20 ` [PATCH v5 6/9] vfs: Add sb_want_write() function to get vfsmount from a given sb Qu Wenruo
2015-01-30 19:11 ` David Sterba
2015-01-30 9:20 ` [PATCH v5 7/9] btrfs: Move btrfs_sysfs_remove_one(fs_info) before transcation thread cleanup Qu Wenruo
2015-01-30 18:56 ` David Sterba
2015-01-30 9:20 ` [PATCH v5 8/9] btrfs: Use sb_want_write() to protect label change Qu Wenruo
2015-01-30 18:34 ` David Sterba
2015-01-30 9:20 ` [PATCH v5 9/9] btrfs: Use sb_want_write() to protect sysfs feature change Qu Wenruo
2015-01-30 18:35 ` David Sterba
2015-01-30 19:17 ` [PATCH v5 0/9] btrfs: Fix freeze/sysfs deadlock in better method David Sterba
2015-01-31 2:30 ` Miao Xie
2015-02-02 1:36 ` Qu Wenruo [this message]
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=54CED4B5.3030702@cn.fujitsu.com \
--to=quwenruo@cn.fujitsu.com \
--cc=dsterba@suse.cz \
--cc=linux-btrfs@vger.kernel.org \
--cc=miaoxie@huawei.com \
--cc=viro@ZenIV.linux.org.uk \
/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).