linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] partitions/ibm.c
@ 2001-02-26 10:15 Holger.Smolinski
  0 siblings, 0 replies; 5+ messages in thread
From: Holger.Smolinski @ 2001-02-26 10:15 UTC (permalink / raw)
  To: Guest section DW, aeb, torvalds, linux-kernel



Andries, others,
Thanks for hacking through the code of fs/partitions/ibm.c.
Your patch does not work at all because you are relying on the
data in the part component of the hd structure, which does not
hold the geometry data of the disk but the data of the partitions
on that disk. Besides that, exactly these data are to be set up
by the code in fs/partitions/ibm.c.
The geometry data is stored in the device driver. As in oposition
to the partition schemes the device drivercan be a loadable
module, fs/partitions.c should not at all directly call or use symbols
from the device driver.
My preferred solution would be having partition schemes as
loadable modules, too. Maybe I'll find some time to post the
approriate patch on this list soon...

Regards,
     Holger Smolinski

IBM Germany
Linux/390 Kernel Development
Schönaicher Str.220, D-71032 Böblingen

>Reading patch-2.4.2 I met a strange amount of crap in
>partitions/ibm.c. It is as if the author does not know
>where the kernel keeps the starting offset of a partition,
>and simulates a HDIO_GETGEO ioctl from user space.
>I think the following patch does the same and removes a lot
>of cruft. (Warning: (i) untested, uncompiled; (ii) pasted
>from another window - tabs will have become spaces.)

>Andries



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] partitions/ibm.c
@ 2001-02-27  0:51 Andries.Brouwer
  0 siblings, 0 replies; 5+ messages in thread
From: Andries.Brouwer @ 2001-02-27  0:51 UTC (permalink / raw)
  To: Andries.Brouwer, Holger.Smolinski; +Cc: dwguest, linux-kernel, torvalds

> your arguments appear correct to me
> I include the patch to be applied from my point of view

Good. Had no time today to look at the details, but at first sight
this is a big improvement. May want to nitpick a little some other time.

Andries

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] partitions/ibm.c
@ 2001-02-26 15:11 Holger.Smolinski
  0 siblings, 0 replies; 5+ messages in thread
From: Holger.Smolinski @ 2001-02-26 15:11 UTC (permalink / raw)
  To: Andries.Brouwer; +Cc: Andries.Brouwer, dwguest, linux-kernel, torvalds

