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 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
