Date: Sat, 24 Mar 2001 17:57:58 -0500 From: tcn <nospam@videotron.ca> 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: <3ABD2676.2070909@videotron.ca> References: <XFMail.010323214917.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <html><head></head><body> 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...<br> <br> Normand Leclerc<br> <a class="moz-txt-link-abbreviated" href="mailto:lecnlercn@videotron.ca">lecnlercn@videotron.ca</a><br> <br> <br> John Baldwin wrote:<br> <blockquote type="cite" cite="mid:XFMail.010323214917.jhb@FreeBSD.org"><pre wrap="">On 24-Mar-01 John Baldwin wrote:<br></pre> <blockquote type="cite"><pre wrap="">On 24-Mar-01 Mike Smith wrote:<br></pre><blockquote type="cite"><blockquote type="cite"><pre wrap=""> Already tried vinum, works fine after modification of dev_t to <br>u_int64_t. I'll change it to uintptr_t tough (cleaner). Hope this gets <br>fixed!<br></pre></blockquote><pre wrap="">This is wrong. In the kernel or in a module, dev_t is an opaque type. <br>In userspace, you use udev_t, not dev_t.<br><br>It sounds like vinum is failing to perform the required conversions when <br>exchanging a dev_t with userland, and the correct fix is going to be to <br>add these.<br></pre></blockquote><pre wrap="">struct drive, struct plex, etc. are shared between userland and the kernel. <br>It<br>looks like they need to use a udev_t, and the kernel will always have to do<br>udev2dev() before using them. Either that or don't export the structures to<br>userland.<br></pre></blockquote> <pre wrap=""><!----><br>Here is an untested patch to change all the data structures vinum exports to<br>userland to use udev_t's instead of dev_t's. It's not necessarily optimal, as<br>udev2dev() can be somewhat expensive I think, but fixing that would mean not<br>exporting private kernel structs to userland like is done now. This patch can<br>be had from <a class="moz-txt-link-freetext" href="http://www.FreeBSD.org/~jhb/patches/vinum.udev.patch">http://www.FreeBSD.org/~jhb/patches/vinum.udev.patch</a>; as well.<br><br></pre> </blockquote> <br> </body></html> --------------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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3ABD2676.2070909>