[-- Attachment #1: Type: text/plain, Size: 552 bytes --]



Andries,
okay, your arguments appear correct to me. Additionally there has some
other work
to be done... I include the patch to be applied from my point of view:
(See attached file: ibmpart.diff)
It includes the adaptions to the dasd_*.c files in drivers/s390/block and
also should
fix the fill_geometry functions to make HDIO_GETGEO work like others.


Gruesse / Regards

Dr. Holger Smolinski
Linux for S/390 Design & Development,
IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, D-71032 Boeblingen
Phone/FAX: +49 7031 16 (x902) - 4652/3456

[-- Attachment #2: ibmpart.diff --]
[-- Type: application/octet-stream, Size: 9700 bytes --]

Index: drivers/s390/block/dasd.c
===================================================================
RCS file: /home/cvs/linux-2.3/drivers/s390/block/dasd.c,v
retrieving revision 1.92
diff -u -r1.92 dasd.c
--- drivers/s390/block/dasd.c	2001/02/22 16:02:44	1.92
+++ drivers/s390/block/dasd.c	2001/02/26 14:55:03
@@ -1979,10 +1979,11 @@
 			break;
 		}
 	case HDIO_GETGEO:{
-			struct hd_geometry geo =
-			{0,};
+			struct hd_geometry geo = {0,};
 			if (device->discipline->fill_geometry)
 				device->discipline->fill_geometry (device, &geo);
+                        geo.start = device->major_info->
+                                gendisk.part[MINOR(inp->i_rdev)].start_sect;
 			rc = copy_to_user ((struct hd_geometry *) data, &geo,
 					   sizeof (struct hd_geometry));
 			if (rc)
@@ -2543,6 +2544,7 @@
 			dd = &major_info->gendisk;
 			dd->sizes[minor] = (device->sizes.blocks <<
 					    device->sizes.s2b_shift) >> 1;
+                       dd->part[minor].start_sect = device->sizes.pt_block;
 			{
 				char buffer[5];
 				sprintf(buffer,"%04X",device->devinfo.devno);
Index: drivers/s390/block/dasd_diag.c
===================================================================
RCS file: /home/cvs/linux-2.3/drivers/s390/block/dasd_diag.c,v
retrieving revision 1.9
diff -u -r1.9 dasd_diag.c
--- drivers/s390/block/dasd_diag.c	2001/02/20 19:24:44	1.9
+++ drivers/s390/block/dasd_diag.c	2001/02/26 14:55:03
@@ -355,19 +360,26 @@
 static int
 dasd_diag_do_analysis (struct dasd_device_t *device)
 {
-	int sb;
 	dasd_diag_private_t *private = (dasd_diag_private_t *) device->private;
 
 	long *label = private->label;
 
 	/* real size of the volume */
 	device->sizes.blocks = label[7];
+        if (private->rdc_data.vdev_class == DEV_CLASS_FBA) {
+		device->sizes.pt_block = 1;
+	} else if (private->rdc_data.vdev_class == DEV_CLASS_ECKD ||
+		   private->rdc_data.vdev_class == DEV_CLASS_CKD) {
+		device->sizes.pt_block = 2;
+	} else {
+		return -EINVAL;
+        }
 	printk (KERN_INFO PRINTK_HEADER
 		"/dev/%s (%04X): capacity (%dkB blks): %ldkB\n",
 		device->name, device->devinfo.devno,
 		(device->sizes.bp_block >> 10),
 		(device->sizes.blocks << device->sizes.s2b_shift) >> 1);
-        free_page(private->label);
+        free_page((long)private->label);
 	return 0;
 }
 
@@ -394,14 +406,6 @@
 	geo->cylinders = cyls;
 	geo->heads = 16;
 	geo->sectors = 128 >> device->sizes.s2b_shift;
-	if (private->rdc_data.vdev_class == DEV_CLASS_FBA) {
-		geo->start = 1;
-	} else if (private->rdc_data.vdev_class == DEV_CLASS_ECKD ||
-		   private->rdc_data.vdev_class == DEV_CLASS_CKD) {
-		geo->start = 2;
-	} else {
-		return -EINVAL;
-	}
 	return rc;
 }
 
Index: drivers/s390/block/dasd_eckd.c
===================================================================
RCS file: /home/cvs/linux-2.3/drivers/s390/block/dasd_eckd.c,v
retrieving revision 1.42
diff -u -r1.42 dasd_eckd.c
--- drivers/s390/block/dasd_eckd.c	2001/02/20 19:24:44	1.42
+++ drivers/s390/block/dasd_eckd.c	2001/02/26 14:55:03
@@ -517,6 +517,7 @@
 		return -EMEDIUMTYPE;
 	}
 	device->sizes.s2b_shift = 0;	/* bits to shift 512 to get a block */
+        device->sizes.pt_block = 2;
 	for (sb = 512; sb < bs; sb = sb << 1)
 		device->sizes.s2b_shift++;
 
@@ -554,7 +555,6 @@
 	geo->cylinders = private->rdc_data.no_cyl;
 	geo->heads = private->rdc_data.trk_per_cyl;
 	geo->sectors = recs_per_track (&(private->rdc_data), 0, device->sizes.bp_block);
-	geo->start = 2;
 	return rc;
 }
 
Index: drivers/s390/block/dasd_fba.c
===================================================================
RCS file: /home/cvs/linux-2.3/drivers/s390/block/dasd_fba.c,v
retrieving revision 1.26
diff -u -r1.26 dasd_fba.c
--- drivers/s390/block/dasd_fba.c	2001/02/20 19:24:44	1.26
+++ drivers/s390/block/dasd_fba.c	2001/02/26 14:55:03
@@ -180,6 +180,7 @@
 		device->sizes.s2b_shift++;
 
 	device->sizes.blocks = (private->rdc_data.blk_bdsa);
+        device->sizes.pt_block = 1;
 
 	return rc;
 }
@@ -204,7 +205,6 @@
 	geo->cylinders = cyls;
 	geo->heads = 16;
 	geo->sectors = 128 >> device->sizes.s2b_shift;
-	geo->start = 1;
 	return rc;
 }
 
Index: fs/partitions/ibm.c
===================================================================
RCS file: /home/cvs/linux-2.3/fs/partitions/ibm.c,v
retrieving revision 1.13
diff -u -r1.13 ibm.c
--- fs/partitions/ibm.c	2001/01/31 17:54:42	1.13
+++ fs/partitions/ibm.c	2001/02/26 14:55:05
@@ -35,28 +35,28 @@
 typedef enum {
   ibm_partition_none = 0,
   ibm_partition_lnx1 = 1,
-  ibm_partition_vol1 = 3,
-  ibm_partition_cms1 = 4
+  ibm_partition_vol1 = 2,
+  ibm_partition_cms1 = 3
 } ibm_partition_t;
 
