From patchwork Wed May 16 22:45:18 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Fernandes X-Patchwork-Id: 10405019 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 48FB060155 for ; Wed, 16 May 2018 22:46:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3A0D228824 for ; Wed, 16 May 2018 22:46:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2E60F2883B; Wed, 16 May 2018 22:46:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 45F5128824 for ; Wed, 16 May 2018 22:46:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751332AbeEPWqK (ORCPT ); Wed, 16 May 2018 18:46:10 -0400 Received: from mail-pl0-f65.google.com ([209.85.160.65]:42447 "EHLO mail-pl0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751153AbeEPWqJ (ORCPT ); Wed, 16 May 2018 18:46:09 -0400 Received: by mail-pl0-f65.google.com with SMTP id u6-v6so1262318pls.9 for ; Wed, 16 May 2018 15:46:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes.org; s=google; h=from:to:cc:subject:date:message-id; bh=Oe+UwTSj5yd95ZI37gAyk6Z1pwAxb9RX/5VheBJGr3c=; b=gbNHf3NGSFkn+EsyqNF0GZe0kXULZap4XxVNgikBXXwFmmO5ci6hbILhSUN6UyooYM gsLil8JGx5GK/SYmS+dtqUvEDY9y5Y+S1J0Kd5J4VDzvYsXBBrDLCG4N/yjpZ8auit1M cqHzEAoxh6XR09fjNB7qOSW0/7Ft/vpyTlfvs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=Oe+UwTSj5yd95ZI37gAyk6Z1pwAxb9RX/5VheBJGr3c=; b=Q39iS/MXfKai5dsO30QnATqTesRoGQ7tjWH7heDpDeIJMxrN5pGmHluDk2DJBwfxdI dZfl5iaqmI7RMJN/kcvu2afkz/2U2Mu9G+OgmU074iFCuTpV29GIkSn14RezcFPTaszP +klv9f44HxyoAPjsQOaIo8lHl24AALYhTNPJivsP3SDqjK/NX3IQ4Id7YoS22u3DKTdr OJ1B6ghPrGm+JzKJ0sSk0JGYSIEvsfOLQYsyTV7K+KJpccd3aidX8cm3YO9dMawGM+G/ F52UO7kQCGxsU9oaqx9EGVokwYjx5pknkUR4IJZsCLvqubFlFGeeGVNELVRNWKClXTSl z11Q== X-Gm-Message-State: ALKqPwd4RjzjiGoAM6MsEqpJi0W+BpAjaWb8wsfQxpJ07HE2/RiqgULb +K8a5/vt2IbhFd+r7VOfVsh/M0QJLhI= X-Google-Smtp-Source: AB8JxZoavwNJWDD/tOIHEIEbpPglC12UtYkf7EB3FesH3Vxz3lYF1nuuv60dmk/PxsiF57mBvFWafw== X-Received: by 2002:a17:902:6a85:: with SMTP id n5-v6mr2745961plk.288.1526510768838; Wed, 16 May 2018 15:46:08 -0700 (PDT) Received: from joelaf.mtv.corp.google.com ([2620:0:1000:1600:3122:ea9c:d178:eb]) by smtp.gmail.com with ESMTPSA id o70-v6sm5515560pfo.49.2018.05.16.15.46.07 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 16 May 2018 15:46:08 -0700 (PDT) From: "Joel Fernandes (Google)" To: linux-kernel@vger.kernel.org Cc: "Joel Fernandes (Google)" , Viresh Kumar , "Rafael J . Wysocki" , Peter Zijlstra , Ingo Molnar , Patrick Bellasi , Juri Lelli , Luca Abeni , Joel Fernandes , linux-pm@vger.kernel.org Subject: [PATCH RFC] schedutil: Allow cpufreq requests to be made even when kthread kicked Date: Wed, 16 May 2018 15:45:18 -0700 Message-Id: <20180516224518.109891-1-joel@joelfernandes.org> X-Mailer: git-send-email 2.17.0.441.gb46fe60e1d-goog Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Currently there is a chance of a schedutil cpufreq update request to be dropped if there is a pending update request. This pending request can be delayed if there is a scheduling delay of the irq_work and the wake up of the schedutil governor kthread. A very bad scenario is when a schedutil request was already just made, such as to reduce the CPU frequency, then a newer request to increase CPU frequency (even sched deadline urgent frequency increase requests) can be dropped, even though the rate limits suggest that its Ok to process a request. This is because of the way the work_in_progress flag is used. This patch improves the situation by allowing new requests to happen even though the old one is still being processed. Note that in this approach, if an irq_work was already issued, we just update next_freq and don't bother to queue another request so there's no extra work being done to make this happen. I had brought up this issue at the OSPM conference and Claudio had a discussion RFC with an alternate approach [1]. I prefer the approach as done in the patch below since it doesn't need any new flags and doesn't cause any other extra overhead. [1] https://patchwork.kernel.org/patch/10384261/ CC: Viresh Kumar CC: Rafael J. Wysocki CC: Peter Zijlstra CC: Ingo Molnar CC: Patrick Bellasi CC: Juri Lelli Cc: Luca Abeni CC: Joel Fernandes CC: linux-pm@vger.kernel.org Signed-off-by: Joel Fernandes (Google) --- Claudio, Could you also test this patch for your usecase? kernel/sched/cpufreq_schedutil.c | 36 +++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index e13df951aca7..a87fc281893d 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -92,9 +92,6 @@ static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time) !cpufreq_can_do_remote_dvfs(sg_policy->policy)) return false; - if (sg_policy->work_in_progress) - return false; - if (unlikely(sg_policy->need_freq_update)) { sg_policy->need_freq_update = false; /* @@ -129,8 +126,11 @@ static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time, policy->cur = next_freq; trace_cpu_frequency(next_freq, smp_processor_id()); } else { - sg_policy->work_in_progress = true; - irq_work_queue(&sg_policy->irq_work); + /* Don't queue request if one was already queued */ + if (!sg_policy->work_in_progress) { + sg_policy->work_in_progress = true; + irq_work_queue(&sg_policy->irq_work); + } } } @@ -291,6 +291,15 @@ static void sugov_update_single(struct update_util_data *hook, u64 time, ignore_dl_rate_limit(sg_cpu, sg_policy); + /* + * For slow-switch systems, single policy requests can't run at the + * moment if the governor thread is already processing a pending + * frequency switch request, this can be fixed by acquiring update_lock + * while updating next_freq and work_in_progress but we prefer not to. + */ + if (sg_policy->work_in_progress) + return; + if (!sugov_should_update_freq(sg_policy, time)) return; @@ -382,13 +391,24 @@ sugov_update_shared(struct update_util_data *hook, u64 time, unsigned int flags) static void sugov_work(struct kthread_work *work) { struct sugov_policy *sg_policy = container_of(work, struct sugov_policy, work); + unsigned int freq; + unsigned long flags; + + /* + * Hold sg_policy->update_lock shortly to handle the case where: + * incase sg_policy->next_freq is read here, and then updated by + * sugov_update_shared just before work_in_progress is set to false + * here, we may miss queueing the new update. + */ + raw_spin_lock_irqsave(&sg_policy->update_lock, flags); + freq = sg_policy->next_freq; + sg_policy->work_in_progress = false; + raw_spin_unlock_irqrestore(&sg_policy->update_lock, flags); mutex_lock(&sg_policy->work_lock); - __cpufreq_driver_target(sg_policy->policy, sg_policy->next_freq, + __cpufreq_driver_target(sg_policy->policy, freq, CPUFREQ_RELATION_L); mutex_unlock(&sg_policy->work_lock); - - sg_policy->work_in_progress = false; } static void sugov_irq_work(struct irq_work *irq_work)