fix ioctl problems after the recent physio changes

in some drivers including wd and scsi.

- physio: if a caller provided a buf, stick to use it
  because some drivers use it as an identifier.
- sprinkle simple_locks.
- scsistrategy: rather than issueing an async request and
  waiting for its completion, simply issue a sync request.
  the way to wait for the completion had an assumption that
  B_CALL is never used.  it isn't the case after the recent
  physio() changes.

pointed/analyzed/tested by Martin Husemann.
This commit is contained in:
yamt 2005-10-31 14:36:41 +00:00
parent f243f4debb
commit 8217506e75
2 changed files with 48 additions and 35 deletions

View File

@ -1,4 +1,4 @@
/* $NetBSD: scsipi_ioctl.c,v 1.52 2005/02/01 00:19:34 reinoud Exp $ */
/* $NetBSD: scsipi_ioctl.c,v 1.53 2005/10/31 14:36:41 yamt Exp $ */
/*-
* Copyright (c) 1998, 2004 The NetBSD Foundation, Inc.
@ -44,7 +44,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: scsipi_ioctl.c,v 1.52 2005/02/01 00:19:34 reinoud Exp $");
__KERNEL_RCSID(0, "$NetBSD: scsipi_ioctl.c,v 1.53 2005/10/31 14:36:41 yamt Exp $");
#include "opt_compat_freebsd.h"
#include "opt_compat_netbsd.h"
@ -202,7 +202,6 @@ scsipi_user_done(struct scsipi_xfer *xs)
screq->retsts = SCCMD_UNKNOWN;
break;
}
biodone(bp); /* we're waiting on it in scsi_strategy() */
if (xs->xs_control & XS_CTL_ASYNC) {
s = splbio();
@ -235,7 +234,6 @@ scsistrategy(struct buf *bp)
struct scsipi_periph *periph;
int error;
int flags = 0;
int s;
si = si_find(bp);
if (si == NULL) {
@ -281,25 +279,15 @@ scsistrategy(struct buf *bp)
error = scsipi_command(periph, (void *)screq->cmd, screq->cmdlen,
(void *)bp->b_data, screq->datalen,
0, /* user must do the retries *//* ignored */
screq->timeout, bp, flags | XS_CTL_USERCMD | XS_CTL_ASYNC);
/* because there is a bp, scsi_scsipi_cmd will return immediatly */
if (error)
goto bad;
SC_DEBUG(periph, SCSIPI_DB3, ("about to sleep\n"));
s = splbio();
while ((bp->b_flags & B_DONE) == 0)
tsleep(bp, PRIBIO, "scistr", 0);
splx(s);
SC_DEBUG(periph, SCSIPI_DB3, ("back from sleep\n"));
return;
screq->timeout, bp, flags | XS_CTL_USERCMD);
bad:
bp->b_flags |= B_ERROR;
bp->b_error = error;
if (error) {
bp->b_flags |= B_ERROR;
bp->b_error = error;
}
biodone(bp);
return;
}
/*

View File

@ -1,4 +1,4 @@
/* $NetBSD: kern_physio.c,v 1.64 2005/10/30 09:17:02 yamt Exp $ */
/* $NetBSD: kern_physio.c,v 1.65 2005/10/31 14:36:41 yamt Exp $ */
/*-
* Copyright (c) 1982, 1986, 1990, 1993
@ -71,7 +71,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: kern_physio.c,v 1.64 2005/10/30 09:17:02 yamt Exp $");
__KERNEL_RCSID(0, "$NetBSD: kern_physio.c,v 1.65 2005/10/31 14:36:41 yamt Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@ -98,6 +98,11 @@ struct workqueue *physio_workqueue;
* buffer descriptors.
*/
/* abuse these members/flags of struct buf */
#define b_running b_freelistindex
#define b_eomoffset b_lblkno
#define B_DONTFREE B_AGE
/*
* allocate a buffer structure for use in physical I/O.
*/
@ -112,6 +117,7 @@ getphysbuf(void)
splx(s);
BUF_INIT(bp);
bp->b_error = 0;
bp->b_flags = B_BUSY;
return(bp);
}
@ -123,6 +129,10 @@ putphysbuf(struct buf *bp)
{
int s;
if ((bp->b_flags & B_DONTFREE) != 0) {
return;
}
if (__predict_false(bp->b_flags & B_WANTED))
panic("putphysbuf: private buf B_WANTED");
s = splbio();
@ -130,10 +140,6 @@ putphysbuf(struct buf *bp)
splx(s);
}
/* abuse these members of struct buf */
#define b_running b_freelistindex
#define b_eomoffset b_lblkno
static void
physio_done(struct work *wk, void *dummy)
{
@ -257,6 +263,7 @@ physio(void (*strategy)(struct buf *), struct buf *obp, dev_t dev, int flags,
size_t todo;
struct buf *bp = NULL;
struct buf *mbp;
int concurrency = PHYSIO_CONCURRENCY - 1;
RUN_ONCE(&physio_initialized, physio_init);
@ -266,26 +273,29 @@ physio(void (*strategy)(struct buf *), struct buf *obp, dev_t dev, int flags,
if (obp != NULL) {
/* [raise the processor priority level to splbio;] */
s = splbio();
simple_lock(&obp->b_interlock);
/* [while the buffer is marked busy] */
while (obp->b_flags & B_BUSY) {
/* [mark the buffer wanted] */
obp->b_flags |= B_WANTED;
/* [wait until the buffer is available] */
tsleep(obp, PRIBIO+1, "physbuf", 0);
ltsleep(obp, PRIBIO+1, "physbuf", 0, &bp->b_interlock);
}
/* Mark it busy, so nobody else will use it. */
obp->b_flags |= B_BUSY;
obp->b_flags = B_BUSY | B_DONTFREE;
/* [lower the priority level] */
simple_unlock(&obp->b_interlock);
splx(s);
concurrency = 0; /* see "XXXkludge" comment below */
}
mbp = getphysbuf();
mbp->b_resid = uio->uio_resid;
mbp->b_running = 0;
mbp->b_flags = 0;
PHOLD(l);
@ -297,13 +307,20 @@ physio(void (*strategy)(struct buf *), struct buf *obp, dev_t dev, int flags,
error = mbp->b_error;
goto done_locked;
}
error = physio_wait(mbp, PHYSIO_CONCURRENCY - 1,
"physio1");
error = physio_wait(mbp, concurrency, "physio1");
if (error) {
goto done_locked;
}
simple_unlock(&mbp->b_interlock);
bp = getphysbuf();
if (obp != NULL) {
/*
* XXXkludge
* some drivers use "obp" as an identifier.
*/
bp = obp;
} else {
bp = getphysbuf();
}
bp->b_dev = dev;
bp->b_proc = p;
bp->b_private = mbp;
@ -316,7 +333,8 @@ physio(void (*strategy)(struct buf *), struct buf *obp, dev_t dev, int flags,
* "Set by physio for raw transfers.", in addition
* to the "busy" and read/write flag.)
*/
bp->b_flags = B_BUSY | B_PHYS | B_RAW | B_CALL | flags;
bp->b_flags = (bp->b_flags & B_DONTFREE) |
B_BUSY | B_PHYS | B_RAW | B_CALL | flags;
bp->b_iodone = physio_biodone;
/* [set up the buffer for a maximum-sized transfer] */
@ -403,15 +421,22 @@ done_locked:
* Also, if we had to steal it, give it back.
*/
if (obp != NULL) {
s = splbio();
KASSERT((obp->b_flags & B_BUSY) != 0);
KASSERT((obp->b_flags & B_DONTFREE) != 0);
/*
* [if another process is waiting for the raw I/O buffer,
* wake up processes waiting to do physical I/O;
*/
if (obp->b_flags & B_WANTED) {
s = splbio();
simple_lock(&obp->b_interlock);
obp->b_flags &=
~(B_BUSY | B_PHYS | B_RAW | B_CALL | B_DONTFREE);
if ((obp->b_flags & B_WANTED) != 0) {
obp->b_flags &= ~B_WANTED;
wakeup(obp);
}
simple_unlock(&obp->b_interlock);
splx(s);
}
PRELE(l);