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

From: Nick Piggin
Date: Tue May 30 2006 - 01:08:23 EST


Linus Torvalds wrote:

On Tue, 30 May 2006, Nick Piggin wrote:

I guess so. Is plugging still needed now that the IO layer should
get larger requests?


Why do you think the IO layer should get larger requests?

For workloads where plugging helps (ie. lots of smaller, contiguous
requests going into the IO layer), should be pretty good these days
due to multiple readahead and writeback.


I really don't understand why people dislike plugging. It's obviously superior to non-plugged variants, exactly because it starts the IO only when _needed_, not at some random "IO request feeding point" boundary.

If you submit IO, it will be needed at some point in time. The benefit
of plugging is to hold it there in anticipation of more IO to merge
with this request.

Not sure what you mean by IO request feeding point...?


In other words, plugging works _correctly_ regardless of any random up-stream patterns. That's the kind of algorithms we want, we do NOT want to have the IO layer depend on upstream always doing the "Right Thing(tm)".

It is a heuristic. I don't see anything obviously correct or incorrect
about it. If you were to submit some asynch requests and _knew_ that
no other requests would be merged with them, plugging would be the
wrong thing to do because the disks would be needlessly idle.


So exactly why would you want to remove it?

Because of this bug ;)

I'm not saying it would never be of any benefit; I was hoping that it
might be unneeded and getting rid of it would fix this bug and
simplify a lot too.

Anyway, clearly a plug friendly bugfix needs to be implemented
regardless.

--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com -
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/