Re: [PATCH v2 07/31] arm64: Process management
On Fri, Sep 14, 2012 at 06:33:52PM +0100, Catalin Marinas wrote: > On Wed, Aug 15, 2012 at 12:50:13AM +0100, Olof Johansson wrote: > > On Tue, Aug 14, 2012 at 06:52:08PM +0100, Catalin Marinas wrote: > > > > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > > > new file mode 100644 > > > index 000..c4a4e1c > > > --- /dev/null > > > +++ b/arch/arm64/kernel/process.c > > > @@ -0,0 +1,416 @@ > > > > [...] > > > +/* > > > + * Function pointers to optional machine specific functions > > > + */ > > > +void (*pm_power_off)(void); > > > +EXPORT_SYMBOL(pm_power_off); > > > + > > > +void (*pm_restart)(const char *cmd); > > > +EXPORT_SYMBOL_GPL(pm_restart); > > [...] > > > +void (*pm_idle)(void) = default_idle; > > > +EXPORT_SYMBOL(pm_idle); > > > > Does it really make sense to export these to modules? > > Changed them to _GPL now but we may still want them exported to modules. > Who knows, we may just have the SoC code in a loadable module (from > initramfs). Ok, keeping the option open for that makes some sense. > > I find the powerpc way of having a machine descriptor structure with these > > (and other) function pointers in it a bit cleaner, since it gives you > > one place to plug it all in. I'd recommend that you consider doing that > > here as well, for these three and potentially other cases in the future. > > > > (See arch/powerpc/include/asm/machdep.h, struct machdep_calls). > > At some point we may add them but currently I want the SoC code to be > minimal. It's not about the size of the code, it's about having one place where the SoC divergence is exposed instead of adding hooks here and there. But agreed, it could be moved to over time. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/31] arm64: Process management
On Fri, Sep 14, 2012 at 06:33:52PM +0100, Catalin Marinas wrote: On Wed, Aug 15, 2012 at 12:50:13AM +0100, Olof Johansson wrote: On Tue, Aug 14, 2012 at 06:52:08PM +0100, Catalin Marinas wrote: diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c new file mode 100644 index 000..c4a4e1c --- /dev/null +++ b/arch/arm64/kernel/process.c @@ -0,0 +1,416 @@ [...] +/* + * Function pointers to optional machine specific functions + */ +void (*pm_power_off)(void); +EXPORT_SYMBOL(pm_power_off); + +void (*pm_restart)(const char *cmd); +EXPORT_SYMBOL_GPL(pm_restart); [...] +void (*pm_idle)(void) = default_idle; +EXPORT_SYMBOL(pm_idle); Does it really make sense to export these to modules? Changed them to _GPL now but we may still want them exported to modules. Who knows, we may just have the SoC code in a loadable module (from initramfs). Ok, keeping the option open for that makes some sense. I find the powerpc way of having a machine descriptor structure with these (and other) function pointers in it a bit cleaner, since it gives you one place to plug it all in. I'd recommend that you consider doing that here as well, for these three and potentially other cases in the future. (See arch/powerpc/include/asm/machdep.h, struct machdep_calls). At some point we may add them but currently I want the SoC code to be minimal. It's not about the size of the code, it's about having one place where the SoC divergence is exposed instead of adding hooks here and there. But agreed, it could be moved to over time. -Olof -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/31] arm64: Process management
On Wed, Aug 15, 2012 at 12:50:13AM +0100, Olof Johansson wrote: > On Tue, Aug 14, 2012 at 06:52:08PM +0100, Catalin Marinas wrote: > > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > > new file mode 100644 > > index 000..c4a4e1c > > --- /dev/null > > +++ b/arch/arm64/kernel/process.c > > @@ -0,0 +1,416 @@ > > [...] > > +/* > > + * Function pointers to optional machine specific functions > > + */ > > +void (*pm_power_off)(void); > > +EXPORT_SYMBOL(pm_power_off); > > + > > +void (*pm_restart)(const char *cmd); > > +EXPORT_SYMBOL_GPL(pm_restart); > [...] > > +void (*pm_idle)(void) = default_idle; > > +EXPORT_SYMBOL(pm_idle); > > Does it really make sense to export these to modules? Changed them to _GPL now but we may still want them exported to modules. Who knows, we may just have the SoC code in a loadable module (from initramfs). > I find the powerpc way of having a machine descriptor structure with these > (and other) function pointers in it a bit cleaner, since it gives you > one place to plug it all in. I'd recommend that you consider doing that > here as well, for these three and potentially other cases in the future. > > (See arch/powerpc/include/asm/machdep.h, struct machdep_calls). At some point we may add them but currently I want the SoC code to be minimal. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/31] arm64: Process management
On Wed, Aug 15, 2012 at 12:50:13AM +0100, Olof Johansson wrote: On Tue, Aug 14, 2012 at 06:52:08PM +0100, Catalin Marinas wrote: diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c new file mode 100644 index 000..c4a4e1c --- /dev/null +++ b/arch/arm64/kernel/process.c @@ -0,0 +1,416 @@ [...] +/* + * Function pointers to optional machine specific functions + */ +void (*pm_power_off)(void); +EXPORT_SYMBOL(pm_power_off); + +void (*pm_restart)(const char *cmd); +EXPORT_SYMBOL_GPL(pm_restart); [...] +void (*pm_idle)(void) = default_idle; +EXPORT_SYMBOL(pm_idle); Does it really make sense to export these to modules? Changed them to _GPL now but we may still want them exported to modules. Who knows, we may just have the SoC code in a loadable module (from initramfs). I find the powerpc way of having a machine descriptor structure with these (and other) function pointers in it a bit cleaner, since it gives you one place to plug it all in. I'd recommend that you consider doing that here as well, for these three and potentially other cases in the future. (See arch/powerpc/include/asm/machdep.h, struct machdep_calls). At some point we may add them but currently I want the SoC code to be minimal. -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/31] arm64: Process management
On Wed, Aug 15, 2012 at 02:53:01PM +0100, Arnd Bergmann wrote: > On Tuesday 14 August 2012, Catalin Marinas wrote: > > > +#define THREAD_SIZE_ORDER 1 > > +#define THREAD_SIZE8192 > > +#define THREAD_START_SP(THREAD_SIZE - 16) > > THREAD_SIZE_ORDER looks wrong for 64kb-page kernels. It also doesn't seem to > be used, so better remove it. It's used in kernel/fork.c if THREAD_SIZE >= PAGE_SIZE. I'll define it conditionally. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/31] arm64: Process management
On Wed, Aug 15, 2012 at 02:53:01PM +0100, Arnd Bergmann wrote: On Tuesday 14 August 2012, Catalin Marinas wrote: +#define THREAD_SIZE_ORDER 1 +#define THREAD_SIZE8192 +#define THREAD_START_SP(THREAD_SIZE - 16) THREAD_SIZE_ORDER looks wrong for 64kb-page kernels. It also doesn't seem to be used, so better remove it. It's used in kernel/fork.c if THREAD_SIZE = PAGE_SIZE. I'll define it conditionally. -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/31] arm64: Process management
On 2012-08-14 at 19:52:08 +0200, Catalin Marinas
wrote:
> +void cpu_idle(void)
> +{
> + local_fiq_enable();
> +
> + /* endless idle loop with no priority at all */
> + while (1) {
> + tick_nohz_idle_enter();
> + rcu_idle_enter();
> + while (!need_resched()) {
> + /*
> + * We need to disable interrupts here to ensure
> + * we don't miss a wakeup call.
> + */
> + local_irq_disable();
> + if (!need_resched()) {
> + stop_critical_timings();
> + pm_idle();
> + start_critical_timings();
> + /*
> + * pm_idle functions should always return
> + * with IRQs enabled.
> + */
> + WARN_ON(irqs_disabled());
> + } else {
> + local_irq_enable();
> + }
> + }
> + rcu_idle_exit();
> + tick_nohz_idle_exit();
> + preempt_enable_no_resched();
> + schedule();
> + preempt_disable();
You could use schedule_preempt_disabled() instead of the above 3 calls.
See http://lkml.kernel.org/n/tip-24swm5zut3h9c4a6s46x8...@git.kernel.org
Cheers
Tobias
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/31] arm64: Process management
On 2012-08-14 at 19:52:08 +0200, Catalin Marinas catalin.mari...@arm.com
wrote:
+void cpu_idle(void)
+{
+ local_fiq_enable();
+
+ /* endless idle loop with no priority at all */
+ while (1) {
+ tick_nohz_idle_enter();
+ rcu_idle_enter();
+ while (!need_resched()) {
+ /*
+ * We need to disable interrupts here to ensure
+ * we don't miss a wakeup call.
+ */
+ local_irq_disable();
+ if (!need_resched()) {
+ stop_critical_timings();
+ pm_idle();
+ start_critical_timings();
+ /*
+ * pm_idle functions should always return
+ * with IRQs enabled.
+ */
+ WARN_ON(irqs_disabled());
+ } else {
+ local_irq_enable();
+ }
+ }
+ rcu_idle_exit();
+ tick_nohz_idle_exit();
+ preempt_enable_no_resched();
+ schedule();
+ preempt_disable();
You could use schedule_preempt_disabled() instead of the above 3 calls.
See http://lkml.kernel.org/n/tip-24swm5zut3h9c4a6s46x8...@git.kernel.org
Cheers
Tobias
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/31] arm64: Process management
On Tuesday 14 August 2012, Catalin Marinas wrote: > +#define THREAD_SIZE_ORDER1 > +#define THREAD_SIZE 8192 > +#define THREAD_START_SP (THREAD_SIZE - 16) THREAD_SIZE_ORDER looks wrong for 64kb-page kernels. It also doesn't seem to be used, so better remove it. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/31] arm64: Process management
On Tuesday 14 August 2012, Catalin Marinas wrote: +#define THREAD_SIZE_ORDER1 +#define THREAD_SIZE 8192 +#define THREAD_START_SP (THREAD_SIZE - 16) THREAD_SIZE_ORDER looks wrong for 64kb-page kernels. It also doesn't seem to be used, so better remove it. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/31] arm64: Process management
Hi,
On Tue, Aug 14, 2012 at 06:52:08PM +0100, Catalin Marinas wrote:
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> new file mode 100644
> index 000..c4a4e1c
> --- /dev/null
> +++ b/arch/arm64/kernel/process.c
> @@ -0,0 +1,416 @@
[...]
> +/*
> + * Function pointers to optional machine specific functions
> + */
> +void (*pm_power_off)(void);
> +EXPORT_SYMBOL(pm_power_off);
> +
> +void (*pm_restart)(const char *cmd);
> +EXPORT_SYMBOL_GPL(pm_restart);
[...]
> +void (*pm_idle)(void) = default_idle;
> +EXPORT_SYMBOL(pm_idle);
Does it really make sense to export these to modules?
I find the powerpc way of having a machine descriptor structure with these
(and other) function pointers in it a bit cleaner, since it gives you
one place to plug it all in. I'd recommend that you consider doing that
here as well, for these three and potentially other cases in the future.
(See arch/powerpc/include/asm/machdep.h, struct machdep_calls).
> +void machine_halt(void)
> +{
> + machine_shutdown();
> + while (1);
> +}
> +
> +void machine_power_off(void)
> +{
> + machine_shutdown();
> + if (pm_power_off)
> + pm_power_off();
> +}
Printing something here along the lines of "System halted, OK to power off"
is useful.
-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/31] arm64: Process management
Hi,
On Tue, Aug 14, 2012 at 06:52:08PM +0100, Catalin Marinas wrote:
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
new file mode 100644
index 000..c4a4e1c
--- /dev/null
+++ b/arch/arm64/kernel/process.c
@@ -0,0 +1,416 @@
[...]
+/*
+ * Function pointers to optional machine specific functions
+ */
+void (*pm_power_off)(void);
+EXPORT_SYMBOL(pm_power_off);
+
+void (*pm_restart)(const char *cmd);
+EXPORT_SYMBOL_GPL(pm_restart);
[...]
+void (*pm_idle)(void) = default_idle;
+EXPORT_SYMBOL(pm_idle);
Does it really make sense to export these to modules?
I find the powerpc way of having a machine descriptor structure with these
(and other) function pointers in it a bit cleaner, since it gives you
one place to plug it all in. I'd recommend that you consider doing that
here as well, for these three and potentially other cases in the future.
(See arch/powerpc/include/asm/machdep.h, struct machdep_calls).
+void machine_halt(void)
+{
+ machine_shutdown();
+ while (1);
+}
+
+void machine_power_off(void)
+{
+ machine_shutdown();
+ if (pm_power_off)
+ pm_power_off();
+}
Printing something here along the lines of System halted, OK to power off
is useful.
-Olof
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
