From owner-svn-src-head@freebsd.org Fri Apr 15 04:48:57 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id EC0DDAECFAB for ; Fri, 15 Apr 2016 04:48:57 +0000 (UTC) (envelope-from steven.hartland@multiplay.co.uk) Received: from mail-wm0-x229.google.com (mail-wm0-x229.google.com [IPv6:2a00:1450:400c:c09::229]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 8419E12DA for ; Fri, 15 Apr 2016 04:48:57 +0000 (UTC) (envelope-from steven.hartland@multiplay.co.uk) Received: by mail-wm0-x229.google.com with SMTP id v188so14597095wme.1 for ; Thu, 14 Apr 2016 21:48:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=multiplay-co-uk.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-transfer-encoding; bh=tq7M5wmGqbWs9jUwENXcDwDoD109JVIoPkR4uxs2N1E=; b=UKkMUZ8CS+6B9EYb4neEEyo/q5rwEhhGqDjLOt2aTiLoyNdlkOMENhLPLJ6sXkKbRx XuUbMv0e3EUfbYL0fxfVK1LdFXXU1kpeCsCMW9xqt/8hom3hTSWucZrfmMm2b1HB39M3 lywSSIntHpO+4Xm4YqvXgSzWpZCnVoI9moOTfJGJNZVeBhbiRSLJM4u0/7nUB+B+7X5y 7Vn8UfLJ6X2aQEAlhQ7+F/HL0UI0R87gvtopNrmtB1P6rp+CFCikmIRO18ylN+QkXsI4 DOYTMTRORI4U8CzfX97MEb1OV5HxD4zGNZWSkCoL7OnZioOwJc5sArQlVcpw2FQks/S0 Ca6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=tq7M5wmGqbWs9jUwENXcDwDoD109JVIoPkR4uxs2N1E=; b=EzPfcxTft4n0vpJvbd99PLoT5adCVcV0Y66tUanX9Sx9+kzxMCKiocY1EyhDKeNV3N jX8ZKZqbHqLsue7rNOz7WPi6TnHCm7TzJK4U7o4Tt80K+gy7OH3kogo1hrW57kkTp3X+ ckOl8LBl3+rt1NjoJHt5Bi9tKk8FN7IrzuICwNdhteN3R3UuRnDbBgPAUB6K4PzNzioE lRkRLKMFkSqpwbjY2lAILTXK5amCuKOUI4tJsC7mqu3dU+XExLFcvZnLgMg61PWQkQ1f 2mMg8xxY7d2sPcCIvk/I9ozyFbXJHVg2GVGKHh34fQn7q/SBVEsG9qTb5pSMVgq3U0Se CtYw== X-Gm-Message-State: AOPr4FWEs3opyT31lY6Aw03GxeC3vjKgGFoIBLOnFzRtR4MvJzhzE/u6BiHXMimzpj89OKq8 X-Received: by 10.194.2.210 with SMTP id 18mr21526154wjw.55.1460695736104; Thu, 14 Apr 2016 21:48:56 -0700 (PDT) Received: from [10.10.1.58] (liv3d.labs.multiplay.co.uk. [82.69.141.171]) by smtp.gmail.com with ESMTPSA id s6sm27481948wjy.31.2016.04.14.21.48.55 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 14 Apr 2016 21:48:55 -0700 (PDT) Subject: Re: svn commit: r298002 - in head/sys: cam cam/ata cam/scsi conf dev/ahci To: Warner Losh , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201604142147.u3ELlwYo052010@repo.freebsd.org> From: Steven Hartland Message-ID: <571072B7.4040909@multiplay.co.uk> Date: Fri, 15 Apr 2016 05:48:55 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <201604142147.u3ELlwYo052010@repo.freebsd.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Apr 2016 04:48:58 -0000 Great to see this hitting the tree Warner, I have a few questions inline below. On 14/04/2016 22:47, Warner Losh wrote: > Author: imp > Date: Thu Apr 14 21:47:58 2016 > New Revision: 298002 > URL: https://svnweb.freebsd.org/changeset/base/298002 > > Log: > New CAM I/O scheduler for FreeBSD. The default I/O scheduler is the same > as before. The common scheduling bits have moved from inline code in > each of the CAM periph drivers into a library that implements the > default scheduling. > > In addition, a number of rate-limiting and I/O preference options can > be enabled by adding CAM_IOSCHED_NETFLIX to your config file. A number > of extra stats are also maintained. CAM_IOSCHED_NETFLIX isn't on by > default because it uses a separate BIO_READ and BIO_WRITE queue, so > doesn't honor BIO_ORDERED between these two types of operations. We > already didn't honor it for BIO_DELETE, and we don't depend on > BIO_ORDERED between reads and writes anywhere in the system (it is > currently used with BIO_FLUSH in ZFS to make sure some writes are > complete before others start and as a poor-man's soft dependency in > one place in UFS where we won't be issuing READs until after the > operation completes). However, out of an abundance of caution, it > isn't enabled by default. > > Plus, this also brings in NCQ TRIM support for those SSDs that support > it. A black list is also provided for known rogues that use NCQ trim > as an excuse to corrupt the drive. It was difficult to separate out > into a separate commit. > > This code has run in production at Netflix for over a year now. > > Sponsored by: Netflix, Inc > Differential Revision: https://reviews.freebsd.org/D4609 snip... > @@ -844,7 +920,7 @@ adadump(void *arg, void *virtual, vm_off > 0, > NULL, > 0, > - ada_default_timeout*1000); > + 5*1000); Not a fan of random constants, is there a reason why this is now just 5 instead if ada_default_timeout, merge issue perhaps? snip... > @@ -1898,6 +2154,31 @@ out: > static int > adaerror(union ccb *ccb, u_int32_t cam_flags, u_int32_t sense_flags) > { > + struct ada_softc *softc; > + struct cam_periph *periph; > + > + periph = xpt_path_periph(ccb->ccb_h.path); > + softc = (struct ada_softc *)periph->softc; > + > + switch (ccb->ccb_h.status & CAM_STATUS_MASK) { > + case CAM_CMD_TIMEOUT: > +#ifdef CAM_IO_STATS > + softc->timeouts++; > +#endif > + break; > + case CAM_REQ_ABORTED: > + case CAM_REQ_CMP_ERR: > + case CAM_REQ_TERMIO: > + case CAM_UNREC_HBA_ERROR: > + case CAM_DATA_RUN_ERR: > + case CAM_ATA_STATUS_ERROR: > +#ifdef CAM_IO_STATS > + softc->errors++; > +#endif > + break; > + default: > + break; > + } Am I missing something or does this entire switch do nothing unless CAM_IO_STATS is set and hence could all be under the #ifdef? Regards Steve