All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Benjamin LaHaise <bcrl@kvack.org>
Cc: Kent Overstreet <kmo@daterainc.com>,
	Mike Galbraith <umgwanakikbuti@gmail.com>,
	Lai Jiangshan <laijs@cn.fujitsu.com>,
	RT <linux-rt-users@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Subject: Re: [RFC PATCH V2] rt/aio: fix rcu garbage collection might_sleep() splat
Date: Mon, 16 Feb 2015 18:59:10 +0100	[thread overview]
Message-ID: <20150216175910.GH21649@linutronix.de> (raw)
In-Reply-To: <20140625152445.GS23137@kvack.org>

* Benjamin LaHaise | 2014-06-25 11:24:45 [-0400]:

>I finally have some time to look at this patch in detail.  I'd rather do the 
>below variant that does what Kent suggested.  Mike, can you confirm that 
>this fixes the issue you reported?  It's on top of my current aio-next tree 
>at git://git.kvack.org/~bcrl/aio-next.git .  If that's okay, I'll queue it 
>up.  Does this bug fix need to end up in -stable kernels as well or would it 
>end up in the -rt tree?

This looks smaller compared to the RCU version. Since recently I have
simple-workqueue in -RT so I could replace schedule_work() with it. And
looking at the code I would have to do this change to free_ioctx_users()
and free_ioctx_reqs(). So here is what I am about to add for next -RT.

Ach. It compiles and I've never seen that splat so any feedback is
welcome :)

diff --git a/fs/aio.c b/fs/aio.c
index 14b93159ef83..551fcfe3fd58 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -40,6 +40,7 @@
 #include <linux/ramfs.h>
 #include <linux/percpu-refcount.h>
 #include <linux/mount.h>
+#include <linux/work-simple.h>
 
 #include <asm/kmap_types.h>
 #include <asm/uaccess.h>
@@ -110,7 +111,7 @@ struct kioctx {
 	struct page		**ring_pages;
 	long			nr_pages;
 
-	struct work_struct	free_work;
+	struct swork_event	free_work;
 
 	/*
 	 * signals when all in-flight requests are done
@@ -226,6 +227,7 @@ static int __init aio_setup(void)
 		.mount		= aio_mount,
 		.kill_sb	= kill_anon_super,
 	};
+	BUG_ON(swork_get());
 	aio_mnt = kern_mount(&aio_fs);
 	if (IS_ERR(aio_mnt))
 		panic("Failed to create aio fs mount.");
@@ -505,9 +507,9 @@ static int kiocb_cancel(struct kiocb *kiocb)
 	return cancel(kiocb);
 }
 
-static void free_ioctx(struct work_struct *work)
+static void free_ioctx(struct swork_event *sev)
 {
-	struct kioctx *ctx = container_of(work, struct kioctx, free_work);
+	struct kioctx *ctx = container_of(sev, struct kioctx, free_work);
 
 	pr_debug("freeing %p\n", ctx);
 
@@ -526,8 +528,8 @@ static void free_ioctx_reqs(struct percpu_ref *ref)
 	if (ctx->requests_done)
 		complete(ctx->requests_done);
 
-	INIT_WORK(&ctx->free_work, free_ioctx);
-	schedule_work(&ctx->free_work);
+	INIT_SWORK(&ctx->free_work, free_ioctx);
+	swork_queue(&ctx->free_work);
 }
 
 /*
@@ -535,9 +537,9 @@ static void free_ioctx_reqs(struct percpu_ref *ref)
  * and ctx->users has dropped to 0, so we know no more kiocbs can be submitted -
  * now it's safe to cancel any that need to be.
  */
-static void free_ioctx_users(struct percpu_ref *ref)
+static void free_ioctx_users_work(struct swork_event *sev)
 {
-	struct kioctx *ctx = container_of(ref, struct kioctx, users);
+	struct kioctx *ctx = container_of(sev, struct kioctx, free_work);
 	struct kiocb *req;
 
 	spin_lock_irq(&ctx->ctx_lock);
@@ -556,6 +558,14 @@ static void free_ioctx_users(struct percpu_ref *ref)
 	percpu_ref_put(&ctx->reqs);
 }
 
+static void free_ioctx_users(struct percpu_ref *ref)
+{
+	struct kioctx *ctx = container_of(ref, struct kioctx, users);
+
+	INIT_SWORK(&ctx->free_work, free_ioctx_users_work);
+	swork_queue(&ctx->free_work);
+}
+
 static int ioctx_add_table(struct kioctx *ctx, struct mm_struct *mm)
 {
 	unsigned i, new_nr;
-- 
2.1.4


      parent reply	other threads:[~2015-02-16 17:59 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-08  8:35 [RFC PATCH] rt/aio: fix rcu garbage collection might_sleep() splat Mike Galbraith
2014-06-09  2:08 ` Lai Jiangshan
2014-06-09  3:17   ` Mike Galbraith
2014-06-09  6:22     ` Mike Galbraith
2014-06-09  9:04       ` Pavel Vasilyev
2014-06-10  3:47   ` [RFC PATCH V2] " Mike Galbraith
2014-06-10 17:50     ` Benjamin LaHaise
2014-06-11  4:10       ` Mike Galbraith
2014-06-12 20:26       ` Kent Overstreet
2014-06-25 15:24         ` Benjamin LaHaise
2014-06-26  7:37           ` Mike Galbraith
2014-06-26 16:42             ` Benjamin LaHaise
2015-02-16 17:59           ` Sebastian Andrzej Siewior [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=20150216175910.GH21649@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=bcrl@kvack.org \
    --cc=kmo@daterainc.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=umgwanakikbuti@gmail.com \
    /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 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.