Messages in this thread | | | Date | Fri, 8 Jan 2016 13:13:30 -0500 | From | Tejun Heo <> | Subject | Re: [PATCH 3/5] cfq-iosched: Allow sync noidle workloads to preempt each other |
| |
On Thu, Jan 07, 2016 at 04:28:14PM +0100, Jan Kara wrote: > From: Jan Kara <jack@suse.com> > > The original idea with preemption of sync noidle queues (introduced in > commit 718eee0579b8 "cfq-iosched: fairness for sync no-idle queues") was > that we service all sync noidle queues together, we don't idle on any of > the queues individually and we idle only if there is no sync noidle > queue to be served. This intention also matches the original test: > > if (cfqd->serving_type == SYNC_NOIDLE_WORKLOAD > && new_cfqq->service_tree == cfqq->service_tree) > return true; > > However since at that time cfqq->service_tree was not set for idling > queues, this test was unreliable and was replaced in commit e4a229196a7c > "cfq-iosched: fix no-idle preemption logic" by: > > if (cfqd->serving_type == SYNC_NOIDLE_WORKLOAD && > cfqq_type(new_cfqq) == SYNC_NOIDLE_WORKLOAD && > new_cfqq->service_tree->count == 1) > return true; > > That was a reliable test but was actually doing something different - > now we preempt sync noidle queue only if the new queue is the only one > busy in the service tree. > > These days cfq queue is kept in service tree even if it is idling and > thus the original check would be safe again. But since we actually check > that cfq queues are in the same cgroup, of the same priority class and > workload type (sync noidle), we know that new_cfqq is fine to preempt > cfqq. So just remove the service tree check. > > Signed-off-by: Jan Kara <jack@suse.com>
Acked-by: Tejun Heo <tj@kernel.org>
Thanks.
-- tejun
| |