From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Axtens Subject: Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states Date: Tue, 09 Apr 2019 00:25:42 +1000 Message-ID: <87lg0kwp3t.fsf@dja-thinkpad.axtens.net> References: <20190405091647.4169-1-huntbag@linux.vnet.ibm.com> <20190405091647.4169-2-huntbag@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain Return-path: In-Reply-To: <20190405091647.4169-2-huntbag@linux.vnet.ibm.com> Sender: linux-kernel-owner@vger.kernel.org To: linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-pm@vger.kernel.org Cc: rjw@rjwysocki.net, daniel.lezcano@linaro.org, mpe@ellerman.id.au, ego@linux.vnet.ibm.com, Abhishek Goel List-Id: linux-pm@vger.kernel.org Hi, Sorry, just realised another thing I wanted to ask: > @@ -442,6 +442,26 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev, > } > } > > > +#ifdef CPUIDLE_FLAG_AUTO_PROMOTION Why is this based on CPUIDLE_FLAG_ rather than CONFIG_CPU_IDLE_? Won't this always be true, given that the flag is defined regardless of the config option in the header? > + if (drv->states[idx].flags & CPUIDLE_FLAG_AUTO_PROMOTION) { > + /* > + * Timeout is intended to be defined as sum of target residency > + * of next available state, entry latency and exit latency. If > + * time interval equal to timeout is spent in current state, > + * and if it is a shallow lite state, we may want to auto- > + * promote from such state. > + */ Regards, Daniel > + for (i = idx + 1; i < drv->state_count; i++) { > + if (drv->states[i].disabled || > + dev->states_usage[i].disable) > + continue; > + *timeout = drv->states[i].target_residency + > + 2 * drv->states[i].exit_latency; > + break; > + } > + } > +#endif > + > return idx; > } > > diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h > index 3b3947232..84d76d1ec 100644 > --- a/include/linux/cpuidle.h > +++ b/include/linux/cpuidle.h > @@ -72,6 +72,13 @@ struct cpuidle_state { > #define CPUIDLE_FLAG_POLLING BIT(0) /* polling state */ > #define CPUIDLE_FLAG_COUPLED BIT(1) /* state applies to multiple cpus */ > #define CPUIDLE_FLAG_TIMER_STOP BIT(2) /* timer is stopped on this state */ > +/* > + * State with only and only fast state bit set don't even lose user context. > + * But such states prevent other sibling threads from thread folding benefits. > + * And hence we don't want to stay for too long in such states and want to > + * auto-promote from it. > + */ > +#define CPUIDLE_FLAG_AUTO_PROMOTION BIT(3) > > struct cpuidle_device_kobj; > struct cpuidle_state_kobj; > @@ -243,7 +250,8 @@ struct cpuidle_governor { > > int (*select) (struct cpuidle_driver *drv, > struct cpuidle_device *dev, > - bool *stop_tick); > + bool *stop_tick, unsigned long > + *timeout); > void (*reflect) (struct cpuidle_device *dev, int index); > }; > > -- > 2.17.1 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 136D3C10F14 for ; Mon, 8 Apr 2019 14:25:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CA85421473 for ; Mon, 8 Apr 2019 14:25:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=axtens.net header.i=@axtens.net header.b="qY5Aj+jj" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726930AbfDHOZs (ORCPT ); Mon, 8 Apr 2019 10:25:48 -0400 Received: from mail-pg1-f195.google.com ([209.85.215.195]:38987 "EHLO mail-pg1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726952AbfDHOZs (ORCPT ); Mon, 8 Apr 2019 10:25:48 -0400 Received: by mail-pg1-f195.google.com with SMTP id k3so7423594pga.6 for ; Mon, 08 Apr 2019 07:25:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=axtens.net; s=google; h=from:to:cc:subject:in-reply-to:references:date:message-id :mime-version; bh=P4vKQmbK14vtbIuUEndCl5MfnUaB3hSyiz0CQzCAQZw=; b=qY5Aj+jjp/s1OSKgwZzU3l7+6LRruW6kkAAmrM7wKfUAdxwjQQxdeNZ/RTPhazCg3J H+cnE0oznA7ZjQ86s8U5XSbCO9iuZBNwagfDSqpGOeGZQ2AnyTGLShNvbaQaPBb+iw/K Hdkouht+z8nSthWNBPdK2PD3K/MmxfyYTaxsY= 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:in-reply-to:references:date :message-id:mime-version; bh=P4vKQmbK14vtbIuUEndCl5MfnUaB3hSyiz0CQzCAQZw=; b=JnT1QBB1524Wxokx6KUEWqa/jDYYFMopUGak6OX9Kw+DjLtyE8QmL3LRyuevTM+KNW gchhp4xdTjLzUiYzVqU9udrfdpsyTxMwCh6RlF7+b1MPK++pILY6xDTDi5HGIRl4pejd jjtGAmDHpVf0GaIVxesBls7FIo5QfTOXYP2JBWofr2mqpM9t+7ah4t+ZngS+LIbsHdBc D8VUsRkf3yfzz//JYaIpQcC9hQGiWmQ9Wge0l5Kgm9xtDikwqvBhoTWlLlLQKf9u5yrt w2YkLVry0s0QIjj8VwYdpNGqkYJ+7tLUh1iZjRRiu2BZ6G0T4EWOGwvWyGbMWMlnSJQg 3HGw== X-Gm-Message-State: APjAAAV/YOeaREYQ5sRT/VrlOBtY+H6QNMjxyjNOEM1agFP/u1JlfFwN sGO1t8ClopGEObDDRymDGc/8hA== X-Google-Smtp-Source: APXvYqzsA1AXiPykN+pw3sx+WRcap8ev3d4GcOkTSI8xZZNomj0hdsu8k/FPqteD9t9mt5/PsBrx4A== X-Received: by 2002:a63:f212:: with SMTP id v18mr27873814pgh.231.1554733547282; Mon, 08 Apr 2019 07:25:47 -0700 (PDT) Received: from localhost (124-171-136-51.dyn.iinet.net.au. [124.171.136.51]) by smtp.gmail.com with ESMTPSA id z13sm43143504pgc.25.2019.04.08.07.25.45 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 08 Apr 2019 07:25:46 -0700 (PDT) From: Daniel Axtens To: Abhishek Goel , linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-pm@vger.kernel.org Cc: rjw@rjwysocki.net, daniel.lezcano@linaro.org, mpe@ellerman.id.au, ego@linux.vnet.ibm.com, Abhishek Goel Subject: Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states In-Reply-To: <20190405091647.4169-2-huntbag@linux.vnet.ibm.com> References: <20190405091647.4169-1-huntbag@linux.vnet.ibm.com> <20190405091647.4169-2-huntbag@linux.vnet.ibm.com> Date: Tue, 09 Apr 2019 00:25:42 +1000 Message-ID: <87lg0kwp3t.fsf@dja-thinkpad.axtens.net> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org Message-ID: <20190408142542.bAALCyi1mIxIUvoB2KKi-s184X1GdBHk9L87pKGq6Uk@z> Hi, Sorry, just realised another thing I wanted to ask: > @@ -442,6 +442,26 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev, > } > } > > > +#ifdef CPUIDLE_FLAG_AUTO_PROMOTION Why is this based on CPUIDLE_FLAG_ rather than CONFIG_CPU_IDLE_? Won't this always be true, given that the flag is defined regardless of the config option in the header? > + if (drv->states[idx].flags & CPUIDLE_FLAG_AUTO_PROMOTION) { > + /* > + * Timeout is intended to be defined as sum of target residency > + * of next available state, entry latency and exit latency. If > + * time interval equal to timeout is spent in current state, > + * and if it is a shallow lite state, we may want to auto- > + * promote from such state. > + */ Regards, Daniel > + for (i = idx + 1; i < drv->state_count; i++) { > + if (drv->states[i].disabled || > + dev->states_usage[i].disable) > + continue; > + *timeout = drv->states[i].target_residency + > + 2 * drv->states[i].exit_latency; > + break; > + } > + } > +#endif > + > return idx; > } > > diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h > index 3b3947232..84d76d1ec 100644 > --- a/include/linux/cpuidle.h > +++ b/include/linux/cpuidle.h > @@ -72,6 +72,13 @@ struct cpuidle_state { > #define CPUIDLE_FLAG_POLLING BIT(0) /* polling state */ > #define CPUIDLE_FLAG_COUPLED BIT(1) /* state applies to multiple cpus */ > #define CPUIDLE_FLAG_TIMER_STOP BIT(2) /* timer is stopped on this state */ > +/* > + * State with only and only fast state bit set don't even lose user context. > + * But such states prevent other sibling threads from thread folding benefits. > + * And hence we don't want to stay for too long in such states and want to > + * auto-promote from it. > + */ > +#define CPUIDLE_FLAG_AUTO_PROMOTION BIT(3) > > struct cpuidle_device_kobj; > struct cpuidle_state_kobj; > @@ -243,7 +250,8 @@ struct cpuidle_governor { > > int (*select) (struct cpuidle_driver *drv, > struct cpuidle_device *dev, > - bool *stop_tick); > + bool *stop_tick, unsigned long > + *timeout); > void (*reflect) (struct cpuidle_device *dev, int index); > }; > > -- > 2.17.1