Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 10 Mar 2016 18:58:50 -0700
From:      Warner Losh <imp@bsdimp.com>
To:        Bryan Drewery <bdrewery@freebsd.org>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-head@freebsd.org,  svn-src-all@freebsd.org, Warner Losh <imp@freebsd.org>
Subject:   Re: svn commit: r296589 - head/sys/dev/fdc
Message-ID:  <CANCZdfq5zHhaR5NvasJRTz%2BVUk1kq-cMb6SxNrYeEr2-nV6wdw@mail.gmail.com>
In-Reply-To: <56E1F72D.7000002@FreeBSD.org>
References:  <201603100033.u2A0X6uN027771@repo.freebsd.org> <56E1F72D.7000002@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mar 10, 2016 3:37 PM, "Bryan Drewery" <bdrewery@freebsd.org> 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.

> 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.

Warner



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfq5zHhaR5NvasJRTz%2BVUk1kq-cMb6SxNrYeEr2-nV6wdw>