From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755922AbcEQOzk (ORCPT ); Tue, 17 May 2016 10:55:40 -0400 Received: from mail-wm0-f47.google.com ([74.125.82.47]:37219 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752879AbcEQOzi (ORCPT ); Tue, 17 May 2016 10:55:38 -0400 Subject: Re: [PATCH] cpuidle: Fix coupled state parameter in cpuidle_enter() To: rjw@rjwysocki.net References: <1463496840-2858-1-git-send-email-daniel.lezcano@linaro.org> Cc: gregory.clement@free-electrons.com, linux-pm@vger.kernel.org, linux@armlinux.org.uk, paul.burton@imgtec.com, stable@vger.kernel.org, open list From: Daniel Lezcano Message-ID: <573B30E7.6040802@linaro.org> Date: Tue, 17 May 2016 16:55:35 +0200 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1463496840-2858-1-git-send-email-daniel.lezcano@linaro.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/17/2016 04:54 PM, Daniel Lezcano wrote: > The commit 0b89e9aa2856 rightfully fixed a regression by letting the couple > idle state framework to handle local interrupt enabling when the cpu is > exiting an idle state. > > The current code check if the idle state is a coupled one and, if so, it will > let the couple code to enable the interrupt. This way, it can decrement the > ready-count before handling the interrupts. This mechanism prevents the other > cpus to wait for a cpu which is handling interrupts. > > But the check is done against the state index returned by the back end driver > 'enter' functions which could be different from the initial index passed as > parameter to the cpuidle_enter_state() function. > > entered_state = target_state->enter(dev, drv, index); > > [ ... ] > > if (!cpuidle_state_is_coupled(drv, entered_state)) > local_irq_enable(); > > [ ... ] > > If the 'index' is referring to a coupled idle state but the 'entered_state' > is *not* coupled, then the interrupts are enabled again. All cpus blocked on > the sync barrier may busy loop longer if the cpu has interrupts to handle > before decrementing the ready-count. Thus consuming more energy than saving. > > Fixes: 0b89e9aa2856 "cpuidle: delay enabling interrupts until all coupled CPUs leave idle" > Cc: stable@vger.kernel.org > Signed-off-by: Daniel Lezcano > --- I don't have a board available with a driver doing couple idle state, so I wasn't able to test this patch. -- Daniel