2001-10-30 23:54:05

by Denis Zaitsev

[permalink] [raw]
Subject: [PATCH] init/main.c/root_dev_names - another one #ifdef

The idea is to include the SCSI disk's names into the list only if we
compile the kernel with the corresponding support. It's not very
serious patch, but to put the things in order... Linus, please, apply
it.

--- init/main.c.orig Wed Oct 31 02:23:48 2001
+++ init/main.c Wed Oct 31 02:44:02 2001
@@ -166,6 +166,7 @@
{ "hdr", 0x5A40 },
{ "hds", 0x5B00 },
{ "hdt", 0x5B40 },
+#ifdef CONFIG_BLK_DEV_SD
{ "sda", 0x0800 },
{ "sdb", 0x0810 },
{ "sdc", 0x0820 },
@@ -182,6 +183,7 @@
{ "sdn", 0x08d0 },
{ "sdo", 0x08e0 },
{ "sdp", 0x08f0 },
+#endif
{ "ada", 0x1c00 },
{ "adb", 0x1c10 },
{ "adc", 0x1c20 },


2001-10-31 00:08:55

by Alan

[permalink] [raw]
Subject: Re: [PATCH] init/main.c/root_dev_names - another one #ifdef

> The idea is to include the SCSI disk's names into the list only if we
> compile the kernel with the corresponding support. It's not very
> serious patch, but to put the things in order... Linus, please, apply
> it.

It took that out deliberately a few months back. The ifdefs in fact
break stuff

Firstly the array is __init so is discarded on boot

Secondly if you have an initrd containing the scsi driver layers then you
can specify root=sda quite legitimately.


Alan

2001-10-31 00:14:26

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] init/main.c/root_dev_names - another one #ifdef


On Wed, 31 Oct 2001, Alan Cox wrote:
>
> It took that out deliberately a few months back. The ifdefs in fact
> break stuff
>
> Firstly the array is __init so is discarded on boot

I think that array really is broken. We should get the name association
from the array that "register_blkdev()" maintains, I'm sure. That way
random stupid driver X doesn't need to touch a common init/main.c file,
which I find personally offensive.

Linus

2001-10-31 00:23:15

by Alan

[permalink] [raw]
Subject: Re: [PATCH] init/main.c/root_dev_names - another one #ifdef

> > Firstly the array is __init so is discarded on boot
>
> I think that array really is broken. We should get the name association
> from the array that "register_blkdev()" maintains, I'm sure. That way
> random stupid driver X doesn't need to touch a common init/main.c file,
> which I find personally offensive.

For 2.5 definitely. Right now we'd have to be very careful to process
the string -> number conversion after initrd had run (ie at the root change)
and also add a mechanism for aliases for the names we don't quite match

2001-10-31 01:29:40

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] init/main.c/root_dev_names - another one #ifdef

On Oct 30, 2001 16:12 -0800, Linus Torvalds wrote:
> On Wed, 31 Oct 2001, Alan Cox wrote:
> > It took that out deliberately a few months back. The ifdefs in fact
> > break stuff
> >
> > Firstly the array is __init so is discarded on boot
>
> I think that array really is broken. We should get the name association
> from the array that "register_blkdev()" maintains, I'm sure. That way
> random stupid driver X doesn't need to touch a common init/main.c file,
> which I find personally offensive.

I have a patch from way back (not mine) which allows the name to be
generated by the block device driver code. Let me know if you are
interested. This also fixes the ugly "lvma" "lvmb" devices that are
in /proc/partitions.

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/

2001-10-31 01:37:00

by Denis Zaitsev

[permalink] [raw]
Subject: Re: [PATCH] init/main.c/root_dev_names - another one #ifdef

On Wed, Oct 31, 2001 at 12:15:10AM +0000, Alan Cox wrote:
> Secondly if you have an initrd containing the scsi driver layers then you
> can specify root=sda quite legitimately.

This sounds strange, as I don't mean the SCSI compiled-into-bzImage,
but the SCSI support at all.

2001-10-31 17:29:12

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] init/main.c/root_dev_names - another one #ifdef

