linux-kernel - Re: [patch for 2.6.30 2/2] arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c: avoid cross-CPU interrupts

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] [day] [month] [year] [list]

Message-Id: <20090419202014.9a511aa9.akpm@linux-foundation.org>
Date:	Sun, 19 Apr 2009 20:20:14 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Len Brown <lenb@...nel.org>
Cc:	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	davej@...hat.com, efault@....de, len.brown@...el.com,
	mingo@...e.hu, rusty@...tcorp.com.au, tglx@...utronix.de,
	venkatesh.pallipadi@...el.com, yakui.zhao@...el.com,
	yanmin_zhang@...ux.intel.com
Subject: Re: [patch for 2.6.30 2/2]
 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c: avoid cross-CPU interrupts

On Sun, 19 Apr 2009 22:57:24 -0400 (EDT) Len Brown <lenb@...nel.org> wrote:

> 
> 
> > +++ a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> > @@ -197,9 +197,22 @@ static void do_drv_write(void *_cmd)
> >  
> >  static void drv_read(struct drv_cmd *cmd)
> >  {
> > -	cmd->val = 0;
> > +	int target_cpu;		/* The CPU on which to perform thr rdmsr() */
> > +	int this_cpu;
> > +
> > +	/*
> > +	 * If the current CPU is in cmd->mask then run the rdmsr() on this
> > +	 * CPU to avoid the cross-cpu interrupt.
> > +	 */
> > +	this_cpu = get_cpu();
> > +	if (cpu_isset(this_cpu, *(cmd->mask)))
> > +		target_cpu = this_cpu;
> > +	else
> > +		target_cpu = cpumask_any(cmd->mask);
> >  
> > -	smp_call_function_single(cpumask_any(cmd->mask), do_drv_read, cmd, 1);
> > +	cmd->val = 0;
> > +	smp_call_function_single(target_cpu, do_drv_read, cmd, 1);
> > +	put_cpu();
> >  }
> >  
> >  static void drv_write(struct drv_cmd *cmd)
> > _
> 
> Rather than this patch I would expect we would want to either:
> 
> A. as we went to the trouble to detect the local case
>    in drv_read, why call smp_call_function at all for that case?

Sure, that would work.

I felt it was a little cleaner to always delegate the call to
smp_call_function() rather than open-coding smp_call_function()'s
internal implementation details at this site.  We'd need to do:

	local_irq_disable();		/* Because this is what smp_call_function_single() does */
	do_drv_read(...);
	local_irq_enable();

> or
> 
> B. optimize smp_call_function_single to beneift all users
>    instead of just this customer.

Yep.  That would be a new smp_call_function_any() which takes a cpumask
rather than a single CPU number.  I think Rusty was cooking something
up..

--
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/