From owner-freebsd-alpha Sat Mar 24 14:58:14 2001 Delivered-To: freebsd-alpha@freebsd.org Received: from VL-MS-MR003.sc1.videotron.ca (relais.videotron.ca [24.201.245.36]) by hub.freebsd.org (Postfix) with ESMTP id B892B37B719; Sat, 24 Mar 2001 14:58:01 -0800 (PST) (envelope-from nospam@videotron.ca) Received: from neutrino.quantum.net ([24.201.229.37]) by VL-MS-MR003.sc1.videotron.ca (Netscape Messaging Server 4.15) with ESMTP id GAQ54O02.05N; Sat, 24 Mar 2001 17:58:00 -0500 Received: from videotron.ca (client32.quantum.net [192.168.56.32]) by neutrino.quantum.net (8.12.0.Beta5/8.12.0.Beta5) with ESMTP id f2OMvvmD028952; Sat, 24 Mar 2001 17:57:58 -0500 (EST) Message-ID: <3ABD2676.2070909@videotron.ca> Date: Sat, 24 Mar 2001 17:57:58 -0500 From: tcn Reply-To: leclercn@videotron.ca User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; m18) Gecko/20010131 Netscape6/6.01 X-Accept-Language: en MIME-Version: 1.0 To: John Baldwin Cc: leclercn@videotron.ca, grog@FreeBSD.org, freebsd-alpha@FreeBSD.org, Mike Smith Subject: Re: dev_t size mismatch kernel / userland References: Content-Type: multipart/mixed; boundary="------------090506040908000906010705" Sender: owner-freebsd-alpha@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org This is a multi-part message in MIME format. --------------090506040908000906010705 Content-Type: multipart/alternative; boundary="------------040508000509070700000303" --------------040508000509070700000303 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit This would be a patch for the -stable 4.3 BETA branch. I am currently testing it... So far, vinum is up, initialization is done on my mirror drive... Normand Leclerc lecnlercn@videotron.ca John Baldwin wrote: > 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. > --------------040508000509070700000303 Content-Type: text/html; charset=us-ascii Content-Transfer-Encoding: 7bit This would be a patch for the -stable 4.3 BETA branch. I am currently testing it... So far, vinum is up, initialization is done on my mirror drive...

Normand Leclerc
lecnlercn@videotron.ca

John Baldwin wrote:

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.

