2001-11-07 22:11:47

by Pete Zaitcev

[permalink] [raw]
Subject: Patch for kernel.real-root-dev on s390

Hello,

what do you think about the attached patch. Without it,
a sysctl to kernel.real-root-dev corrupts adjacent memory.
Anyone cares to comment?

-- Pete

diff -ur -X dontdiff linux-2.4.9-13.1/drivers/block/rd.c linux-2.4.9-13.1pt1/drivers/block/rd.c
--- linux-2.4.9-13.1/drivers/block/rd.c Tue Nov 6 19:19:21 2001
+++ linux-2.4.9-13.1pt1/drivers/block/rd.c Tue Nov 6 23:43:49 2001
@@ -704,9 +704,6 @@

static void __init rd_load_disk(int n)
{
-#ifdef CONFIG_BLK_DEV_INITRD
- extern kdev_t real_root_dev;
-#endif

if (rd_doload == 0)
return;
diff -ur -X dontdiff linux-2.4.9-13.1/include/linux/fs.h linux-2.4.9-13.1pt1/include/linux/fs.h
--- linux-2.4.9-13.1/include/linux/fs.h Tue Nov 6 19:19:10 2001
+++ linux-2.4.9-13.1pt1/include/linux/fs.h Tue Nov 6 23:46:44 2001
@@ -1471,7 +1471,7 @@
extern void mount_root(void);

#ifdef CONFIG_BLK_DEV_INITRD
-extern kdev_t real_root_dev;
+extern unsigned int real_root_dev;
extern int change_root(kdev_t, const char *);
#endif

diff -ur -X dontdiff linux-2.4.9-13.1/init/main.c linux-2.4.9-13.1pt1/init/main.c
--- linux-2.4.9-13.1/init/main.c Tue Nov 6 19:19:19 2001
+++ linux-2.4.9-13.1pt1/init/main.c Tue Nov 6 23:46:12 2001
@@ -126,7 +126,7 @@
int rows, cols;

#ifdef CONFIG_BLK_DEV_INITRD
-kdev_t real_root_dev;
+unsigned int real_root_dev; /* do_proc_dointvec cannot handle kdev_t */
#endif

int root_mountflags = MS_RDONLY;


2001-11-10 02:49:02

by Ulrich Weigand

[permalink] [raw]
Subject: Re: Patch for kernel.real-root-dev on s390

Pete Zaitcev wrote:

>what do you think about the attached patch. Without it,
>a sysctl to kernel.real-root-dev corrupts adjacent memory.

I agree that this looks broken, but I don't see why it
would be s390 specific. The clobber of adjacent memory
happens on all architectures, and on all big endian systems
the value read is incorrect even if adjacent memory happens
to be 0.

However, I'm not convinced the patch is a proper fix; it
will cause the MAJOR and MINOR macros to be applied to a
variable not of type kdev_t, which happens to work now but will
break if the definition of kdev_t is changed to a structure
or pointer type (as it probably will at some point in the
future, if I recall the various discussions correctly).

What about either
- adding support for kdev_t values to procfs
or
- keeping two int variables real_root_major and
real_root_minor ?

Bye,
Ulrich

--
Dr. Ulrich Weigand
[email protected]

2001-11-10 18:00:06

by Pete Zaitcev

[permalink] [raw]
Subject: Re: Patch for kernel.real-root-dev on s390

> From: Ulrich Weigand <[email protected]>
> Date: Sat, 10 Nov 2001 03:48:33 +0100 (MET)
> Cc: [email protected]

> I agree that this looks broken, but I don't see why it
> would be s390 specific. The clobber of adjacent memory
> happens on all architectures, and on all big endian systems
> the value read is incorrect even if adjacent memory happens
> to be 0.

Probably alignment restrictions do not allow anything interesting
to happen. I know now that ppc people complained about it.

> However, I'm not convinced the patch is a proper fix; it
> will cause the MAJOR and MINOR macros to be applied to a
> variable not of type kdev_t, which happens to work now but will
> break if the definition of kdev_t is changed to a structure
> or pointer type (as it probably will at some point in the
> future, if I recall the various discussions correctly).
>
> What about either
> - adding support for kdev_t values to procfs
> or

I thought that would be the right thing to do when kdev_t is changed.
Currently, I do not know how to change it. Guy Streeter told me
that someone floated an insanely ugly patch that special-cased
shorts into do_proc_dointvec(), and I did not like that approach
too much. Once the structure of new kdev_t is known, the
do_proc_kdev_t may be defined, but I think it makes no sense
to jump the gun now.

> - keeping two int variables real_root_major and
> real_root_minor ?

Who knows if we are going to have majors and minors at all.
Suppose Gooch and Viro give us a decent devfs, or something.

An alternative may be to redo the initrd interface, for instance
have /proc/real-root-path instead of real-root-dev (and no sysctl),
I did not have time to explore all implications of that way.

-- Pete