Re: [GIT PULL] SCSI fixes for 2.6.32-rc3

From: James Bottomley
Date: Mon Oct 12 2009 - 10:20:39 EST


On Mon, 2009-10-12 at 15:06 +0200, Ingo Molnar wrote:
> * James Bottomley <James.Bottomley@xxxxxxx> wrote:
>
> > On Fri, 2009-10-09 at 11:15 +0200, Ingo Molnar wrote:
> > > * Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > > On Thu, 8 Oct 2009, Theodore Tso wrote:
> > > > >
> > > > > So would it be acceptable to merge the 50 kloc of crap _during_ the
> > > > > merge window?
> > > >
> > > > Yes. I actually looked at the driver (since I had pulled it - I've
> > > > unpulled it but am still mulling it over), and while I think it looked
> > > > huge and overly complex, it by no means gave me the kinds of vibes I
> > > > get from some "obviously-ported-from-windows-with-no-clue" drivers.
> > > >
> > > > So at least from my quick look I didn't get the feeling that the
> > > > driver was "evil". For me, it's a timing issue. I hate getting big
> > > > pull requests after -rc1 is out, and I really don't like the feeling
> > > > that people are just ignoring the merge window.
> > > >
> > > > That said, if somebody wants to look more closely at the driver, and
> > > > then wants to convince people that it should have gone through
> > > > "staging", feel free. But that's not what I've personally been arguing
> > > > about.
> > >
> > > Greg, what's your take on the quality of this new driver? Do you have
> > > some time to do a review of this with drivers/staging/ versus drivers/
> > > glasses on? The Git URI is at:
> >
> > To me, the matter of staging versus actual tree isn't a quality issue
> > (otherwise we'd be shifting ~75% of SCSI drivers to staging, depending
> > on whose view of "quality" was being used). [...]
>
> I think you need to update your notion of what goes into
> drivers/staging/ - these days it's primarily about code/implementation
> quality (Greg please correct me if i'm wrong about that).
>
> Driver ABIs are distinctly down the priority list.

Not for me they're not. We worked hard to unify exposed ABIs for
drivers, so this is the most important user visible feature. We can
clean up code in the drivers tree, that's easy. We can't break the user
visible ABI of a supported driver without causing a lot of pain to its
user community.

> > [...] It's an ABI issue. If we would have to change the user visible
> > ABI while the driver was being cleaned up, I'd want it in staging to
> > warn users to expect these problems. Although we couldn't clean up
> > everything, I did make sure this driver plugs correctly into the
> > standard linux FC ABI before putting it in the SCSI tree, so there are
> > no ABI changes anticipated even though there will likely be a lot of
> > code changes. Therefore, the correct clean up path for this one is
> > through the SCSI tree.
>
> What kind of significant ABI does this driver expose? If this question
> even comes up then it's using the wrong kind of ABI i think. Drivers
> should almost never expose significant new ABIs.

The answer was in the email fragment you quoted: none on its own. It
plugs into the standard FC transport ABI now.

However, there are various storage bodies who see creating user ABIs as
part of their job. The way these tend to get implemented is in the
driver because that's the way it was done in UNIX (each driver rolls its
own). Part of my job is going through drivers and taking them out
again ... while ensuring we at least have a way to perform whatever task
it was in a uniform manner for all drivers.

> If it's a storage management ABI then that should have been done at a
> higher level - possibly as a system call, but at minimum as a general
> facility.

It is: it's implemented in the FC transport class, where it's uniform
for all FC drivers.

James

> (Anyway ... this cannot be argued without knowing what specific ABI you
> mean here.)
>
> Ingo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/