Date: Tue, 30 Apr 2013 04:16:49 +0100 From: "Steven Hartland" <killing@multiplay.co.uk> To: "Steven Hartland" <smh@freebsd.org>, "Kenneth D. Merry" <ken@kdm.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r249939 - head/sys/cam/scsi Message-ID: <8C38141DA56442218C71EDCBA9790CF8@multiplay.co.uk> References: <201304261617.r3QGH58Q048395@svn.freebsd.org> <20130429225641.GA1375@nargothrond.kdm.org> <F701AE7B956C498AB89649768B748D81@multiplay.co.uk>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. ------=_NextPart_000_0415_01CE4559.890A90E0 Content-Type: text/plain; format=flowed; charset="iso-8859-1"; reply-type=response Content-Transfer-Encoding: 7bit ----- Original Message ----- From: "Steven Hartland" >> On Fri, Apr 26, 2013 at 16:17:05 +0000, Steven Hartland wrote: >>> Author: smh >>> Date: Fri Apr 26 16:17:04 2013 >>> New Revision: 249939 >>> URL: http://svnweb.freebsd.org/changeset/base/249939 >>> >>> Log: >>> Added available delete methods discovery during device probe, including the >>> maximum sizes for said methods, which are used when processing BIO_DELETE >>> requests. This includes updating UNMAP support discovery to be based on >>> SBC-3 T10/1799-D Revision 31 specification. >>> Added ATA TRIM support to cam scsi devices via ATA Pass-Through(16) >>> sys/cam/scsi/scsi_da.c: >>> - Added ATA Data Set Management TRIM support via ATA Pass-Through(16) >>> as a delete_method >>> >> >> This adds a lot of unnecessary verbosity for devices that don't support ATA >> passthrough. For example: >> >> (da7:iscsi4:0:0:0): ATA COMMAND PASS THROUGH(16). CDB: 85 08 0a 00 00 02 00 00 00 00 00 00 00 40 ec 00 >> (da7:iscsi4:0:0:0): CAM status: SCSI Status Error >> (da7:iscsi4:0:0:0): SCSI status: Check Condition >> (da7:iscsi4:0:0:0): Retrying command (per sense data) >> (2:2:0:0): ATA COMMAND PASS THROUGH(16). CDB: 85 08 0a 00 00 02 00 00 00 00 00 00 00 40 ec 00 >> (2:2:0:0): Tag: 0x00f6, Type: 1 >> (2:2:0:0): CTL Status: SCSI Error >> (2:2:0:0): SCSI Status: Check Condition >> (2:2:0:0): SCSI sense: ILLEGAL REQUEST asc:20,0 (Invalid command operation code) >> (2:2:0:0): Command byte 0 is invalid >> >> (da8:iscsi4:0:0:1): ATA COMMAND PASS THROUGH(16). CDB: 85 08 0a 00 00 02 00 00 00 00 00 00 00 40 ec 00 >> (da8:iscsi4:0:0:1): CAM status: SCSI Status Error >> (da8:iscsi4:0:0:1): SCSI status: Check Condition >> (da8:iscsi4:0:0:1): Retrying command (per sense data) >> (da8:iscsi4:0:0:1): ATA COMMAND PASS THROUGH(16). CDB: 85 08 0a 00 00 02 00 00 00 00 00 00 00 40 ec 00 >> (da8:iscsi4:0:0:1): CAM status: SCSI Status Error >> (da8:iscsi4:0:0:1): SCSI status: Check Condition >> (da8:iscsi4:0:0:1): Error 5, Retries exhausted >> >> That is with CTL and and trasz's new iSCSI initiator, but you should see it >> with any CTL configuration. (And probably with any controller or device >> that doesn't support ATA passthrough.) >> >> So, please: >> - Check for the presence of VPD page 0x89 before sending an ATA >> passthrough command. The spec (sat3r03 in this case) says that >> it "shall" be implemented, so I think we can count on that. >> - If the target returns an illegal request sense key, don't retry >> again. The target will keep returning illegal request > > Thanks for the report Ken I'll check this on a card I know doesn't support > pass-through. I checked with an areca controller, which I know doesn't support pass16, and all is quiet during boot / probe. I can provoke output at the app layer with camcontrol identify da0, which is to be expected, but nothing in /var/log/messages. daerror handler in scsi_da.c definitely has SF_QUIET_IR so Illegal Request should never be output. I can't provoke this using standard CAM devices even when they don't support ATA Pass-Through (16) so I think this is actually an issue with CTL: 1. Being verbose when it shouldn't (ctl_process_done - line 12571?) 2. Retrying Illegal Request commands when it shouldn't (I didn't experince this on r250032) The attached patch implements a check for ATA Information VPD before using ATA Pass-Through (16). It's had limited testing but I have confirmed it eliminates the use of pass16 under CTL and still enables ATA TRIM under on mpt for SATA disks, so looks promising. If you could test and let me know if it works for you Ken that would be appreciated. The test case I used for CTL was:- kldload ctl ctladm create -b ramdisk -s 10485760 ctladm port -o on As a side note while test "kldload ctl" caused kernel panic the once. If anyone's interested the trace was:- #0 doadump (textdump=0) at pcpu.h:231 #1 0xffffffff802f6d6e in db_dump (dummy=<value optimized out>, dummy2=0, dummy3=0, dummy4=0x0) at /usr/home/smh/freebsd/base/head/sys/ddb/db_command.c:543 #2 0xffffffff802f683d in db_command (cmd_table=<value optimized out>) at /usr/home/smh/freebsd/base/head/sys/ddb/db_command.c:449 #3 0xffffffff802f65b4 in db_command_loop () at /usr/home/smh/freebsd/base/head/sys/ddb/db_command.c:502 #4 0xffffffff802f8f50 in db_trap (type=<value optimized out>, code=0) at /usr/home/smh/freebsd/base/head/sys/ddb/db_main.c:231 #5 0xffffffff805c0df3 in kdb_trap (type=9, code=0, tf=<value optimized out>) at /usr/home/smh/freebsd/base/head/sys/kern/subr_kdb.c:654 #6 0xffffffff8075580a in trap_fatal (frame=0xffffff823b8dc790, eva=<value optimized out>) at /usr/home/smh/freebsd/base/head/sys/amd64/amd64/trap.c:867 #7 0xffffffff807554b7 in trap (frame=<value optimized out>) at /usr/home/smh/freebsd/base/head/sys/amd64/amd64/trap.c:224 #8 0xffffffff8073e1f2 in calltrap () at /usr/home/smh/freebsd/base/head/sys/amd64/amd64/exception.S:228 #9 0xffffffff8029c860 in cam_periph_alloc (periph_ctor=0xffffffff802af410 <proberegister>, periph_oninvalidate=0xfffffe0019cfa200, periph_dtor=<value optimized out>, periph_start=0xfffffe0015980a90, name=<value optimized out>, type=2159638184, path=0xfffffe0015ad79a0, ac_callback=<value optimized out>, code=<value optimized out>) at /usr/home/smh/freebsd/base/head/sys/cam/cam_periph.c:227 #10 0xffffffff802aed6b in scsi_scan_lun (request_ccb=<value optimized out>) at /usr/home/smh/freebsd/base/head/sys/cam/scsi/scsi_xpt.c:2266 #11 0xffffffff802b2859 in scsi_scan_bus (periph=0x0, request_ccb=0xfffffe00234df000) at /usr/home/smh/freebsd/base/head/sys/cam/scsi/scsi_xpt.c:1969 #12 0xffffffff802a66c5 in xpt_scanner_thread (dummy=<value optimized out>) at /usr/home/smh/freebsd/base/head/sys/cam/cam_xpt.c:2411 #13 0xffffffff8055cde4 in fork_exit (callout=0xffffffff802a6650 <xpt_scanner_thread>, arg=0x0, frame=0xffffff823b8dcc00) at /usr/home/smh/freebsd/base/head/sys/kern/kern_fork.c:991 #14 0xffffffff8073e72e in fork_trampoline () at /usr/home/smh/freebsd/base/head/sys/amd64/amd64/exception.S:602 #15 0x0000000000000000 in ?? () Regards Steve ================================================ This e.mail is private and confidential between Multiplay (UK) Ltd. and the person or entity to whom it is addressed. In the event of misdirection, the recipient is prohibited from using, copying, printing or otherwise disseminating it or any information contained in it. In the event of misdirection, illegible or incomplete transmission please telephone +44 845 868 1337 or return the E.mail to postmaster@multiplay.co.uk. ------=_NextPart_000_0415_01CE4559.890A90E0 Content-Type: application/octet-stream; name="cam-scsi-delete-ata-info.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="cam-scsi-delete-ata-info.patch" Use the existence of ATA Information VPD to determine if we should = attempt=0A= to query ATA functionality via ATA Pass-Through (16) as this page is = defined=0A= as "must" for SATL devices, hence indicating that the device is at least=0A= likely to support Pass-Through (16).=0A= =0A= This eliminates errors produced by CTL when ATA Pass-Through (16) fails.=0A= =0A= Output details about supported and choosen delete method when verbose = booted.=0A= Index: sys/cam/scsi/scsi_all.h=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A= --- sys/cam/scsi/scsi_all.h (revision 250032)=0A= +++ sys/cam/scsi/scsi_all.h (working copy)=0A= @@ -1430,6 +1430,12 @@=0A= };=0A= =0A= /*=0A= + * ATA Information VPD Page based on=0A= + * T10/2126-D Revision 04=0A= + */=0A= +#define SVPD_ATA_INFORMATION 0x89=0A= +=0A= +/*=0A= * Block Device Characteristics VPD Page based on=0A= * T10/1799-D Revision 31=0A= */=0A= Index: sys/cam/scsi/scsi_da.c=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A= --- sys/cam/scsi/scsi_da.c (revision 250033)=0A= +++ sys/cam/scsi/scsi_da.c (working copy)=0A= @@ -918,6 +918,7 @@=0A= da_delete_methods delete_method);=0A= static void dadeletemethodchoose(struct da_softc *softc,=0A= da_delete_methods default_method);=0A= +static void dadeletemethodauto(struct cam_periph *periph);=0A= =0A= static periph_ctor_t daregister;=0A= static periph_dtor_t dacleanup;=0A= @@ -1680,6 +1681,47 @@=0A= }=0A= =0A= static void=0A= +dadeletemethodauto(struct cam_periph *periph)=0A= +{=0A= + struct da_softc *softc;=0A= +=0A= + softc =3D (struct da_softc *)periph->softc;=0A= +=0A= + dadeletemethodchoose(softc, DA_DELETE_NONE);=0A= +=0A= + if (bootverbose && (softc->flags & DA_FLAG_PROBED) =3D=3D 0) {=0A= + char buf[80];=0A= + int i, sep;=0A= +=0A= + snprintf(buf, sizeof(buf), "Delete methods: <");=0A= + sep =3D 0;=0A= + for (i =3D DA_DELETE_MIN; i <=3D DA_DELETE_MAX; i++) {=0A= + if (softc->delete_available & (1 << i)) {=0A= + if (sep) {=0A= + strlcat(buf, ",", sizeof(buf));=0A= + } else {=0A= + sep =3D 1;=0A= + }=0A= + strlcat(buf, da_delete_method_names[i],=0A= + sizeof(buf));=0A= + if (i =3D=3D softc->delete_method) {=0A= + strlcat(buf, "(*)", sizeof(buf));=0A= + }=0A= + }=0A= + }=0A= + if (sep =3D=3D 0) {=0A= + if (softc->delete_method =3D=3D DA_DELETE_NONE) =0A= + strlcat(buf, "NONE(*)", sizeof(buf));=0A= + else=0A= + strlcat(buf, "DISABLED(*)", sizeof(buf));=0A= + }=0A= + strlcat(buf, ">", sizeof(buf));=0A= + printf("%s%d: %s\n", periph->periph_name,=0A= + periph->unit_number, buf);=0A= + }=0A= +}=0A= +=0A= +static void=0A= dadeletemethodchoose(struct da_softc *softc, da_delete_methods = default_method)=0A= {=0A= int i, delete_method;=0A= @@ -2457,6 +2499,21 @@=0A= {=0A= struct ata_params *ata_params;=0A= =0A= + if (!scsi_vpd_supported_page(periph, SVPD_ATA_INFORMATION)) {=0A= + dadeletemethodauto(periph);=0A= +=0A= + xpt_release_ccb(start_ccb);=0A= + softc->state =3D DA_STATE_NORMAL;=0A= + daschedule(periph);=0A= + wakeup(&softc->disk->d_mediasize);=0A= + if ((softc->flags & DA_FLAG_PROBED) =3D=3D 0) {=0A= + softc->flags |=3D DA_FLAG_PROBED;=0A= + cam_periph_unhold(periph);=0A= + } else=0A= + cam_periph_release_locked(periph);=0A= + break;=0A= + }=0A= +=0A= ata_params =3D (struct ata_params*)=0A= malloc(sizeof(*ata_params), M_SCSIDA, M_NOWAIT|M_ZERO);=0A= =0A= @@ -3134,7 +3191,7 @@=0A= }=0A= =0A= free(ata_params, M_SCSIDA);=0A= - dadeletemethodchoose(softc, DA_DELETE_NONE);=0A= + dadeletemethodauto(periph);=0A= /*=0A= * Since our peripheral may be invalidated by an error=0A= * above or an external event, we must release our CCB=0A= ------=_NextPart_000_0415_01CE4559.890A90E0--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8C38141DA56442218C71EDCBA9790CF8>