In article <[email protected]> you wrote:
> I have a patch from way back (not mine) which allows the name to be
> generated by the block device driver code. Let me know if you are
> interested. This also fixes the ugly "lvma" "lvmb" devices that are
> in /proc/partitions.

In OpenGFS CVS we have such a patch as well.
Version for -ac is attached.

Christoph

--
Of course it doesn't work. We've performed a software upgrade.

diff -uNr -Xdontdiff ../master/linux-2.4.10-ac4/drivers/ide/ide-probe.c linux/drivers/ide/ide-probe.c
--- ../master/linux-2.4.10-ac4/drivers/ide/ide-probe.c Tue Oct 30 14:54:22 2001
+++ linux/drivers/ide/ide-probe.c Wed Oct 31 18:21:20 2001
@@ -759,6 +759,8 @@
}
minors = units * (1<<PARTN_BITS);
gd = kmalloc (sizeof(struct gendisk), GFP_KERNEL);
+ /* XXX: check for nulptr deref */
+ memset(gd, 0, sizeof(*gd));
gd->sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
gd->part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
bs = kmalloc (minors*sizeof(int), GFP_KERNEL);
diff -uNr -Xdontdiff ../master/linux-2.4.10-ac4/fs/partitions/check.c linux/fs/partitions/check.c
--- ../master/linux-2.4.10-ac4/fs/partitions/check.c Tue Oct 30 14:54:29 2001
+++ linux/fs/partitions/check.c Wed Oct 31 18:20:40 2001
@@ -99,6 +99,9 @@
unsigned int unit = (minor >> hd->minor_shift);
unsigned int part = (minor & ((1 << hd->minor_shift) -1 ));

+ if (hd->device_names)
+ return hd->device_names[minor];
+
if ((unit < hd->nr_real) && hd->part[minor].de) {
int pos;

diff -uNr -Xdontdiff ../master/linux-2.4.10-ac4/include/linux/genhd.h linux/include/linux/genhd.h
--- ../master/linux-2.4.10-ac4/include/linux/genhd.h Tue Oct 30 14:54:31 2001
+++ linux/include/linux/genhd.h Wed Oct 31 18:20:54 2001
@@ -83,6 +83,8 @@

devfs_handle_t *de_arr; /* one per physical disc */
char *flags; /* one per physical disc */
+#define _HAVE_GENDISK_DEVICE_NAMES
+ char **device_names;
};

/* drivers/block/genhd.c */

2001-10-31 18:20:54

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] init/main.c/root_dev_names - another one #ifdef

On Oct 31, 2001 18:28 +0100, Christoph Hellwig wrote:
> In OpenGFS CVS we have such a patch as well.
> Version for -ac is attached.
>
> diff -uNr -Xdontdiff ../master/linux-2.4.10-ac4/fs/partitions/check.c linux/fs/partitions/check.c
> --- ../master/linux-2.4.10-ac4/fs/partitions/check.c Tue Oct 30 14:54:29 2001
> +++ linux/fs/partitions/check.c Wed Oct 31 18:20:40 2001
> @@ -99,6 +99,9 @@
> unsigned int unit = (minor >> hd->minor_shift);
> unsigned int part = (minor & ((1 << hd->minor_shift) -1 ));
>
> + if (hd->device_names)
> + return hd->device_names[minor];
> +
> if ((unit < hd->nr_real) && hd->part[minor].de) {
> int pos;
>

This seems kind of ugly - an array holding each device name? The patch
I have rather puts a function to generate the device names when needed
(which is probably not very often, unless GFS does something wierd).

I take it your patch is only the "bare bones" part which shows what is
changed? "My" patch removes all of the junk from partitions/check.c
and puts it where it belongs - into the device drivers. Patch originally
by Brian Kress <[email protected]>, dated Nov 24, 2000. Probably won't
apply cleanly, but it is clear enough to see what it is doing. If there
is interest, I could rediff against a current kernel.

Note that (not included with this patch, I noticed it later) one needs
to change the maximum disk_name buffer length to 128 because of LVM names.
If you don't have LVM, or don't have LV names > 64 chars, it is OK to test.

Cheers, Andreas
=========================================================================
diff -u --recursive linux-2.4.0-test11/drivers/block/DAC960.c
linux-2.4.0-test11-ppfix/drivers/block/DAC960.c
--- linux-2.4.0-test11/drivers/block/DAC960.c Mon Nov 20 15:17:25 2000
+++ linux-2.4.0-test11-ppfix/drivers/block/DAC960.c Thu Nov 23 14:29:37
2000
@@ -1885,6 +1885,21 @@
}


