Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 23 Mar 2001 21:49:17 -0800 (PST)
From:      John Baldwin <jhb@FreeBSD.org>
To:        John Baldwin <jhb@FreeBSD.org>
Cc:        leclercn@videotron.ca, grog@FreeBSD.org, freebsd-alpha@FreeBSD.org, Mike Smith <msmith@FreeBSD.org>
Subject:   Re: dev_t size mismatch kernel / userland
Message-ID:  <XFMail.010323214917.jhb@FreeBSD.org>
In-Reply-To: <XFMail.010323214512.jhb@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format
--_=XFMail.1.4.0.FreeBSD:010323214914:1177=_
Content-Type: text/plain; charset=us-ascii


On 24-Mar-01 John Baldwin wrote:
> 
> On 24-Mar-01 Mike Smith wrote:
>>>   Already tried vinum, works fine after modification of dev_t to 
>>> u_int64_t.  I'll change it to uintptr_t tough (cleaner).  Hope this gets 
>>> fixed!
>> 
>> This is wrong.  In the kernel or in a module, dev_t is an opaque type.  
>> In userspace, you use udev_t, not dev_t.
>> 
>> It sounds like vinum is failing to perform the required conversions when 
>> exchanging a dev_t with userland, and the correct fix is going to be to 
>> add these.
> 
> struct drive, struct plex, etc. are shared between userland and the kernel. 
> It
> looks like they need to use a udev_t, and the kernel will always have to do
> udev2dev() before using them.  Either that or don't export the structures to
> userland.

Here is an untested patch to change all the data structures vinum exports to
userland to use udev_t's instead of dev_t's.  It's not necessarily optimal, as
udev2dev() can be somewhat expensive I think, but fixing that would mean not
exporting private kernel structs to userland like is done now.  This patch can
be had from http://www.FreeBSD.org/~jhb/patches/vinum.udev.patch as well.

-- 

John Baldwin <jhb@FreeBSD.org> -- http://www.FreeBSD.org/~jhb/
PGP Key: http://www.baldwin.cx/~john/pgpkey.asc
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

--_=XFMail.1.4.0.FreeBSD:010323214914:1177=_
Content-Disposition: attachment; filename="vinum.udev.patch"
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
 charset=us-ascii; name=vinum.udev.patch; SizeOnDisk=10206

Index: vinumconfig.c
===================================================================
RCS file: /usr/cvs/src/sys/dev/vinum/vinumconfig.c,v
retrieving revision 1.40
diff -u -r1.40 vinumconfig.c
--- vinumconfig.c	2001/02/20 12:14:01	1.40
+++ vinumconfig.c	2001/03/24 05:37:49
@@ -734,7 +734,7 @@
 	    sd->sectors);
     if (sd->plexno >= 0)
 	PLEX[sd->plexno].subdisks--;			    /* one less subdisk */
-    destroy_dev(sd->dev);
+    destroy_dev(udev2dev(sd->dev, 0));
     bzero(sd, sizeof(struct sd));			    /* and clear it out */
     sd->state = sd_unallocated;
     vinum_conf.subdisks_used--;				    /* one less sd */
@@ -812,7 +812,7 @@
 	Free(plex->sdnos);
     if (plex->lock)
 	Free(plex->lock);
-    destroy_dev(plex->dev);
+    destroy_dev(udev2dev(plex->dev, 0));
     bzero(plex, sizeof(struct plex));			    /* and clear it out */
     plex->state = plex_unallocated;
 }
@@ -883,7 +883,7 @@
     struct volume *vol;
 
     vol = &VOL[volno];
-    destroy_dev(vol->dev);
+    destroy_dev(udev2dev(vol->dev, 0));
     bzero(vol, sizeof(struct volume));			    /* and clear it out */
     vol->state = volume_unallocated;
 }
@@ -1223,8 +1223,8 @@
     if (sd->sectors < 0)
 	throw_rude_remark(EINVAL, "sd %s has no length spec", sd->name);
 
