Attachment 130276 Details for Bug 100234 – fs/pstore: Perform erase from a worker

[patch] fs/pstore: Perform erase from a worker

0001-fs-pstore-Perform-erase-from-a-worker.patch (text/plain), 7.05 KB, created by Chris Wilson on 2017-03-16 22:03:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2017-03-16 22:03:10 UTC

Size: 7.05 KB

patch

obsolete

>From ddf77e34427df6f235ea9732da734e6642fd3c6e Mon Sep 17 00:00:00 2001
>From: Chris Wilson <chris@chris-wilson.co.uk>
>Date: Thu, 16 Mar 2017 21:44:33 +0000
>Subject: [PATCH] fs/pstore: Perform erase from a worker
>
>In order to prevent a cyclic recursion between psi->read_mutex and the
>inode_lock, we need to move the pse->erase to a worker.
>
>[  605.374955] ======================================================
>[  605.381281] [ INFO: possible circular locking dependency detected ]
>[  605.387679] 4.11.0-rc2-CI-CI_DRM_2352+ #1 Not tainted
>[  605.392826] -------------------------------------------------------
>[  605.399196] rm/7298 is trying to acquire lock:
>[  605.403720]  (&psinfo->read_mutex){+.+.+.}, at: [<ffffffff813e183f>] pstore_unlink+0x3f/0xa0
>[  605.412300]
>[  605.412300] but task is already holding lock:
>[  605.418237]  (&sb->s_type->i_mutex_key#14){++++++}, at: [<ffffffff812157ec>] vfs_unlink+0x4c/0x19
>0
>[  605.427397]
>[  605.427397] which lock already depends on the new lock.
>[  605.427397]
>[  605.435770]
>[  605.435770] the existing dependency chain (in reverse order) is:
>[  605.443396]
>[  605.443396] -> #1 (&sb->s_type->i_mutex_key#14){++++++}:
>[  605.450347]        lock_acquire+0xc9/0x220
>[  605.454551]        down_write+0x3f/0x70
>[  605.458484]        pstore_mkfile+0x1f4/0x460
>[  605.462835]        pstore_get_records+0x17a/0x320
>[  605.467664]        pstore_fill_super+0xa4/0xc0
>[  605.472205]        mount_single+0x89/0xb0
>[  605.476314]        pstore_mount+0x13/0x20
>[  605.480411]        mount_fs+0xf/0x90
>[  605.484122]        vfs_kern_mount+0x66/0x170
>[  605.488464]        do_mount+0x190/0xd50
>[  605.492397]        SyS_mount+0x90/0xd0
>[  605.496212]        entry_SYSCALL_64_fastpath+0x1c/0xb1
>[  605.501496]
>[  605.501496] -> #0 (&psinfo->read_mutex){+.+.+.}:
>[  605.507747]        __lock_acquire+0x1ac0/0x1bb0
>[  605.512401]        lock_acquire+0xc9/0x220
>[  605.516594]        __mutex_lock+0x6e/0x990
>[  605.520755]        mutex_lock_nested+0x16/0x20
>[  605.525279]        pstore_unlink+0x3f/0xa0
>[  605.529465]        vfs_unlink+0xb5/0x190
>[  605.533477]        do_unlinkat+0x24c/0x2a0
>[  605.537672]        SyS_unlinkat+0x16/0x30
>[  605.541781]        entry_SYSCALL_64_fastpath+0x1c/0xb1
>[  605.547067]
>[  605.547067] other info that might help us debug this:
>[  605.547067]
>[  605.555221]  Possible unsafe locking scenario:
>[  605.555221]
>[  605.561280]        CPU0                    CPU1
>[  605.565883]        ----                    ----
>[  605.570502]   lock(&sb->s_type->i_mutex_key#14);
>[  605.575217]                                lock(&psinfo->read_mutex);
>[  605.581803]                                lock(&sb->s_type->i_mutex_key#14);
>[  605.589159]   lock(&psinfo->read_mutex);
>[  605.593156]
>[  605.593156]  *** DEADLOCK ***
>[  605.593156]
>[  605.599214] 3 locks held by rm/7298:
>[  605.602896]  #0:  (sb_writers#11){.+.+..}, at: [<ffffffff8122e85f>] mnt_want_write+0x1f/0x50
>[  605.611490]  #1:  (&sb->s_type->i_mutex_key#14/1){+.+...}, at: [<ffffffff8121a7dc>] do_unlinkat+0
>x11c/0x2a0
>[  605.621417]  #2:  (&sb->s_type->i_mutex_key#14){++++++}, at: [<ffffffff812157ec>] vfs_unlink+0x4c
>/0x190
>[  605.630995]
>[  605.630995] stack backtrace:
>[  605.635450] CPU: 7 PID: 7298 Comm: rm Not tainted 4.11.0-rc2-CI-CI_DRM_2352+ #1
>[  605.642999] Hardware name: Gigabyte Technology Co., Ltd. Z170X-UD5/Z170X-UD5-CF, BIOS F21 01/06/2
>017
>[  605.652305] Call Trace:
>[  605.654814]  dump_stack+0x67/0x92
>[  605.658184]  print_circular_bug+0x1e0/0x2e0
>[  605.662465]  __lock_acquire+0x1ac0/0x1bb0
>[  605.666634]  ? retint_kernel+0x2d/0x2d
>[  605.670456]  lock_acquire+0xc9/0x220
>[  605.674112]  ? pstore_unlink+0x3f/0xa0
>[  605.677970]  ? pstore_unlink+0x3f/0xa0
>[  605.681818]  __mutex_lock+0x6e/0x990
>[  605.685456]  ? pstore_unlink+0x3f/0xa0
>[  605.689791]  ? pstore_unlink+0x3f/0xa0
>[  605.694124]  ? vfs_unlink+0x4c/0x190
>[  605.698310]  mutex_lock_nested+0x16/0x20
>[  605.702859]  pstore_unlink+0x3f/0xa0
>[  605.707021]  vfs_unlink+0xb5/0x190
>[  605.711024]  do_unlinkat+0x24c/0x2a0
>[  605.715194]  SyS_unlinkat+0x16/0x30
>[  605.719275]  entry_SYSCALL_64_fastpath+0x1c/0xb1
>[  605.724543] RIP: 0033:0x7f8b08073ed7
>[  605.728676] RSP: 002b:00007ffe70eff628 EFLAGS: 00000206 ORIG_RAX: 0000000000000107
>[  605.736929] RAX: ffffffffffffffda RBX: ffffffff8147ea93 RCX: 00007f8b08073ed7
>[  605.744711] RDX: 0000000000000000 RSI: 0000000001450000 RDI: 00000000ffffff9c
>[  605.752512] RBP: ffffc9000338ff88 R08: 0000000000000003 R09: 0000000000000000
>[  605.760276] R10: 000000000000015e R11: 0000000000000206 R12: 0000000000000000
>[  605.768040] R13: 00007ffe70eff750 R14: 000000000144ff70 R15: 0000000001451230
>[  605.775800]  ? __this_cpu_preempt_check+0x13/0x20
>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100234
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>Cc: Kees Cook <keescook@chromium.org>
>Cc: Anton Vorontsov <anton@enomsg.org>
>Cc: Colin Cross <ccross@android.com>
>Cc: Tony Luck <tony.luck@intel.com>
>---
> fs/pstore/inode.c    | 37 ++++++++++++++++++++++++++++++-------
> fs/pstore/platform.c |  2 ++
> 2 files changed, 32 insertions(+), 7 deletions(-)
>
>diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
>index 57c0646..a68a001 100644
>--- a/fs/pstore/inode.c
>+++ b/fs/pstore/inode.c
>@@ -186,6 +186,26 @@ static const struct file_operations pstore_file_operations = {
> 	.release	= seq_release,
> };
> 
>+struct pstore_unlink_work {
>+	struct work_struct work;
>+	struct dentry *dentry;
>+};
>+
>+static void pstore_unlink_work(struct work_struct *work)
>+{
>+	struct pstore_unlink_work *arg = container_of(work, typeof(*arg), work);
>+	struct dentry *dentry = arg->dentry;
>+	struct pstore_private *p = d_inode(dentry)->i_private;
>+
>+	mutex_lock(&p->psi->read_mutex);
>+	p->psi->erase(p->type, p->id, p->count,
>+		      d_inode(dentry)->i_ctime, p->psi);
>+	mutex_unlock(&p->psi->read_mutex);
>+
>+	dput(dentry);
>+	kfree(arg);
>+}
>+
> /*
>  * When a file is unlinked from our file system we call the
>  * platform driver to erase the record from persistent store.
>@@ -193,20 +213,23 @@ static const struct file_operations pstore_file_operations = {
> static int pstore_unlink(struct inode *dir, struct dentry *dentry)
> {
> 	struct pstore_private *p = d_inode(dentry)->i_private;
>+	struct pstore_unlink_work *work;
> 	int err;
> 
> 	err = pstore_check_syslog_permissions(p);
> 	if (err)
> 		return err;
> 
>-	if (p->psi->erase) {
>-		mutex_lock(&p->psi->read_mutex);
>-		p->psi->erase(p->type, p->id, p->count,
>-			      d_inode(dentry)->i_ctime, p->psi);
>-		mutex_unlock(&p->psi->read_mutex);
>-	} else {
>+	if (!p->psi->erase)
> 		return -EPERM;
>-	}
>+
>+	work = kmalloc(sizeof(*work), GFP_KERNEL);
>+	if (!work)
>+		return -ENOMEM;
>+
>+	work->dentry = dget_dlock(dentry);
>+	INIT_WORK(&work->work, pstore_unlink_work);
>+	schedule_work(&work->work);
> 
> 	return simple_unlink(dir, dentry);
> }
>diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
>index efab7b6..6ffca5a 100644
>--- a/fs/pstore/platform.c
>+++ b/fs/pstore/platform.c
>@@ -731,6 +731,8 @@ EXPORT_SYMBOL_GPL(pstore_register);
> 
> void pstore_unregister(struct pstore_info *psi)
> {
>+	flush_scheduled_work();
>+
> 	if (psi->flags & PSTORE_FLAGS_PMSG)
> 		pstore_unregister_pmsg();
> 	if (psi->flags & PSTORE_FLAGS_FTRACE)
>-- 
>2.7.4
>

Actions: View | Splinter Review

Attachments on bug 100234: 130276

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.