On 10/16/2014 06:27 AM, Jan Kara wrote: > Hello, > > one of our customers was complaining that elv_attempt_insert_merge() > merges two requests (via blk_attempt_req_merge()) without asking IO > scheduler for permission (->elevator_allow_merge_fn() callback). Now for > them this is a problem because of their custom IO scheduler but looking > into the code this can result in somewhat suboptimal behavior for CFQ as > well (merging two requests from different IO contexts, possibly merging > sync & async request). What do others think about this? > > Regarding possible fix, we cannot really call ->elevator_allow_merge_fn() > because that assumes it is called from a context of a process submitting the > passed bio. So we would need to create a separate allow merge callback for > this.
It would need a new (rq to rq) merge hook, if they have a custom IO scheduler, they should submit a change to allow that kind of behaviour. Outside of potentially mixing sync and async IO (which seems like something that should rarely/never happen), not sure I see a lot of downsides. And that case could be explicitly checked in attempt_merge() or blk_attempt_req_merge() without having to define a new hook to catch that specific case. For the hook, cfq would lookup the io contexts and compare, and basically disallow any merge that crosses a cfq io context boundary. But given that I would only expect these types of merges to happen very rarely, the sync vs async check would be good enough for me. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
