From patchwork Tue May 12 16:30:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: ahaslam@baylibre.com X-Patchwork-Id: 6389621 Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id CCE84BEEE1 for ; Tue, 12 May 2015 16:30:42 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E0081203E3 for ; Tue, 12 May 2015 16:30:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B453720390 for ; Tue, 12 May 2015 16:30:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753527AbbELQaj (ORCPT ); Tue, 12 May 2015 12:30:39 -0400 Received: from mail-ob0-f178.google.com ([209.85.214.178]:34480 "EHLO mail-ob0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753185AbbELQaj (ORCPT ); Tue, 12 May 2015 12:30:39 -0400 Received: by obfe9 with SMTP id e9so9814506obf.1 for ; Tue, 12 May 2015 09:30:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:date:message-id:subject:from:to :content-type; bh=ZoSptPiIVS4den7zQAKZUc59eTy8qHI8FYJsqFUshJ0=; b=Sc89dnn0rgVb7XjQf4F5zzFAQiiC6RJVskNIUbsbMnEPN8y04sFylbDPMGqY74k5Bm pXA6Fqp5kh6CMT3IWgqDGMtTjQT4yde4obIemf/XAAhjEkMtoJpmbI4aYUk5W3hWFaPP uZQ7xSSZ2K8ZASNu1ZZASmhfcYAwtTJQ+dnzACsf+S7YWu6YMUC373ehIR+7a/WBfnBD igSXJwwLjIjLxLm7cugZK1ATzSbFyFkk66HE4jh/gMOgEQ5/t+uT9inROKtkA1uToI2x tp8ZU05ADA2nrWfWDeBwG9YLvGM2utiu1iXJA/ZXVmXKzH2xGTDNebD+eR+ti/smtuA7 yXWg== X-Gm-Message-State: ALoCoQmbs/BJqUrRrt3oPICzUr5m/TiZrdDJVfHr7tLPotsNOb/aAmqwWX295VyyHhcyBw9N9JfY MIME-Version: 1.0 X-Received: by 10.60.155.196 with SMTP id vy4mr12589373oeb.55.1431448238404; Tue, 12 May 2015 09:30:38 -0700 (PDT) Received: by 10.76.98.193 with HTTP; Tue, 12 May 2015 09:30:38 -0700 (PDT) Date: Tue, 12 May 2015 18:30:38 +0200 Message-ID: Subject: validate callback for menu governor From: Axel Haslam To: Lina Iyer , Kevin Hilman , Ulf Hansson , Geert Uytterhoeven , "Rafael J. Wysocki" , Linux PM list , =?UTF-8?Q?Bartosz_Go=C5=82aszewski?= , Benoit Cousson Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi, Recently a question about cpuidle and cluster off states came up in a separate discussion [1]. The question is if it would be useful to add a "validate" callback in the cpuidle menu governor. Cpuidle drivers could use this callback to check if entering a cluster off state by the "last man" would not violate the target residency requirements. i think the same problem is explained at the top of ./drivers/cpuidle/cpuidle-big_little.c, so im sorry if this was addressed before. To illustrate the scenario, on a system where cpu0 and cpu1 are in the same cluster and where the target residency time for the cluster off state is 5ms, imagine that: cpu 1 enters the cluster off state at time t0. the predicted wakeup by the governor for this cpu is in 20ms cpu 0 enters the cluster off state at t0 + 19ms. the predicted wakeup is in 30ms. in this case the cluster off state will actually be entered 1ms before cpu1 is predicted to wake up, and the cluster off residency constraint would not be met. To avoid this, before entering the cluster off state, the last cpu could "validate" that the predicted wakeup time of all the cpus in his cluster *still* don't violate the cluster off state requirements. maybe this is a corner case that does not justify the added logic, which ends up being more expensive. a simple prototype looks something like this: Regards Axel [1] http://marc.info/?l=linux-pm&m=143143190116868&w=2 --- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index b8a5fa1..bdd3211 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -20,6 +20,7 @@ #include #include #include +#include /* * Please note when changing the tuning values: @@ -124,6 +125,7 @@ struct menu_device { unsigned int next_timer_us; unsigned int predicted_us; + unsigned int predicted_time_us; unsigned int bucket; unsigned int correction_factor[BUCKETS]; unsigned int intervals[INTERVALS]; @@ -276,6 +278,45 @@ again: goto again; } +/* Check if the idle state is still valid for a given cpu */ +static bool menu_validate(struct cpuidle_driver *drv, + struct cpuidle_device *dev, + int cpu, + int state) +{ + unsigned int now_us; + int last_state_idx = CPUIDLE_DRIVER_STATE_START - 1; + struct menu_device *data; + unsigned int remaining_us; + int i; + + now_us = ktime_to_us(ktime_get_raw()); + data = per_cpu_ptr(&menu_devices, cpu); + remaining_us = data->predicted_time_us - now_us; + + /* Get the new recomended state for this cpu */ + for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) { + struct cpuidle_state *s = &drv->states[i]; + struct cpuidle_state_usage *su = &dev->states_usage[i]; + + if (s->disabled || su->disable) + continue; + + if (s->target_residency > remaining_us) + continue; + + last_state_idx = i; + + /* state (or deeper) is still allowed */ + if (last_state_idx >= state) + return true; + } + + /* the cpu doest not allow for the requested state */ + return false; + +} + /** * menu_select - selects the next idle state to enter * @drv: cpuidle driver containing state data @@ -352,6 +393,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) data->last_state_idx = i; } + data->predicted_time_us = ktime_to_us(ktime_get_raw()) + + data->predicted_us; return data->last_state_idx; } @@ -470,6 +513,7 @@ static struct cpuidle_governor menu_governor = { .enable = menu_enable_device, .select = menu_select, .reflect = menu_reflect, + .validate = menu_validate, .owner = THIS_MODULE, }; diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h index 9c5e892..e917fa9 100644 --- a/include/linux/cpuidle.h +++ b/include/linux/cpuidle.h @@ -226,6 +226,11 @@ struct cpuidle_governor { struct cpuidle_device *dev); void (*reflect) (struct cpuidle_device *dev, int index); + bool (*validate) (struct cpuidle_driver *drv, + struct cpuidle_device *dev, + int cpu, + int state); + struct module *owner; };