+char *DAC960_disk_name(struct gendisk *hd, int minor, char *buf)
+
+{
+ int ctlr = hd->major - DAC960_MAJOR;
+ int disk = minor >> hd->minor_shift;
+ int part = minor & ((1 << hd->minor_shift) - 1);
+
+ if (part)
+ sprintf(buf, "%s/c%dd%dp%d", hd->major_name, ctlr, disk, part);
+ else
+ sprintf(buf, "%s/c%dd%d", hd->major_name, ctlr, disk);
+ return buf;
+}
+
+
/*
DAC960_RegisterBlockDevice registers the Block Device structures
associated with Controller.
@@ -1945,6 +1960,7 @@
Controller->GenericDiskInfo.nr_real = Controller->LogicalDriveCount;
Controller->GenericDiskInfo.next = NULL;
Controller->GenericDiskInfo.fops = &DAC960_BlockDeviceOperations;
+ Controller->GenericDiskInfo.hd_name = DAC960_disk_name;
/*
Install the Generic Disk Information structure at the end of the
list.
*/
diff -u --recursive linux-2.4.0-test11/drivers/block/cciss.c
linux-2.4.0-test11-ppfix/drivers/block/cciss.c
--- linux-2.4.0-test11/drivers/block/cciss.c Mon Nov 20 15:17:25 2000
+++ linux-2.4.0-test11-ppfix/drivers/block/cciss.c Thu Nov 23 14:22:55
2000
@@ -1749,6 +1749,20 @@
kfree(size_buff);
}

+char *cciss_disk_name(struct gendisk *hd, int minor, char *buf)
+
+{
+ int ctlr = hd->major - COMPAQ_CISS_MAJOR;
+ int disk = minor >> hd->minor_shift;
+ int part = minor & ((1 << hd->minor_shift) - 1);
+
+ if (part)
+ sprintf(buf, "%s/c%dd%dp%d", hd->major_name, ctlr, disk, part);
+ else
+ sprintf(buf, "%s/c%dd%d", hd->major_name, ctlr, disk);
+ return buf;
+}
+
/*
* This is it. Find all the controllers and register them. I really
hate
* stealing all these major device numbers.
@@ -1851,6 +1865,7 @@
hba[i]->gendisk.part = hba[i]->hd;
hba[i]->gendisk.sizes = hba[i]->sizes;
hba[i]->gendisk.nr_real = hba[i]->num_luns;
+ hba[i]->gendisk.hd_name = cciss_disk_name;

/* Get on the disk list */
hba[i]->gendisk.next = gendisk_head;
diff -u --recursive linux-2.4.0-test11/drivers/block/cpqarray.c
linux-2.4.0-test11-ppfix/drivers/block/cpqarray.c
--- linux-2.4.0-test11/drivers/block/cpqarray.c Mon Nov 20 15:20:29 2000
+++ linux-2.4.0-test11-ppfix/drivers/block/cpqarray.c Thu Nov 23
14:14:03 2000
@@ -362,6 +362,20 @@
}
#endif /* MODULE */

