real_root_dev
`real_root_dev' must be `int', not `kdev_t'. The reason for this is that we still have (in ): typedef unsigned short kdev_t; while kernel/sysctl.c has the following line: {KERN_REALROOTDEV, "real-root-dev", &real_root_dev, sizeof(int), 0644, NULL, &proc_dointvec}, This causes a nice endianness problem on big-endian machines. It can even cause weird results on little-endian machines where the alignment rule for int is a 2 byte-boundary. We've had this patch in the m68k tree since ages (at least 2.0.x), so it's time to share it to the world :-) I think it was originally by Andreas Schwab. --- linux-2.4.0-test10-pre3/init/main.c Sat Sep 23 17:31:22 2000 +++ geert-real_root_dev-2.4.0-test10-pre3/init/main.c Thu Oct 19 21:39:07 2000 @@ -124,7 +124,7 @@ int rows, cols; #ifdef CONFIG_BLK_DEV_INITRD -kdev_t real_root_dev; +int real_root_dev; /* MUST be int for sysctl! */ #endif int root_mountflags = MS_RDONLY; @@ -717,7 +717,7 @@ #ifdef CONFIG_BLK_DEV_INITRD root_mountflags = real_root_mountflags; - if (mount_initrd && ROOT_DEV != real_root_dev + if (mount_initrd && ROOT_DEV != (kdev_t)real_root_dev && MAJOR(ROOT_DEV) == RAMDISK_MAJOR && MINOR(ROOT_DEV) == 0) { int error; int i, pid; @@ -725,9 +725,9 @@ pid = kernel_thread(do_linuxrc, "/linuxrc", SIGCHLD); if (pid>0) while (pid != wait(&i)); - if (MAJOR(real_root_dev) != RAMDISK_MAJOR - || MINOR(real_root_dev) != 0) { - error = change_root(real_root_dev,"/initrd"); + if (MAJOR((kdev_t)real_root_dev) != RAMDISK_MAJOR + || MINOR((kdev_t)real_root_dev) != 0) { + error = change_root((kdev_t)real_root_dev,"/initrd"); if (error) printk(KERN_ERR "Change root to /initrd: " "error %d\n",error); --- linux-2.4.0-test10-pre3/include/linux/devfs_fs_kernel.h Tue Jul 18 14:07:34 2000 +++ geert-real_root_dev-2.4.0-test10-pre3/include/linux/devfs_fs_kernel.h Thu +Oct 19 21:38:06 2000 @@ -46,7 +46,7 @@ #ifdef CONFIG_BLK_DEV_INITRD -# define ROOT_DEVICE_NAME ((real_root_dev ==ROOT_DEV) ? root_device_name:NULL) +# define ROOT_DEVICE_NAME (((kdev_t)real_root_dev ==ROOT_DEV) ? +root_device_name:NULL) #else # define ROOT_DEVICE_NAME root_device_name #endif --- linux-2.4.0-test10-pre3/include/linux/fs.h Sat Oct 14 19:03:33 2000 +++ geert-real_root_dev-2.4.0-test10-pre3/include/linux/fs.hThu Oct 19 21:38:21 +2000 @@ -1228,7 +1228,7 @@ extern void mount_root(void); #ifdef CONFIG_BLK_DEV_INITRD -extern kdev_t real_root_dev; +extern int real_root_dev; extern int change_root(kdev_t, const char *); #endif --- linux-2.4.0-test10-pre3/drivers/block/rd.c Tue Jul 18 14:08:53 2000 +++ geert-real_root_dev-2.4.0-test10-pre3/drivers/block/rd.cThu Oct 19 21:37:58 +2000 @@ -704,7 +704,7 @@ static void __init rd_load_disk(int n) { #ifdef CONFIG_BLK_DEV_INITRD - extern kdev_t real_root_dev; + extern int real_root_dev; #endif if (rd_doload == 0) @@ -712,7 +712,7 @@ if (MAJOR(ROOT_DEV) != FLOPPY_MAJOR #ifdef CONFIG_BLK_DEV_INITRD - && MAJOR(real_root_dev) != FLOPPY_MAJOR + && MAJOR((kdev_t)real_root_dev) != FLOPPY_MAJOR #endif ) return; @@ -724,8 +724,8 @@ #ifdef CONFIG_MAC_FLOPPY if(MAJOR(ROOT_DEV) == FLOPPY_MAJOR) swim3_fd_eject(MINOR(ROOT_DEV)); - else if(MAJOR(real_root_dev) == FLOPPY_MAJOR) - swim3_fd_eject(MINOR(real_root_dev)); + else if(MAJOR((kdev_t)real_root_dev) == FLOPPY_MAJOR) + swim3_fd_eject(MINOR((kdev_t)real_root_dev)); #endif printk(KERN_NOTICE "VFS: Insert root floppy disk to be loaded into RAM disk and press ENTER\n"); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [EMAIL PROTECTED] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: real_root_dev
On Mon, Nov 27, 2000 at 10:27:00PM +0100, Marcus Sundberg wrote: > This reminded me of an old bug which apparently still hasn't been > fixed (not in 2.2 at least). In init/main.c we have: > > extern int rd_image_start;/* starting block # of image */ > #ifdef CONFIG_BLK_DEV_INITRD > kdev_t real_root_dev; > #endif > #endif > > int root_mountflags = MS_RDONLY; > > and then in kernel/sysctl.c: > > #ifdef CONFIG_BLK_DEV_INITRD > {KERN_REALROOTDEV, "real-root-dev", &real_root_dev, sizeof(int), >0644, NULL, &proc_dointvec}, > #endif > > Because rd_image_start and root_mountflags are both int-aligned, > this happens to work on little endian platforms. On big endian > platforms however writing a value in the range 0-65535 to > /proc/sys/kernel/real-root-dev will place 0 in real_root_dev, > and the actual value in the two padding bytes... > > Unfortunately proc_dointvec() doesn't support shorts, so what is > the correct fix? Changing: > kdev_t real_root_dev; > into > int real_root-dev; > is a perfectly working solution, but is it acceptable? If you compile the kernel and use an integral type for kdev_t, perhaps. On the other hand, I usually use a pointer type for kdev_t, and then this entire sysctl construction is broken. Andries - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: real_root_dev
On Thu, Oct 19, 2000 at 09:50:48PM +0200, Geert Uytterhoeven wrote: > > `real_root_dev' must be `int', not `kdev_t'. > > - if (MAJOR(real_root_dev) != RAMDISK_MAJOR > + if (MAJOR((kdev_t)real_root_dev) != RAMDISK_MAJOR Ach, Geert, how painful to behold! Never forget: a kdev_t is a pointer to a structure, and MAJOR takes a field of this structure. Casting an integer to a structure is ridiculous. There are functions to_kdev_t etc to do the conversion (and these may involve lookup in a hash table). Please keep the source as much as possible kdev_t clean. At some point in time, I hope 2.5.1, we must change, and all such cruft would have to be fixed again. Andries - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: real_root_dev
On Fri, 20 Oct 2000, Andries Brouwer wrote: > On Thu, Oct 19, 2000 at 09:50:48PM +0200, Geert Uytterhoeven wrote: > > > > `real_root_dev' must be `int', not `kdev_t'. > > > > - if (MAJOR(real_root_dev) != RAMDISK_MAJOR > > + if (MAJOR((kdev_t)real_root_dev) != RAMDISK_MAJOR > > Ach, Geert, how painful to behold! > > Never forget: a kdev_t is a pointer to a structure, > and MAJOR takes a field of this structure. > Casting an integer to a structure is ridiculous. > There are functions to_kdev_t etc to do the conversion > (and these may involve lookup in a hash table). Well, that's what the _comments_ in say: | However, for the time being we let kdev_t be almost the same as dev_t: | | typedef struct { unsigned short major, minor; } kdev_t; But the actual definition is | typedef unsigned short kdev_t; Which is incompatible with taking the address of a kdev_t object and assuming it has the same size as an int, which doesn't equal to any of the `admissible operations on an object of type kdev_t', as per . > Please keep the source as much as possible kdev_t clean. > At some point in time, I hope 2.5.1, we must change, > and all such cruft would have to be fixed again. So what do you suggest to fix the bug related to sysctl of real_root_dev? Just disable it completely? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [EMAIL PROTECTED] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/