Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964925AbbFJQHg (ORCPT ); Wed, 10 Jun 2015 12:07:36 -0400 Received: from ip4-83-240-67-251.cust.nbox.cz ([83.240.67.251]:36014 "EHLO ip4-83-240-18-248.cust.nbox.cz" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S933793AbbFJP1m (ORCPT ); Wed, 10 Jun 2015 11:27:42 -0400 From: Jiri Slaby To: stable@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Oleg Nesterov , Benjamin LaHaise , Jiri Slaby Subject: [PATCH 3.12 009/111] aio: change exit_aio() to load mm->ioctx_table once and avoid rcu_read_lock() Date: Wed, 10 Jun 2015 17:25:58 +0200 Message-Id: X-Mailer: git-send-email 2.4.2 In-Reply-To: <93091169a673f49c2574cddf1ef858cf0704f646.1433943052.git.jslaby@suse.cz> References: <93091169a673f49c2574cddf1ef858cf0704f646.1433943052.git.jslaby@suse.cz> In-Reply-To: References: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4155 Lines: 128 From: Oleg Nesterov 3.12-stable review patch. If anyone has any objections, please let me know. =============== commit 4b70ac5fd9b58bfaa5f25b4ea48f528aefbf3308 upstream. On 04/30, Benjamin LaHaise wrote: > > > - ctx->mmap_size = 0; > > - > > - kill_ioctx(mm, ctx, NULL); > > + if (ctx) { > > + ctx->mmap_size = 0; > > + kill_ioctx(mm, ctx, NULL); > > + } > > Rather than indenting and moving the two lines changing mmap_size and the > kill_ioctx() call, why not just do "if (!ctx) ... continue;"? That reduces > the number of lines changed and avoid excessive indentation. OK. To me the code looks better/simpler with "if (ctx)", but this is subjective of course, I won't argue. The patch still removes the empty line between mmap_size = 0 and kill_ioctx(), we reset mmap_size only for kill_ioctx(). But feel free to remove this change. ------------------------------------------------------------------------------- Subject: [PATCH v3 1/2] aio: change exit_aio() to load mm->ioctx_table once and avoid rcu_read_lock() 1. We can read ->ioctx_table only once and we do not read rcu_read_lock() or even rcu_dereference(). This mm has no users, nobody else can play with ->ioctx_table. Otherwise the code is buggy anyway, if we need rcu_read_lock() in a loop because ->ioctx_table can be updated then kfree(table) is obviously wrong. 2. Update the comment. "exit_mmap(mm) is coming" is the good reason to avoid munmap(), but another reason is that we simply can't do vm_munmap() unless current->mm == mm and this is not true in general, the caller is mmput(). 3. We do not really need to nullify mm->ioctx_table before return, probably the current code does this to catch the potential problems. But in this case RCU_INIT_POINTER(NULL) looks better. Signed-off-by: Oleg Nesterov Signed-off-by: Benjamin LaHaise Signed-off-by: Jiri Slaby --- fs/aio.c | 44 +++++++++++++++++--------------------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/fs/aio.c b/fs/aio.c index 7bdf3467bf24..aafb96629f49 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -801,38 +801,25 @@ EXPORT_SYMBOL(wait_on_sync_kiocb); */ void exit_aio(struct mm_struct *mm) { - struct kioctx_table *table; - struct kioctx *ctx; - unsigned i = 0; + struct kioctx_table *table = rcu_dereference_raw(mm->ioctx_table); + int i; - while (1) { + if (!table) + return; + + for (i = 0; i < table->nr; ++i) { + struct kioctx *ctx = table->table[i]; struct completion requests_done = COMPLETION_INITIALIZER_ONSTACK(requests_done); - rcu_read_lock(); - table = rcu_dereference(mm->ioctx_table); - - do { - if (!table || i >= table->nr) { - rcu_read_unlock(); - rcu_assign_pointer(mm->ioctx_table, NULL); - if (table) - kfree(table); - return; - } - - ctx = table->table[i++]; - } while (!ctx); - - rcu_read_unlock(); - + if (!ctx) + continue; /* - * We don't need to bother with munmap() here - - * exit_mmap(mm) is coming and it'll unmap everything. - * Since aio_free_ring() uses non-zero ->mmap_size - * as indicator that it needs to unmap the area, - * just set it to 0; aio_free_ring() is the only - * place that uses ->mmap_size, so it's safe. + * We don't need to bother with munmap() here - exit_mmap(mm) + * is coming and it'll unmap everything. And we simply can't, + * this is not necessarily our ->mm. + * Since kill_ioctx() uses non-zero ->mmap_size as indicator + * that it needs to unmap the area, just set it to 0. */ ctx->mmap_size = 0; @@ -841,6 +828,9 @@ void exit_aio(struct mm_struct *mm) /* Wait until all IO for the context are done. */ wait_for_completion(&requests_done); } + + RCU_INIT_POINTER(mm->ioctx_table, NULL); + kfree(table); } static void put_reqs_available(struct kioctx *ctx, unsigned nr) -- 2.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/