Hello all,
For my new coretemp driver[1], I need to execute the rdmsr on particular
processor. There is no such "global" function for that in the kernel so far.
The per CPU msr_read and msr_write are used in following drivers:
msr.c (it is static there now)
k8-edac.c (duplicated right now -> driver in -mm)
coretemp.c (my new Core temperature sensor -> driver [1])
Question is how make an access to that functions. Enclosed patch does simple
EXPORT_SYMBOL_GPL for them, but then both drivers (k8-edac.c and coretemp.c)
would depend on the MSR driver. The ultimate solution would be to move this type
of function to separate module, but perhaps this is just bit overkill?
Any ideas what would be the best solution?
I would like to make sure that module refcounting is done in module.c, so I
don't need to handle this in my patch.
Thanks,
Rudolf
Please CC me, I'm not on all lists.
[1] http://lists.lm-sensors.org/pipermail/lm-sensors/2006-December/018420.html
Dave Jones wrote:
>
> Exposing the guts of the msr driver like that doesn't seem too clean.
> For in-kernel use, why not just add something like this..
> (note:not even compile tested)..
>
Well, that *is* the guts of the MSR driver.
> void rdmsr_on_cpu(unsigned int cpu, unsigned long msr, unsigned long *lo, unsigned long *hi)
> {
> cpumask_t oldmask;
>
> oldmask = current->cpus_allowed;
> set_cpus_allowed(current, cpumask_of_cpu(cpu));
>
> rdmsr(msr, &lo, &hi);
>
> set_cpus_allowed(current, oldmask);
> }
>
[The above doesn't work, by the way. This approach was discussed a long
time ago, and vetoed due to the potential for deadlock.]
-hpa
On Wed, Dec 13, 2006 at 10:45:13PM +0100, Rudolf Marek wrote:
> Hello all,
>
> For my new coretemp driver[1], I need to execute the rdmsr on particular
> processor. There is no such "global" function for that in the kernel so far.
>
> The per CPU msr_read and msr_write are used in following drivers:
>
> msr.c (it is static there now)
> k8-edac.c (duplicated right now -> driver in -mm)
> coretemp.c (my new Core temperature sensor -> driver [1])
>
> Question is how make an access to that functions. Enclosed patch does simple
> EXPORT_SYMBOL_GPL for them, but then both drivers (k8-edac.c and coretemp.c)
> would depend on the MSR driver. The ultimate solution would be to move this type
> of function to separate module, but perhaps this is just bit overkill?
Exposing the guts of the msr driver like that doesn't seem too clean.
For in-kernel use, why not just add something like this..
(note:not even compile tested)..
void rdmsr_on_cpu(unsigned int cpu, unsigned long msr, unsigned long *lo, unsigned long *hi)
{
cpumask_t oldmask;
oldmask = current->cpus_allowed;
set_cpus_allowed(current, cpumask_of_cpu(cpu));
rdmsr(msr, &lo, &hi);
set_cpus_allowed(current, oldmask);
}
Dave
--
http://www.codemonkey.org.uk
On Wed, Dec 13, 2006 at 02:19:52PM -0800, H. Peter Anvin wrote:
> > void rdmsr_on_cpu(unsigned int cpu, unsigned long msr, unsigned long *lo, unsigned long *hi)
> > {
> > cpumask_t oldmask;
> >
> > oldmask = current->cpus_allowed;
> > set_cpus_allowed(current, cpumask_of_cpu(cpu));
> >
> > rdmsr(msr, &lo, &hi);
> >
> > set_cpus_allowed(current, oldmask);
> > }
> >
>
> [The above doesn't work, by the way. This approach was discussed a long
> time ago, and vetoed due to the potential for deadlock.]
Can you explain this a little further? I'm fairly certain
there are places in the kernel already doing this (or similar).
In fact, I cut-n-pasted most of the above from similar code in the
powernow-k8 driver. What exactly can we deadlock on?
Dave
--
http://www.codemonkey.org.uk
Dave Jones wrote:
>
> Can you explain this a little further? I'm fairly certain
> there are places in the kernel already doing this (or similar).
> In fact, I cut-n-pasted most of the above from similar code in the
> powernow-k8 driver. What exactly can we deadlock on?
>
I wanted to change the MSR driver to do the above, and Alan Cox objected
that with realtime priority routines and/or user set affinity, that code
might never be executed, so I retained the IPI-based code (which
executes at the target processor at interrupt priority.)
-hpa
Rudolf Marek wrote:
> Hello all,
>
> For my new coretemp driver[1], I need to execute the rdmsr on particular
> processor. There is no such "global" function for that in the kernel so
> far.
>
> The per CPU msr_read and msr_write are used in following drivers:
>
> msr.c (it is static there now)
> k8-edac.c (duplicated right now -> driver in -mm)
> coretemp.c (my new Core temperature sensor -> driver [1])
>
> Question is how make an access to that functions. Enclosed patch does
> simple EXPORT_SYMBOL_GPL for them, but then both drivers (k8-edac.c and
> coretemp.c) would depend on the MSR driver. The ultimate solution would
> be to move this type
> of function to separate module, but perhaps this is just bit overkill?
>
> Any ideas what would be the best solution?
>
For now I think you could just export these and allow the dependency.
I've been meaning to rewrite the MSR and CPUID drivers to use a common
core, which would also allow invoking nnostandard CPUID and msrs which
need the entire register file to be set; that should probably be
included in that.
In fact, I've made that change something like four times (it seems to be
an airplane project that I never get around to submitting), so I should
actually get it finished and sent in.
-hpa