public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Fix for the bug reported by Herbert on 2.6.17-rc2
@ 2006-04-25  2:35 sekharan
  2006-04-25  2:35 ` [PATCH 1/3] Remove __devinitdata from notifier block definitions sekharan
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: sekharan @ 2006-04-25  2:35 UTC (permalink / raw)
  To: akpm, torvalds
  Cc: sekharan, linux-kernel, herbert, linux-xfs, xfs-masters, stern

Herbert Poetzl (herbert_at_13thfloor.at) got a oops in 2.6.17-rc2 in
notifier_chain_register(), when mounting an xfs filesystem (oops
info at the end).

I started debugging the oops and found that quite a few users of
notifier call chains incorrectly use init section for notifier_block
data structure definitions and notifier_call function definitions.

I went through all the usages of both of those (199 files in total),
and fixed all of them.

I could reproduce the problem that Herbert has reported without these
patches and verified that they do not occur when these patches are
applied.

Herbert, please try this patchset and reply.

chandra
--

This patchset has 3 patches

remove_devinitdata - removes __devinit_data from all the defintions
	of notifier_block
remove_devinit - removed __devinit and __cpuinit from all the definitions
	of notifier_call
check_init_data - Adds an ASSERT to notifier_chain_register() to make 
	sure that the notifier_block and the notifier_call are not in
	the init section.

---- oops report by Herbert


[   64.289157] BUG: unable to handle kernel paging request at virtual address c056a680
[   64.290085]  printing eip:
[   64.290402] c0129290
[   64.290686] *pde = 005bd027
[   64.291037] *pte = 0056a000
[   64.291504] Oops: 0000 [#1]
[   64.291823] SMP DEBUG_PAGEALLOC
[   64.292820] Modules linked in:
[   64.293453] CPU:    0
[   64.293485] EIP:    0060:[<c0129290>]    Not tainted VLI
[   64.293529] EFLAGS: 00000286   (2.6.17-rc2 #1) 
[   64.295055] EIP is at notifier_chain_register+0x20/0x50
[   64.295648] eax: c056a678   ebx: cf5e23f8   ecx: 00000000   edx: c04bea9c
[   64.296362] esi: cf5e23f8   edi: cffc5000   ebp: cf5e2800   esp: cffdad5c
[   64.297140] ds: 007b   es: 007b   ss: 0068
[   64.297613] Process mount (pid: 34, threadinfo=cffda000 task=cff7e570)
[   64.298258] Stack: <0>c04bea80 c0129454 c04bea9c cf5e23f8 cf5e2000 cf5e2000 c01367f7 c04bea80 
[   64.299558]        cf5e23f8 c02d4b26 cf5e23f8 00000404 cf5e2000 cfd1f520 cffc5000 c02d1f53 
[   64.300700]        cf5e2000 00000001 c02e65ef 00000424 00000001 cffc5000 cfd1f520 c02f2880 
[   64.301841] Call Trace:
[   64.302278]  <c0129454> blocking_notifier_chain_register+0x54/0x90   <c01367f7> register_cpu_notifier+0x17/0x20
[   64.303684]  <c02d4b26> xfs_icsb_init_counters+0x46/0xb0   <c02d1f53> xfs_mount_init+0x23/0x160
[   64.304844]  <c02e65ef> kmem_zalloc+0x1f/0x50   <c02f2880> bhv_insert_all_vfsops+0x10/0x50
[   64.305940]  <c02f1f65> xfs_fs_fill_super+0x35/0x1f0   <c0313e97> snprintf+0x27/0x30
[   64.307124]  <c01a24f4> disk_name+0x64/0xc0   <c0168f1f> sb_set_blocksize+0x1f/0x50
[   64.308140]  <c0168869> get_sb_bdev+0x109/0x160   <c02f2150> xfs_fs_get_sb+0x30/0x40
[   64.309129]  <c02f1f30> xfs_fs_fill_super+0x0/0x1f0   <c0168b10> do_kern_mount+0xa0/0x160
[   64.310156]  <c0181187> do_new_mount+0x77/0xc0   <c018184f> do_mount+0x1bf/0x230
[   64.311177]  <c03f4a68> iret_exc+0x3d4/0x6ab   <c0181633> copy_mount_options+0x63/0xc0
[   64.312246]  <c03f427f> lock_kernel+0x2f/0x50   <c0181c5f> sys_mount+0x9f/0xe0
[   64.313237]  <c0102b27> syscall_call+0x7/0xb  
[   64.313917] Code: 90 90 90 90 90 90 90 90 90 90 90 53 8b 54 24 08 8b 5c 24 0c 8b 02 85 c0 74 31 8b 4b 08 8d b4 26 00 00 00
00 8d bc 27 00 00 00 00 <3b> 48 08 7f 1b 8d 50 04 8b 40 04 85 c0 75 f1 31 c0 eb 0d 90 90 
[   64.318371] EIP: [<c0129290>] notifier_chain_register+0x20/0x50 SS:ESP 0068:cffdad5c



-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] Remove __devinitdata from notifier block definitions
  2006-04-25  2:35 [PATCH 0/3] Fix for the bug reported by Herbert on 2.6.17-rc2 sekharan
@ 2006-04-25  2:35 ` sekharan
  2006-04-25  2:35 ` [PATCH 2/3] Remove __devinit and __cpuinit from notifier_call definitions sekharan
  2006-04-25  2:35 ` [PATCH 3/3] Assert notifier_block and notifier_call are not in init section sekharan
  2 siblings, 0 replies; 12+ messages in thread
