Messages in this thread | | | Date | Tue, 1 Feb 2011 19:52:25 +0100 | From | Tejun Heo <> | Subject | Re: [RFC PATCH 4/3] block: skip elevator initialization for flush requests |
| |
Hello,
On Tue, Feb 01, 2011 at 12:38:46PM -0500, Mike Snitzer wrote: > > I thought about doing it this way but I think we're burying the > > REQ_FLUSH|REQ_FUA test logic too deep. get_request() shouldn't > > "magically" know not to allocate elevator data. > > There is already a considerable amount of REQ_FLUSH|REQ_FUA special > casing magic sprinkled though-out the block layer. Why is this > get_request() change the case that goes too far?
After the reimplementation, FLUSH implementation seems to be pretty well isolated. Also, having REQ_FLUSH logic in the issue and completion paths is logical and preventing them from leaking to other places sounds like a good idea.
> > The decision should > > be made higher in the stack and passed down to get_request(). e.g. if > > REQ_SORTED is set in @rw, elevator data is allocated; otherwise, not. > > Considering REQ_SORTED is set in elv_insert(), well after get_request() > is called, I'm not seeing what you're suggesting.
I was suggesting using REQ_SORTED in @rw parameter to indicate "this request may be sorted and thus needs elevator data allocation".
> Anyway, I agree that ideally we'd have a mechanism to explicitly > short-circuit elevator initialization. But doing so in a meaningful way > would likely require a fair amount of refactoring of get_request* and > its callers. I'll come back to this and have another look but my gut is > this interface churn wouldn't _really_ help -- all things considered.
I don't know. I agree that it's not a critical issue but, to me, subjectively of course, it feels a bit too subtle. The sharing of fields using unions is already subtle enough. I with that at least the allocation switching would be obvious and explicit. The combined subtleties scare me.
Thank you.
-- tejun
| |