+char *ida_disk_name(struct gendisk *hd, int minor, char *buf)
+
+{
+ int ctlr = hd->major - COMPAQ_SMART2_MAJOR;
+ int disk = minor >> hd->minor_shift;
+ int part = minor & ((1 << hd->minor_shift) - 1);
+
+ if (part)
+ sprintf(buf, "%s/c%dd%dp%d", hd->major_name, ctlr, disk, part);
+ else
+ sprintf(buf, "%s/c%dd%d", hd->major_name, ctlr, disk);
+ return buf;
+}
+
/*
* This is it. Find all the controllers and register them. I really
hate
* stealing all these major device numbers.
@@ -512,6 +526,7 @@
ida_gendisk[i].part = ida + (i*256);
ida_gendisk[i].sizes = ida_sizes + (i*256);
ida_gendisk[i].nr_real = 0;
+ ida_gendisk[i].hd_name = ida_disk_name;

/* Get on the disk list */
ida_gendisk[i].next = gendisk_head;
diff -u --recursive linux-2.4.0-test11/drivers/ide/ide-probe.c
linux-2.4.0-test11-ppfix/drivers/ide/ide-probe.c
--- linux-2.4.0-test11/drivers/ide/ide-probe.c Thu Aug 3 19:29:49 2000
+++ linux-2.4.0-test11-ppfix/drivers/ide/ide-probe.c Thu Nov 23 12:32:16
2000
@@ -726,6 +726,40 @@
return 0;
}

+char* ide_disk_name(struct gendisk *hd, int minor, char *buf)
+
+{
+ int unit = (minor >> hd->minor_shift) + 'a';
+ unsigned int part = minor & ((1 << hd->minor_shift) - 1);
+
+ switch (hd->major) {
+ case IDE9_MAJOR:
+ unit += 2;
+ case IDE8_MAJOR:
+ unit += 2;
+ case IDE7_MAJOR:
+ unit += 2;
+ case IDE6_MAJOR:
+ unit += 2;
+ case IDE5_MAJOR:
+ unit += 2;
+ case IDE4_MAJOR:
+ unit += 2;
+ case IDE3_MAJOR:
+ unit += 2;
+ case IDE2_MAJOR:
+ unit += 2;
+ case IDE1_MAJOR:
+ unit += 2;
+ case IDE0_MAJOR:
+ }
+ if (part)
+ sprintf(buf, "hd%c%d", unit, part);
+ else
+ sprintf(buf, "hd%c", unit);
+ return buf;
+}
+
/*
* init_gendisk() (as opposed to ide_geninit) is called for each major
device,
* after probing for drives, to allocate partition tables and other
data
@@ -781,6 +815,7 @@
gd->fops = ide_fops; /* file operations */
gd->de_arr = kmalloc (sizeof *gd->de_arr * units, GFP_KERNEL);
gd->flags = kmalloc (sizeof *gd->flags * units, GFP_KERNEL);
+ gd->hd_name = ide_disk_name;
if (gd->de_arr)
memset (gd->de_arr, 0, sizeof *gd->de_arr * units);
if (gd->flags)
diff -u --recursive linux-2.4.0-test11/drivers/md/lvm.c
linux-2.4.0-test11-ppfix/drivers/md/lvm.c
--- linux-2.4.0-test11/drivers/md/lvm.c Mon Nov 20 15:20:30 2000
+++ linux-2.4.0-test11-ppfix/drivers/md/lvm.c Thu Nov 23 07:47:20 2000
@@ -213,9 +213,7 @@
&lvm_proc_get_info;
#endif

-#ifdef LVM_HD_NAME
-void lvm_hd_name(char *, int);
-#endif
+char *lvm_hd_name(struct gendisk *, int, char *);
/* End external function prototypes */


@@ -230,9 +228,6 @@
int lvm_snapshot_alloc(lv_t *);
void lvm_snapshot_release(lv_t *);

-#ifdef LVM_HD_NAME
-extern void (*lvm_hd_name_ptr) (char *, int);
-#endif
static int lvm_map(struct buffer_head *, int);
static int lvm_do_lock_lvm(void);
static int lvm_do_le_remap(vg_t *, void *);
@@ -397,11 +392,6 @@
lvm_gendisk.next = NULL;
}

