| Message ID | 1334698746-11529-1-git-send-email-yorksun@freescale.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kumar Gala |
| Headers | show |
On 04/17/2012 04:39 PM, York Sun wrote: > The timebase synchronization is only necessary if we need to reset a > separate core. Currently only KEXEC and CPU hotplug require resetting > a single core. The following code should be in the condition of > CONFIG_KEXEC or CONFIG_HOTPLUG_CPU > > .give_timebase = smp_generic_give_timebase, > .take_timebase = smp_generic_take_timebase, > > Signed-off-by: York Sun <yorksun@freescale.com> > Acked-by: Li Yang <leoli@freescale.com> > --- > arch/powerpc/platforms/85xx/smp.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c > index 56942af..868c6d7 100644 > --- a/arch/powerpc/platforms/85xx/smp.c > +++ b/arch/powerpc/platforms/85xx/smp.c > @@ -192,8 +192,10 @@ struct smp_ops_t smp_85xx_ops = { > .cpu_disable = generic_cpu_disable, > .cpu_die = generic_cpu_die, > #endif > +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU) > .give_timebase = smp_generic_give_timebase, > .take_timebase = smp_generic_take_timebase, > +#endif > }; > > #ifdef CONFIG_KEXEC Note that this is only a temporary fix, that assumes the environments where tbsync is problematic[1] (virtualization and simulation) do not enable CONFIG_KEXEC or CONFIG_HOTPLUG_CPU. Eventually the sync should be done via CCSR like in U-Boot, and the decision on whether to do it should be runtime. -Scott [1] More so than usual.
On Apr 17, 2012, at 4:39 PM, York Sun wrote: > The timebase synchronization is only necessary if we need to reset a > separate core. Currently only KEXEC and CPU hotplug require resetting > a single core. The following code should be in the condition of > CONFIG_KEXEC or CONFIG_HOTPLUG_CPU > > .give_timebase = smp_generic_give_timebase, > .take_timebase = smp_generic_take_timebase, This doesn't explain why you are putting the #ifdef back, only under what conditions it applies. > > Signed-off-by: York Sun <yorksun@freescale.com> > Acked-by: Li Yang <leoli@freescale.com> > --- > arch/powerpc/platforms/85xx/smp.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c > index 56942af..868c6d7 100644 > --- a/arch/powerpc/platforms/85xx/smp.c > +++ b/arch/powerpc/platforms/85xx/smp.c > @@ -192,8 +192,10 @@ struct smp_ops_t smp_85xx_ops = { > .cpu_disable = generic_cpu_disable, > .cpu_die = generic_cpu_die, > #endif > +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU) > .give_timebase = smp_generic_give_timebase, > .take_timebase = smp_generic_take_timebase, > +#endif > }; > > #ifdef CONFIG_KEXEC > -- > 1.7.0.4 > > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev
On Apr 17, 2012, at 5:17 PM, Scott Wood wrote: > On 04/17/2012 04:39 PM, York Sun wrote: >> The timebase synchronization is only necessary if we need to reset a >> separate core. Currently only KEXEC and CPU hotplug require resetting >> a single core. The following code should be in the condition of >> CONFIG_KEXEC or CONFIG_HOTPLUG_CPU >> >> .give_timebase = smp_generic_give_timebase, >> .take_timebase = smp_generic_take_timebase, >> >> Signed-off-by: York Sun <yorksun@freescale.com> >> Acked-by: Li Yang <leoli@freescale.com> >> --- >> arch/powerpc/platforms/85xx/smp.c | 2 ++ >> 1 files changed, 2 insertions(+), 0 deletions(-) >> >> diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c >> index 56942af..868c6d7 100644 >> --- a/arch/powerpc/platforms/85xx/smp.c >> +++ b/arch/powerpc/platforms/85xx/smp.c >> @@ -192,8 +192,10 @@ struct smp_ops_t smp_85xx_ops = { >> .cpu_disable = generic_cpu_disable, >> .cpu_die = generic_cpu_die, >> #endif >> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU) >> .give_timebase = smp_generic_give_timebase, >> .take_timebase = smp_generic_take_timebase, >> +#endif >> }; >> >> #ifdef CONFIG_KEXEC > > Note that this is only a temporary fix, that assumes the environments > where tbsync is problematic[1] (virtualization and simulation) do not > enable CONFIG_KEXEC or CONFIG_HOTPLUG_CPU. Eventually the sync should > be done via CCSR like in U-Boot, and the decision on whether to do it > should be runtime. Is the thinking with the CCSR like u-boot sync after boot would resync to some non-zero TB value? I'm guessing the idea is doing such a rsync w/TB stopped in the system is quick enough that the freezing of it will not be noticed. One should measure this and see what it looks like in core cycles and how it scales based on # of cores. (could use alternate TB as a counter to measure). - k
On 04/18/2012 09:15 AM, Kumar Gala wrote: > > On Apr 17, 2012, at 5:17 PM, Scott Wood wrote: > >> On 04/17/2012 04:39 PM, York Sun wrote: >>> The timebase synchronization is only necessary if we need to reset a >>> separate core. Currently only KEXEC and CPU hotplug require resetting >>> a single core. The following code should be in the condition of >>> CONFIG_KEXEC or CONFIG_HOTPLUG_CPU >>> >>> .give_timebase = smp_generic_give_timebase, >>> .take_timebase = smp_generic_take_timebase, >>> >>> Signed-off-by: York Sun <yorksun@freescale.com> >>> Acked-by: Li Yang <leoli@freescale.com> >>> --- >>> arch/powerpc/platforms/85xx/smp.c | 2 ++ >>> 1 files changed, 2 insertions(+), 0 deletions(-) >>> >>> diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c >>> index 56942af..868c6d7 100644 >>> --- a/arch/powerpc/platforms/85xx/smp.c >>> +++ b/arch/powerpc/platforms/85xx/smp.c >>> @@ -192,8 +192,10 @@ struct smp_ops_t smp_85xx_ops = { >>> .cpu_disable = generic_cpu_disable, >>> .cpu_die = generic_cpu_die, >>> #endif >>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU) >>> .give_timebase = smp_generic_give_timebase, >>> .take_timebase = smp_generic_take_timebase, >>> +#endif >>> }; >>> >>> #ifdef CONFIG_KEXEC >> >> Note that this is only a temporary fix, that assumes the environments >> where tbsync is problematic[1] (virtualization and simulation) do not >> enable CONFIG_KEXEC or CONFIG_HOTPLUG_CPU. Eventually the sync should >> be done via CCSR like in U-Boot, and the decision on whether to do it >> should be runtime. > > Is the thinking with the CCSR like u-boot sync after boot would resync to some non-zero TB value? Yes, while all timebases are stopped, the core coming out of reset will be set to the timebase value of the other cores. > I'm guessing the idea is doing such a rsync w/TB stopped in the > system is quick enough that the freezing of it will not be noticed. Quick enough is relative, but remember that the context is things like kexec and deep sleep, which are already quite timing-intrusive. > One should measure this and see what it looks like in core cycles and > how it scales based on # of cores. (could use alternate TB as a > counter to measure). I have a hard time imagining it being slower than the generic tbsync, but in any case it's really the only sane option we have in cases where we must reset individual cores -- I'm not sure how relevant such benchmarking would be, other than "nice to know". -Scott
diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c index 56942af..868c6d7 100644 --- a/arch/powerpc/platforms/85xx/smp.c +++ b/arch/powerpc/platforms/85xx/smp.c @@ -192,8 +192,10 @@ struct smp_ops_t smp_85xx_ops = { .cpu_disable = generic_cpu_disable, .cpu_die = generic_cpu_die, #endif +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU) .give_timebase = smp_generic_give_timebase, .take_timebase = smp_generic_take_timebase, +#endif }; #ifdef CONFIG_KEXEC
The timebase synchronization is only necessary if we need to reset a separate core. Currently only KEXEC and CPU hotplug require resetting a single core. The following code should be in the condition of CONFIG_KEXEC or CONFIG_HOTPLUG_CPU .give_timebase = smp_generic_give_timebase, .take_timebase = smp_generic_take_timebase, Signed-off-by: York Sun <yorksun@freescale.com> Acked-by: Li Yang <leoli@freescale.com> --- arch/powerpc/platforms/85xx/smp.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)