-    sd->dev = make_dev(&vinum_cdevsw, VINUMRMINOR(VINUM_SD_TYPE, sdno),
-	UID_ROOT, GID_WHEEL, S_IRUSR|S_IWUSR, "vinum/sd/%s", sd->name);
+    sd->dev = dev2udev(make_dev(&vinum_cdevsw, VINUMRMINOR(VINUM_SD_TYPE, sdno)
,
+	UID_ROOT, GID_WHEEL, S_IRUSR|S_IWUSR, "vinum/sd/%s", sd->name));
     if (state != sd_unallocated)			    /* we had a specific state to set */
 	sd->state = state;				    /* do it now */
     else if (sd->state == sd_unallocated)		    /* no, nothing set yet, */
@@ -1382,8 +1382,8 @@
     if (plex->organization == plex_disorg)
 	throw_rude_remark(EINVAL, "No plex organization specified");
 
-    plex->dev = make_dev(&vinum_cdevsw, VINUMRMINOR(VINUM_PLEX_TYPE, plexno),
-	UID_ROOT, GID_WHEEL, S_IRUSR|S_IWUSR, "vinum/plex/%s", plex->name);
+    plex->dev = dev2udev(make_dev(&vinum_cdevsw, VINUMRMINOR(VINUM_PLEX_TYPE, p
lexno),
+	UID_ROOT, GID_WHEEL, S_IRUSR|S_IWUSR, "vinum/plex/%s", plex->name));
 
     if ((plex->volno < 0)				    /* we don't have a volume */
     &&(!detached))					    /* and we wouldn't object */
@@ -1544,8 +1544,8 @@
 	vol->size = max(vol->size, PLEX[vol->plex[i]].length);
 
     vinum_conf.volumes_used++;				    /* one more in use */
-    vol->dev = make_dev(&vinum_cdevsw, VINUMRMINOR(VINUM_VOLUME_TYPE, volno),
-	UID_ROOT, GID_WHEEL, S_IRUSR|S_IWUSR, "vinum/vol/%s", vol->name);
+    vol->dev = dev2udev(make_dev(&vinum_cdevsw, VINUMRMINOR(VINUM_VOLUME_TYPE, 
volno),
+	UID_ROOT, GID_WHEEL, S_IRUSR|S_IWUSR, "vinum/vol/%s", vol->name));
 }
 
 /*
Index: vinumio.c
===================================================================
RCS file: /usr/cvs/src/sys/dev/vinum/vinumio.c,v
retrieving revision 1.65
diff -u -r1.65 vinumio.c
--- vinumio.c	2001/01/20 03:46:19	1.65
+++ vinumio.c	2001/03/24 05:44:00
@@ -56,6 +56,7 @@
     int unit;
     char *dname;
     struct cdevsw *dsw;					    /* pointer to cdevsw entry */
+    dev_t dev;
 
     if (bcmp(drive->devicename, "/dev/", 5))		    /* device name doesn't start 
with /dev */
 	return ENOENT;					    /* give up */
@@ -71,7 +72,7 @@
      * moving the system to devfs.
      */
     dname = &drive->devicename[5];
-    drive->dev = NULL;					    /* no device yet */
+    drive->dev = ENOUDEV;				    /* no device yet */
 
     /* Find the device */
     if (bcmp(dname, "ad", 2) == 0)			    /* IDE disk */
@@ -128,16 +129,17 @@
     if ((devminor & 7) == 2)				    /* partition c */
 	return ENOTTY;					    /* not buying that */
 
-    drive->dev = makedev(devmajor, devminor);		    /* find the device */
-    if (drive->dev == NULL)				    /* didn't find anything */
+    dev = makedev(devmajor, devminor);			    /* find the device */
+    if (drive->dev == NOUDEV)				    /* didn't find anything */
 	return ENODEV;
 