From: sekharan @ 2006-04-25  2:35 UTC (permalink / raw)
  To: akpm, torvalds
  Cc: sekharan, linux-kernel, herbert, linux-xfs, xfs-masters, stern

Few of the notifier_chain_register() callers use __initdata in
the definition of notifier_block data structure.  It is incorrect as 
the data structure should be available after the initializations (they
do not unregister them during initializations).

This was leading to oops when notifier_chain_register() call is invoked
for those callback chains after initialization.

This patch fixes all such usages to _not_ have the notifier_block data
structure in the init data section.
--
Signed-Off-By: Chandra Seetharaman <sekharan@us.ibm.com>

 arch/powerpc/kernel/sysfs.c        |    2 +-
 arch/s390/appldata/appldata_base.c |    2 +-
 block/ll_rw_blk.c                  |    2 +-
 kernel/hrtimer.c                   |    2 +-
 kernel/rcupdate.c                  |    2 +-
 kernel/sched.c                     |    2 +-
 kernel/softirq.c                   |    2 +-
 kernel/softlockup.c                |    2 +-
 kernel/timer.c                     |    2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

Index: linux-2617-rc2/arch/powerpc/kernel/sysfs.c
===================================================================
--- linux-2617-rc2.orig/arch/powerpc/kernel/sysfs.c	2006-04-24 06:56:50.000000000 -0700
+++ linux-2617-rc2/arch/powerpc/kernel/sysfs.c	2006-04-24 07:11:37.000000000 -0700
@@ -297,7 +297,7 @@ static int __devinit sysfs_cpu_notify(st
 	return NOTIFY_OK;
 }
 
