[PATCH] s390 appldata_base: Misc cpuinit annotations and bugfix

[PATCH] s390 appldata_base: Misc cpuinit annotations and bugfix

[Satyam Sharma]

appldata_offline_cpu() is only called from __cpuinit-marked hotplug
notifier callback and from the __exit-marked module_exit function,
therefore candidate for __cpuexit.

BTW the __exit module_exit function appldata_exit() of this driver
fails to unregister_hotcpu_notifier() the notifier_block that was
registered by appldata_init() during module startup. This will lead
to oops if hotplug notification comes after module has been unloaded.
Let's fix this by unregistering the notifier appropriately (before
appldata_offline_cpu()'ing the CPUs).

Signed-off-by: Satyam Sharma <satyam@infradead.org>

---

 arch/s390/appldata/appldata_base.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/s390/appldata/appldata_base.c b/arch/s390/appldata/appldata_base.c
index 62391fb..a355d81 100644
--- a/arch/s390/appldata/appldata_base.c
+++ b/arch/s390/appldata/appldata_base.c
@@ -547,8 +547,7 @@ static void __cpuinit appldata_online_cpu(int cpu)
 	spin_unlock(&appldata_timer_lock);
 }
 
-static void
-appldata_offline_cpu(int cpu)
+static void __cpuexit appldata_offline_cpu(int cpu)
 {
 	del_virt_timer(&per_cpu(appldata_timer, cpu));
 	if (atomic_dec_and_test(&appldata_expire_count)) {
@@ -560,9 +559,9 @@ appldata_offline_cpu(int cpu)
 	spin_unlock(&appldata_timer_lock);
 }
 
-static int __cpuinit
-appldata_cpu_notify(struct notifier_block *self,
-		    unsigned long action, void *hcpu)
+static int __cpuinit appldata_cpu_notify(struct notifier_block *self,
+					 unsigned long action,
+					 void *hcpu)
 {
 	switch (action) {
 	case CPU_ONLINE:
@@ -646,6 +645,8 @@ static void __exit appldata_exit(void)
 	}
 	spin_unlock(&appldata_ops_lock);
 
+	unregister_hotcpu_notifier(&appldata_nb);
+
 	for_each_online_cpu(i)
 		appldata_offline_cpu(i);
 

[PATCH] s390 appldata_base: Remove module_exit function and modular stuff

[Satyam Sharma]

On Thu, 23 Aug 2007, Satyam Sharma wrote:
> 
> BTW the __exit module_exit function appldata_exit() of this driver
> fails to unregister_hotcpu_notifier() the notifier_block that was
> registered by appldata_init() during module startup. This will lead
> to oops if hotplug notification comes after module has been unloaded.
> Let's fix this by unregistering the notifier appropriately (before
> appldata_offline_cpu()'ing the CPUs).

Heh, no wonder we never saw this oops before. This code can never be
built as a module! :-) But why does it define and implement an __exit
marked module_exit(appldata_exit) function in that case ?!

[ On top of previous patch: ]


[PATCH] s390 appldata_base: Remove module_exit function and modular stuff

arch/s390/Kconfig tells us that CONFIG_APPLDATA_BASE is bool and hence
can never be built modular. Given this, defining appldata_exit() function
is pointless (and wasteful, actually). Remove all that.

Previous patch annotated appldata_offline_cpu() as __cpuexit, but now
with the __exit function appldata_exit() gone, the only callsite that
references it is __cpuinit, so this function can also be __cpuinit,
thereby saving space when HOTPLUG_CPU=n.

Signed-off-by: Satyam Sharma <satyam@infradead.org>

---

 arch/s390/appldata/appldata_base.c |   48 +-----------------------------------
 1 files changed, 1 insertions(+), 47 deletions(-)

diff --git a/arch/s390/appldata/appldata_base.c b/arch/s390/appldata/appldata_base.c
index a355d81..e74a5fb 100644
--- a/arch/s390/appldata/appldata_base.c
+++ b/arch/s390/appldata/appldata_base.c
@@ -547,7 +547,7 @@ static void __cpuinit appldata_online_cpu(int cpu)
 	spin_unlock(&appldata_timer_lock);
 }
 
