Re: [PATCH 0/1] aio: change exit_aio() to load mm->ioctx_table once and avoid rcu_read_lock()

2014-04-29 Thread Kent Overstreet
On Tue, Apr 29, 2014 at 08:39:30PM +0200, Oleg Nesterov wrote: > 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

Re: [PATCH 0/1] aio: change exit_aio() to load mm->ioctx_table once and avoid rcu_read_lock()

2014-04-29 Thread Oleg Nesterov
On 04/29, Benjamin LaHaise wrote: > > On Tue, Apr 29, 2014 at 08:39:15PM +0200, Oleg Nesterov wrote: > > > > Kent, could you also explain kioctx->dead is atomic_t? This looks > > pointless? afaics atomic_ buys nothing in this case. > > If it wasn't atomic, it would need to be volatile. Then

Re: [PATCH 0/1] aio: change exit_aio() to load mm->ioctx_table once and avoid rcu_read_lock()

2014-04-29 Thread Benjamin LaHaise
On Tue, Apr 29, 2014 at 08:39:15PM +0200, Oleg Nesterov wrote: > Completely untested and I know nothing about aio, thus needs an ack > from maintainers or should be ignored. > > But exit_aio() looks "obviously unnecessarily overcomplicated", I was > really puzzled when I looked at this code by

[PATCH 0/1] aio: change exit_aio() to load mm->ioctx_table once and avoid rcu_read_lock()

2014-04-29 Thread Oleg Nesterov
Completely untested and I know nothing about aio, thus needs an ack from maintainers or should be ignored. But exit_aio() looks "obviously unnecessarily overcomplicated", I was really puzzled when I looked at this code by accident. Kent, could you also explain kioctx->dead is atomic_t? This

Re: [PATCH 0/1] aio: change exit_aio() to load mm->ioctx_table once and avoid rcu_read_lock()

2014-04-29 Thread Oleg Nesterov
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

[PATCH 0/1] aio: change exit_aio() to load mm-ioctx_table once and avoid rcu_read_lock()

2014-04-29 Thread Oleg Nesterov
Completely untested and I know nothing about aio, thus needs an ack from maintainers or should be ignored. But exit_aio() looks obviously unnecessarily overcomplicated, I was really puzzled when I looked at this code by accident. Kent, could you also explain kioctx-dead is atomic_t? This looks

Re: [PATCH 0/1] aio: change exit_aio() to load mm-ioctx_table once and avoid rcu_read_lock()

2014-04-29 Thread Oleg Nesterov
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

Re: [PATCH 0/1] aio: change exit_aio() to load mm-ioctx_table once and avoid rcu_read_lock()

2014-04-29 Thread Benjamin LaHaise
On Tue, Apr 29, 2014 at 08:39:15PM +0200, Oleg Nesterov wrote: Completely untested and I know nothing about aio, thus needs an ack from maintainers or should be ignored. But exit_aio() looks obviously unnecessarily overcomplicated, I was really puzzled when I looked at this code by accident.

Re: [PATCH 0/1] aio: change exit_aio() to load mm-ioctx_table once and avoid rcu_read_lock()

2014-04-29 Thread Oleg Nesterov
On 04/29, Benjamin LaHaise wrote: On Tue, Apr 29, 2014 at 08:39:15PM +0200, Oleg Nesterov wrote: Kent, could you also explain kioctx-dead is atomic_t? This looks pointless? afaics atomic_ buys nothing in this case. If it wasn't atomic, it would need to be volatile. Then ACCESS_ONCE() in

Re: [PATCH 0/1] aio: change exit_aio() to load mm-ioctx_table once and avoid rcu_read_lock()

2014-04-29 Thread Kent Overstreet
On Tue, Apr 29, 2014 at 08:39:30PM +0200, Oleg Nesterov wrote: 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