-static struct notifier_block __devinitdata sysfs_cpu_nb = {
+static struct notifier_block sysfs_cpu_nb = {
 	.notifier_call	= sysfs_cpu_notify,
 };
 
Index: linux-2617-rc2/arch/s390/appldata/appldata_base.c
===================================================================
--- linux-2617-rc2.orig/arch/s390/appldata/appldata_base.c	2006-04-24 06:56:50.000000000 -0700
+++ linux-2617-rc2/arch/s390/appldata/appldata_base.c	2006-04-24 06:57:34.000000000 -0700
@@ -652,7 +652,7 @@ appldata_cpu_notify(struct notifier_bloc
 	return NOTIFY_OK;
 }
 
-static struct notifier_block __devinitdata appldata_nb = {
+static struct notifier_block appldata_nb = {
 	.notifier_call = appldata_cpu_notify,
 };
 
Index: linux-2617-rc2/block/ll_rw_blk.c
===================================================================
--- linux-2617-rc2.orig/block/ll_rw_blk.c	2006-04-19 10:17:08.000000000 -0700
+++ linux-2617-rc2/block/ll_rw_blk.c	2006-04-24 06:58:19.000000000 -0700
@@ -3385,7 +3385,7 @@ static int blk_cpu_notify(struct notifie
 }
 
 
-static struct notifier_block __devinitdata blk_cpu_notifier = {
+static struct notifier_block blk_cpu_notifier = {
 	.notifier_call	= blk_cpu_notify,
 };
 
Index: linux-2617-rc2/kernel/hrtimer.c
===================================================================
--- linux-2617-rc2.orig/kernel/hrtimer.c	2006-04-19 10:17:15.000000000 -0700
+++ linux-2617-rc2/kernel/hrtimer.c	2006-04-24 07:11:37.000000000 -0700
@@ -860,7 +860,7 @@ static int __devinit hrtimer_cpu_notify(
 	return NOTIFY_OK;
 }
 
-static struct notifier_block __devinitdata hrtimers_nb = {
+static struct notifier_block hrtimers_nb = {
 	.notifier_call = hrtimer_cpu_notify,
 };
 
Index: linux-2617-rc2/kernel/softirq.c
===================================================================
--- linux-2617-rc2.orig/kernel/softirq.c	2006-04-19 10:17:15.000000000 -0700
+++ linux-2617-rc2/kernel/softirq.c	2006-04-24 07:11:37.000000000 -0700
@@ -484,7 +484,7 @@ static int __devinit cpu_callback(struct
 	return NOTIFY_OK;
 }
 
-static struct notifier_block __devinitdata cpu_nfb = {
+static struct notifier_block cpu_nfb = {
 	.notifier_call = cpu_callback
 };
 
Index: linux-2617-rc2/kernel/rcupdate.c
===================================================================
--- linux-2617-rc2.orig/kernel/rcupdate.c	2006-04-24 06:58:55.000000000 -0700
+++ linux-2617-rc2/kernel/rcupdate.c	2006-04-24 07:11:37.000000000 -0700
@@ -537,7 +537,7 @@ static int __devinit rcu_cpu_notify(stru
 	return NOTIFY_OK;
 }
 
-static struct notifier_block __devinitdata rcu_nb = {
+static struct notifier_block rcu_nb = {
 	.notifier_call	= rcu_cpu_notify,
 };
 
Index: linux-2617-rc2/kernel/softlockup.c
===================================================================
--- linux-2617-rc2.orig/kernel/softlockup.c	2006-04-19 10:17:15.000000000 -0700
+++ linux-2617-rc2/kernel/softlockup.c	2006-04-24 07:00:06.000000000 -0700
@@ -140,7 +140,7 @@ cpu_callback(struct notifier_block *nfb,
 	return NOTIFY_OK;
 }
 
-static struct notifier_block __devinitdata cpu_nfb = {
+static struct notifier_block cpu_nfb = {
 	.notifier_call = cpu_callback
 };
 
Index: linux-2617-rc2/kernel/timer.c
===================================================================
--- linux-2617-rc2.orig/kernel/timer.c	2006-04-24 06:54:40.000000000 -0700
+++ linux-2617-rc2/kernel/timer.c	2006-04-24 07:11:37.000000000 -0700
@@ -1334,7 +1334,7 @@ static int __devinit timer_cpu_notify(st
 	return NOTIFY_OK;
 }
 
-static struct notifier_block __devinitdata timers_nb = {
+static struct notifier_block timers_nb = {
 	.notifier_call	= timer_cpu_notify,
 };
 
Index: linux-2617-rc2/kernel/sched.c
===================================================================
--- linux-2617-rc2.orig/kernel/sched.c	2006-04-24 06:54:41.000000000 -0700
+++ linux-2617-rc2/kernel/sched.c	2006-04-24 07:11:45.000000000 -0700
@@ -4814,7 +4814,7 @@ static int migration_call(struct notifie
 /* Register at highest priority so that task migration (migrate_all_tasks)
  * happens before everything else.
  */
-static struct notifier_block __devinitdata migration_notifier = {
+static struct notifier_block migration_notifier = {
 	.notifier_call = migration_call,
 	.priority = 10
 };

-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/3] Remove __devinit and __cpuinit from notifier_call definitions
  2006-04-25  2:35 [PATCH 0/3] Fix for the bug reported by Herbert on 2.6.17-rc2 sekharan
  2006-04-25  2:35 ` [PATCH 1/3] Remove __devinitdata from notifier block definitions sekharan
@ 2006-04-25  2:35 ` sekharan
  2006-04-25  2:35 ` [PATCH 3/3] Assert notifier_block and notifier_call are not in init section sekharan
  2 siblings, 0 replies; 12+ messages in thread
From: sekharan @ 2006-04-25  2:35 UTC (permalink / raw)
  To: akpm, torvalds
  Cc: sekharan, linux-kernel, herbert, linux-xfs, xfs-masters, stern

Few of the notifier_chain_register() callers use __init in the definition
of notifier_call.  It is incorrect as the function definition should be
available after the initializations (they do not unregister them during
initializations).

This patch fixes all such usages to _not_ have the notifier_call __init
section.
--
Signed-Off-By: Chandra Seetharaman <sekharan@us.ibm.com>

 arch/i386/kernel/cpu/intel_cacheinfo.c |    2 +-
 arch/i386/kernel/cpuid.c               |    2 +-
 arch/i386/kernel/msr.c                 |    2 +-
 arch/ia64/kernel/palinfo.c             |    2 +-
 arch/ia64/kernel/salinfo.c             |    2 +-
 arch/ia64/kernel/topology.c            |    2 +-
 arch/powerpc/kernel/sysfs.c            |    2 +-
 arch/x86_64/kernel/mce.c               |    2 +-
 arch/x86_64/kernel/mce_amd.c           |    2 +-
 drivers/base/topology.c                |    2 +-
 drivers/cpufreq/cpufreq.c              |    2 +-
 kernel/hrtimer.c                       |    2 +-
 kernel/profile.c                       |    2 +-
 kernel/rcupdate.c                      |    2 +-
 kernel/softirq.c                       |    2 +-
 kernel/softlockup.c                    |    2 +-
 kernel/timer.c                         |    2 +-
 kernel/workqueue.c                     |    2 +-
 mm/page_alloc.c                        |    2 +-
 mm/slab.c                              |    2 +-
 mm/vmscan.c                            |    2 +-
 21 files changed, 21 insertions(+), 21 deletions(-)

Index: linux-2617-rc2/arch/i386/kernel/cpuid.c
===================================================================
--- linux-2617-rc2.orig/arch/i386/kernel/cpuid.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/arch/i386/kernel/cpuid.c	2006-04-24 08:35:08.000000000 -0700
@@ -168,7 +168,7 @@ static int cpuid_class_device_create(int
 	return err;
 }
 
-static int __devinit cpuid_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
+static int cpuid_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
 
Index: linux-2617-rc2/arch/i386/kernel/msr.c
===================================================================
--- linux-2617-rc2.orig/arch/i386/kernel/msr.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/arch/i386/kernel/msr.c	2006-04-24 08:35:08.000000000 -0700
@@ -251,7 +251,7 @@ static int msr_class_device_create(int i
 	return err;
 }
 
-static int __devinit msr_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
+static int msr_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
 
Index: linux-2617-rc2/arch/ia64/kernel/palinfo.c
===================================================================
--- linux-2617-rc2.orig/arch/ia64/kernel/palinfo.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/arch/ia64/kernel/palinfo.c	2006-04-24 08:35:08.000000000 -0700
@@ -959,7 +959,7 @@ remove_palinfo_proc_entries(unsigned int
 	}
 }
 
-static int __devinit palinfo_cpu_callback(struct notifier_block *nfb,
+static int palinfo_cpu_callback(struct notifier_block *nfb,
 								unsigned long action,
 								void *hcpu)
 {
Index: linux-2617-rc2/arch/powerpc/kernel/sysfs.c
===================================================================
--- linux-2617-rc2.orig/arch/powerpc/kernel/sysfs.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/arch/powerpc/kernel/sysfs.c	2006-04-24 08:35:08.000000000 -0700
@@ -279,7 +279,7 @@ static void unregister_cpu_online(unsign
 }
 #endif /* CONFIG_HOTPLUG_CPU */
 
-static int __devinit sysfs_cpu_notify(struct notifier_block *self,
+static int sysfs_cpu_notify(struct notifier_block *self,
 				      unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned int)(long)hcpu;
Index: linux-2617-rc2/kernel/hrtimer.c
===================================================================
--- linux-2617-rc2.orig/kernel/hrtimer.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/kernel/hrtimer.c	2006-04-24 08:35:08.000000000 -0700
@@ -836,7 +836,7 @@ static void migrate_hrtimers(int cpu)
 }
 #endif /* CONFIG_HOTPLUG_CPU */
 
-static int __devinit hrtimer_cpu_notify(struct notifier_block *self,
+static int hrtimer_cpu_notify(struct notifier_block *self,
 					unsigned long action, void *hcpu)
 {
 	long cpu = (long)hcpu;
Index: linux-2617-rc2/kernel/profile.c
===================================================================
--- linux-2617-rc2.orig/kernel/profile.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/kernel/profile.c	2006-04-24 08:35:08.000000000 -0700
@@ -299,7 +299,7 @@ out:
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
-static int __devinit profile_cpu_callback(struct notifier_block *info,
+static int profile_cpu_callback(struct notifier_block *info,
 					unsigned long action, void *__cpu)
 {
 	int node, cpu = (unsigned long)__cpu;
Index: linux-2617-rc2/kernel/rcupdate.c
===================================================================
--- linux-2617-rc2.orig/kernel/rcupdate.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/kernel/rcupdate.c	2006-04-24 10:21:20.000000000 -0700
@@ -520,7 +520,7 @@ static void __devinit rcu_online_cpu(int
 	tasklet_init(&per_cpu(rcu_tasklet, cpu), rcu_process_callbacks, 0UL);
 }
 
-static int __devinit rcu_cpu_notify(struct notifier_block *self, 
+static int rcu_cpu_notify(struct notifier_block *self,
 				unsigned long action, void *hcpu)
 {
 	long cpu = (long)hcpu;
Index: linux-2617-rc2/kernel/softirq.c
===================================================================
--- linux-2617-rc2.orig/kernel/softirq.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/kernel/softirq.c	2006-04-24 10:19:15.000000000 -0700
@@ -446,7 +446,7 @@ static void takeover_tasklets(unsigned i
 }
 #endif /* CONFIG_HOTPLUG_CPU */
 
-static int __devinit cpu_callback(struct notifier_block *nfb,
+static int cpu_callback(struct notifier_block *nfb,
 				  unsigned long action,
 				  void *hcpu)
 {
Index: linux-2617-rc2/kernel/timer.c
===================================================================
--- linux-2617-rc2.orig/kernel/timer.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/kernel/timer.c	2006-04-24 10:21:43.000000000 -0700
@@ -1314,7 +1314,7 @@ static void __devinit migrate_timers(int
 }
 #endif /* CONFIG_HOTPLUG_CPU */
 
-static int __devinit timer_cpu_notify(struct notifier_block *self, 
+static int timer_cpu_notify(struct notifier_block *self,
 				unsigned long action, void *hcpu)
 {
 	long cpu = (long)hcpu;
Index: linux-2617-rc2/kernel/workqueue.c
===================================================================
--- linux-2617-rc2.orig/kernel/workqueue.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/kernel/workqueue.c	2006-04-24 08:35:08.000000000 -0700
@@ -547,7 +547,7 @@ static void take_over_work(struct workqu
 }
 
 /* We're holding the cpucontrol mutex here */
-static int __devinit workqueue_cpu_callback(struct notifier_block *nfb,
+static int workqueue_cpu_callback(struct notifier_block *nfb,
 				  unsigned long action,
 				  void *hcpu)
 {
Index: linux-2617-rc2/mm/slab.c
===================================================================
--- linux-2617-rc2.orig/mm/slab.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/mm/slab.c	2006-04-24 08:35:08.000000000 -0700
@@ -1036,7 +1036,7 @@ static inline void free_alien_cache(stru
 
 #endif
 
-static int __devinit cpuup_callback(struct notifier_block *nfb,
+static int cpuup_callback(struct notifier_block *nfb,
 				    unsigned long action, void *hcpu)
 {
 	long cpu = (long)hcpu;
Index: linux-2617-rc2/arch/ia64/kernel/topology.c
===================================================================
--- linux-2617-rc2.orig/arch/ia64/kernel/topology.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/arch/ia64/kernel/topology.c	2006-04-24 08:35:08.000000000 -0700
@@ -429,7 +429,7 @@ static int __cpuinit cache_remove_dev(st
  * When a cpu is hot-plugged, do a check and initiate
  * cache kobject if necessary
  */
-static int __cpuinit cache_cpu_callback(struct notifier_block *nfb,
+static int cache_cpu_callback(struct notifier_block *nfb,
 		unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
Index: linux-2617-rc2/arch/i386/kernel/cpu/intel_cacheinfo.c
===================================================================
--- linux-2617-rc2.orig/arch/i386/kernel/cpu/intel_cacheinfo.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/arch/i386/kernel/cpu/intel_cacheinfo.c	2006-04-24 08:35:08.000000000 -0700
@@ -642,7 +642,7 @@ static void __cpuexit cache_remove_dev(s
 	return;
 }
 
-static int __cpuinit cacheinfo_cpu_callback(struct notifier_block *nfb,
+static int cacheinfo_cpu_callback(struct notifier_block *nfb,
 					unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
Index: linux-2617-rc2/arch/x86_64/kernel/mce_amd.c
===================================================================
--- linux-2617-rc2.orig/arch/x86_64/kernel/mce_amd.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/arch/x86_64/kernel/mce_amd.c	2006-04-24 08:35:08.000000000 -0700
@@ -482,7 +482,7 @@ static void threshold_remove_device(unsi
 #endif
 
 /* get notified when a cpu comes on/off */
-static __cpuinit int threshold_cpu_callback(struct notifier_block *nfb,
+static int threshold_cpu_callback(struct notifier_block *nfb,
 					    unsigned long action, void *hcpu)
 {
 	/* cpu was unsigned int to begin with */
Index: linux-2617-rc2/drivers/base/topology.c
===================================================================
--- linux-2617-rc2.orig/drivers/base/topology.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/drivers/base/topology.c	2006-04-24 08:35:08.000000000 -0700
@@ -107,7 +107,7 @@ static int __cpuinit topology_remove_dev
 	return 0;
 }
 
-static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
+static int topology_cpu_callback(struct notifier_block *nfb,
 		unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
Index: linux-2617-rc2/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-2617-rc2.orig/drivers/cpufreq/cpufreq.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/drivers/cpufreq/cpufreq.c	2006-04-24 08:35:08.000000000 -0700
@@ -1497,7 +1497,7 @@ int cpufreq_update_policy(unsigned int c
 }
 EXPORT_SYMBOL(cpufreq_update_policy);
 
-static int __cpuinit cpufreq_cpu_callback(struct notifier_block *nfb,
+static int cpufreq_cpu_callback(struct notifier_block *nfb,
 					unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
Index: linux-2617-rc2/mm/page_alloc.c
===================================================================
--- linux-2617-rc2.orig/mm/page_alloc.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/mm/page_alloc.c	2006-04-24 08:35:08.000000000 -0700
@@ -1960,7 +1960,7 @@ static inline void free_zone_pagesets(in
 	}
 }
 
-static int __cpuinit pageset_cpuup_callback(struct notifier_block *nfb,
+static int pageset_cpuup_callback(struct notifier_block *nfb,
 		unsigned long action,
 		void *hcpu)
 {
Index: linux-2617-rc2/mm/vmscan.c
===================================================================
--- linux-2617-rc2.orig/mm/vmscan.c	2006-04-24 08:34:25.000000000 -0700
+++ linux-2617-rc2/mm/vmscan.c	2006-04-24 08:35:08.000000000 -0700
@@ -1328,7 +1328,7 @@ repeat:
    not required for correctness.  So if the last cpu in a node goes
    away, we get changed to run anywhere: as the first one comes back,
    restore their cpu bindings. */
-static int __devinit cpu_callback(struct notifier_block *nfb,
+static int cpu_callback(struct notifier_block *nfb,
 				  unsigned long action, void *hcpu)
 {
 	pg_data_t *pgdat;
Index: linux-2617-rc2/kernel/softlockup.c
===================================================================
--- linux-2617-rc2.orig/kernel/softlockup.c	2006-04-24 07:00:06.000000000 -0700
+++ linux-2617-rc2/kernel/softlockup.c	2006-04-24 10:19:33.000000000 -0700
@@ -104,7 +104,7 @@ static int watchdog(void * __bind_cpu)
 /*
  * Create/destroy watchdog threads as CPUs come and go:
  */
-static int __devinit
+static int
 cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 {
 	int hotcpu = (unsigned long)hcpu;
Index: linux-2617-rc2/arch/ia64/kernel/salinfo.c
===================================================================
--- linux-2617-rc2.orig/arch/ia64/kernel/salinfo.c	2006-03-19 21:53:29.000000000 -0800
+++ linux-2617-rc2/arch/ia64/kernel/salinfo.c	2006-04-24 09:48:39.000000000 -0700
@@ -572,7 +572,7 @@ static struct file_operations salinfo_da
 };
 
 #ifdef	CONFIG_HOTPLUG_CPU
-static int __devinit
+static int
 salinfo_cpu_callback(struct notifier_block *nb, unsigned long action, void *hcpu)
 {
 	unsigned int i, cpu = (unsigned long)hcpu;
Index: linux-2617-rc2/arch/x86_64/kernel/mce.c
===================================================================
--- linux-2617-rc2.orig/arch/x86_64/kernel/mce.c	2006-04-19 10:17:08.000000000 -0700
+++ linux-2617-rc2/arch/x86_64/kernel/mce.c	2006-04-24 09:49:08.000000000 -0700
@@ -629,7 +629,7 @@ static __cpuinit void mce_remove_device(
 #endif
 
 /* Get notified when a cpu comes on/off. Be hotplug friendly. */
-static __cpuinit int
+static int
 mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;

-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/3] Assert notifier_block and notifier_call are not in init section
  2006-04-25  2:35 [PATCH 0/3] Fix for the bug reported by Herbert on 2.6.17-rc2 sekharan
  2006-04-25  2:35 ` [PATCH 1/3] Remove __devinitdata from notifier block definitions sekharan
  2006-04-25  2:35 ` [PATCH 2/3] Remove __devinit and __cpuinit from notifier_call definitions sekharan
@ 2006-04-25  2:35 ` sekharan
  2006-04-25  2:47   ` Linus Torvalds
  2 siblings, 1 reply; 12+ messages in thread
From: sekharan @ 2006-04-25  2:35 UTC (permalink / raw)
  To: akpm, torvalds
  Cc: sekharan, linux-kernel, herbert, linux-xfs, xfs-masters, stern

Andrew,

	Feel free to drop this patch if you think it is not needed.

regards,

chandra
--

This patch ASSERTS that the notifier_block data structure and the
notifier_call funtion are not in the init section.

--
Signed-Off-By: Chandra Seetharaman <sekharan@us.ibm.com>

 sys.c |    7 +++++++
 1 files changed, 7 insertions(+)

Index: linux-2617-rc2/kernel/sys.c
===================================================================
--- linux-2617-rc2.orig/kernel/sys.c	2006-04-24 11:29:30.000000000 -0700
+++ linux-2617-rc2/kernel/sys.c	2006-04-24 11:36:35.000000000 -0700
@@ -97,6 +97,11 @@ int cad_pid = 1;
 
 static BLOCKING_NOTIFIER_HEAD(reboot_notifier_list);
 
+static inline int addr_in_init_section(void *addr)
+{
+	return ((addr >= (void *)__init_begin) && (addr < (void *)__init_end));
+}
+
 /*
  *	Notifier chain core routines.  The exported routines below
  *	are layered on top of these, with appropriate locking added.
@@ -105,6 +110,8 @@ static BLOCKING_NOTIFIER_HEAD(reboot_not
 static int notifier_chain_register(struct notifier_block **nl,
 		struct notifier_block *n)
 {
+	BUG_ON(addr_in_init_section(n)
+		 || addr_in_init_section(n->notifier_call));
 	while ((*nl) != NULL) {
 		if (n->priority > (*nl)->priority)
 			break;

-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] Assert notifier_block and notifier_call are not in init section
  2006-04-25  2:35 ` [PATCH 3/3] Assert notifier_block and notifier_call are not in init section sekharan
@ 2006-04-25  2:47   ` Linus Torvalds
  2006-04-25  2:50     ` Linus Torvalds
  2006-04-25 19:01     ` Chandra Seetharaman
  0 siblings, 2 replies; 12+ messages in thread
From: Linus Torvalds @ 2006-04-25  2:47 UTC (permalink / raw)
  To: sekharan; +Cc: akpm, linux-kernel, herbert, linux-xfs, xfs-masters, stern



On Mon, 24 Apr 2006, sekharan@us.ibm.com wrote:
> 
> 	Feel free to drop this patch if you think it is not needed.

It's incorrect.

The init section will be free'd, and as a result can be re-allocated to 
other uses. Thus testing that data is not in the init-section makes no 
sense.

Testing for _code_ not being in the init section can be sensible, since 
code never gets re-allocated (modulo module code, but that's never in the 
init section). So checking the "notifier_call" part may be sensible, but 
checking the notifier block data pointer definitely is not.

Patches 1-2 applied.

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] Assert notifier_block and notifier_call are not in init section
  2006-04-25  2:47   ` Linus Torvalds
@ 2006-04-25  2:50     ` Linus Torvalds
  2006-04-25 19:01     ` Chandra Seetharaman
  1 sibling, 0 replies; 12+ messages in thread
From: Linus Torvalds @ 2006-04-25  2:50 UTC (permalink / raw)
  To: sekharan; +Cc: akpm, linux-kernel, herbert, linux-xfs, xfs-masters, stern



On Mon, 24 Apr 2006, Linus Torvalds wrote:
> 
> Patches 1-2 applied.

Actually, I take that back. The second one doesn't apply to my tree, so I 
assume it's against the -mm tree, and that I'll get this series through 
Andrew.

Btw, can you fix your mailer to put your real name in your email address, 
please?

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] Assert notifier_block and notifier_call are not in init section
  2006-04-25  2:47   ` Linus Torvalds
  2006-04-25  2:50     ` Linus Torvalds
@ 2006-04-25 19:01     ` Chandra Seetharaman
  2006-04-25 19:16       ` Linus Torvalds
  1 sibling, 1 reply; 12+ messages in thread
From: Chandra Seetharaman @ 2006-04-25 19:01 UTC (permalink / raw)
  To: Linus Torvalds, akpm
  Cc: linux-kernel, herbert, linux-xfs, xfs-masters, Alan Stern

On Mon, 2006-04-24 at 19:47 -0700, Linus Torvalds wrote:
> 
> On Mon, 24 Apr 2006, sekharan@us.ibm.com wrote:
> > 
> > 	Feel free to drop this patch if you think it is not needed.
> 
> It's incorrect.
> 
> The init section will be free'd, and as a result can be re-allocated to 
> other uses. Thus testing that data is not in the init-section makes no 
> sense.
>
> Testing for _code_ not being in the init section can be sensible, since 
> code never gets re-allocated (modulo module code, but that's never in the 
> init section). So checking the "notifier_call" part may be sensible, but 
> checking the notifier block data pointer definitely is not.

Two questions:
1) related to this patch: Do you want me to generate a patch that
asserts only notifier calls ?

2) Unrelated to this patch: If the _code_ section is never reallocated
or reused, what is the purpose of putting _code_ in the init section ?
Only to make sure that the init calls are called in order ?

Thanks

chandra
PS: I fixed my mailer to put my name. sorry about that.

> 
> Patches 1-2 applied.
> 
> 		Linus
-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] Assert notifier_block and notifier_call are not in init section
  2006-04-25 19:01     ` Chandra Seetharaman
@ 2006-04-25 19:16       ` Linus Torvalds
  2006-04-25 20:26         ` Alan Stern
  2006-04-25 22:33         ` Chandra Seetharaman
  0 siblings, 2 replies; 12+ messages in thread
From: Linus Torvalds @ 2006-04-25 19:16 UTC (permalink / raw)
  To: Chandra Seetharaman
  Cc: akpm, linux-kernel, herbert, linux-xfs, xfs-masters, Alan Stern



On Tue, 25 Apr 2006, Chandra Seetharaman wrote:
> 
> Two questions:
> 1) related to this patch: Do you want me to generate a patch that
> asserts only notifier calls ?

I don't really have any strong preferences. It seems a bit strange that 
we'd do it for notifiers but not for other people. It might be better to 
try to build it into the build system itself, and get it through the 
_normal_ "section checking".

One way to do that would be to make the "register_notifier()" thing just 
create this dummy asm() that just puts the arguments into a section that 
doesn't even get loaded, but that cna be checked.

> 2) Unrelated to this patch: If the _code_ section is never reallocated
> or reused, what is the purpose of putting _code_ in the init section ?
> Only to make sure that the init calls are called in order ?

No, the code section is re-used, it's just never re-used for any other 
code (since we don't generate code on the fly). So if you pass in a 
function pointer, you know that if it's in the init section, it means that 
init-code that was discarded.

But if you pass in a data pointer, you'll never know if it's a data 
pointer to the original init-code section, or if it was a data pointer 
that was just dynamically allocated after the init-code section was freed.

> PS: I fixed my mailer to put my name. sorry about that.

Looks good.

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] Assert notifier_block and notifier_call are not in init section
  2006-04-25 19:16       ` Linus Torvalds
@ 2006-04-25 20:26         ` Alan Stern
  2006-04-25 20:38           ` Linus Torvalds
  2006-04-25 22:33         ` Chandra Seetharaman
  1 sibling, 1 reply; 12+ messages in thread
From: Alan Stern @ 2006-04-25 20:26 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Chandra Seetharaman, akpm, linux-kernel, herbert, linux-xfs,
	xfs-masters

On Tue, 25 Apr 2006, Linus Torvalds wrote:

> > 2) Unrelated to this patch: If the _code_ section is never reallocated
> > or reused, what is the purpose of putting _code_ in the init section ?
> > Only to make sure that the init calls are called in order ?
> 
> No, the code section is re-used, it's just never re-used for any other 
> code (since we don't generate code on the fly). So if you pass in a 
> function pointer, you know that if it's in the init section, it means that 
> init-code that was discarded.

What about loadable modules?  Is their code never loaded into memory that
used to be part of an init section?

Alan Stern


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] Assert notifier_block and notifier_call are not in init section
  2006-04-25 20:26         ` Alan Stern
@ 2006-04-25 20:38           ` Linus Torvalds
  2006-04-25 20:54             ` Randy.Dunlap
  0 siblings, 1 reply; 12+ messages in thread
From: Linus Torvalds @ 2006-04-25 20:38 UTC (permalink / raw)
  To: Alan Stern
  Cc: Chandra Seetharaman, akpm, linux-kernel, herbert, linux-xfs,
	xfs-masters



On Tue, 25 Apr 2006, Alan Stern wrote:
> 
> What about loadable modules?  Is their code never loaded into memory that
> used to be part of an init section?

Their code might _physically_ reside in a re-allocation of an init 
section, but will have a virtual address far away (and it would be the 
virtual address that you'd see if you took the address of a function).

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] Assert notifier_block and notifier_call are not in init section
  2006-04-25 20:38           ` Linus Torvalds
@ 2006-04-25 20:54             ` Randy.Dunlap
  0 siblings, 0 replies; 12+ messages in thread
From: Randy.Dunlap @ 2006-04-25 20:54 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: stern, sekharan, akpm, linux-kernel, herbert, linux-xfs,
	xfs-masters

On Tue, 25 Apr 2006 13:38:22 -0700 (PDT) Linus Torvalds wrote:

> 
> 
> On Tue, 25 Apr 2006, Alan Stern wrote:
> > 
> > What about loadable modules?  Is their code never loaded into memory that
> > used to be part of an init section?
> 
> Their code might _physically_ reside in a re-allocation of an init 
> section, but will have a virtual address far away (and it would be the 
> virtual address that you'd see if you took the address of a function).

and the freed init data area is poisoned (in 386, x86_64, powerpc, and
sparc64).

---
~Randy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] Assert notifier_block and notifier_call are not in init section
  2006-04-25 19:16       ` Linus Torvalds
  2006-04-25 20:26         ` Alan Stern
@ 2006-04-25 22:33         ` Chandra Seetharaman
  1 sibling, 0 replies; 12+ messages in thread
From: Chandra Seetharaman @ 2006-04-25 22:33 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: akpm, linux-kernel, herbert, linux-xfs, xfs-masters, Alan Stern

On Tue, 2006-04-25 at 12:16 -0700, Linus Torvalds wrote:
> 
> On Tue, 25 Apr 2006, Chandra Seetharaman wrote:
> > 
> > Two questions:
> > 1) related to this patch: Do you want me to generate a patch that
> > asserts only notifier calls ?
> 
> I don't really have any strong preferences. It seems a bit strange that 
> we'd do it for notifiers but not for other people. It might be better to 
> try to build it into the build system itself, and get it through the 
> _normal_ "section checking".

I 'll hold off for now then.
> 
> One way to do that would be to make the "register_notifier()" thing just 
> create this dummy asm() that just puts the arguments into a section that 
> doesn't even get loaded, but that cna be checked.
> 
> > 2) Unrelated to this patch: If the _code_ section is never reallocated
> > or reused, what is the purpose of putting _code_ in the init section ?
> > Only to make sure that the init calls are called in order ?
> 
> No, the code section is re-used, it's just never re-used for any other 
> code (since we don't generate code on the fly). So if you pass in a 
> function pointer, you know that if it's in the init section, it means that 
> init-code that was discarded.
> 
> But if you pass in a data pointer, you'll never know if it's a data 
> pointer to the original init-code section, or if it was a data pointer 
> that was just dynamically allocated after the init-code section was freed.

Thanks for the clarification. 
> 
> > PS: I fixed my mailer to put my name. sorry about that.
> 
> Looks good.
> 
> 		Linus
-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2006-04-25 22:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-04-25  2:35 [PATCH 0/3] Fix for the bug reported by Herbert on 2.6.17-rc2 sekharan
2006-04-25  2:35 ` [PATCH 1/3] Remove __devinitdata from notifier block definitions sekharan
2006-04-25  2:35 ` [PATCH 2/3] Remove __devinit and __cpuinit from notifier_call definitions sekharan
2006-04-25  2:35 ` [PATCH 3/3] Assert notifier_block and notifier_call are not in init section sekharan
2006-04-25  2:47   ` Linus Torvalds
2006-04-25  2:50     ` Linus Torvalds
2006-04-25 19:01     ` Chandra Seetharaman
2006-04-25 19:16       ` Linus Torvalds
2006-04-25 20:26         ` Alan Stern
2006-04-25 20:38           ` Linus Torvalds
2006-04-25 20:54             ` Randy.Dunlap
2006-04-25 22:33         ` Chandra Seetharaman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox