Full conversion to early_initcall() interface, remove old interface.

PATCH In review Eduard - Gabriel Munteanu 3 participants 8 messages Jun 23, 2008 – Jun 27, 2008 Scheduler

Share & tools

Eduard - Gabriel Munteanu

author

2008-06-23 12:32 UTC #0 lore raw

PATCH 2/3 Full conversion to early_initcall() interface, remove old interface.

A previous patch added the early_initcall(), to allow a cleaner hooking of pre-SMP initcalls. Now we remove the older interface, converting all existing users to the new one.

Signed-off-by Eduard - Gabriel Munteanu <[email protected]>

---
 include/linux/sched.h |    9 ---------
 init/main.c           |   22 +---------------------
 kernel/sched.c        |    6 +++++-
 kernel/softirq.c      |    4 +++-
 kernel/softlockup.c   |   27 ++++++++++++++++++++++++---
 5 files changed, 33 insertions(+), 35 deletions(-)

include/linux/sched.h +0 -9

index c5d3f84..efd8877 100644
@@ -291,7 +291,6 @@ extern void sched_show_task(struct task_struct *p);
 
 #ifdef CONFIG_DETECT_SOFTLOCKUP
 extern void softlockup_tick(void);
-extern void spawn_softlockup_task(void);
 extern void touch_softlockup_watchdog(void);
 extern void touch_all_softlockup_watchdogs(void);
 extern unsigned long  softlockup_thresh;

@@ -2200,14 +2199,6 @@ static inline void inc_syscw(struct task_struct *tsk)
 }
 #endif
 
-#ifdef CONFIG_SMP
-void migration_init(void);
-#else
-static inline void migration_init(void)
-{
-}
-#endif
-
 #ifndef TASK_SIZE_OF
 #define TASK_SIZE_OF(tsk)	TASK_SIZE
 #endif

init/main.c +1 ~~-21~~

index c5397f6..057f364 100644
@@ -766,16 +766,7 @@ static void __init do_basic_setup(void)
 	do_initcalls();
 }
 
-static int __initdata nosoftlockup;
-
-static int __init nosoftlockup_setup(char *str)
-{
-	nosoftlockup = 1;
-	return 1;
-}
-__setup("nosoftlockup", nosoftlockup_setup);
-
-static void __init __do_pre_smp_initcalls(void)
+static void __init do_pre_smp_initcalls(void)
 {
 	initcall_t *call;

@@ -783,16 +774,6 @@ static void __init __do_pre_smp_initcalls(void)
 		do_one_initcall(*call);
 }
 