-    drive->dev->si_iosize_max = DFLTPHYS;
-    dsw = devsw(drive->dev);
+    drive->dev = dev2udev(dev);
+    dev->si_iosize_max = DFLTPHYS;
+    dsw = devsw(dev);
     if (dsw == NULL)
 	drive->lasterror = ENOENT;
     else
-	drive->lasterror = (dsw->d_open) (drive->dev, FWRITE, 0, NULL);
+	drive->lasterror = (dsw->d_open) (dev, FWRITE, 0, NULL);
 
     if (drive->lasterror != 0) {			    /* failed */
 	drive->state = drive_down;			    /* just force it down */
@@ -206,6 +208,8 @@
 int
 init_drive(struct drive *drive, int verbose)
 {
+    dev_t dev;
+
     if (drive->devicename[0] != '/') {
 	drive->lasterror = EINVAL;
 	log(LOG_ERR, "vinum: Can't open drive without drive name\n");
@@ -215,7 +219,8 @@
     if (drive->lasterror)
 	return drive->lasterror;
 
-    drive->lasterror = (*devsw(drive->dev)->d_ioctl) (drive->dev,
+    dev = udev2dev(drive->dev, 0);
+    drive->lasterror = (*devsw(dev)->d_ioctl) (dev,
 	DIOCGPART,
 	(caddr_t) & drive->partinfo,
 	FREAD,
@@ -260,12 +265,15 @@
 void
 close_locked_drive(struct drive *drive)
 {
+    dev_t dev;
+
     /*
      * If we can't access the drive, we can't flush
      * the queues, which spec_close() will try to
      * do.  Get rid of them here first.
      */
-    drive->lasterror = (*devsw(drive->dev)->d_close) (drive->dev, 0, 0, NULL);
+    dev = udev2dev(drive->dev, 0);
+    drive->lasterror = (*devsw(dev)->d_close) (dev, 0, 0, NULL);
     drive->flags &= ~VF_OPEN;				    /* no longer open */
 }
 
@@ -321,7 +329,7 @@
 	bp = geteblk(len);				    /* get a buffer header */
 	bp->b_flags = 0;
 	bp->b_iocmd = flag;
-	bp->b_dev = drive->dev;				    /* device */
+	bp->b_dev = udev2dev(drive->dev, 0);		    /* device */
 	bp->b_blkno = offset / drive->partinfo.disklab->d_secsize; /* block number */
 	bp->b_saveaddr = bp->b_data;
 	bp->b_data = buf;
@@ -606,6 +614,7 @@
     struct vinum_hdr *vhdr;				    /* and as header */
     char *config;					    /* point to config data */
     int wlabel_on;					    /* to set writing label on/off */
+    dev_t dev;
 
     /* don't save the configuration while we're still working on it */
     if (vinum_conf.flags & VF_CONFIGURING)
@@ -627,6 +636,7 @@
 	if (drive->state > drive_referenced) {
 	    LOCKDRIVE(drive);				    /* don't let it change */
 
+	    dev = udev2dev(drive->dev, 0);
 	    /*
 	     * First, do some drive consistency checks.  Some
 	     * of these are kludges, others require a process
@@ -656,7 +666,7 @@
 		if ((drive->state != drive_unallocated)
 		    && (drive->state != drive_referenced)) { /* and it's a real drive */
 		    wlabel_on = 1;			    /* enable writing the label */
-		    error = (*devsw(drive->dev)->d_ioctl) (drive->dev, /* make the label writ
eable */
+		    error = (*devsw(dev)->d_ioctl) (dev,    /* make the label writeable */
 			DIOCWLABEL,
 			(caddr_t) & wlabel_on,
 			FWRITE,
@@ -669,7 +679,7 @@
 			error = write_drive(drive, config, MAXCONFIG, VINUM_CONFIG_OFFSET + MAXCONFI
G);	/* second copy */
 		    wlabel_on = 0;			    /* enable writing the label */
 		    if (error == 0)
-			error = (*devsw(drive->dev)->d_ioctl) (drive->dev, /* make the label non-wri
teable again */
+			error = (*devsw(dev)->d_ioctl) (dev, /* make the label non-writeable again *
/
 			    DIOCWLABEL,
 			    (caddr_t) & wlabel_on,
 			    FWRITE,
Index: vinumrequest.c
===================================================================
RCS file: /usr/cvs/src/sys/dev/vinum/vinumrequest.c,v
retrieving revision 1.54
diff -u -r1.54 vinumrequest.c
--- vinumrequest.c	2001/01/10 05:07:52	1.54
+++ vinumrequest.c	2001/03/24 05:34:23
@@ -818,7 +818,7 @@
      * when the drive is dead.
      */
     if ((rqe->flags & XFR_BAD_SUBDISK) == 0)		    /* subdisk is accessible, */
-	bp->b_dev = DRIVE[rqe->driveno].dev;		    /* drive device */
+	bp->b_dev = udev2dev(DRIVE[rqe->driveno].dev, 0);   /* drive device */
     bp->b_blkno = rqe->sdoffset + sd->driveoffset;	    /* start address */
     bp->b_bcount = rqe->buflen << DEV_BSHIFT;		    /* number of bytes to transf
er */
     bp->b_resid = bp->b_bcount;				    /* and it's still all waiting */
@@ -934,7 +934,7 @@
     sbp->b.b_bufsize = bp->b_bufsize;			    /* buffer size */
     sbp->b.b_bcount = bp->b_bcount;			    /* number of bytes to transfer */
     sbp->b.b_resid = bp->b_resid;			    /* and amount waiting */
-    sbp->b.b_dev = DRIVE[sd->driveno].dev;		    /* device */
+    sbp->b.b_dev = udev2dev(DRIVE[sd->driveno].dev, 0);	    /* device */
     sbp->b.b_data = bp->b_data;				    /* data buffer */
     sbp->b.b_blkno = bp->b_blkno + sd->driveoffset;
     sbp->b.b_iodone = sdio_done;			    /* come here on completion */
Index: vinumvar.h
===================================================================
RCS file: /usr/cvs/src/sys/dev/vinum/vinumvar.h,v
retrieving revision 1.38
diff -u -r1.38 vinumvar.h
--- vinumvar.h	2001/02/20 22:07:36	1.38
+++ vinumvar.h	2001/03/24 05:32:31
@@ -396,7 +396,7 @@
     u_int64_t bytes_read;				    /* number of bytes read */
     u_int64_t bytes_written;				    /* number of bytes written */
     char devicename[MAXDRIVENAME];			    /* name of the slice it's on */
-    dev_t dev;						    /* device information */
+    udev_t dev;						    /* device information */
     struct vinum_label label;				    /* and the label information */
     struct partinfo partinfo;				    /* partition information */
     int freelist_size;					    /* number of entries alloced in free list */
@@ -451,7 +451,7 @@
     int init_blocksize;					    /* init block size (bytes) */
     int init_interval;					    /* and time to wait between transfers */
     char name[MAXSDNAME];				    /* name of subdisk */
-    dev_t dev;
+    udev_t dev;
 };
 
 /*** Plex definitions ***/
@@ -499,7 +499,7 @@
     u_int64_t multistripe;				    /* requests that needed more than one stripe 
*/
     int sddowncount;					    /* number of subdisks down */
     char name[MAXPLEXNAME];				    /* name of plex */
-    dev_t dev;
+    udev_t dev;
 };
 
 /*** Volume definitions ***/
@@ -539,7 +539,7 @@
     int plex[MAXPLEX];					    /* index of plexes */
     char name[MAXVOLNAME];				    /* name of volume */
     struct disklabel label;				    /* for DIOCGPART */
-    dev_t dev;
+    udev_t dev;
 };
 
 /*

--_=XFMail.1.4.0.FreeBSD:010323214914:1177=_--
End of MIME message

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-alpha" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.010323214917.jhb>