Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1303662 > unrolled thread

[PATCH 3/5] cfq-iosched: Allow sync noidle workloads to preempt each other

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 3/5] cfq-iosched: Allow sync noidle workloads to preempt each other Jan Kara <jack@suse.cz> - 2016-01-07 16:30 +0100
    Re: [PATCH 3/5] cfq-iosched: Allow sync noidle workloads to preempt  each other Tejun Heo <tj@kernel.org> - 2016-01-08 19:20 +0100

#1303662 — [PATCH 3/5] cfq-iosched: Allow sync noidle workloads to preempt each other

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>
---
 block/cfq-iosched.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index b78a82acf99c..6c8bd1025c62 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -3969,7 +3969,6 @@ cfq_should_preempt(struct cfq_data *cfqd, struct cfq_queue *new_cfqq,
 	/* Allow preemption only if we are idling on sync-noidle tree */
 	if (cfqd->serving_wl_type == SYNC_NOIDLE_WORKLOAD &&
 	    cfqq_type(new_cfqq) == SYNC_NOIDLE_WORKLOAD &&
-	    new_cfqq->service_tree->count == 2 &&
 	    RB_EMPTY_ROOT(&cfqq->sort_list))
 		return true;
 
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1304818 — 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web