Re: [rfc][patch] remove racy sync_page?

[Nick Piggin <nickpiggin@yahoo.com.au>]

Linus Torvalds wrote:

> Tell me WHERE you can unplug in that sequence. I will tell you where you 
> can NOT unplug:

...

>  - you can NOT just unplug in the path _after_ "readpage()", because the 
>    IO may have been started by SOMEBODY ELSE that just did read-ahead, and 
>    didn't unplug (on _purpose_ - the whole point of doing read-ahead is to 
>    allow concurrent IO execution, so a read-aheader that unplugs is broken 
>    by definition)

Umm, this happens with the current lock_page() after readpage. And
with per-task plugs, you do not unplug anybody else.

Sure it isn't perfect (you can still concoct a sequence of concurrent
tasks doing funny things that result in a slightly suboptimal request
pattern), but to me it sounds as good or better in some cases than
what we have now.

> So what is your alternative? Put the explicit unplug at every possible 
> occurrence of lock_page() (and keep in mind that some of them don't want 
> it: we only want it when the lock-page will block, which is not always 
> true. Some people lock the page not because it's under IO, and they're 
> waiting for it to be unlocked, but because it's dirty and they're going to 
> start IO on it - the lock_page() generally won't block on that, and if 
> it doesn't, we don't want to kick previous IO).

I keep telling you. Put the unplug after submission of IO. Not before
waiting for IO.

I'll give you one example of how it could be better (feel free to ask
for others too). Your sys_readahead(). Someone asks to readahead 1MB
of data, currently if the queue is empty, that's going to sit around
for 4ms before starting the read. 4ms later, the app comes back hoping
for the request to have finished. Unfortunately it takes another 4ms.
Throughput is halved.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com