From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753948AbbFJCrn (ORCPT ); Tue, 9 Jun 2015 22:47:43 -0400 Received: from mail-pd0-f171.google.com ([209.85.192.171]:33683 "EHLO mail-pd0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752183AbbFJCrf (ORCPT ); Tue, 9 Jun 2015 22:47:35 -0400 Date: Wed, 10 Jun 2015 11:47:27 +0900 From: Tejun Heo To: Jeff Moyer Cc: axboe@kernel.dk, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, vgoyal@redhat.com, avanzini.arianna@gmail.com Subject: Re: [PATCH 7/8] cfq-iosched: fold cfq_find_alloc_queue() into cfq_get_queue() Message-ID: <20150610024727.GD11955@mtj.duckdns.org> References: <1433753973-23684-1-git-send-email-tj@kernel.org> <1433753973-23684-8-git-send-email-tj@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hey, Jeff. On Tue, Jun 09, 2015 at 10:40:02AM -0400, Jeff Moyer wrote: > The resulting code (introduced by the last patch, I know) is not ideal: > > rcu_read_lock(); > cfqg = cfq_lookup_create_cfqg(cfqd, bio_blkcg(bio)); > if (!cfqg) { > cfqq = &cfqd->oom_cfqq; > goto out; > } > > if (!is_sync) { > if (!ioprio_valid(cic->ioprio)) { > struct task_struct *tsk = current; > ioprio = task_nice_ioprio(tsk); > ioprio_class = task_nice_ioclass(tsk); > } > async_cfqq = cfq_async_queue_prio(cfqd, ioprio_class, > ioprio); > cfqq = *async_cfqq; > if (cfqq) > goto out; > } > > As you mentioned, we don't need to lookup the cfqg for the async queue. > What's more is we could fallback to the oom_cfqq even if we had an > existing async cfqq. I'm guessing you structured the code this way to > make the error path cleaner. I don't think it's a big deal, as it > should be a rare occurrence, so... In this patch, the lookup seems unnecessary for the async case but the change is required for the following changes because async queues are moved from cfq_data to cfq_group, so we can't determine async queues w/o looking up cfqg at all and we'll have to fall back to oom_cfqq if cfqg lookup fails (cuz there's no system-wide async queues). Thanks. -- tejun