-static void __init do_pre_smp_initcalls(void)
-{
-	extern int spawn_ksoftirqd(void);
-
-	migration_init();
-	spawn_ksoftirqd();
-	if (!nosoftlockup)
-		spawn_softlockup_task();
-}
-
 static void run_init_process(char *init_filename)
 {
 	argv_init[0] = init_filename;

@@ -864,7 +845,6 @@ static int __init kernel_init(void * unused)
 
 	smp_prepare_cpus(setup_max_cpus);
 
-	__do_pre_smp_initcalls();
 	do_pre_smp_initcalls();
 
 	smp_init();

kernel/sched.c +5 -1

index b048ad8..ccddfdd 100644
@@ -6175,7 +6175,7 @@ static struct notifier_block __cpuinitdata migration_notifier = {
 	.priority = 10
 };
 
-void __init migration_init(void)
+static int __init migration_init(void)
 {
 	void *cpu = (void *)(long)smp_processor_id();
 	int err;

@@ -6185,7 +6185,11 @@ void __init migration_init(void)
 	BUG_ON(err == NOTIFY_BAD);
 	migration_call(&migration_notifier, CPU_ONLINE, cpu);
 	register_cpu_notifier(&migration_notifier);
+
+	return err;
 }
+
+early_initcall(migration_init);
 #endif
 
 #ifdef CONFIG_SMP

kernel/softirq.c +3 -1

index 36e0617..2ef08c7 100644
@@ -659,7 +659,7 @@ static struct notifier_block __cpuinitdata cpu_nfb = {
 	.notifier_call = cpu_callback
 };
 
-__init int spawn_ksoftirqd(void)
+static __init int spawn_ksoftirqd(void)
 {
 	void *cpu = (void *)(long)smp_processor_id();
 	int err = cpu_callback(&cpu_nfb, CPU_UP_PREPARE, cpu);

@@ -670,6 +670,8 @@ __init int spawn_ksoftirqd(void)
 	return 0;
 }
 
+early_initcall(spawn_ksoftirqd);
+
 #ifdef CONFIG_SMP
 /*
  * Call a function on all processors

kernel/softlockup.c +24 -3

index c828c23..1653802 100644
@@ -306,14 +306,35 @@ static struct notifier_block __cpuinitdata cpu_nfb = {
 	.notifier_call = cpu_callback
 };
 
-__init void spawn_softlockup_task(void)
+static int __initdata nosoftlockup;
+
+static int __init nosoftlockup_setup(char *str)
+{
+	nosoftlockup = 1;
+	return 1;
+}
+__setup("nosoftlockup", nosoftlockup_setup);
+
+static int __init spawn_softlockup_task(void)
 {
 	void *cpu = (void *)(long)smp_processor_id();
-	int err = cpu_callback(&cpu_nfb, CPU_UP_PREPARE, cpu);
+	int err;
+
+	if (nosoftlockup)
+		return 0;
 
-	BUG_ON(err == NOTIFY_BAD);
+	err = cpu_callback(&cpu_nfb, CPU_UP_PREPARE, cpu);
+	if (err == NOTIFY_BAD) {
+		BUG();
+		return 1;
+	}
 	cpu_callback(&cpu_nfb, CPU_ONLINE, cpu);
 	register_cpu_notifier(&cpu_nfb);
 
 	atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
+
+	return 0;
 }
+
+early_initcall(spawn_softlockup_task);
+

Signature · 1 line

1.5.5.4

Andrew Morton

2008-06-24 00:28 UTC #1 lore raw

Quoted 3 lines · On Mon, 23 Jun 2008 15:30:56 +0300 Eduard - Gabriel Munteanu…

A previous patch added the early_initcall(), to allow a cleaner hooking of pre-SMP initcalls. Now we remove the older interface, converting all existing users to the new one.

The patch failed to convert init_call_single_data() because you're working against a two-month-old codebase. Patches against linux-next are more appropriate, particularly late in -rc.

Please check that what I committed actually worked, thanks.

Eduard - Gabriel Munteanu

author

2008-06-25 09:03 UTC #2 lore raw

PATCH 2/3 Full conversion to early_initcall() interface, remove old interface.

A previous patch added the early_initcall(), to allow a cleaner hooking of pre-SMP initcalls. Now we remove the older interface, converting all existing users to the new one.

Signed-off-by Eduard - Gabriel Munteanu <[email protected]>

---
 include/linux/sched.h |   12 ------------
 include/linux/smp.h   |    8 --------
 init/main.c           |   23 +----------------------
 kernel/sched.c        |    6 +++++-
 kernel/smp.c          |    6 +++++-
 kernel/softirq.c      |    4 +++-
 kernel/softlockup.c   |   27 ++++++++++++++++++++++++---
 7 files changed, 38 insertions(+), 48 deletions(-)

include/linux/sched.h +0 ~~-12~~

index 239df23..495996d 100644
@@ -292,7 +292,6 @@ extern void sched_show_task(struct task_struct *p);
 
 #ifdef CONFIG_DETECT_SOFTLOCKUP
 extern void softlockup_tick(void);
-extern void spawn_softlockup_task(void);
 extern void touch_softlockup_watchdog(void);
 extern void touch_all_softlockup_watchdogs(void);
 extern unsigned int  softlockup_panic;

@@ -304,9 +303,6 @@ extern int softlockup_thresh;
 static inline void softlockup_tick(void)
 {
 }
-static inline void spawn_softlockup_task(void)
-{
-}
 static inline void touch_softlockup_watchdog(void)
 {
 }

@@ -2211,14 +2207,6 @@ static inline void inc_syscw(struct task_struct *tsk)
 }
 #endif
 
-#ifdef CONFIG_SMP
-void migration_init(void);
-#else
-static inline void migration_init(void)
-{
-}
-#endif
-
 #ifndef TASK_SIZE_OF
 #define TASK_SIZE_OF(tsk)	TASK_SIZE
 #endif

include/linux/smp.h +0 -8

index 4d4c6ad..3c26b21 100644
@@ -75,12 +75,7 @@ void __smp_call_function_single(int cpuid, struct call_single_data *data);
 #ifdef CONFIG_USE_GENERIC_SMP_HELPERS
 void generic_smp_call_function_single_interrupt(void);
 void generic_smp_call_function_interrupt(void);
-void init_call_single_data(void);
 extern spinlock_t call_function_lock;
-#else
-static inline void init_call_single_data(void)
-{
-}
 #endif
 
 /*

@@ -138,9 +133,6 @@ static inline void smp_send_reschedule(int cpu) { }
 })
 #define smp_call_function_mask(mask, func, info, wait) \
 			(up_smp_call_function(func, info))
-static inline void init_call_single_data(void)
-{
-}
 #endif /* !SMP */
 
 /*

init/main.c +1 ~~-22~~

index 9e34d92..bfbfadd 100644
@@ -776,16 +776,7 @@ static void __init do_basic_setup(void)
 	do_initcalls();
 }
 
-static int __initdata nosoftlockup;
-
-static int __init nosoftlockup_setup(char *str)
-{
-	nosoftlockup = 1;
-	return 1;
-}
-__setup("nosoftlockup", nosoftlockup_setup);
-
-static void __init __do_pre_smp_initcalls(void)
+static void __init do_pre_smp_initcalls(void)
 {
 	initcall_t *call;

@@ -793,17 +784,6 @@ static void __init __do_pre_smp_initcalls(void)
 		do_one_initcall(*call);
 }
 
-static void __init do_pre_smp_initcalls(void)
-{
-	extern int spawn_ksoftirqd(void);
-
-	init_call_single_data();
-	migration_init();
-	spawn_ksoftirqd();
-	if (!nosoftlockup)
-		spawn_softlockup_task();
-}
-
 static void run_init_process(char *init_filename)
 {
 	argv_init[0] = init_filename;

@@ -884,7 +864,6 @@ static int __init kernel_init(void * unused)
 
 	smp_prepare_cpus(setup_max_cpus);
 
-	__do_pre_smp_initcalls();
 	do_pre_smp_initcalls();
 
 	smp_init();

kernel/sched.c +5 -1

index 306f7f6..3c987f3 100644
@@ -6244,7 +6244,7 @@ static struct notifier_block __cpuinitdata migration_notifier = {
 	.priority = 10
 };
 
-void __init migration_init(void)
+static int __init migration_init(void)
 {
 	void *cpu = (void *)(long)smp_processor_id();
 	int err;

@@ -6254,7 +6254,11 @@ void __init migration_init(void)
 	BUG_ON(err == NOTIFY_BAD);
 	migration_call(&migration_notifier, CPU_ONLINE, cpu);
 	register_cpu_notifier(&migration_notifier);
+
+	return err;
 }
+
+early_initcall(migration_init);
 #endif
 
 #ifdef CONFIG_SMP

kernel/smp.c +5 -1

index a0c9cb0..f121e1d 100644
@@ -33,7 +33,7 @@ struct call_single_queue {
 	spinlock_t lock;
 };
 
-void __cpuinit init_call_single_data(void)
+static int __cpuinit init_call_single_data(void)
 {
 	int i;

@@ -43,8 +43,12 @@ void __cpuinit init_call_single_data(void)
 		spin_lock_init(&q->lock);
 		INIT_LIST_HEAD(&q->list);
 	}
+
+	return 0;
 }
 
+early_initcall(init_call_single_data);
+
 static void csd_flag_wait(struct call_single_data *data)
 {
 	/* Wait for response */

kernel/softirq.c +3 -1

index 0592568..efab66a 100644
@@ -658,7 +658,7 @@ static struct notifier_block __cpuinitdata cpu_nfb = {
 	.notifier_call = cpu_callback
 };
 
-__init int spawn_ksoftirqd(void)
+static int __init spawn_ksoftirqd(void)
 {
 	void *cpu = (void *)(long)smp_processor_id();
 	int err = cpu_callback(&cpu_nfb, CPU_UP_PREPARE, cpu);

@@ -669,6 +669,8 @@ __init int spawn_ksoftirqd(void)
 	return 0;
 }
 
+early_initcall(spawn_ksoftirqd);
+
 #ifdef CONFIG_SMP
 /*
  * Call a function on all processors

kernel/softlockup.c +24 -3

index 6b682d8..393d310 100644
@@ -335,14 +335,35 @@ static struct notifier_block __cpuinitdata cpu_nfb = {
 	.notifier_call = cpu_callback
 };
 
-__init void spawn_softlockup_task(void)
+static int __initdata nosoftlockup;
+
+static int __init nosoftlockup_setup(char *str)
+{
+	nosoftlockup = 1;
+	return 1;
+}
+__setup("nosoftlockup", nosoftlockup_setup);
+
+static int __init spawn_softlockup_task(void)
 {
 	void *cpu = (void *)(long)smp_processor_id();
-	int err = cpu_callback(&cpu_nfb, CPU_UP_PREPARE, cpu);
+	int err;
+
+	if (nosoftlockup)
+		return 0;
 
-	BUG_ON(err == NOTIFY_BAD);
+	err = cpu_callback(&cpu_nfb, CPU_UP_PREPARE, cpu);
+	if (err == NOTIFY_BAD) {
+		BUG();
+		return 1;
+	}
 	cpu_callback(&cpu_nfb, CPU_ONLINE, cpu);
 	register_cpu_notifier(&cpu_nfb);
 
 	atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
+
+	return 0;
 }
+
+early_initcall(spawn_softlockup_task);
+

Signature · 1 line

1.5.5.4

Eduard - Gabriel Munteanu

author

2008-06-25 09:07 UTC #3 lore raw

Hi,

Please merge this instead, it's based on linux-next. Your change wasn't enough.

And I don't think Tom Zanussi should be Cc-ed on patches 1 and 2, these are mere prereqs for the 3rd. So add the Cc for Tom just for the third. You could add yourself as Cc to the other two, or whoever you think is responsible for such core stuff.

Cheers,
Eduard

Johannes Weiner

2008-06-27 10:55 UTC #4 lore raw

In reply to Eduard - Gabriel Munteanu

Hi Eduard,

Quoted 14 lines · Eduard - Gabriel Munteanu

A previous patch added the early_initcall(), to allow a cleaner hooking of pre-SMP initcalls. Now we remove the older interface, converting all existing users to the new one.
Signed-off-by Eduard - Gabriel Munteanu <[email protected]>
---
 include/linux/sched.h |   12 ------------
 include/linux/smp.h   |    8 --------
 init/main.c           |   23 +----------------------
 kernel/sched.c        |    6 +++++-
 kernel/smp.c          |    6 +++++-
 kernel/softirq.c      |    4 +++-
 kernel/softlockup.c   |   27 ++++++++++++++++++++++++---
 7 files changed, 38 insertions(+), 48 deletions(-)

...

Quoted 14 lines

kernel/softirq.c +1 -1

index 0592568..efab66a 100644
@@ -658,7 +658,7 @@ static struct notifier_block __cpuinitdata cpu_nfb = {
 	.notifier_call = cpu_callback
 };
 
-__init int spawn_ksoftirqd(void)
+static int __init spawn_ksoftirqd(void)
 {
 	void *cpu = (void *)(long)smp_processor_id();
 	int err = cpu_callback(&cpu_nfb, CPU_UP_PREPARE, cpu);

kernel/softirq.c __init int spawn_ksoftirqd(void) · line 670

Quoted 2 lines

kernel/softirq.c +0 -0

@@ -669,6 +669,8 @@ __init int spawn_ksoftirqd(void)
 	return 0;
 }

You forgot to remove the declaration from linux/interrupt.h.

Andrew, the following fix is needed for -mmotm:

From: Johannes Weiner <[email protected]>
Subject: full-conversion-to-early_initcall-interface-remove-old-interface-fix

Original patch made spawn_softirqd() static but failed to remove the global definition. Do so now.

Signed-off-by Johannes Weiner <[email protected]>

---

include/linux/interrupt.h +0 -1

index 30da779..62aa4f8 100644
@@ -299,7 +299,6 @@ extern void softirq_init(void);
 #define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL << (nr)); } while (0)
 extern void raise_softirq_irqoff(unsigned int nr);
 extern void raise_softirq(unsigned int nr);
-extern int spawn_ksoftirqd(void);
 
 
 /* Tasklets --- multithreaded analogue of BHs.

Eduard - Gabriel Munteanu

author

2008-06-27 11:28 UTC #5 lore raw

Quoted 1 line · On Fri, 27 Jun 2008 12:54:21 +0200 Johannes Weiner <[email protected]> wrote:

Hi Eduard,

Hi.

Quoted 1 line

You forgot to remove the declaration from linux/interrupt.h.

The last patch was made against linux-next, as Andrew Morton suggested, where spawn_ksoftirqd() was declared in the scope of __do_pre_smp_initcall():
@@ -793,17 +784,6 @@ static void __init __do_pre_smp_initcalls(void)
do_one_initcall(*call);
}

-static void __init do_pre_smp_initcalls(void)
-{
- extern int spawn_ksoftirqd(void);
-
- init_call_single_data();
- migration_init();
- spawn_ksoftirqd();
- if (!nosoftlockup)
- spawn_softlockup_task();
-}
-

It may be that some other patch in -mmotm moves that into include/linux/interrupt.h. In linux-next with my patch, running

$ find . -name \*.h | xargs grep "spawn_ksoftirqd"
shows nothing.

Thanks for spotting this difference.

(Using -mmotm + quilt myself is an additional headache, since git already does what I want in terms of patch (read commits) management; that's why I prefered linux-next.)

Quoted 17 lines

Andrew, the following fix is needed for -mmotm:
From: Johannes Weiner <[email protected]>
Subject:
full-conversion-to-early_initcall-interface-remove-old-interface-fix
Original patch made spawn_softirqd() static but failed to remove the global definition. Do so now.
Signed-off-by Johannes Weiner <[email protected]>
---
include/linux/interrupt.h +0 -0
index 30da779..62aa4f8 100644

include/linux/interrupt.h extern void softirq_init(void); · line 304

Quoted 7 lines

include/linux/interrupt.h +0 -1

@@ -299,7 +299,6 @@ extern void softirq_init(void);
 #define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL <<
 (nr)); } while (0) extern void raise_softirq_irqoff(unsigned int nr);
 extern void raise_softirq(unsigned int nr);
-extern int spawn_ksoftirqd(void);
 
 
 /* Tasklets --- multithreaded analogue of BHs.

Cheers,
Eduard

Andrew Morton

2008-06-27 18:46 UTC #6 lore raw

Quoted 30 lines · On Fri, 27 Jun 2008 14:28:00 +0300 Eduard - Gabriel Munteanu…

Quoted 1 line · On Fri, 27 Jun 2008 12:54:21 +0200 Johannes Weiner <[email protected]> wrote:
Hi Eduard,
Hi.
Quoted 1 line
You forgot to remove the declaration from linux/interrupt.h.
The last patch was made against linux-next, as Andrew Morton suggested, where spawn_ksoftirqd() was declared in the scope of __do_pre_smp_initcall():
(quoted diff) +0 ~~-11~~
@@ -793,17 +784,6 @@ static void __init __do_pre_smp_initcalls(void)
 		do_one_initcall(*call);
 }
 
-static void __init do_pre_smp_initcalls(void)
-{
-	extern int spawn_ksoftirqd(void);
-
-	init_call_single_data();
-	migration_init();
-	spawn_ksoftirqd();
-	if (!nosoftlockup)
-		spawn_softlockup_task();
-}
-
 
It may be that some other patch in -mmotm moves that into include/linux/interrupt.h. In linux-next with my patch, running

Quoted 2 lines

$ find . -name \*.h | xargs grep "spawn_ksoftirqd"
shows nothing.

Yes, I dropped that patch as it's no longer relevant.

Quoted 5 lines

Thanks for spotting this difference.
(Using -mmotm + quilt myself is an additional headache, since git already does what I want in terms of patch (read commits) management; that's why I prefered linux-next.)

It is a bit of a pain. I avoid asking people to raise patches against -mm unless it's really necessary.

Eduard - Gabriel Munteanu

author

2008-06-27 19:42 UTC #7 lore raw

Quoted 2 lines · On Fri, 27 Jun 2008 11:45:15 -0700 Andrew Morton <[email protected]>…

It is a bit of a pain. I avoid asking people to raise patches against -mm unless it's really necessary.

Just my 2 cents... you said in a 2006 discussion that you don't use Git because you modify and drop patches often. I'd like to point out that newer Git releases support interactive rebasing, which can do very nice things:
- reorder commits (also adds '<<<', '===', '>>>' merge-like stuff when that fails, so you can fix it easier)
- squash a commit into the previous -> solves the "patch that fixes another patch" problem.
- edit commits different than HEAD
That is all doable with 'git-rebase -i'.

But maybe you got used to quilt and changing requires more effort at the beginning, so I'm not going to argue too much.

I hope this helps.

Cheers,
Eduard