]> git.itanic.dy.fi Git - linux-stable/commit
aio: change exit_aio() to load mm->ioctx_table once and avoid rcu_read_lock()
authorOleg Nesterov <oleg@redhat.com>
Wed, 30 Apr 2014 17:02:48 +0000 (19:02 +0200)
committerBenjamin LaHaise <bcrl@kvack.org>
Tue, 24 Jun 2014 22:10:24 +0000 (18:10 -0400)
commit4b70ac5fd9b58bfaa5f25b4ea48f528aefbf3308
treeef245ebab7daea1b368d20faa3314ab99a799022
parentedfbbf388f293d70bf4b7c0bc38774d05e6f711a
aio: change exit_aio() to load mm->ioctx_table once and avoid rcu_read_lock()

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 <oleg@redhat.com>
Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
fs/aio.c