Re: [BUG] raid5 link error? (was [PATCH] raid5 fix after xor.c cleanup)

From: Neil Brown (neilb@cse.unsw.edu.au)
Date: Sun Nov 19 2000 - 19:41:11 EST


On Saturday November 18, spaans@delft.corps.nl wrote:
> On Sat, Nov 18, 2000 at 12:35:36PM +0100, Jasper Spaans wrote:
>
> > Hmm, next time I'll need to eat my own dogfood -- this patch doesn't work,
> > it only compiles. Don't use it.
>
> It seems to me the original code was correct, but the linking isn't in the
> right order and the initcalls are in the wrong order (ie,
> raid5.c::raid5_init() is being called before xor.c::calibrate_xor_block() --
> any linking gurus out there who can help me out on this one?
>
> Feeling a bit of a schizo, but getting used to my brown paper bag,
>
> Regards

Yep... the link order is important isn't it.
At least three pairs of eyes looked and the changes to xor and no of
us spotted this dependancy - just shows how important beta testers are
- thanks.

The following patch changes the link order in the Makefile so that xor
is initiailised before md tries to autostart anything.
It also takes the theme a bit further and uses module_init/module_exit
to init and shutdown the raid personalities. This allows us to remove
the explicit calls to raidXX_init from md.c, which is nice.

I have tested this patch both
   1/ monolithic kernel and autodetecting an array
   2/ md and all personalities modules

and it works fine.

It reduced the "#ifdef" count of the kernel by 8, which must be a good
thing :-)

NeilBrown

--- ./drivers/md/Makefile 2000/11/19 22:25:54 1.1
+++ ./drivers/md/Makefile 2000/11/20 00:33:08 1.2
@@ -16,11 +16,13 @@
 obj-n :=
 obj- :=
 
-obj-$(CONFIG_BLK_DEV_MD) += md.o
+# Note: link order is important.
+# md personalities must come before md, and xor before raid5
 obj-$(CONFIG_MD_LINEAR) += linear.o
 obj-$(CONFIG_MD_RAID0) += raid0.o
 obj-$(CONFIG_MD_RAID1) += raid1.o
-obj-$(CONFIG_MD_RAID5) += raid5.o xor.o
+obj-$(CONFIG_MD_RAID5) += xor.o raid5.o
+obj-$(CONFIG_BLK_DEV_MD) += md.o
 obj-$(CONFIG_BLK_DEV_LVM) += lvm-mod.o
 
 # Translate to Rules.make lists.
--- ./drivers/md/md.c 2000/11/19 22:27:17 1.1
+++ ./drivers/md/md.c 2000/11/20 00:33:08 1.2
@@ -3576,12 +3576,6 @@
         create_proc_read_entry("mdstat", 0, NULL, md_status_read_proc, NULL);
 #endif
 }
-void hsm_init (void);
-void translucent_init (void);
-void linear_init (void);
-void raid0_init (void);
-void raid1_init (void);
-void raid5_init (void);
 
 int md__init md_init (void)
 {
@@ -3617,18 +3611,6 @@
         md_register_reboot_notifier(&md_notifier);
         raid_table_header = register_sysctl_table(raid_root_table, 1);
 
-#ifdef CONFIG_MD_LINEAR
- linear_init ();
-#endif
-#ifdef CONFIG_MD_RAID0
- raid0_init ();
-#endif
-#ifdef CONFIG_MD_RAID1
- raid1_init ();
-#endif
-#ifdef CONFIG_MD_RAID5
- raid5_init ();
-#endif
         md_geninit();
         return (0);
 }
--- ./drivers/md/linear.c 2000/11/19 22:33:21 1.1
+++ ./drivers/md/linear.c 2000/11/20 00:33:08 1.2
@@ -190,24 +190,16 @@
         status: linear_status,
 };
 
-#ifndef MODULE
-
-void md__init linear_init (void)
-{
- register_md_personality (LINEAR, &linear_personality);
-}
-
-#else
-
-int init_module (void)
+static int md__init linear_init (void)
 {
- return (register_md_personality (LINEAR, &linear_personality));
+ return register_md_personality (LINEAR, &linear_personality);
 }
 
-void cleanup_module (void)
+static void linear_exit (void)
 {
         unregister_md_personality (LINEAR);
 }
 
-#endif
 
+module_init(linear_init);
+module_exit(linear_exit);
--- ./drivers/md/raid0.c 2000/11/19 22:39:37 1.1
+++ ./drivers/md/raid0.c 2000/11/20 00:33:08 1.2
@@ -333,24 +333,17 @@
         status: raid0_status,
 };
 
-#ifndef MODULE
-
-void raid0_init (void)
-{
- register_md_personality (RAID0, &raid0_personality);
-}
-
-#else
-
-int init_module (void)
+static int md__init raid0_init (void)
 {
- return (register_md_personality (RAID0, &raid0_personality));
+ return register_md_personality (RAID0, &raid0_personality);
 }
 
-void cleanup_module (void)
+void raid0_exit (void)
 {
         unregister_md_personality (RAID0);
 }
 
-#endif
+module_init(raid0_init);
+module_exit(raid0_exit);
+
 
--- ./drivers/md/raid1.c 2000/11/19 22:39:42 1.1
+++ ./drivers/md/raid1.c 2000/11/20 00:33:08 1.2
@@ -1882,19 +1882,16 @@
         sync_request: raid1_sync_request
 };
 
-int raid1_init (void)
+static int md__init raid1_init (void)
 {
         return register_md_personality (RAID1, &raid1_personality);
 }
 
-#ifdef MODULE
-int init_module (void)
-{
- return raid1_init();
-}
-
-void cleanup_module (void)
+static void raid1_exit (void)
 {
         unregister_md_personality (RAID1);
 }
-#endif
+
+module_init(raid1_init);
+module_exit(raid1_exit);
+
--- ./drivers/md/raid5.c 2000/11/19 22:39:46 1.1
+++ ./drivers/md/raid5.c 2000/11/20 00:33:08 1.2
@@ -2352,19 +2352,16 @@
         sync_request: raid5_sync_request
 };
 
-int raid5_init (void)
+static int md__init raid5_init (void)
 {
         return register_md_personality (RAID5, &raid5_personality);
 }
 
-#ifdef MODULE
-int init_module (void)
-{
- return raid5_init();
-}
-
-void cleanup_module (void)
+static void raid5_exit (void)
 {
         unregister_md_personality (RAID5);
 }
-#endif
+
+module_init(raid5_init);
+module_exit(raid5_exit);
+
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Nov 23 2000 - 21:00:18 EST