From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756350AbcAHSNf (ORCPT ); Fri, 8 Jan 2016 13:13:35 -0500 Received: from mail-yk0-f181.google.com ([209.85.160.181]:35960 "EHLO mail-yk0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756334AbcAHSNc (ORCPT ); Fri, 8 Jan 2016 13:13:32 -0500 Date: Fri, 8 Jan 2016 13:13:30 -0500 From: Tejun Heo To: Jan Kara Cc: axboe@kernel.dk, linux-kernel@vger.kernel.org, Jeff Moyer , Jan Kara Subject: Re: [PATCH 3/5] cfq-iosched: Allow sync noidle workloads to preempt each other Message-ID: <20160108181330.GW1898@mtj.duckdns.org> References: <1452180496-18483-1-git-send-email-jack@suse.cz> <1452180496-18483-4-git-send-email-jack@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1452180496-18483-4-git-send-email-jack@suse.cz> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 07, 2016 at 04:28:14PM +0100, Jan Kara wrote: > From: Jan Kara > > 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 Acked-by: Tejun Heo Thanks. -- tejun