-#ifdef LVM_HD_NAME
- /* reference from drivers/block/genhd.c */
- lvm_hd_name_ptr = lvm_hd_name;
-#endif
-
blk_init_queue(BLK_DEFAULT_QUEUE(MAJOR_NR), DEVICE_REQUEST);
blk_queue_make_request(BLK_DEFAULT_QUEUE(MAJOR_NR),
lvm_make_request_fn);
blk_queue_pluggable(BLK_DEFAULT_QUEUE(MAJOR_NR),
lvm_plug_device_noop);
@@ -460,11 +450,6 @@
remove_proc_entry(LVM_NAME, &proc_root);
#endif

-#ifdef LVM_HD_NAME
- /* reference from linux/drivers/block/genhd.c */
- lvm_hd_name_ptr = NULL;
-#endif
-
printk(KERN_INFO "%s -- Module successfully deactivated\n", lvm_name);

return;
@@ -1448,24 +1433,22 @@
* internal support functions
*/

-#ifdef LVM_HD_NAME
/*
* generate "hard disk" name
*/
-void lvm_hd_name(char *buf, int minor)
+char *lvm_hd_name(struct gendisk *hd, int minor, char *buf)
{
int len = 0;
lv_t *lv_ptr;

if (vg[VG_BLK(minor)] == NULL ||
(lv_ptr = vg[VG_BLK(minor)]->lv[LV_BLK(minor)]) == NULL)
- return;
+ return buf;
len = strlen(lv_ptr->lv_name) - 5;
memcpy(buf, &lv_ptr->lv_name[5], len);
buf[len] = 0;
- return;
+ return buf;
}
-#endif


/*
@@ -2515,6 +2498,8 @@

blksize_size[MAJOR_NR] = lvm_blocksizes;
blk_size[MAJOR_NR] = lvm_size;
+
+ lvm_gendisk.hd_name=lvm_hd_name;

return;
} /* lvm_gen_init() */
diff -u --recursive linux-2.4.0-test11/drivers/md/md.c
linux-2.4.0-test11-ppfix/drivers/md/md.c
--- linux-2.4.0-test11/drivers/md/md.c Mon Nov 20 15:20:30 2000
+++ linux-2.4.0-test11-ppfix/drivers/md/md.c Thu Nov 23 07:56:06 2000
@@ -113,6 +113,8 @@
extern struct block_device_operations md_fops;
static devfs_handle_t devfs_handle;

+char *md_disk_name(struct gendisk*, int, char *);
+
static struct gendisk md_gendisk=
{
major: MD_MAJOR,
@@ -125,6 +127,7 @@
real_devices: NULL,
next: NULL,
fops: &md_fops,
+ hd_name: md_disk_name,
};

