From patchwork Fri May 18 18:55:01 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Fernandes X-Patchwork-Id: 10411913 X-Patchwork-Delegate: rjw@sisk.pl 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 EBF81602C2 for ; Fri, 18 May 2018 18:55:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D875C28A8C for ; Fri, 18 May 2018 18:55:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CA8AE28A8E; Fri, 18 May 2018 18:55:25 +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=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL 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 B314B28A8C for ; Fri, 18 May 2018 18:55:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752008AbeERSzX (ORCPT ); Fri, 18 May 2018 14:55:23 -0400 Received: from mail-pg0-f44.google.com ([74.125.83.44]:37072 "EHLO mail-pg0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751589AbeERSzW (ORCPT ); Fri, 18 May 2018 14:55:22 -0400 Received: by mail-pg0-f44.google.com with SMTP id a13-v6so3686532pgu.4 for ; Fri, 18 May 2018 11:55:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=LGZLW3AYcIHo8xYpveO5bq4qClMMQzpok1BLzSUcW6w=; b=E7r5nvOxshc+HiCtbna38B0KjE2ljDGZHqVhMGYZL6jI1BVxNjB1yspP5UU6m/qBAn LhwIyfwI0g6rPFgmLPaHu/wa29/yQJffxsDekbFShU4m4e/TFWoKV1eD5DAsIfdyv8uB GUet4XKIF0z6C2E4q7+EWRO5nU7RrHE8Md1v4KkVpTO1CqO5+6Oxmn/zBV6cwRTbNR99 6PpaUpH+c6KUsxwFfHrhsKhpBdWzhMtYxIrz/Zp+I762duAF2800TVinDqZ1/6AJGy1y E9uPwKRr67CJaoiXDdxO5W7attqRHphlkfvNivDbB0u2gFBcqcgl9IkOEjSco4TOUoSY Dq/A== 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=LGZLW3AYcIHo8xYpveO5bq4qClMMQzpok1BLzSUcW6w=; b=tj3TOLIFs/N3HsAW892k1ppxOQ1gXC8k9QkpOsfCqjK6u4jTKiDjTcKnhAbfW1uLUC qS0xJKKcyAyaF2TLvGRehdb/ENXYN+1jk9yrNlUSPhmvoziUIzV/zAp1dp4WrO/VImMp BfA1v1IcCsSIsYBamivFEwo73WBty6fyw10Hg4wkFSUhFj5uDd74zyO9KOl2kWLCZsrE 3pULQAqmZj9gpRqZtK2V0Tn63xXb/FD9AARm7YfSCY6SM/LG0BWY0xZvkllJ7I82KTYq VWe7jJn9l/S3P0+4nIwJY6sHd93J7rXpbG/koSknLfbutxmLUcJvu8q26bGSiDjg2txy C/lQ== X-Gm-Message-State: ALKqPwffJqg2H7aq6A6Kqq1WAXaGI51qEKn61G8z84onnCmfPLfKm37B mDfvTmsBers53IilswzRwUPSEg== X-Google-Smtp-Source: AB8JxZpQjPazoOQxBobMRQw5uXxwNpiZO9GmNiJ7scQ1ghvHiLBXGAxGuFPWNHRhNyE0BYenQL6v5A== X-Received: by 2002:a62:f24a:: with SMTP id y10-v6mr10573179pfl.75.1526669721738; Fri, 18 May 2018 11:55:21 -0700 (PDT) Received: from joelaf.mtv.corp.google.com ([2620:0:1000:1600:3122:ea9c:d178:eb]) by smtp.gmail.com with ESMTPSA id x2-v6sm11658061pgc.53.2018.05.18.11.55.20 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 May 2018 11:55:20 -0700 (PDT) From: "Joel Fernandes (Google.)" X-Google-Original-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 , Todd Kjos , claudio@evidence.eu.com, kernel-team@android.com, linux-pm@vger.kernel.org Subject: [PATCH v2] schedutil: Allow cpufreq requests to be made even when kthread kicked Date: Fri, 18 May 2018 11:55:01 -0700 Message-Id: <20180518185501.173552-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 From: "Joel Fernandes (Google)" 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/ LGTMed-by: Viresh Kumar LGTMed-by: Juri Lelli 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: Todd Kjos CC: claudio@evidence.eu.com CC: kernel-team@android.com CC: linux-pm@vger.kernel.org Signed-off-by: Joel Fernandes (Google) Acked-by: Viresh Kumar --- v1 -> v2: Minor style related changes. kernel/sched/cpufreq_schedutil.c | 34 ++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index e13df951aca7..5c482ec38610 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; /* @@ -128,7 +125,7 @@ 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 { + } else if (!sg_policy->work_in_progress) { sg_policy->work_in_progress = true; irq_work_queue(&sg_policy->irq_work); } @@ -291,6 +288,13 @@ 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 update is in progress, unless we acquire update_lock. + */ + if (sg_policy->work_in_progress) + return; + if (!sugov_should_update_freq(sg_policy, time)) return; @@ -382,13 +386,27 @@ 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. + * + * Note: If a work was queued after the update_lock is released, + * sugov_work will just be called again by kthread_work code; and the + * request will be proceed before the sugov thread sleeps. + */ + 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_RELATION_L); + __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)