+static char* part_names[] = { [ibm_partition_none] = "(nonl)",
+			      [ibm_partition_lnx1] = "LNX1",
+			      [ibm_partition_vol1] = "VOL1",
+			      [ibm_partition_cms1] = "CMS1"};
+
 static ibm_partition_t
 get_partition_type ( char * type )
 {
-        static char lnx[5]="LNX1";
-        static char vol[5]="VOL1";
-        static char cms[5]="CMS1";
-        if ( ! strncmp ( lnx, "LNX1",4 ) ) {
-                ASCEBC(lnx,4);
-                ASCEBC(vol,4);
-                ASCEBC(cms,4);
-        }
-        if ( ! strncmp (type,lnx,4) ||
-             ! strncmp (type,"LNX1",4) )
-                return ibm_partition_lnx1;
-        if ( ! strncmp (type,vol,4) )
-                return ibm_partition_vol1;
-        if ( ! strncmp (type,cms,4) )
-                return ibm_partition_cms1;
+	int i;
+	char temp[5];
+	for ( i = 1; i < 4; i ++) {
+		strncpy(temp,part_names[i],5);
+		ASCEBC(temp,5);
+		/* both ASCII and EBCDIC are allowed */
+		if ( ! strncmp (type,temp,4) || 
+		     ! strncmp (type,part_names[i],4) ) 
+			return i;
+	}
         return ibm_partition_none;
 }
 
@@ -66,13 +66,9 @@
 {
 	struct buffer_head *bh;
 	ibm_partition_t partition_type;
-	char type[5] = {0,};
-	char name[7] = {0,};
-	struct hd_geometry geo;
-	mm_segment_t old_fs;
+	char type[5] = "notyp";
+	char name[7] = "nolbl";
 	int blocksize;
-	struct file *filp = NULL;
-	struct inode *inode = NULL;
 	int offset, size;
 
 	blocksize = hardsect_size[MAJOR(dev)][MINOR(dev)];
@@ -80,37 +76,8 @@
 		return 0;
 	}
 	set_blocksize(dev, blocksize);  /* OUCH !! */
-
-	/* find out offset of volume label (partn table) */
-	inode = get_empty_inode();
-	inode -> i_rdev = dev;
-#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,3,98))
-	inode -> i_bdev = bdget(kdev_t_to_nr(dev));
-#endif /* KERNEL_VERSION */
-	filp = (struct file *)kmalloc (sizeof(struct file),GFP_KERNEL);
-	if (!filp)
-		return 0;
-	memset(filp,0,sizeof(struct file));
-	filp ->f_mode = 1; /* read only */
-	blkdev_open(inode,filp);
-	old_fs=get_fs();
-	set_fs(KERNEL_DS);
-#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,3,98))
-	inode-> i_bdev -> bd_op->ioctl (inode, filp, HDIO_GETGEO, (unsigned long)(&geo));
-#else
-	filp->f_op->ioctl (inode, filp, HDIO_GETGEO, (unsigned long)(&geo));
-#endif /* KERNEL_VERSION */
-	set_fs(old_fs);
-#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0))
-        blkdev_put(inode->i_bdev,BDEV_FILE);
-#elif (LINUX_VERSION_CODE > KERNEL_VERSION(2,3,98))
-	blkdev_close(inode,filp);
-#else
-	blkdev_release(inode);
-#endif /* LINUX_VERSION_CODE */
 