/*
@@ -215,6 +218,14 @@
MOD_INC_USE_COUNT;

return mddev;
+}
+
+char * md_disk_name(struct gendisk *hd, int minor, char* buf)
+
+{
+ int unit = (minor >> hd->minor_shift) + '0';
+ sprintf(buf, "%s%c", hd->major_name, unit);
+ return buf;
}

struct gendisk * find_gendisk (kdev_t dev)
diff -u --recursive linux-2.4.0-test11/drivers/scsi/sd.c
linux-2.4.0-test11-ppfix/drivers/scsi/sd.c
--- linux-2.4.0-test11/drivers/scsi/sd.c Mon Nov 20 15:17:18 2000
+++ linux-2.4.0-test11-ppfix/drivers/scsi/sd.c Thu Nov 23 09:53:01 2000
@@ -1013,6 +1013,30 @@
return i;
}

+char *scsi_disk_name(struct gendisk *hd, int minor, char *buf)
+
+{
+ int unit = (minor >> hd->minor_shift) + 'a';
+ unsigned int part = minor & ((1 << hd->minor_shift) - 1);
+
+ if (hd->major >= SCSI_DISK1_MAJOR && hd->major <= SCSI_DISK7_MAJOR) {
+ unit = unit + (hd->major - SCSI_DISK1_MAJOR + 1) * 16;
+ if (unit > 'z') {
+ unit -= 'z' + 1;
+ sprintf(buf, "sd%c%c", 'a' + unit / 26, 'a' + unit % 26);
+ if (part)
+ sprintf(buf + 4, "%d", part);
+ return buf;
+ }
+ }
+
+ if (part)
+ sprintf(buf, "sd%c%d", unit, part);
+ else
+ sprintf(buf, "sd%c", unit);
+ return buf;
+}
+
/*
* The sd_init() function looks at all SCSI drives present, determines
* their size, and reads partition table entries for them.
@@ -1109,6 +1133,7 @@
sd_gendisks[i].next = sd_gendisks + i + 1;
sd_gendisks[i].real_devices =
(void *) (rscsi_disks + i * SCSI_DISKS_PER_MAJOR);
+ sd_gendisks[i].hd_name=scsi_disk_name;
}

LAST_SD_GENDISK.next = NULL;
diff -u --recursive linux-2.4.0-test11/fs/partitions/check.c
linux-2.4.0-test11-ppfix/fs/partitions/check.c
--- linux-2.4.0-test11/fs/partitions/check.c Mon Nov 20 15:17:27 2000
+++ linux-2.4.0-test11-ppfix/fs/partitions/check.c Thu Nov 23 14:30:45
2000
@@ -83,11 +83,10 @@
*/
char *disk_name (struct gendisk *hd, int minor, char *buf)
{
- unsigned int part;
const char *maj = hd->major_name;
int unit = (minor >> hd->minor_shift) + 'a';
+ unsigned int part = minor & ((1 << hd->minor_shift) - 1);

- part = minor & ((1 << hd->minor_shift) - 1);
if (hd->part[minor].de) {
int pos;

@@ -95,77 +94,8 @@
if (pos >= 0)
return buf + pos;
}
- /*
- * IDE devices use multiple major numbers, but the drives
- * are named as: {hda,hdb}, {hdc,hdd}, {hde,hdf}, {hdg,hdh}..
- * This requires special handling here.
- */
- switch (hd->major) {
- case IDE9_MAJOR:
- unit += 2;
- case IDE8_MAJOR:
- unit += 2;
- case IDE7_MAJOR:
- unit += 2;
- case IDE6_MAJOR:
- unit += 2;
- case IDE5_MAJOR:
- unit += 2;
- case IDE4_MAJOR:
- unit += 2;
- case IDE3_MAJOR:
- unit += 2;
- case IDE2_MAJOR:
- unit += 2;
- case IDE1_MAJOR:
- unit += 2;
- case IDE0_MAJOR:
- maj = "hd";
- break;
- case MD_MAJOR:
- unit -= 'a'-'0';
- break;
- }
- if (hd->major >= SCSI_DISK1_MAJOR && hd->major <= SCSI_DISK7_MAJOR) {
- unit = unit + (hd->major - SCSI_DISK1_MAJOR + 1) * 16;
- if (unit > 'z') {
- unit -= 'z' + 1;
- sprintf(buf, "sd%c%c", 'a' + unit / 26, 'a' + unit % 26);
- if (part)
- sprintf(buf + 4, "%d", part);
- return buf;
- }
- }
- if (hd->major >= COMPAQ_SMART2_MAJOR && hd->major <=
COMPAQ_SMART2_MAJOR+7) {
- int ctlr = hd->major - COMPAQ_SMART2_MAJOR;
- int disk = minor >> hd->minor_shift;
- int part = minor & (( 1 << hd->minor_shift) - 1);
- if (part == 0)
- sprintf(buf, "%s/c%dd%d", maj, ctlr, disk);
- else
- sprintf(buf, "%s/c%dd%dp%d", maj, ctlr, disk, part);
- return buf;
- }
- if (hd->major >= COMPAQ_CISS_MAJOR && hd->major <=
COMPAQ_CISS_MAJOR+7) {
- int ctlr = hd->major - COMPAQ_CISS_MAJOR;
- int disk = minor >> hd->minor_shift;
- int part = minor & (( 1 << hd->minor_shift) - 1);
- if (part == 0)
- sprintf(buf, "%s/c%dd%d", maj, ctlr, disk);
- else
- sprintf(buf, "%s/c%dd%dp%d", maj, ctlr, disk,
part);
- return buf;
- }
- if (hd->major >= DAC960_MAJOR && hd->major <= DAC960_MAJOR+7) {
- int ctlr = hd->major - DAC960_MAJOR;
- int disk = minor >> hd->minor_shift;
- int part = minor & (( 1 << hd->minor_shift) - 1);
- if (part == 0)
- sprintf(buf, "%s/c%dd%d", maj, ctlr, disk);
- else
- sprintf(buf, "%s/c%dd%dp%d", maj, ctlr, disk, part);
- return buf;
- }
+ if (hd->hd_name) return hd->hd_name(hd, minor, buf);
+
if (part)
sprintf(buf, "%s%c%d", maj, unit, part);
else
diff -u --recursive linux-2.4.0-test11/include/linux/genhd.h
linux-2.4.0-test11-ppfix/include/linux/genhd.h
--- linux-2.4.0-test11/include/linux/genhd.h Mon Nov 20 15:31:01 2000
+++ linux-2.4.0-test11-ppfix/include/linux/genhd.h Thu Nov 23 07:44:35
2000
@@ -72,6 +72,8 @@

devfs_handle_t *de_arr; /* one per physical disc */
char *flags; /* one per physical disc */
+
+ char *(*hd_name) (struct gendisk *, int, char *);
};
#endif /* __KERNEL__ */
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/

