* [PATCH] orangefs: free superblock when mount fails
@ 2017-04-14 18:22 Martin Brandenburg
2017-04-14 18:43 ` Mike Marshall
0 siblings, 1 reply; 2+ messages in thread
From: Martin Brandenburg @ 2017-04-14 18:22 UTC (permalink / raw)
To: torvalds, hubcap, viro, linux-fsdevel, linux-kernel
Cc: Martin Brandenburg, stable
Otherwise lockdep says:
[ 1337.483798] ================================================
[ 1337.483999] [ BUG: lock held when returning to user space! ]
[ 1337.484252] 4.11.0-rc6 #19 Not tainted
[ 1337.484423] ------------------------------------------------
[ 1337.484626] mount/14766 is leaving the kernel with locks still held!
[ 1337.484841] 1 lock held by mount/14766:
[ 1337.485017] #0: (&type->s_umount_key#33/1){+.+.+.}, at: [<ffffffff8124171f>] sget_userns+0x2af/0x520
Caught by xfstests generic/413 which tried to mount with the unsupported
mount option dax. Then xfstests generic/422 ran sync which deadlocks.
Signed-off-by: Martin Brandenburg <martin@omnibond.com>
Cc: stable@vger.kernel.org
---
fs/orangefs/devorangefs-req.c | 9 +++++++--
fs/orangefs/orangefs-kernel.h | 1 +
fs/orangefs/super.c | 23 ++++++++++++++++-------
3 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/fs/orangefs/devorangefs-req.c b/fs/orangefs/devorangefs-req.c
index c4ab6fdf17a0..e1534c9bab16 100644
--- a/fs/orangefs/devorangefs-req.c
+++ b/fs/orangefs/devorangefs-req.c
@@ -208,14 +208,19 @@ static ssize_t orangefs_devreq_read(struct file *file,
continue;
/*
* Skip ops whose filesystem we don't know about unless
- * it is being mounted.
+ * it is being mounted or unmounted. It is possible for
+ * a filesystem we don't know about to be unmounted if
+ * it fails to mount in the kernel after userspace has
+ * been sent the mount request.
*/
/* XXX: is there a better way to detect this? */
} else if (ret == -1 &&
!(op->upcall.type ==
ORANGEFS_VFS_OP_FS_MOUNT ||
op->upcall.type ==
- ORANGEFS_VFS_OP_GETATTR)) {
+ ORANGEFS_VFS_OP_GETATTR ||
+ op->upcall.type ==
+ ORANGEFS_VFS_OP_FS_UMOUNT)) {
gossip_debug(GOSSIP_DEV_DEBUG,
"orangefs: skipping op tag %llu %s\n",
llu(op->tag), get_opname_string(op));
diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
index 5e48a0be9761..8afac46fcc87 100644
--- a/fs/orangefs/orangefs-kernel.h
+++ b/fs/orangefs/orangefs-kernel.h
@@ -249,6 +249,7 @@ struct orangefs_sb_info_s {
char devname[ORANGEFS_MAX_SERVER_ADDR_LEN];
struct super_block *sb;
int mount_pending;
+ int no_list;
struct list_head list;
};
diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index cd261c8de53a..629d8c917fa6 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -493,7 +493,7 @@ struct dentry *orangefs_mount(struct file_system_type *fst,
if (ret) {
d = ERR_PTR(ret);
- goto free_op;
+ goto free_sb_and_op;
}
/*
@@ -519,6 +519,9 @@ struct dentry *orangefs_mount(struct file_system_type *fst,
spin_unlock(&orangefs_superblocks_lock);
op_release(new_op);
+ /* Must be removed from the list now. */
+ ORANGEFS_SB(sb)->no_list = 0;
+
if (orangefs_userspace_version >= 20906) {
new_op = op_alloc(ORANGEFS_VFS_OP_FEATURES);
if (!new_op)
@@ -533,6 +536,10 @@ struct dentry *orangefs_mount(struct file_system_type *fst,
return dget(sb->s_root);
+free_sb_and_op:
+ /* Will call orangefs_kill_sb with sb not in list. */
+ ORANGEFS_SB(sb)->no_list = 1;
+ deactivate_locked_super(sb);
free_op:
gossip_err("orangefs_mount: mount request failed with %d\n", ret);
if (ret == -EINVAL) {
@@ -558,12 +565,14 @@ void orangefs_kill_sb(struct super_block *sb)
*/
orangefs_unmount_sb(sb);
- /* remove the sb from our list of orangefs specific sb's */
-
- spin_lock(&orangefs_superblocks_lock);
- __list_del_entry(&ORANGEFS_SB(sb)->list); /* not list_del_init */
- ORANGEFS_SB(sb)->list.prev = NULL;
- spin_unlock(&orangefs_superblocks_lock);
+ if (!ORANGEFS_SB(sb)->no_list) {
+ /* remove the sb from our list of orangefs specific sb's */
+ spin_lock(&orangefs_superblocks_lock);
+ /* not list_del_init */
+ __list_del_entry(&ORANGEFS_SB(sb)->list);
+ ORANGEFS_SB(sb)->list.prev = NULL;
+ spin_unlock(&orangefs_superblocks_lock);
+ }
/*
* make sure that ORANGEFS_DEV_REMOUNT_ALL loop that might've seen us
--
2.11.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] orangefs: free superblock when mount fails
2017-04-14 18:22 [PATCH] orangefs: free superblock when mount fails Martin Brandenburg
@ 2017-04-14 18:43 ` Mike Marshall
0 siblings, 0 replies; 2+ messages in thread
From: Mike Marshall @ 2017-04-14 18:43 UTC (permalink / raw)
To: Martin Brandenburg; +Cc: Linus Torvalds, Al Viro, linux-fsdevel, LKML, stable
ACK.
I tried to mount orangefs with a nonsense option and got:
[96967.205842] ================================================
[96967.206439] [ BUG: lock held when returning to user space! ]
[96967.207046] 4.10.0-00008-g554ce8b #2 Not tainted
[96967.207531] ------------------------------------------------
[96967.208130] mount/6371 is leaving the kernel with locks still held!
[96967.208797] 1 lock held by mount/6371:
[96967.209211] #0: (&type->s_umount_key#52/1){+.+.+.}, at:
[<ffffffffbe2a1782>] sget_userns+0x2d2/0x510
and then I typed sync and it wedged...
After applying Martin's patch, the nonsense mount option merely caused
the mount to fail without sickening the kernel...
-Mike
On Fri, Apr 14, 2017 at 2:22 PM, Martin Brandenburg <martin@omnibond.com> wrote:
> Otherwise lockdep says:
>
> [ 1337.483798] ================================================
> [ 1337.483999] [ BUG: lock held when returning to user space! ]
> [ 1337.484252] 4.11.0-rc6 #19 Not tainted
> [ 1337.484423] ------------------------------------------------
> [ 1337.484626] mount/14766 is leaving the kernel with locks still held!
> [ 1337.484841] 1 lock held by mount/14766:
> [ 1337.485017] #0: (&type->s_umount_key#33/1){+.+.+.}, at: [<ffffffff8124171f>] sget_userns+0x2af/0x520
>
> Caught by xfstests generic/413 which tried to mount with the unsupported
> mount option dax. Then xfstests generic/422 ran sync which deadlocks.
>
> Signed-off-by: Martin Brandenburg <martin@omnibond.com>
> Cc: stable@vger.kernel.org
> ---
> fs/orangefs/devorangefs-req.c | 9 +++++++--
> fs/orangefs/orangefs-kernel.h | 1 +
> fs/orangefs/super.c | 23 ++++++++++++++++-------
> 3 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/fs/orangefs/devorangefs-req.c b/fs/orangefs/devorangefs-req.c
> index c4ab6fdf17a0..e1534c9bab16 100644
> --- a/fs/orangefs/devorangefs-req.c
> +++ b/fs/orangefs/devorangefs-req.c
> @@ -208,14 +208,19 @@ static ssize_t orangefs_devreq_read(struct file *file,
> continue;
> /*
> * Skip ops whose filesystem we don't know about unless
> - * it is being mounted.
> + * it is being mounted or unmounted. It is possible for
> + * a filesystem we don't know about to be unmounted if
> + * it fails to mount in the kernel after userspace has
> + * been sent the mount request.
> */
> /* XXX: is there a better way to detect this? */
> } else if (ret == -1 &&
> !(op->upcall.type ==
> ORANGEFS_VFS_OP_FS_MOUNT ||
> op->upcall.type ==
> - ORANGEFS_VFS_OP_GETATTR)) {
> + ORANGEFS_VFS_OP_GETATTR ||
> + op->upcall.type ==
> + ORANGEFS_VFS_OP_FS_UMOUNT)) {
> gossip_debug(GOSSIP_DEV_DEBUG,
> "orangefs: skipping op tag %llu %s\n",
> llu(op->tag), get_opname_string(op));
> diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
> index 5e48a0be9761..8afac46fcc87 100644
> --- a/fs/orangefs/orangefs-kernel.h
> +++ b/fs/orangefs/orangefs-kernel.h
> @@ -249,6 +249,7 @@ struct orangefs_sb_info_s {
> char devname[ORANGEFS_MAX_SERVER_ADDR_LEN];
> struct super_block *sb;
> int mount_pending;
> + int no_list;
> struct list_head list;
> };
>
> diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
> index cd261c8de53a..629d8c917fa6 100644
> --- a/fs/orangefs/super.c
> +++ b/fs/orangefs/super.c
> @@ -493,7 +493,7 @@ struct dentry *orangefs_mount(struct file_system_type *fst,
>
> if (ret) {
> d = ERR_PTR(ret);
> - goto free_op;
> + goto free_sb_and_op;
> }
>
> /*
> @@ -519,6 +519,9 @@ struct dentry *orangefs_mount(struct file_system_type *fst,
> spin_unlock(&orangefs_superblocks_lock);
> op_release(new_op);
>
> + /* Must be removed from the list now. */
> + ORANGEFS_SB(sb)->no_list = 0;
> +
> if (orangefs_userspace_version >= 20906) {
> new_op = op_alloc(ORANGEFS_VFS_OP_FEATURES);
> if (!new_op)
> @@ -533,6 +536,10 @@ struct dentry *orangefs_mount(struct file_system_type *fst,
>
> return dget(sb->s_root);
>
> +free_sb_and_op:
> + /* Will call orangefs_kill_sb with sb not in list. */
> + ORANGEFS_SB(sb)->no_list = 1;
> + deactivate_locked_super(sb);
> free_op:
> gossip_err("orangefs_mount: mount request failed with %d\n", ret);
> if (ret == -EINVAL) {
> @@ -558,12 +565,14 @@ void orangefs_kill_sb(struct super_block *sb)
> */
> orangefs_unmount_sb(sb);
>
> - /* remove the sb from our list of orangefs specific sb's */
> -
> - spin_lock(&orangefs_superblocks_lock);
> - __list_del_entry(&ORANGEFS_SB(sb)->list); /* not list_del_init */
> - ORANGEFS_SB(sb)->list.prev = NULL;
> - spin_unlock(&orangefs_superblocks_lock);
> + if (!ORANGEFS_SB(sb)->no_list) {
> + /* remove the sb from our list of orangefs specific sb's */
> + spin_lock(&orangefs_superblocks_lock);
> + /* not list_del_init */
> + __list_del_entry(&ORANGEFS_SB(sb)->list);
> + ORANGEFS_SB(sb)->list.prev = NULL;
> + spin_unlock(&orangefs_superblocks_lock);
> + }
>
> /*
> * make sure that ORANGEFS_DEV_REMOUNT_ALL loop that might've seen us
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-04-14 18:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-14 18:22 [PATCH] orangefs: free superblock when mount fails Martin Brandenburg
2017-04-14 18:43 ` Mike Marshall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.