-	size = hd -> sizes[MINOR(dev)]<<1;
-	if ( ( bh = bread( dev, geo.start, blocksize) ) != NULL ) {
+	if ( ( bh = bread( dev, first_sector, blocksize) ) != NULL ) {
 		strncpy ( type,bh -> b_data, 4);
 		strncpy ( name,bh -> b_data + 4, 6);
         } else {
@@ -120,42 +87,28 @@
 		EBCASC(name,6);
 	}
 	switch ( partition_type = get_partition_type(type) ) {
-	case ibm_partition_lnx1: 
-		offset = (geo.start + 1);
-		printk ( "(LNX1)/%6s:",name);
-		break;
-	case ibm_partition_vol1:
-		offset = 0;
-		size = 0;
-		printk ( "(VOL1)/%6s:",name);
-		break;
 	case ibm_partition_cms1:
-		printk ( "(CMS1)/%6s:",name);
-		if (* (((long *)bh->b_data) + 13) == 0) {
-			/* disk holds a CMS filesystem */
-			offset = (geo.start + 1);
-			printk ("(CMS)");
-		} else {
+		if (* (((long *)bh->b_data) + 13) != 0) {
 			/* disk is reserved minidisk */
-			// mdisk_setup_data.size[i] =
-			// (label[7] - 1 - label[13]) *
-			// (label[3] >> 9) >> 1;
 			long *label=(long*)bh->b_data;
 			blocksize = label[3];
 			offset = label[13];
 			size = (label[7]-1)*(blocksize>>9); 
 			printk ("(MDSK)");
-		}
-		break;
+			break;
+		} /* !!! else branch is fallthrough !!! */
+	case ibm_partition_lnx1: 
 	case ibm_partition_none:
-		printk ( "(nonl)/      :");
-		offset = (geo.start+1);
+		offset = (first_sector + 1);
+		size = hd -> sizes[MINOR(dev)]<<1;
 		break;
+	case ibm_partition_vol1: /* fall through !!! */
 	default:
 		offset = 0;
 		size = 0;
 		
 	}
+	printk ( "%6s/%6s:",part_names[partition_type],name);
 #if (LINUX_VERSION_CODE > KERNEL_VERSION(2,3,98))
 	add_gd_partition( hd, MINOR(dev), 0,size);
 	add_gd_partition( hd, MINOR(dev) + 1, offset * (blocksize >> 9),
Index: include/asm-s390/dasd.h
===================================================================
RCS file: /home/cvs/linux-2.3/include/asm-s390/dasd.h,v
retrieving revision 1.16
diff -u -r1.16 dasd.h
--- include/asm-s390/dasd.h	2001/02/20 19:24:44	1.16
+++ include/asm-s390/dasd.h	2001/02/26 14:55:06
@@ -195,6 +195,7 @@
 	unsigned long blocks; /* size of volume in blocks */
 	unsigned int bp_block; /* bytes per block */
 	unsigned int s2b_shift; /* log2 (bp_block/512) */
+        unsigned int pt_block; /* from which block to read the partn table */
 } dasd_sizes_t;
 
 /* 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] partitions/ibm.c
@ 2001-02-26 13:05 Andries.Brouwer
  0 siblings, 0 replies; 5+ messages in thread
From: Andries.Brouwer @ 2001-02-26 13:05 UTC (permalink / raw)
  To: Andries.Brouwer, Holger.Smolinski, dwguest, linux-kernel, torvalds

    From Holger.Smolinski@de.ibm.com Mon Feb 26 12:10:59 2001

    Andries, others,
    Thanks for hacking through the code of fs/partitions/ibm.c.
    Your patch does not work at all because you are relying on the
    data in the part component of the hd structure, which does not
    hold the geometry data of the disk but the data of the partitions
    on that disk.

Hmm. To me "geometry" means things with sectors, heads and cylinders -
something you do not need at all. You only need to know whether you
have to read sector 1 or 2 from this disk.

    Besides that, exactly these data are to be set up
    by the code in fs/partitions/ibm.c.

No.
ibm_partition() is called from check_partition(), which does
	first_sector = hd->part[MINOR(dev)].start_sect;
and then calls ibm_partition() with first_sector as third parameter.
Clearly, this assumes that hd->part[MINOR(dev)].start_sect
has a value already.

The "start" field of the struct returned by HDIO_GETGEO does not
tell us where the partition table lives.
It tells us where the partition starts. Maybe there is no table.
For an entire disk the answer will be zero.

Thus, I think the present setup of ibm_partition() is broken.
(If I have a disk with ibm partition, then it seems right now
it cannot be moved to some ide or scsi machine because the
information you want is returned only by the
	device->discipline->fill_geometry()
call in dasd.c, and not by the HDIO_GETGEO of any other driver.)

Andries


[And, of course, similarly, these fill_geometry() routines are broken.]


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] partitions/ibm.c
@ 2001-02-24  3:38 Guest section DW
  0 siblings, 0 replies; 5+ messages in thread
From: Guest section DW @ 2001-02-24  3:38 UTC (permalink / raw)
  To: Linux390, aeb, torvalds, linux-kernel

Reading patch-2.4.2 I met a strange amount of crap in
partitions/ibm.c. It is as if the author does not know
where the kernel keeps the starting offset of a partition,
and simulates a HDIO_GETGEO ioctl from user space.
I think the following patch does the same and removes a lot
of cruft. (Warning: (i) untested, uncompiled; (ii) pasted
from another window - tabs will have become spaces.)

Andries

-----

diff -u --recursive --new-file ../linux-2.4.2/linux/fs/partitions/ibm.c ./linux/
fs/partitions/ibm.c
--- ../linux-2.4.2/linux/fs/partitions/ibm.c    Sat Feb 24 03:44:02 2001
+++ ./linux/fs/partitions/ibm.c Sat Feb 24 04:04:33 2001
@@ -60,56 +60,22 @@
 }
 
 int 
-ibm_partition(struct gendisk *hd, kdev_t dev, unsigned long first_sector, int
-first_part_minor)
+ibm_partition(struct gendisk *hd, kdev_t dev, unsigned long first_sector,
+             int first_part_minor)
 {
        struct buffer_head *bh;
        ibm_partition_t partition_type;
        char type[5] = {0,};
        char name[7] = {0,};
-       struct hd_geometry geo;
-       mm_segment_t old_fs;
        int blocksize;
-       struct file *filp = NULL;
-       struct inode *inode = NULL;
-       int offset, size;
+       int start, offset, size;
 
-       blocksize = hardsect_size[MAJOR(dev)][MINOR(dev)];
-       if ( blocksize <= 0 ) {
-               return 0;
-       }
+       blocksize = get_hardsect_size(dev);
        set_blocksize(dev, blocksize);  /* OUCH !! */
 
-       /* find out offset of volume label (partn table) */
-       inode = get_empty_inode();
-       inode -> i_rdev = dev;
-#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,3,98))
-       inode -> i_bdev = bdget(kdev_t_to_nr(dev));
-#endif /* KERNEL_VERSION */
-       filp = (struct file *)kmalloc (sizeof(struct file),GFP_KERNEL);
-       if (!filp)
-               return 0;
-       memset(filp,0,sizeof(struct file));
-       filp ->f_mode = 1; /* read only */
-       blkdev_open(inode,filp);
-       old_fs=get_fs();
-       set_fs(KERNEL_DS);
-#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,3,98))
-       inode-> i_bdev -> bd_op->ioctl (inode, filp, HDIO_GETGEO, (unsigned long
)(&geo));
-#else
-       filp->f_op->ioctl (inode, filp, HDIO_GETGEO, (unsigned long)(&geo));
-#endif /* KERNEL_VERSION */
-       set_fs(old_fs);
-#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0))
-        blkdev_put(inode->i_bdev,BDEV_FILE);
-#elif (LINUX_VERSION_CODE > KERNEL_VERSION(2,3,98))
-       blkdev_close(inode,filp);
-#else
-       blkdev_release(inode);
-#endif /* LINUX_VERSION_CODE */
-
-       size = hd -> sizes[MINOR(dev)]<<1;
-       if ( ( bh = bread( dev, geo.start, blocksize) ) != NULL ) {
+       start = hd->part[MINOR(dev)].start_sect;
+       size = hd->sizes[MINOR(dev)] << 1;
+       if ( ( bh = bread( dev, start, blocksize) ) != NULL ) {
                strncpy ( type,bh -> b_data, 4);
                strncpy ( name,bh -> b_data + 4, 6);
         } else {
@@ -120,7 +86,7 @@
        }
        switch ( partition_type = get_partition_type(type) ) {
        case ibm_partition_lnx1: 
-               offset = (geo.start + 1);
+               offset = start + 1;
                printk ( "(LNX1)/%6s:",name);
                break;
        case ibm_partition_vol1:
@@ -132,7 +98,7 @@
                printk ( "(CMS1)/%6s:",name);
                if (* (((long *)bh->b_data) + 13) == 0) {
                        /* disk holds a CMS filesystem */
-                       offset = (geo.start + 1);
+                       offset = start + 1;
                        printk ("(CMS)");
                } else {
                        /* disk is reserved minidisk */
@@ -148,22 +114,18 @@
                break;
        case ibm_partition_none:
                printk ( "(nonl)/      :");
-               offset = (geo.start+1);
+               offset = start+1;
                break;
        default:
                offset = 0;
                size = 0;
 
        }
-#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,3,98))
+
        add_gd_partition( hd, MINOR(dev), 0,size);
        add_gd_partition( hd, MINOR(dev) + 1, offset * (blocksize >> 9),
                          size-offset*(blocksize>>9));
-#else
-       add_partition( hd, MINOR(dev), 0,size,0);
-       add_partition( hd, MINOR(dev) + 1, offset * (blocksize >> 9),
-                         size-offset*(blocksize>>9) ,0 );
-#endif /* LINUX_VERSION */
+
        printk ( "\n" );
        bforget(bh);
        return 1;

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2001-02-27  0:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-02-26 10:15 [PATCH] partitions/ibm.c Holger.Smolinski
  -- strict thread matches above, loose matches on Subject: below --
2001-02-27  0:51 Andries.Brouwer
2001-02-26 15:11 Holger.Smolinski
2001-02-26 13:05 Andries.Brouwer
2001-02-24  3:38 Guest section DW

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).