From owner-svn-src-all@freebsd.org Fri Mar 11 05:14:59 2016 Return-Path: Delivered-To: svn-src-all@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 BAB05ACB89B for ; Fri, 11 Mar 2016 05:14:59 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-ig0-x232.google.com (mail-ig0-x232.google.com [IPv6:2607:f8b0:4001:c05::232]) (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 828D3B85 for ; Fri, 11 Mar 2016 05:14:59 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-ig0-x232.google.com with SMTP id vf5so2210618igb.0 for ; Thu, 10 Mar 2016 21:14:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc; bh=4/HmTQ7eL7GsPzWGwAIIXp/S6O6+80NKBA+n5khHARY=; b=VxKtVRncmdncPX/nS4jdXlwSRtnTI0zp1YaHYl4J9X1sFv2YPwX0IqiSwoSg/t+TYX +PhzfcS4gSTrqKJGtQ2jmwBRE8J8TDdpB+JocewI8jJm2gFrWCMEDoa6DQZ//bgaXU1W E5EevM4W0dFJL+mECR6vVKWmuucxv0R01GtLomWQdeSgHkqve7sJXV7l0od7h2bv9mHa lrAk7NangFw4tTNRsfC0XcHZTSKZ4D4yuXhR16BLQQpdE4FoWyQDCCmYbGgLVxlFxzC1 Tk2ydYnqmRI3UPwjEkcmlveNOyQmjqZvOHv2Ek2O+SGqfgjVumplQAUYRZBICCW3szTs 2U3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:date :message-id:subject:from:to:cc; bh=4/HmTQ7eL7GsPzWGwAIIXp/S6O6+80NKBA+n5khHARY=; b=N3F8EqawFhP5JIRLmx2093K6t2kA7/fzJfo/aUq3QXS9fZSWaPPsjbQSCE3Q9MA95j R+OMb5tiqoGt9yv4UrHFV15LLp0B4VZDAvC8vTsBIrS/5uwWiFSNuIPphjLbEUD7XMID hfOvrEmqT0xG9vaFxYgDa72Y2nKdol+fRH0FksA/YoMHqVYEVgdYqU8LjrPKUDuPe2EO djN0bCQk7iXVhAqZQPZNCaoC5dBHLSzbXLDupM8detKSIZcJUg7BVKma9rg9LPXufgQI 6/BnZxR0PsGYqTVP9TbfvEjIrU7mzfD3aNMz1kYV15TQbiat/9clSsDMP5zu8sakRrCC o4RA== X-Gm-Message-State: AD7BkJIWZud2Epu1QGbySDYew0iUfR5uo2TaaDBWVzCIRr2H5DgW8ef6KoPaFNSBT6CYaaYOqUhnXA36bFh55w== MIME-Version: 1.0 X-Received: by 10.50.50.134 with SMTP id c6mr621284igo.57.1457673298900; Thu, 10 Mar 2016 21:14:58 -0800 (PST) Sender: wlosh@bsdimp.com Received: by 10.36.65.230 with HTTP; Thu, 10 Mar 2016 21:14:58 -0800 (PST) X-Originating-IP: [50.253.99.174] In-Reply-To: References: <201603100033.u2A0X6uN027771@repo.freebsd.org> <56E1F72D.7000002@FreeBSD.org> Date: Thu, 10 Mar 2016 22:14:58 -0700 X-Google-Sender-Auth: WV8n-HYY6g1x_1Ylvym17lBDaOk Message-ID: Subject: Re: svn commit: r296589 - head/sys/dev/fdc From: Warner Losh To: Bryan Drewery Cc: src-committers , "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , Warner Losh Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.21 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.21 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: Fri, 11 Mar 2016 05:14:59 -0000 On Thu, Mar 10, 2016 at 6:58 PM, Warner Losh wrote: > > On Mar 10, 2016 3:37 PM, "Bryan Drewery" wrote: > > > > On 3/9/16 4:33 PM, Warner Losh wrote: > > > Author: imp > > > Date: Thu Mar 10 00:33:06 2016 > > > New Revision: 296589 > > > URL: https://svnweb.freebsd.org/changeset/base/296589 > > > > > > Log: > > > Stop assuming that bio_cmd is a bit field. > > > > > > Differential Revision: https://reviews.freebsd.org/D5587 > > > > > > Modified: > > > head/sys/dev/fdc/fdc.c > > > > > > Modified: head/sys/dev/fdc/fdc.c > > > > ============================================================================== > > > --- head/sys/dev/fdc/fdc.c Thu Mar 10 00:27:10 2016 (r296588) > > > +++ head/sys/dev/fdc/fdc.c Thu Mar 10 00:33:06 2016 (r296589) > > > @@ -941,7 +941,7 @@ fdc_worker(struct fdc_data *fdc) > > > /* Disable ISADMA if we bailed while it was active */ > > > if (fd != NULL && (fd->flags & FD_ISADMA)) { > > > isa_dmadone( > > > - bp->bio_cmd & BIO_READ ? ISADMA_READ : ISADMA_WRITE, > > > + bp->bio_cmd == BIO_READ ? ISADMA_READ : ISADMA_WRITE, > > > > I think we should have some kind of file (like ports CHANGES) that lists > > subtle KPI changes. This and the bio bzero change were easily missed > > and could lead to who-knows-what downstream for vendors or even > > out-of-tree modules. > > True. However, these have never been documented one way or another.... > > And this change isn't a change yet... > > I'd love a kpi change file. This is but one of many. We'd need someone > clueful to watch the tree and remind people to add things to it. > > I'm also working on documenting our storage api so that people know better > what is defined, vs what's there and subject to change. > Re-reading this, I wasn't very clear: I think we need this file. I think we need someone else (not me) to spearhead it and police changes I think that the sooner we start the better. Can I get a volunteer? > > Btw there are some missed still: > > > > ./dev/mfi/mfi.c: switch (bio->bio_cmd & 0x03) { > > ./dev/mfi/mfi.c: switch (bio->bio_cmd & 0x03) { > > That makes me sad. Code like that has never been guaranteed. Bare > constants.... shudder. > Now that I've had a closer look at the code, it doesn't actually assume that BIO_READ and BIO_WRITE are bits. It only assumes that their values are <= 3. I'll fix that. That's also a bad assumption, but it's a different kind of bad. Warner