From patchwork Thu May 7 16:56:20 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Lezcano X-Patchwork-Id: 48134 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-lb0-f198.google.com (mail-lb0-f198.google.com [209.85.217.198]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 9E4AF20553 for ; Thu, 7 May 2015 16:57:23 +0000 (UTC) Received: by lbbrr5 with SMTP id rr5sf14057777lbb.3 for ; Thu, 07 May 2015 09:57:22 -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:delivered-to:from:to:cc:subject :date:message-id:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=iveoyVXZ8/hde50TMjDLMepYm372Du8L/gsnkem7Epo=; b=QAxewFlbefd4TQF31yOLKiE3hS5gZ/Tum8OEHgsng1yE77EILOKkfoiuf4xqzYr4Sl vYymHKv+p52FhVidJ7Fa1dKtLHrqpRlYRRumSW1oiBeTWO6AGIjGeleEuZmdPevfoY3u xr1tSKHCx2HwvGEMiTgHWJA6sDufPKFzDIyP1+WFOV3SKXyH9MoUPQYBEQTSlyEJ1TP7 I/Xrv4xxzCgc7+5Ry5ukBhdD2OeDMOO8bYGA/SCo5s/8nuxD+SHFyeG79Xhb8KA6UB9M ObL/g7Xby/Ajst6UaiUMNfG+sTc/GKH+eLZwThFEHEK7IOLwqyfpVU3609OYYUjd5H4j kCGA== X-Gm-Message-State: ALoCoQns77raFdRpMl7Y76fOFE1MscNeT3NpfXPS8zj8I4QCabXOtUmyBkDCJxe7byZkmDKi80kZ X-Received: by 10.113.11.3 with SMTP id ee3mr2664491lbd.9.1431017842471; Thu, 07 May 2015 09:57:22 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.23.232 with SMTP id p8ls238418laf.13.gmail; Thu, 07 May 2015 09:57:22 -0700 (PDT) X-Received: by 10.152.184.101 with SMTP id et5mr3800179lac.43.1431017842319; Thu, 07 May 2015 09:57:22 -0700 (PDT) Received: from mail-la0-f42.google.com (mail-la0-f42.google.com. [209.85.215.42]) by mx.google.com with ESMTPS id lv6si1642417lac.86.2015.05.07.09.57.21 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 07 May 2015 09:57:21 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.42 as permitted sender) client-ip=209.85.215.42; Received: by layy10 with SMTP id y10so35244095lay.0 for ; Thu, 07 May 2015 09:57:21 -0700 (PDT) X-Received: by 10.112.199.133 with SMTP id jk5mr3958934lbc.32.1431017841879; Thu, 07 May 2015 09:57:21 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.108.230 with SMTP id hn6csp38890lbb; Thu, 7 May 2015 09:57:20 -0700 (PDT) X-Received: by 10.68.135.73 with SMTP id pq9mr8592511pbb.46.1431017839826; Thu, 07 May 2015 09:57:19 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id py8si3505273pdb.157.2015.05.07.09.57.18; Thu, 07 May 2015 09:57:19 -0700 (PDT) Received-SPF: none (google.com: linux-acpi-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751461AbbEGQ5R (ORCPT + 7 others); Thu, 7 May 2015 12:57:17 -0400 Received: from mail-wg0-f54.google.com ([74.125.82.54]:34537 "EHLO mail-wg0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751197AbbEGQ5Q (ORCPT ); Thu, 7 May 2015 12:57:16 -0400 Received: by wgic8 with SMTP id c8so23228923wgi.1 for ; Thu, 07 May 2015 09:57:15 -0700 (PDT) X-Received: by 10.180.218.137 with SMTP id pg9mr8246601wic.79.1431017835220; Thu, 07 May 2015 09:57:15 -0700 (PDT) Received: from localhost.localdomain (sju31-1-78-210-255-2.fbx.proxad.net. [78.210.255.2]) by mx.google.com with ESMTPSA id q4sm4313637wja.24.2015.05.07.09.57.13 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 07 May 2015 09:57:14 -0700 (PDT) From: Daniel Lezcano To: daniel.lezcano@linaro.org Cc: "Rafael J. Wysocki" , Len Brown , Mohammad Merajul Islam Molla , Tejun Heo , Christoph Lameter , Mel Gorman , Javi Merino , Jiri Kosina , Antonio Ospite , linux-acpi@vger.kernel.org (open list:ACPI), linux-kernel@vger.kernel.org (open list), linux-pm@vger.kernel.org (open list:CPUIDLE DRIVERS) Subject: [RFC PATCH] cpuidle: Remove the default poll idle loop Date: Thu, 7 May 2015 18:56:20 +0200 Message-Id: <1431017784-19599-1-git-send-email-daniel.lezcano@linaro.org> X-Mailer: git-send-email 1.9.1 Sender: linux-acpi-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: daniel.lezcano@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.42 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , The poll idle loop is useful only for the *menu* governor: when there is a timer about to shutdown very soon (less than 5us), then we default to the poll idle if no other idle state is found, otherwise the 'hlt' state is the default. The poll idle state is x86 specific, hence leading to the DRIVER_START index hell all around the code and prone to buggy code. I have been looking on a x86 24 cpus system, how many times this state is called. It appears to be very rarely used with a recent kernel. Furthermore the poll code is more than 7 years old and now very fast idle instruction exists (less than 1us). Another pitfall with the poll idle state is when the governor makes a bad prediction and selects this state while the idle state duration is much more longer, that results in an excessive energy consumption. Remove this state and the related code as it brings more complexity for a very discutable benefit. Signed-off-by: Daniel Lezcano --- drivers/acpi/processor_idle.c | 8 ++++---- drivers/cpuidle/cpuidle.c | 6 +++--- drivers/cpuidle/driver.c | 32 -------------------------------- drivers/cpuidle/governors/ladder.c | 12 ++++++------ drivers/cpuidle/governors/menu.c | 13 ++----------- include/linux/cpuidle.h | 6 ------ 6 files changed, 15 insertions(+), 62 deletions(-) diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 39e0c8e..4fbbe07 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -799,7 +799,7 @@ static int acpi_idle_enter(struct cpuidle_device *dev, if (cx->type != ACPI_STATE_C1) { if (acpi_idle_fallback_to_c1(pr) && num_online_cpus() > 1) { - index = CPUIDLE_DRIVER_STATE_START; + index = 0; cx = per_cpu(acpi_cstate[index], dev->cpu); } else if (cx->type == ACPI_STATE_C3 && pr->flags.bm_check) { if (cx->bm_sts_skip || !acpi_idle_bm_check()) { @@ -863,7 +863,7 @@ struct cpuidle_driver acpi_idle_driver = { static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr, struct cpuidle_device *dev) { - int i, count = CPUIDLE_DRIVER_STATE_START; + int i, count = 0; struct acpi_processor_cx *cx; if (!pr->flags.power_setup_done) @@ -908,7 +908,7 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr, */ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr) { - int i, count = CPUIDLE_DRIVER_STATE_START; + int i, count = 0; struct acpi_processor_cx *cx; struct cpuidle_state *state; struct cpuidle_driver *drv = &acpi_idle_driver; @@ -920,7 +920,7 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr) return -EINVAL; drv->safe_state_index = -1; - for (i = CPUIDLE_DRIVER_STATE_START; i < CPUIDLE_STATE_MAX; i++) { + for (i = 0; i < CPUIDLE_STATE_MAX; i++) { drv->states[i].name[0] = '\0'; drv->states[i].desc[0] = '\0'; } diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 61c417b..ffdbf1f 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -65,7 +65,7 @@ int cpuidle_play_dead(void) return -ENODEV; /* Find lowest-power state that supports long-term idle */ - for (i = drv->state_count - 1; i >= CPUIDLE_DRIVER_STATE_START; i--) + for (i = drv->state_count - 1; i >= 0; i--) if (drv->states[i].enter_dead) return drv->states[i].enter_dead(dev, i); @@ -76,9 +76,9 @@ static int find_deepest_state(struct cpuidle_driver *drv, struct cpuidle_device *dev, bool freeze) { unsigned int latency_req = 0; - int i, ret = freeze ? -1 : CPUIDLE_DRIVER_STATE_START - 1; + int i, ret = freeze ? -1 : 0; - for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) { + for (i = 0; i < drv->state_count; i++) { struct cpuidle_state *s = &drv->states[i]; struct cpuidle_state_usage *su = &dev->states_usage[i]; diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c index 5db1478..8b51220 100644 --- a/drivers/cpuidle/driver.c +++ b/drivers/cpuidle/driver.c @@ -177,36 +177,6 @@ static void __cpuidle_driver_init(struct cpuidle_driver *drv) } } -#ifdef CONFIG_ARCH_HAS_CPU_RELAX -static int poll_idle(struct cpuidle_device *dev, - struct cpuidle_driver *drv, int index) -{ - local_irq_enable(); - if (!current_set_polling_and_test()) { - while (!need_resched()) - cpu_relax(); - } - current_clr_polling(); - - return index; -} - -static void poll_idle_init(struct cpuidle_driver *drv) -{ - struct cpuidle_state *state = &drv->states[0]; - - snprintf(state->name, CPUIDLE_NAME_LEN, "POLL"); - snprintf(state->desc, CPUIDLE_DESC_LEN, "CPUIDLE CORE POLL IDLE"); - state->exit_latency = 0; - state->target_residency = 0; - state->power_usage = -1; - state->enter = poll_idle; - state->disabled = false; -} -#else -static void poll_idle_init(struct cpuidle_driver *drv) {} -#endif /* !CONFIG_ARCH_HAS_CPU_RELAX */ - /** * __cpuidle_register_driver: register the driver * @drv: a valid pointer to a struct cpuidle_driver @@ -240,8 +210,6 @@ static int __cpuidle_register_driver(struct cpuidle_driver *drv) on_each_cpu_mask(drv->cpumask, cpuidle_setup_broadcast_timer, (void *)1, 1); - poll_idle_init(drv); - return 0; } diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c index 401c010..304e08c 100644 --- a/drivers/cpuidle/governors/ladder.c +++ b/drivers/cpuidle/governors/ladder.c @@ -96,13 +96,13 @@ static int ladder_select_state(struct cpuidle_driver *drv, } /* consider demotion */ - if (last_idx > CPUIDLE_DRIVER_STATE_START && + if (last_idx > 0 && (drv->states[last_idx].disabled || dev->states_usage[last_idx].disable || drv->states[last_idx].exit_latency > latency_req)) { int i; - for (i = last_idx - 1; i > CPUIDLE_DRIVER_STATE_START; i--) { + for (i = last_idx - 1; i > 0; i--) { if (drv->states[i].exit_latency <= latency_req) break; } @@ -110,7 +110,7 @@ static int ladder_select_state(struct cpuidle_driver *drv, return i; } - if (last_idx > CPUIDLE_DRIVER_STATE_START && + if (last_idx > 0 && last_residency < last_state->threshold.demotion_time) { last_state->stats.demotion_count++; last_state->stats.promotion_count = 0; @@ -137,9 +137,9 @@ static int ladder_enable_device(struct cpuidle_driver *drv, struct ladder_device_state *lstate; struct cpuidle_state *state; - ldev->last_state_idx = CPUIDLE_DRIVER_STATE_START; + ldev->last_state_idx = 0; - for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) { + for (i = 0; i < drv->state_count; i++) { state = &drv->states[i]; lstate = &ldev->states[i]; @@ -151,7 +151,7 @@ static int ladder_enable_device(struct cpuidle_driver *drv, if (i < drv->state_count - 1) lstate->threshold.promotion_time = state->exit_latency; - if (i > CPUIDLE_DRIVER_STATE_START) + if (i > 0) lstate->threshold.demotion_time = state->exit_latency; } diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index b8a5fa1..5c5e7db 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -294,7 +294,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) data->needs_update = 0; } - data->last_state_idx = CPUIDLE_DRIVER_STATE_START - 1; + data->last_state_idx = 0; /* Special case when user has set very strict latency requirement */ if (unlikely(latency_req == 0)) @@ -327,19 +327,10 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) latency_req = interactivity_req; /* - * We want to default to C1 (hlt), not to busy polling - * unless the timer is happening really really soon. - */ - if (data->next_timer_us > 5 && - !drv->states[CPUIDLE_DRIVER_STATE_START].disabled && - dev->states_usage[CPUIDLE_DRIVER_STATE_START].disable == 0) - data->last_state_idx = CPUIDLE_DRIVER_STATE_START; - - /* * Find the idle state with the lowest power while satisfying * our constraints. */ - for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) { + for (i = 0; i < drv->state_count; i++) { struct cpuidle_state *s = &drv->states[i]; struct cpuidle_state_usage *su = &dev->states_usage[i]; diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h index 9c5e892..351b652 100644 --- a/include/linux/cpuidle.h +++ b/include/linux/cpuidle.h @@ -236,10 +236,4 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov) {return 0;} #endif -#ifdef CONFIG_ARCH_HAS_CPU_RELAX -#define CPUIDLE_DRIVER_STATE_START 1 -#else -#define CPUIDLE_DRIVER_STATE_START 0 -#endif - #endif /* _LINUX_CPUIDLE_H */