--------------040508000509070700000303-- --------------090506040908000906010705 Content-Type: text/plain; name="vinum.udev.patch2" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="vinum.udev.patch2" Only in .: org Only in .: vinum.udev.patch2 diff -u ./vinumio.c org/vinumio.c --- ./vinumio.c Sat Mar 24 17:35:21 2001 +++ org/vinumio.c Sat Mar 24 17:22:00 2001 @@ -71,7 +71,7 @@ * moving the system to devfs. */ dname = &drive->devicename[5]; - drive->dev = ENODEV; /* no device yet */ + drive->dev = NULL; /* no device yet */ /* Find the device */ if (bcmp(dname, "ad", 2) == 0) /* IDE disk */ @@ -128,16 +128,16 @@ if ((devminor & 7) == 2) /* partition c */ return ENOTTY; /* not buying that */ - drive->dev = dev2udev(makedev(devmajor, devminor)); /* find the device */ - if (drive->dev == ENODEV) /* didn't find anything */ + drive->dev = makedev(devmajor, devminor); /* find the device */ + if (drive->dev == NULL) /* didn't find anything */ return ENODEV; - udev2dev(drive->dev,0)->si_iosize_max = DFLTPHYS; - dsw = devsw(udev2dev(drive->dev,0)); + drive->dev->si_iosize_max = DFLTPHYS; + dsw = devsw(drive->dev); if (dsw == NULL) drive->lasterror = ENOENT; else - drive->lasterror = (dsw->d_open) (udev2dev(drive->dev,0), FWRITE, 0, NULL); + drive->lasterror = (dsw->d_open) (drive->dev, FWRITE, 0, NULL); if (drive->lasterror != 0) { /* failed */ drive->state = drive_down; /* just force it down */ @@ -215,7 +215,7 @@ if (drive->lasterror) return drive->lasterror; - drive->lasterror = (*devsw(udev2dev(drive->dev,0))->d_ioctl) (udev2dev(drive->dev,0), + drive->lasterror = (*devsw(drive->dev)->d_ioctl) (drive->dev, DIOCGPART, (caddr_t) & drive->partinfo, FREAD, @@ -265,7 +265,7 @@ * the queues, which spec_close() will try to * do. Get rid of them here first. */ - drive->lasterror = (*devsw(udev2dev(drive->dev,0))->d_close) (udev2dev(drive->dev,0), 0, 0, NULL); + drive->lasterror = (*devsw(drive->dev)->d_close) (drive->dev, 0, 0, NULL); drive->flags &= ~VF_OPEN; /* no longer open */ } @@ -320,7 +320,7 @@ bp = geteblk(len); /* get a buffer header */ bp->b_flags = flag; - bp->b_dev = udev2dev(drive->dev,0); /* device */ + bp->b_dev = drive->dev; /* device */ bp->b_blkno = offset / drive->partinfo.disklab->d_secsize; /* block number */ bp->b_saveaddr = bp->b_data; bp->b_data = buf; @@ -655,7 +655,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(udev2dev(drive->dev,0))->d_ioctl) (udev2dev(drive->dev,0), /* make the label writeable */ + error = (*devsw(drive->dev)->d_ioctl) (drive->dev, /* make the label writeable */ DIOCWLABEL, (caddr_t) & wlabel_on, FWRITE, @@ -668,7 +668,7 @@ error = write_drive(drive, config, MAXCONFIG, VINUM_CONFIG_OFFSET + MAXCONFIG); /* second copy */ wlabel_on = 0; /* enable writing the label */ if (error == 0) - error = (*devsw(udev2dev(drive->dev,0))->d_ioctl) (udev2dev(drive->dev,0), /* make the label non-writeable again */ + error = (*devsw(drive->dev)->d_ioctl) (drive->dev, /* make the label non-writeable again */ DIOCWLABEL, (caddr_t) & wlabel_on, FWRITE, diff -u ./vinumrequest.c org/vinumrequest.c --- ./vinumrequest.c Sat Mar 24 17:39:09 2001 +++ org/vinumrequest.c Sat Mar 24 17:21:59 2001 @@ -816,7 +816,7 @@ * when the drive is dead. */ if ((rqe->flags & XFR_BAD_SUBDISK) == 0) /* subdisk is accessible, */ - bp->b_dev = udev2dev(DRIVE[rqe->driveno].dev,0); /* drive device */ + bp->b_dev = DRIVE[rqe->driveno].dev; /* drive device */ bp->b_blkno = rqe->sdoffset + sd->driveoffset; /* start address */ bp->b_bcount = rqe->buflen << DEV_BSHIFT; /* number of bytes to transfer */ bp->b_resid = bp->b_bcount; /* and it's still all waiting */ @@ -931,7 +931,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 = udev2dev(DRIVE[sd->driveno].dev,0); /* device */ + sbp->b.b_dev = DRIVE[sd->driveno].dev; /* 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 */ diff -u ./vinumvar.h org/vinumvar.h --- ./vinumvar.h Sat Mar 24 17:22:09 2001 +++ org/vinumvar.h Sat Mar 24 17:22:00 2001 @@ -406,7 +406,7 @@ u_int64_t sectors; /* and length in sectors */ } *freelist; struct partinfo partinfo; /* partition information */ - udev_t dev; /* device information */ + dev_t dev; /* device information */ #ifdef VINUMDEBUG char lockfilename[16]; /* name of file from which we were locked */ int lockline; /* and the line number */ --------------090506040908000906010705-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-alpha" in the body of the message