linux-kernel - Re: [PATCH 5/9] acpi: processor->cpuidle: Only set cpuidle check_bm flag if pr->flags.bm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 12 Jan 2011 02:17:18 -0500 (EST)
From:	Len Brown <lenb@...nel.org>
To:	Thomas Renninger <trenn@...e.de>
Cc:	linux-perf-users@...r.kernel.org, mingo@...e.hu,
	arjan@...ux.intel.com, j-pihet@...com, linux-acpi@...r.kernel.org,
	linux-pm@...ts.linux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/9] acpi: processor->cpuidle: Only set cpuidle check_bm
 flag if pr->flags.bm_check is set

On Fri, 7 Jan 2011, Thomas Renninger wrote:

>  drivers/acpi/processor_idle.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index b28693e..82f74c9 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1039,7 +1039,8 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
>  			snprintf(state->abbr, CPUIDLE_ABBR_LEN, "C3");
>  			state->flags |= CPUIDLE_FLAG_DEEP;
>  			state->flags |= CPUIDLE_FLAG_TIME_VALID;
> -			state->flags |= CPUIDLE_FLAG_CHECK_BM;
> +			if (pr->flags.bm_check)
> +				state->flags |= CPUIDLE_FLAG_CHECK_BM;
>  			state->enter = pr->flags.bm_check ?
>  					acpi_idle_enter_bm :
>  					acpi_idle_enter_simple;
> -- 

While it is logical, this patch is a NO-OP, since nobody
ever checks for state->flags |= CPUIDLE_FLAG_CHECK_BM --
not cpuidle, and not the platform driver.

ie. it would be better to delete use of this flag from this file.

This points out a problem w/ cpuidle, of course.
Who owns and defines state->flags -- cpuidle and its governors,
or the cpuidle drivers?

cpuidle and its governors look only at...
CPUIDLE_FLAG_TIME_VALID and CPUIDLE_FLAG_IGNORE

cpuidle sets, but never looks at, CPUIDLE_FLAG_POLL

only intel_idle uses CPUIDLE_FLAG_TLB_FLUSHED,
so it should probably define it.

only OMAP uses CPUIDLE_FLAG_CHECK_BM,
so it should define it (or create a flag
that actually says what they mean here)

CPUIDLE_FLAG_SHALLOW, CPUIDLE_FLAG_BALANCED, CPUIDLE_FLAG_DEEP
are set in various architectures, but never checked,
so they should be deleted.

thanks,
Len Brown, Intel Open Source Technology Center

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/