Re: [patch] fs: aio fix rcu lookup

[Nick Piggin]

On Thu, Jan 20, 2011 at 8:03 AM, Jeff Moyer <jmoyer@xxxxxxxxxx> wrote:
> Nick Piggin <npiggin@xxxxxxxxx> writes:
>
>> On Thu, Jan 20, 2011 at 7:32 AM, Jeff Moyer <jmoyer@xxxxxxxxxx> wrote:
>>> Nick Piggin <npiggin@xxxxxxxxx> writes:
>>>
>>>> On Thu, Jan 20, 2011 at 6:46 AM, Jeff Moyer <jmoyer@xxxxxxxxxx> wrote:
>>>>> Jeff Moyer <jmoyer@xxxxxxxxxx> writes:
>>>>>
>>>>>> Jan Kara <jack@xxxxxxx> writes:
>>>>>>
>>>>>>>  But there's the second race I describe making it possible
>>>>>>> for new IO to be created after io_destroy() has waited for all IO to
>>>>>>> finish...
>>>>>>
>>>>>> Can't that be solved by introducing memory barriers around the accesses
>>>>>> to ->dead?
>>>>>
>>>>> Upon further consideration, I don't think so.
>>>>>
>>>>> Given the options, I think adding the synchronize rcu to the io_destroy
>>>>> path is the best way forward.  You're already waiting for a bunch of
>>>>> queued I/O to finish, so there is no guarantee that you're going to
>>>>> finish that call quickly.
>>>>
>>>> I think synchronize_rcu() is not something to sprinkle around outside
>>>> very slow paths. It can be done without synchronize_rcu.
>>>
>>> I'm not sure I understand what you're saying.  Do you mean to imply that
>>> io_destroy is not a very slow path?  Because it is.  I prefer a solution
>>> that doesn't re-architecht things in order to solve a theoretical issue
>>> that's never been observed.
>>
>> Even something that happens once per process lifetime, like in fork/exit
>> is not necessarily suitable for RCU.
>
> Now you've really lost me.  ;-)  Processes which utilize the in-kernel
> aio interface typically create an ioctx at process startup, use that for
> submitting all of their io, then destroy it on exit.  Think of a
> database.  Every time you call io_submit, you're doing a lookup of the
> ioctx.
>
>> I don't know exactly how all programs use io_destroy -- of the small
>> number that do, probably an even smaller number would care here. But I
>> don't think it simplifies things enough to use synchronize_rcu for it.
>
> Above it sounded like you didn't think AIO should be using RCU at all.

synchronize_rcu of course, not RCU (typo).

> Here it sounds like you are just against synchronize_rcu.  Which is it?
> And if the latter, then please tell me in what cases you feel one would
> be justified in calling synchronize_rcu.  For now, I simply disagree
> with you.  As I said before, you're already potentially waiting for disk
> I/O to complete.  It doesn't get much worse than that for latency.

I think synchronize_rcu should firstly not be used unless it gives a good
simplification, or speedup in fastpath.

When that is satified, then it is a question of exactly what kind of slow
path it should be used in. I don't think it should be used in process-
synchronous code (eg syscalls) except for error cases, resource
exhaustion, management syscalls (like module unload).

For example "it's waiting for IO anyway" is not a good reason, IMO.
Firstly because it may not be waiting for a 10ms disk IO, it may be
waiting for anything up to an in-RAM device. Secondly because it
could be quite slow depending on the RCU model used.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/