-------- Original Message --------
Subject: Re: [PATCH RFC v3 1/5] Revert "btrfs: add support for
processing pending changes" related commits
From: David Sterba <dste...@suse.cz>
To: Qu Wenruo <quwen...@cn.fujitsu.com>
Date: 2015年01月23日 22:57
On Fri, Jan 23, 2015 at 05:31:41PM +0800, Qu Wenruo wrote:
For mount option change, later patches will introduce copy-n-update
method and rwsem protects to keep mount options consistent during
transaction.
That's a better approach, for the mount options.
I'm glad that you like this method.
Although the description in this patch is outdated, it is now
per-transaction mount option.
Sorry for the confusion.
For sysfs interface to change label/features, it will keep the same
behavior as 'btrfs pro set', so pending changes are also not needed.
This still leaves the transaction commit inside the syfs handler, that
was one of the points not to do that.
The callstack looks safe from, eg. the label handler:
[169148.523158] WARNING: CPU: 1 PID: 2044 at fs/btrfs/sysfs.c:394
btrfs_label_store+0x135/0x190 [btrfs]()
[169148.533925] Modules linked in: btrfs dm_flakey rpcsec_gss_krb5 loop [last
unloaded: btrfs]
[169148.536950] CPU: 1 PID: 2044 Comm: bash Tainted: G W
3.19.0-rc5-default+ #211
[169148.536952] Hardware name: Intel Corporation Santa Rosa platform/Matanzas,
BIOS TSRSCRB1.86C.0047.B00.0610170821 10/17/06
[169148.536954] 000000000000018a ffff88007a753dc8 ffffffff81a9898b
000000000000018a
[169148.536963] 0000000000000000 ffff88007a753e08 ffffffff81077f65
ffff880077fb0100
[169148.536972] ffff880075dc0000 ffff880077fbff00 0000000000000009
ffff880075dc06d0
[169148.536980] Call Trace:
[169148.536983] [<ffffffff81a9898b>] dump_stack+0x4f/0x6c
[169148.536991] [<ffffffff81077f65>] warn_slowpath_common+0x95/0xe0
[169148.537000] [<ffffffff81077fca>] warn_slowpath_null+0x1a/0x20
[169148.537005] [<ffffffffa0052b65>] btrfs_label_store+0x135/0x190 [btrfs]
[169148.537030] [<ffffffff813ed8b7>] kobj_attr_store+0x17/0x20
[169148.537037] [<ffffffff812147ff>] sysfs_kf_write+0x4f/0x70
[169148.537044] [<ffffffff81213cc8>] kernfs_fop_write+0x128/0x180
[169148.537051] [<ffffffff8119f404>] vfs_write+0xd4/0x1d0
[169148.537059] [<ffffffff8119f7b9>] SyS_write+0x59/0xd0
[169148.537070] [<ffffffff81a9f9d2>] system_call_fastpath+0x12/0x17
Lockep shows these locks held:
[169148.537296] 4 locks held by bash/2044:
[169148.537309] #0: (sb_writers#5){.+.+.+}, at: [<ffffffff8119f4e0>]
vfs_write+0x1b0/0x1d0
[169148.537319] #1: (&of->mutex){+.+.+.}, at: [<ffffffff81213c2e>]
kernfs_fop_write+0x8e/0x180
[169148.537330] #2: (s_active#214){.+.+.+}, at: [<ffffffff81213c36>]
kernfs_fop_write+0x96/0x180
[169148.537342] #3: (tasklist_lock){.+.+..}, at: [<ffffffff810b9ed4>]
debug_show_all_locks+0x44/0x1e0
#3 is from lockdep
#2 is not really a lock, annotated vfs atomic counter
#0 is annotated atomic, the freezing barrier
#1 is a kernfs mutex that, afaics it's per file, but I don't like to see
the lock dependency here. That's a lock we can see now, but it's outside
of btrfs or the vfs. It's a matter of precaution.
Thanks for pointing out the problem.
It makes sense to delay it.
But we have btrfs-workqueue, why not put it to "worker" workqueue?
If using this method, we can just wrap btrfs_ioctl_set_fslabel() and
queue it to fs_info->workers.
This can avoid the the lockdep problem, but the behavior is still
inconsistent with the synchronized
ioctl method.
Although not perfect, it should be good enough and still clean enough.
What do you think about such method?
Thanks,
Qu
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
