Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id ; Sun, 19 Nov 2000 20:11:55 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id ; Sun, 19 Nov 2000 20:11:35 -0500 Received: from note.orchestra.cse.unsw.EDU.AU ([129.94.242.29]:32005 "HELO note.orchestra.cse.unsw.EDU.AU") by vger.kernel.org with SMTP id ; Sun, 19 Nov 2000 20:11:31 -0500 From: Neil Brown To: Jasper Spaans Date: Mon, 20 Nov 2000 11:41:11 +1100 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <14872.29479.901021.472890@notabene.cse.unsw.edu.au> Cc: linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org cc: Richard Henderson , MOLNAR Ingo , "David S. Miller" , Linus Torvalds Subject: Re: [BUG] raid5 link error? (was [PATCH] raid5 fix after xor.c cleanup) In-Reply-To: message from Jasper Spaans on Saturday November 18 In-Reply-To: <20001117234144.A14461@spaans.ds9a.nl> <20001118123536.A5674@spaans.ds9a.nl> <20001118235352.D2226@spaans.ds9a.nl> X-Mailer: VM 6.72 under Emacs 20.7.2 X-face: [Gw_3E*Gng}4rRrKRYotwlE?.2|**#s9D 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/