2001-10-31 18:31:32

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] init/main.c/root_dev_names - another one #ifdef

On Wed, Oct 31, 2001 at 11:20:55AM -0700, Andreas Dilger wrote:
> This seems kind of ugly - an array holding each device name? The patch
> I have rather puts a function to generate the device names when needed
> (which is probably not very often, unless GFS does something wierd).

*nod*

> I take it your patch is only the "bare bones" part which shows what is
> changed?

Well, it's a patch that tries to be not intrusive, it just crates the hooks
the two blockdevice drivers in the OpenGFS tree can use.

Christoph

--
Of course it doesn't work. We've performed a software upgrade.

2001-10-31 19:08:17

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] init/main.c/root_dev_names - another one #ifdef

On Oct 31, 2001 19:31 +0100, Christoph Hellwig wrote:
> On Wed, Oct 31, 2001 at 11:20:55AM -0700, Andreas Dilger wrote:
> > This seems kind of ugly - an array holding each device name? The patch
> > I have rather puts a function to generate the device names when needed
> > (which is probably not very often, unless GFS does something wierd).
>
> *nod*
>
> > I take it your patch is only the "bare bones" part which shows what is
> > changed?
>
> Well, it's a patch that tries to be not intrusive, it just crates the hooks
> the two blockdevice drivers in the OpenGFS tree can use.

Well, given that it's intrusive enough to change the gendisk struct
(which my patch does as well), we may as well go whole hog and remove
all of the partition name cruft from where it should not be. I suppose
the issue is whether you are expecting GFS to go into the kernel, or if
you want to keep the diff as small as possible if it will be outside
the kernel for a long time.

The LVM code had a global function to fix the naming issue for LVM
devices in /proc/partitions, but Linus (AFAIK) didn't like it because
it didn't fix the real problem, as the posted patch does, of localizing
the partition name generation to the driver itself. The only wart in
the current system is that we use an external buffer passed into the
driver, and if the driver likes long names (ala LVM) it will overflow.

As I look through the kernel today, there is even more ugliness spread
around with "#ifdef CONFIG_DEVFS" to create strange partition names in
places where it should probably not be.

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/

2001-11-01 02:06:35

by Denis Zaitsev

[permalink] [raw]
Subject: Re: [PATCH] init/main.c/root_dev_names - another one #ifdef

On Tue, Oct 30, 2001 at 04:12:36PM -0800, Linus Torvalds wrote:
> I think that array really is broken. We should get the name association
> from the array that "register_blkdev()" maintains, I'm sure. That way

If this array will be thrown away, am I right that we will lose an
ability to do root=/dev/hda ? As only "new names" will retain? And
by the way, it seems that devfs maintains the neccessary information,
not the array you have mentioned...