Date: Thu, 21 Dec 2000 08:01:28 -0700 From: "Justin T. Gibbs" <gibbs@scsiguy.com> To: Nat Lanza <magus@cs.cmu.edu> Cc: freebsd-scsi@FreeBSD.ORG Subject: Re: Asynchronous commands for the passthrough driver Message-ID: <200012211501.eBLF1SF42268@aslan.scsiguy.com> In-Reply-To: Your message of "20 Dec 2000 02:02:55 EST." <uoc4rzzmv6o.fsf@hurlame.pdl.cs.cmu.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multipart MIME message.
--==_Exmh_-13419104960
Content-Type: text/plain; charset=us-ascii
>I'd like to add support for asynchronous commands to the pass
>driver.
I actually started on an interface for this a *long* time ago, but
never found the time to complete it. I've included the diffs from
that effort which primarily remove the cruft from the read/write
interface that has never been used and defines an interface for
async commands. Implementing the rest of the interface (or even
changing it) is left as an exercise for the reader.
(The SF_RETRY_SELTO changes are from some error recovery cleanups
that are in the pipeline. You can ignore those).
>The simplest solution, and the one I'm currently considering, would be
>to add two new ioctls to the pass driver, CAMIOSENDCOMMAND and
>CAMIOFETCHCOMMAND, and maintain a per-instance queue of completed but
>uncollected CCBs. The first ioctl would enqueue a CCB to the device,
>passing along a completion function that would add the result CCB to
>the pass queue, and return immediately without sleeping the
>caller. The second ioctl would return the first completed CCB on the
>pass queue if there are any CCBs to return, and would either sleep or
>return EAGAIN if the queue is empty.
I chose to implement an interface capable of scheduling and retreiving
whole lists of I/O so a smart application could avoid several uesr/kernel
boundary trips for certain types of applications. The only problem with
this is defining the semantics of the enqueue ioctl if one or more of the
CCBs contain errors. You probably want to validate all ccbs before
really sending any of them and then indicate which ccb failed by updating
the hosts copy of the errant ccb's status field and perhaps returning an
offset or pointer to the user. The other difference in this setup is
that it avoids a copy of the rather large CCB onto the kernel stack
at the time of the ioctl when we'll just have to copy that object
again into malloced storage.
>Alternately, notification could be done through kqueue or maybe a
>poll() on the /dev/pass<n> device.
I suppose an application could have the need to poll on thousands of
pass fds, but not in the near term. We can always add a kqueue interface
for agregating pass events at a later time giving the user the choice of
which one to use. It would also be cool if you could register an arbatrary
signal to fire when commands have completed as well as register a different
signal for exceptional events (bus resets, loop up/down, etc.). The latter
would finally allow you to do everything that you can from within the kernel
from a userland app.
>An additional complication is the issue of data copies for read/write
>requests. In the synchronous case, things are simple -- the
>application making the request is sleeping, so the CAM layer is free
>to copy data to/from the application-provided buffer without worrying
>about what the application might have done to that buffer in the
>meantime. If the application isn't sleeping while the request is being
>processed, there's the possibility that the specified buffer isn't
>available when the CAM layer wants to do the data copy -- maybe that
>buffer is gone, or maybe the application has exited.
This is the same issue that aio has to deal with, and the pass driver
already deals with it. vmapbuf wires the page into the kernel address
space and ensures that it will not be swapped out until vunmapbuf is called.
Oh, the one thing I'd consider changing in my interface is that perhaps
having the user provide a linked list of entries (linked through the
xpt_links in the ccb header) instead of an array of pointers might make
the enqueue simpler on both sides. Don't know though. What do the
listio interfaces do?
--
Justin
--==_Exmh_-13419104960
Content-Type: text/plain ; name="cam.pass.diffs"; charset=us-ascii
Content-Description: cam.pass.diffs
Content-Disposition: attachment; filename="cam.pass.diffs"
Index: cam/scsi/scsi_pass.c
===================================================================
RCS file: /usr/cvs/src/sys/cam/scsi/scsi_pass.c,v
retrieving revision 1.25
diff -c -r1.25 scsi_pass.c
*** cam/scsi/scsi_pass.c 2000/10/30 08:08:00 1.25
--- cam/scsi/scsi_pass.c 2000/11/03 15:00:44
***************
*** 1,5 ****
/*
! * Copyright (c) 1997, 1998 Justin T. Gibbs.
* Copyright (c) 1997, 1998, 1999 Kenneth D. Merry.
* All rights reserved.
*
--- 1,5 ----
/*
! * Copyright (c) 1997, 1998, 2000 Justin T. Gibbs.
* Copyright (c) 1997, 1998, 1999 Kenneth D. Merry.
* All rights reserved.
*
***************
*** 42,47 ****
--- 42,48 ----
#include <cam/cam_ccb.h>
#include <cam/cam_extend.h>
#include <cam/cam_periph.h>
+ #include <cam/cam_queue.h>
#include <cam/cam_xpt_periph.h>
#include <cam/cam_debug.h>
***************
*** 63,81 ****
PASS_CCB_WAITING
} pass_ccb_types;
#define ccb_type ppriv_field0
#define ccb_bp ppriv_ptr1
struct pass_softc {
! pass_state state;
! pass_flags flags;
! u_int8_t pd_type;
! struct bio_queue_head bio_queue;
! union ccb saved_ccb;
! struct devstat device_stats;
! dev_t dev;
};
#ifndef MIN
#define MIN(x,y) ((x<y) ? x : y)
#endif
--- 64,95 ----
PASS_CCB_WAITING
} pass_ccb_types;
+ typedef enum {
+ PASS_MAPPING_ACTIVE = 0x01
+ } pass_mapinfo_flags;
+
+ struct pass_mapping_info {
+ struct cam_periph_map_info mapinfo;
+ union ccb *user_ccb_addr;
+ pass_mapinfo_flags flags;
+ };
+
#define ccb_type ppriv_field0
#define ccb_bp ppriv_ptr1
struct pass_softc {
! pass_state state;
! pass_flags flags;
! u_int8_t pd_type;
! struct ccb_hdr_tailq async_command_queue;
! u_int async_num_active;
! union ccb saved_ccb;
! struct devstat device_stats;
! dev_t dev;
};
+ #define PASS_MAX_ASYNC 255
+
#ifndef MIN
#define MIN(x,y) ((x<y) ? x : y)
#endif
***************
*** 85,91 ****
static d_open_t passopen;
static d_close_t passclose;
static d_ioctl_t passioctl;
- static d_strategy_t passstrategy;
static periph_init_t passinit;
static periph_ctor_t passregister;
--- 99,104 ----
***************
*** 112,123 ****
static struct cdevsw pass_cdevsw = {
/* open */ passopen,
/* close */ passclose,
! /* read */ physread,
! /* write */ physwrite,
/* ioctl */ passioctl,
/* poll */ nopoll,
/* mmap */ nommap,
! /* strategy */ passstrategy,
/* name */ "pass",
/* maj */ PASS_CDEV_MAJOR,
/* dump */ nodump,
--- 125,136 ----
static struct cdevsw pass_cdevsw = {
/* open */ passopen,
/* close */ passclose,
! /* read */ noread,
! /* write */ nowrite,
/* ioctl */ passioctl,
/* poll */ nopoll,
/* mmap */ nommap,
! /* strategy */ nostrategy,
/* name */ "pass",
/* maj */ PASS_CDEV_MAJOR,
/* dump */ nodump,
***************
*** 173,181 ****
static void
passoninvalidate(struct cam_periph *periph)
{
- int s;
struct pass_softc *softc;
- struct bio *q_bp;
struct ccb_setasync csa;
softc = (struct pass_softc *)periph->softc;
--- 186,192 ----
***************
*** 194,218 ****
softc->flags |= PASS_FLAG_INVALID;
/*
! * Although the oninvalidate() routines are always called at
! * splsoftcam, we need to be at splbio() here to keep the buffer
! * queue from being modified while we traverse it.
! */
! s = splbio();
!
! /*
! * Return all queued I/O with ENXIO.
* XXX Handle any transactions queued to the card
* with XPT_ABORT_CCB.
*/
- while ((q_bp = bioq_first(&softc->bio_queue)) != NULL){
- bioq_remove(&softc->bio_queue, q_bp);
- q_bp->bio_resid = q_bp->bio_bcount;
- q_bp->bio_error = ENXIO;
- q_bp->bio_flags |= BIO_ERROR;
- biodone(q_bp);
- }
- splx(s);
if (bootverbose) {
xpt_print_path(periph->path);
--- 205,214 ----
softc->flags |= PASS_FLAG_INVALID;
/*
! * XXX Return all queued I/O with ENXIO.
* XXX Handle any transactions queued to the card
* with XPT_ABORT_CCB.
*/
if (bootverbose) {
xpt_print_path(periph->path);
***************
*** 286,291 ****
--- 282,288 ----
struct pass_softc *softc;
struct ccb_setasync csa;
struct ccb_getdev *cgd;
+ int no_tags;
cgd = (struct ccb_getdev *)arg;
if (periph == NULL) {
***************
*** 310,327 ****
bzero(softc, sizeof(*softc));
softc->state = PASS_STATE_NORMAL;
softc->pd_type = SID_TYPE(&cgd->inq_data);
! bioq_init(&softc->bio_queue);
periph->softc = softc;
-
cam_extend_set(passperiphs, periph->unit_number, periph);
/*
* We pass in 0 for a blocksize, since we don't
* know what the blocksize of this device is, if
* it even has a blocksize.
*/
! devstat_add_entry(&softc->device_stats, "pass", periph->unit_number,
! 0, DEVSTAT_NO_BLOCKSIZE | DEVSTAT_NO_ORDERED_TAGS,
softc->pd_type |
DEVSTAT_TYPE_IF_SCSI |
DEVSTAT_TYPE_PASS,
--- 307,326 ----
bzero(softc, sizeof(*softc));
softc->state = PASS_STATE_NORMAL;
softc->pd_type = SID_TYPE(&cgd->inq_data);
! TAILQ_INIT(&softc->async_command_queue);
periph->softc = softc;
cam_extend_set(passperiphs, periph->unit_number, periph);
+
/*
* We pass in 0 for a blocksize, since we don't
* know what the blocksize of this device is, if
* it even has a blocksize.
*/
! no_tags = (cgd->inq_data.flags & SID_CmdQue) == 0;
! devstat_add_entry(&softc->device_stats, "pass", periph->unit_number, 0,
! DEVSTAT_NO_BLOCKSIZE
! | (no_tags ? DEVSTAT_NO_ORDERED_TAGS : 0),
softc->pd_type |
DEVSTAT_TYPE_IF_SCSI |
DEVSTAT_TYPE_PASS,
***************
*** 448,523 ****
return (0);
}
- /*
- * Actually translate the requested transfer into one the physical driver
- * can understand. The transfer is described by a buf and will include
- * only one physical transfer.
- */
static void
- passstrategy(struct bio *bp)
- {
- struct cam_periph *periph;
- struct pass_softc *softc;
- u_int unit;
- int s;
-
- /*
- * The read/write interface for the passthrough driver doesn't
- * really work right now. So, we just pass back EINVAL to tell the
- * user to go away.
- */
- bp->bio_error = EINVAL;
- goto bad;
-
- /* unit = dkunit(bp->bio_dev); */
- /* XXX KDM fix this */
- unit = minor(bp->bio_dev) & 0xff;
-
- periph = cam_extend_get(passperiphs, unit);
- if (periph == NULL) {
- bp->bio_error = ENXIO;
- goto bad;
- }
- softc = (struct pass_softc *)periph->softc;
-
- /*
- * Odd number of bytes or negative offset
- */
- /* valid request? */
- if (bp->bio_blkno < 0) {
- bp->bio_error = EINVAL;
- goto bad;
- }
-
- /*
- * Mask interrupts so that the pack cannot be invalidated until
- * after we are in the queue. Otherwise, we might not properly
- * clean up one of the buffers.
- */
- s = splbio();
-
- bioq_insert_tail(&softc->bio_queue, bp);
-
- splx(s);
-
- /*
- * Schedule ourselves for performing the work.
- */
- xpt_schedule(periph, /* XXX priority */1);
-
- return;
- bad:
- bp->bio_flags |= BIO_ERROR;
-
- /*
- * Correctly set the buf to indicate a completed xfer
- */
- bp->bio_resid = bp->bio_bcount;
- biodone(bp);
- return;
- }
-
- static void
passstart(struct cam_periph *periph, union ccb *start_ccb)
{
struct pass_softc *softc;
--- 447,453 ----
***************
*** 527,579 ****
switch (softc->state) {
case PASS_STATE_NORMAL:
- {
- struct bio *bp;
-
s = splbio();
! bp = bioq_first(&softc->bio_queue);
! if (periph->immediate_priority <= periph->pinfo.priority) {
! start_ccb->ccb_h.ccb_type = PASS_CCB_WAITING;
! SLIST_INSERT_HEAD(&periph->ccb_list, &start_ccb->ccb_h,
! periph_links.sle);
! periph->immediate_priority = CAM_PRIORITY_NONE;
! splx(s);
! wakeup(&periph->ccb_list);
! } else if (bp == NULL) {
! splx(s);
! xpt_release_ccb(start_ccb);
! } else {
!
! bioq_remove(&softc->bio_queue, bp);
!
! devstat_start_transaction(&softc->device_stats);
!
! /*
! * XXX JGibbs -
! * Interpret the contents of the bp as a CCB
! * and pass it to a routine shared by our ioctl
! * code and passtart.
! * For now, just biodone it with EIO so we don't
! * hang.
! */
! bp->bio_error = EIO;
! bp->bio_flags |= BIO_ERROR;
! bp->bio_resid = bp->bio_bcount;
! biodone(bp);
! bp = bioq_first(&softc->bio_queue);
! splx(s);
!
! xpt_action(start_ccb);
!
! }
! if (bp != NULL) {
! /* Have more work to do, so ensure we stay scheduled */
! xpt_schedule(periph, /* XXX priority */1);
! }
break;
}
- }
}
static void
passdone(struct cam_periph *periph, union ccb *done_ccb)
{
--- 457,473 ----
switch (softc->state) {
case PASS_STATE_NORMAL:
s = splbio();
! start_ccb->ccb_h.ccb_type = PASS_CCB_WAITING;
! SLIST_INSERT_HEAD(&periph->ccb_list, &start_ccb->ccb_h,
! periph_links.sle);
! periph->immediate_priority = CAM_PRIORITY_NONE;
! splx(s);
! wakeup(&periph->ccb_list);
break;
}
}
+
static void
passdone(struct cam_periph *periph, union ccb *done_ccb)
{
***************
*** 583,637 ****
softc = (struct pass_softc *)periph->softc;
csio = &done_ccb->csio;
switch (csio->ccb_h.ccb_type) {
- case PASS_CCB_BUFFER_IO:
- {
- struct bio *bp;
- cam_status status;
- u_int8_t scsi_status;
- devstat_trans_flags ds_flags;
-
- status = done_ccb->ccb_h.status;
- scsi_status = done_ccb->csio.scsi_status;
- bp = (struct bio *)done_ccb->ccb_h.ccb_bp;
- /* XXX handle errors */
- if (!(((status & CAM_STATUS_MASK) == CAM_REQ_CMP)
- && (scsi_status == SCSI_STATUS_OK))) {
- int error;
-
- if ((error = passerror(done_ccb, 0, 0)) == ERESTART) {
- /*
- * A retry was scheuled, so
- * just return.
- */
- return;
- }
-
- /*
- * XXX unfreeze the queue after we complete
- * the abort process
- */
- bp->bio_error = error;
- bp->bio_flags |= BIO_ERROR;
- }
-
- if ((done_ccb->ccb_h.flags & CAM_DIR_MASK) == CAM_DIR_IN)
- ds_flags = DEVSTAT_READ;
- else if ((done_ccb->ccb_h.flags & CAM_DIR_MASK) == CAM_DIR_OUT)
- ds_flags = DEVSTAT_WRITE;
- else
- ds_flags = DEVSTAT_NO_DATA;
-
- devstat_end_transaction_bio(&softc->device_stats, bp);
- biodone(bp);
- break;
- }
case PASS_CCB_WAITING:
- {
/* Caller will release the CCB */
wakeup(&done_ccb->ccb_h.cbfcnp);
return;
}
- }
xpt_release_ccb(done_ccb);
}
--- 477,487 ----
***************
*** 715,720 ****
--- 565,599 ----
break;
}
+ case CAMIOCOMMANDASYNC:
+ {
+ struct pass_command_async *pca;
+ union ccb **pca_buffer;
+ size_t pca_buffer_size;
+ int i;
+
+ pca = (struct pass_command_async *)addr;
+ pca_buffer_size =
+ pca->pca_allocated_entries * sizeof(union ccb *);
+ pca_buffer = malloc(pca_buffer_size, M_TEMP, M_WAITOK);
+ if (pca_buffer == NULL) {
+ error = ENOMEM;
+ break;
+ }
+ error = copyin(pca->pca_buffer, pca_buffer, pca_buffer_size);
+ if (error != 0) {
+ free(pca_buffer, M_TEMP);
+ break;
+ }
+ for (i = 0; i < pca->pca_allocated_entries; i++) {
+
+ }
+ break;
+ }
+ case CAMIOCOMMANDSTATUS:
+ {
+ break;
+ }
default:
error = cam_periph_ioctl(periph, cmd, addr, passerror);
break;
***************
*** 792,799 ****
error = cam_periph_runccb(ccb,
(ccb->ccb_h.flags & CAM_PASS_ERR_RECOVER) ?
passerror : NULL,
! /* cam_flags */ 0,
! /* sense_flags */SF_RETRY_UA | SF_RETRY_SELTO,
&softc->device_stats);
if (need_unmap != 0)
--- 671,678 ----
error = cam_periph_runccb(ccb,
(ccb->ccb_h.flags & CAM_PASS_ERR_RECOVER) ?
passerror : NULL,
! /* cam_flags */ CAM_RETRY_SELTO,
! /* sense_flags */SF_RETRY_UA,
&softc->device_stats);
if (need_unmap != 0)
Index: cam/scsi/scsi_pass.h
===================================================================
RCS file: /usr/cvs/src/sys/cam/scsi/scsi_pass.h,v
retrieving revision 1.3
diff -c -r1.3 scsi_pass.h
*** cam/scsi/scsi_pass.h 1999/08/28 00:40:48 1.3
--- cam/scsi/scsi_pass.h 2000/06/14 11:54:05
***************
*** 1,4 ****
--- 1,5 ----
/*
+ * Copyright (c) 2000 Justin T. Gibbs.
* Copyright (c) 1997, 1999 Kenneth D. Merry.
* All rights reserved.
*
***************
*** 32,38 ****
#include <cam/cam_ccb.h>
! #define CAMIOCOMMAND _IOWR(CAM_VERSION, 2, union ccb)
! #define CAMGETPASSTHRU _IOWR(CAM_VERSION, 3, union ccb)
#endif
--- 33,54 ----
#include <cam/cam_ccb.h>
! /*
! * Convert to using a pointer to a ccb in the next major version.
! * This should allow us to avoid an extra copy of the CCB data.
! */
! #define CAMIOCOMMAND _IOWR(CAM_VERSION, 2, union ccb)
! #define CAMGETPASSTHRU _IOWR(CAM_VERSION, 3, union ccb)
+ struct pass_command_async {
+ union ccb **pca_buffer;
+ u_int32_t pca_allocated_entries;
+ };
+ #define CAMIOCOMMANDASYNC _IOWR(CAM_VERSION, 4, struct pass_command_async)
+ struct pass_command_status {
+ union ccb **pcs_buffer;
+ u_int32_t pcs_allocated_entries;
+ u_int32_t pcs_returned_entries;
+ };
+ #define CAMIOCOMMANDSTATUS _IOWR(CAM_VERSION, 5, struct pass_command_status)
#endif
--==_Exmh_-13419104960--
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-scsi" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200012211501.eBLF1SF42268>