2000-11-20 01:11:55

by NeilBrown

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

On Saturday November 18, [email protected] 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);
+