On Tuesday, September 11, 2012, Daniel Lezcano wrote: > On 09/08/2012 12:06 AM, Rafael J. Wysocki wrote: > > On Friday, September 07, 2012, Rafael J. Wysocki wrote: > >> On Friday, September 07, 2012, Rafael J. Wysocki wrote: > >>> On Friday, September 07, 2012, Daniel Lezcano wrote: > >>>> Currently we have the cpuidle_device field in the acpi_processor_power structure. > >>>> This adds a dependency in processor.h for cpuidle.h. > >>>> > >>>> In order to be consistent with the rest of the drivers and for the per cpu states > >>>> coming right after this patch, this one move out of the acpi_processor_power > >>>> structure the cpuidle_device field. > >>>> > >>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> > >>>> Acked-by: Peter De Schrijver <pdeschrijver@xxxxxxxxxx> > >>>> Tested-by: Peter De Schrijver <pdeschrijver@xxxxxxxxxx> > >>>> --- > >>>> drivers/acpi/processor_idle.c | 25 ++++++++++++++++++------- > >>>> include/acpi/processor.h | 2 -- > >>>> 2 files changed, 18 insertions(+), 9 deletions(-) > >>>> > >>>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > >>>> index de89624..084b1d2 100644 > >>>> --- a/drivers/acpi/processor_idle.c > >>>> +++ b/drivers/acpi/processor_idle.c > >>>> @@ -79,6 +79,8 @@ module_param(bm_check_disable, uint, 0000); > >>>> static unsigned int latency_factor __read_mostly = 2; > >>>> module_param(latency_factor, uint, 0644); > >>>> > >>>> +static DEFINE_PER_CPU(struct cpuidle_device, acpi_cpuidle_device); > >>>> + > >>> > >>> Well. Why are you moving that thing into the percpu memory? It doesn't > >>> have to be per-CPU and storing it there just wastes the room. > >> > >> Sorry, it is per-CPU already, scratch that. > > > > Well, no, it isn't. So I was right originally (boy, that code _is_ confusing). > > > > So originally you had per-CPU pointers called 'processors' that each pointed > > to a struct acpi_processor object created by acpi_processor_add() in slab > > memory. Your patch doesn't touch those pointers, so they are still there. > > Yes, the purpose of this patch is the same as the other patches which is > separate the cpuidle code from the rest of the acpi code. It is part of > the cleanup. > > > In addition to them it creates a number of static per-CPU objects that > > previously were stored in those struct acpi_processor object mentioned above. > > These things need not be stored in percpu memory. > > Agreed, this patch makes the cpuidle driver to consume more per cpu > memory. Is it acceptable to create a per cpu pointer for the cpuidle > devices which will be allocated in the processor_driver init function > like the intel_idle driver ? We keep the same memory consumption while > we are moving the cpuidle specific code the C file. I'm not really sure what you mean hear, care to elaborate? > By the way, most of the cpuidle drivers for ARM are defining a static > cpuidle device structure per cpu. I guess your remark for acpi applies > to them too. Yes, it does. Percpu memory is limited and should only be used for things that really need to be stored there. > Not easy to make all these drivers to converge to the same code scheme ... I agree, but then I'm not sure it's a problem if they are slightly different. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
