From owner-svn-src-all@FreeBSD.ORG Wed Apr 10 23:20:11 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 124C17D7; Wed, 10 Apr 2013 23:20:11 +0000 (UTC) (envelope-from sbruno@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) by mx1.freebsd.org (Postfix) with ESMTP id 04637109; Wed, 10 Apr 2013 23:20:11 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.6/8.14.6) with ESMTP id r3ANKAOX016646; Wed, 10 Apr 2013 23:20:10 GMT (envelope-from sbruno@svn.freebsd.org) Received: (from sbruno@localhost) by svn.freebsd.org (8.14.6/8.14.5/Submit) id r3ANKAKw016642; Wed, 10 Apr 2013 23:20:10 GMT (envelope-from sbruno@svn.freebsd.org) Message-Id: <201304102320.r3ANKAKw016642@svn.freebsd.org> From: Sean Bruno Date: Wed, 10 Apr 2013 23:20:10 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r249348 - in head/sys: conf dev/dpt X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Apr 2013 23:20:11 -0000 Author: sbruno Date: Wed Apr 10 23:20:09 2013 New Revision: 249348 URL: http://svnweb.freebsd.org/changeset/base/249348 Log: options DPT_HANDLE_TIMEOUTS hasn't worked since dpt(4) was converted to CAM somewhere around svn r39402 to r39234. I don't know of anyone who really wants to test these changes, but they only remove the deprecated code in question. This shreds the driver down a bit and *removes* options from the kernel configs. These don't appear to be referenced in the man page, so no need to check it there. PR: kern/44587 Obtained from: Yahoo! Inc. MFC after: 2 weeks Modified: head/sys/conf/NOTES head/sys/conf/options head/sys/dev/dpt/dpt.h head/sys/dev/dpt/dpt_scsi.c Modified: head/sys/conf/NOTES ============================================================================== --- head/sys/conf/NOTES Wed Apr 10 22:12:21 2013 (r249347) +++ head/sys/conf/NOTES Wed Apr 10 23:20:09 2013 (r249348) @@ -1623,20 +1623,10 @@ options ISP_DEFAULT_ROLES=0 # Compaq are actually DPT controllers. # # See src/sys/dev/dpt for debugging and other subtle options. -# DPT_MEASURE_PERFORMANCE Enables a set of (semi)invasive metrics. Various +# DPT_MEASURE_PERFORMANCE Enables a set of (semi)invasive metrics. Various # instruments are enabled. The tools in # /usr/sbin/dpt_* assume these to be enabled. -# DPT_HANDLE_TIMEOUTS Normally device timeouts are handled by the DPT. -# If you want the driver to handle timeouts, enable -# this option. If your system is very busy, this -# option will create more trouble than solve. -# DPT_TIMEOUT_FACTOR Used to compute the excessive amount of time to -# wait when timing out with the above option. # DPT_DEBUG_xxxx These are controllable from sys/dev/dpt/dpt.h -# DPT_LOST_IRQ When enabled, will try, once per second, to catch -# any interrupt that got lost. Seems to help in some -# DPT-firmware/Motherboard combinations. Minimal -# cost, great benefit. # DPT_RESET_HBA Make "reset" actually reset the controller # instead of fudging it. Only enable this if you # are 100% certain you need it. @@ -1645,9 +1635,6 @@ device dpt # DPT options #!CAM# options DPT_MEASURE_PERFORMANCE -#!CAM# options DPT_HANDLE_TIMEOUTS -options DPT_TIMEOUT_FACTOR=4 -options DPT_LOST_IRQ options DPT_RESET_HBA # Modified: head/sys/conf/options ============================================================================== --- head/sys/conf/options Wed Apr 10 22:12:21 2013 (r249347) +++ head/sys/conf/options Wed Apr 10 23:20:09 2013 (r249348) @@ -534,9 +534,6 @@ NATM # DPT driver debug flags DPT_MEASURE_PERFORMANCE opt_dpt.h -DPT_HANDLE_TIMEOUTS opt_dpt.h -DPT_TIMEOUT_FACTOR opt_dpt.h -DPT_LOST_IRQ opt_dpt.h DPT_RESET_HBA opt_dpt.h # Misc debug flags. Most of these should probably be replaced with Modified: head/sys/dev/dpt/dpt.h ============================================================================== --- head/sys/dev/dpt/dpt.h Wed Apr 10 22:12:21 2013 (r249347) +++ head/sys/dev/dpt/dpt.h Wed Apr 10 23:20:09 2013 (r249348) @@ -1110,16 +1110,6 @@ typedef struct dpt_softc { #define DPT_HA_COMMAND_ACTIVE 0x00000080 #define DPT_HA_QUIET 0x00000100 -#ifdef DPT_LOST_IRQ -#define DPT_LOST_IRQ_SET 0x10000000 -#define DPT_LOST_IRQ_ACTIVE 0x20000000 -#endif - -#ifdef DPT_HANDLE_TIMEOUTS -#define DPT_HA_TIMEOUTS_SET 0x40000000 -#define DPT_HA_TIMEOUTS_ACTIVE 0x80000000 -#endif - u_int8_t primary; /* true if primary */ u_int8_t more_support :1, /* HBA supports MORE flag */ Modified: head/sys/dev/dpt/dpt_scsi.c ============================================================================== --- head/sys/dev/dpt/dpt_scsi.c Wed Apr 10 22:12:21 2013 (r249347) +++ head/sys/dev/dpt/dpt_scsi.c Wed Apr 10 23:20:09 2013 (r249348) @@ -1632,9 +1632,6 @@ dpt_intr_locked(dpt_softc_t *dpt) "clear EOC.\n Marking as LOST.\n", dccb->transaction_id); -#ifdef DPT_HANDLE_TIMEOUTS - dccb->state |= DPT_CCB_STATE_MARKED_LOST; -#endif /* This CLEARS the interrupt! */ status = dpt_inb(dpt, HA_RSTATUS); continue; @@ -2524,154 +2521,4 @@ dpt_user_cmd_done(dpt_softc_t * dpt, int return; } -#ifdef DPT_HANDLE_TIMEOUTS -/** - * This function walks down the SUBMITTED queue. - * Every request that is too old gets aborted and marked. - * Since the DPT will complete (interrupt) immediately (what does that mean?), - * We just walk the list, aborting old commands and marking them as such. - * The dpt_complete function will get rid of the that were interrupted in the - * normal manner. - * - * This function needs to run at splcam(), as it interacts with the submitted - * queue, as well as the completed and free queues. Just like dpt_intr() does. - * To run it at any ISPL other than that of dpt_intr(), will mean that dpt_intr - * willbe able to pre-empt it, grab a transaction in progress (towards - * destruction) and operate on it. The state of this transaction will be not - * very clear. - * The only other option, is to lock it only as long as necessary but have - * dpt_intr() spin-wait on it. In a UP environment this makes no sense and in - * a SMP environment, the advantage is dubvious for a function that runs once - * every ten seconds for few microseconds and, on systems with healthy - * hardware, does not do anything anyway. - */ - -static void -dpt_handle_timeouts(dpt_softc_t * dpt) -{ - dpt_ccb_t *ccb; - - if (dpt->state & DPT_HA_TIMEOUTS_ACTIVE) { - device_printf(dpt->dev, "WARNING: Timeout Handling Collision\n"); - return; - } - dpt->state |= DPT_HA_TIMEOUTS_ACTIVE; - - /* Loop through the entire submitted queue, looking for lost souls */ - TAILQ_FIRST(ccb, &&dpt->submitted_ccbs, links) { - struct scsi_xfer *xs; - u_int32_t age, max_age; - - xs = ccb->xs; - age = dpt_time_delta(ccb->command_started, microtime_now); - -#define TenSec 10000000 - - if (xs == NULL) { /* Local, non-kernel call */ - max_age = TenSec; - } else { - max_age = (((xs->timeout * (dpt->submitted_ccbs_count - + DPT_TIMEOUT_FACTOR)) - > TenSec) - ? (xs->timeout * (dpt->submitted_ccbs_count - + DPT_TIMEOUT_FACTOR)) - : TenSec); - } - - /* - * If a transaction is marked lost and is TWICE as old as we - * care, then, and only then do we destroy it! - */ - if (ccb->state & DPT_CCB_STATE_MARKED_LOST) { - /* Remember who is next */ - if (age > (max_age * 2)) { - dpt_Qremove_submitted(dpt, ccb); - ccb->state &= ~DPT_CCB_STATE_MARKED_LOST; - ccb->state |= DPT_CCB_STATE_ABORTED; -#define cmd_name scsi_cmd_name(ccb->eata_ccb.cp_scsi_cmd) - if (ccb->retries++ > DPT_RETRIES) { - device_printf(dpt->dev, - "ERROR: Destroying stale " - "%d (%s)\n" - " on " - "c%db%dt%du%d (%d/%d)\n", - ccb->transaction_id, - cmd_name, - device_get_unit(dpt->dev), - ccb->eata_ccb.cp_channel, - ccb->eata_ccb.cp_id, - ccb->eata_ccb.cp_LUN, age, - ccb->retries); -#define send_ccb &ccb->eata_ccb -#define ESA EATA_SPECIFIC_ABORT - (void) dpt_send_immediate(dpt, - send_ccb, - ESA, - 0, 0); - dpt_Qpush_free(dpt, ccb); - - /* The SCSI layer should re-try */ - xs->error |= XS_TIMEOUT; - xs->flags |= SCSI_ITSDONE; - scsi_done(xs); - } else { - device_printf(dpt->dev, - "ERROR: Stale %d (%s) on " - "c%db%dt%du%d (%d)\n" - " gets another " - "chance(%d/%d)\n", - ccb->transaction_id, - cmd_name, - device_get_unit(dpt->dev), - ccb->eata_ccb.cp_channel, - ccb->eata_ccb.cp_id, - ccb->eata_ccb.cp_LUN, - age, ccb->retries, DPT_RETRIES); - - dpt_Qpush_waiting(dpt, ccb); - dpt_sched_queue(dpt); - } - } - } else { - /* - * This is a transaction that is not to be destroyed - * (yet) But it is too old for our liking. We wait as - * long as the upper layer thinks. Not really, we - * multiply that by the number of commands in the - * submitted queue + 1. - */ - if (!(ccb->state & DPT_CCB_STATE_MARKED_LOST) && - (age != ~0) && (age > max_age)) { - device_printf(dpt->dev, - "ERROR: Marking %d (%s) on " - "c%db%dt%du%d \n" - " as late after %dusec\n", - ccb->transaction_id, - cmd_name, - device_get_unit(dpt->dev), - ccb->eata_ccb.cp_channel, - ccb->eata_ccb.cp_id, - ccb->eata_ccb.cp_LUN, age); - ccb->state |= DPT_CCB_STATE_MARKED_LOST; - } - } - } - - dpt->state &= ~DPT_HA_TIMEOUTS_ACTIVE; -} - -static void -dpt_timeout(void *arg) -{ - dpt_softc_t *dpt = (dpt_softc_t *) arg; - - mtx_assert(&dpt->lock, MA_OWNED); - if (!(dpt->state & DPT_HA_TIMEOUTS_ACTIVE)) - dpt_handle_timeouts(dpt); - - callout_reset(&dpt->timer, hz * 10, dpt_timeout, dpt); -} - -#endif /* DPT_HANDLE_TIMEOUTS */ - #endif