-static void __cpuexit appldata_offline_cpu(int cpu)
+static void __cpuinit appldata_offline_cpu(int cpu)
 {
 	del_virt_timer(&per_cpu(appldata_timer, cpu));
 	if (atomic_dec_and_test(&appldata_expire_count)) {
@@ -607,61 +607,15 @@ static int __init appldata_init(void)
 	register_hotcpu_notifier(&appldata_nb);
 
 	appldata_sysctl_header = register_sysctl_table(appldata_dir_table);
-#ifdef MODULE
-	appldata_dir_table[0].de->owner = THIS_MODULE;
-	appldata_table[0].de->owner = THIS_MODULE;
-	appldata_table[1].de->owner = THIS_MODULE;
-#endif
 
 	P_DEBUG("Base interface initialized.\n");
 	return 0;
 }
 
-/*
- * appldata_exit()
- *
- * stop timer, unregister /proc entries
- */
-static void __exit appldata_exit(void)
-{
-	struct list_head *lh;
-	struct appldata_ops *ops;
-	int rc, i;
-
-	P_DEBUG("Unloading module ...\n");
-	/*
-	 * ops list should be empty, but just in case something went wrong...
-	 */
-	spin_lock(&appldata_ops_lock);
-	list_for_each(lh, &appldata_ops_list) {
-		ops = list_entry(lh, struct appldata_ops, list);
-		rc = appldata_diag(ops->record_nr, APPLDATA_STOP_REC,
-				(unsigned long) ops->data, ops->size,
-				ops->mod_lvl);
-		if (rc != 0) {
-			P_ERROR("STOP DIAG 0xDC for %s failed, "
-				"return code: %d\n", ops->name, rc);
-		}
-	}
-	spin_unlock(&appldata_ops_lock);
-
-	unregister_hotcpu_notifier(&appldata_nb);
-
-	for_each_online_cpu(i)
-		appldata_offline_cpu(i);
-
-	appldata_timer_active = 0;
-
-	unregister_sysctl_table(appldata_sysctl_header);
-
-	destroy_workqueue(appldata_wq);
-	P_DEBUG("... module unloaded!\n");
-}
 /**************************** init / exit <END> ******************************/
 
 
 module_init(appldata_init);
-module_exit(appldata_exit);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Gerald Schaefer");
 MODULE_DESCRIPTION("Linux-VM Monitor Stream, base infrastructure");

Re: [PATCH] s390 appldata_base: Remove module_exit function and modular stuff

[Martin Schwidefsky]

On Thu, 2007-08-23 at 07:51 +0530, Satyam Sharma wrote: 
> > BTW the __exit module_exit function appldata_exit() of this driver
> > fails to unregister_hotcpu_notifier() the notifier_block that was
> > registered by appldata_init() during module startup. This will lead
> > to oops if hotplug notification comes after module has been unloaded.
> > Let's fix this by unregistering the notifier appropriately (before
> > appldata_offline_cpu()'ing the CPUs).
> 
> Heh, no wonder we never saw this oops before. This code can never be
> built as a module! :-) But why does it define and implement an __exit
> marked module_exit(appldata_exit) function in that case ?!

I has been a module once for debugging purposes That the module_exit
function still exists is "historical". We can remove it since it is dead
code.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH] s390 appldata_base: Remove module_exit function and modular stuff

[Satyam Sharma]

Hi Martin,


On Thu, 23 Aug 2007, Martin Schwidefsky wrote:

> On Thu, 2007-08-23 at 07:51 +0530, Satyam Sharma wrote: 
> > > BTW the __exit module_exit function appldata_exit() of this driver
> > > fails to unregister_hotcpu_notifier() the notifier_block that was
> > > registered by appldata_init() during module startup. This will lead
> > > to oops if hotplug notification comes after module has been unloaded.
> > > Let's fix this by unregistering the notifier appropriately (before
> > > appldata_offline_cpu()'ing the CPUs).
> > 
> > Heh, no wonder we never saw this oops before. This code can never be
> > built as a module! :-) But why does it define and implement an __exit
> > marked module_exit(appldata_exit) function in that case ?!
> 
> I has been a module once for debugging purposes That the module_exit
> function still exists is "historical". We can remove it since it is dead
> code.

Thanks, fair enough. Does this mean you're acking them -- I'd prefer both
the patches to be applied separately. Rationale: we should still fix the
appldata_exit() related oops _before_ removing it so that in future if
this needs to be made modular for debugging purpose again, just the
removal patch can be reverted, but the oops would still be fixed in the
commit before it.


Satyam

Re: [PATCH] s390 appldata_base: Remove module_exit function and modular stuff

[Martin Schwidefsky]

On Fri, 2007-08-24 at 04:21 +0530, Satyam Sharma wrote:
> > I has been a module once for debugging purposes That the module_exit
> > function still exists is "historical". We can remove it since it is dead
> > code.
> 
> Thanks, fair enough. Does this mean you're acking them -- I'd prefer both
> the patches to be applied separately. Rationale: we should still fix the
> appldata_exit() related oops _before_ removing it so that in future if
> this needs to be made modular for debugging purpose again, just the
> removal patch can be reverted, but the oops would still be fixed in the
> commit before it.

ACK, the patches are fine.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.