Proposals to change the way all drivers work with SCSI commands

Proposals to change the way all drivers work with SCSI commands

[James Bottomley]

Right at the moment, we're planning to clean up the way SCSI drivers
process commands.  The proposals are essentially:

     1. Get rid of the now unnecessary map_single path (every command is
        either zero transfer or scatter/gather)
     2. use accessors to manipulate the SG lists (mainly so that we can
        alter the implementation without affecting the drivers)

It strikes me that in all of this, we could also consider doing the DMA
mapping inside the mid layer (instead of in every driver).  This is
essentially what libata is already doing ... leading to confusion in
SCSI drivers that use libata for SATA.

So what do people think about this?

James

Re: Proposals to change the way all drivers work with SCSI commands

[FUJITA Tomonori]

From: James Bottomley <James.Bottomley@SteelEye.com>
Subject: Proposals to change the way all drivers work with SCSI commands
Date: Fri, 11 May 2007 13:33:41 -0500

> Right at the moment, we're planning to clean up the way SCSI drivers
> process commands.  The proposals are essentially:
> 
>      1. Get rid of the now unnecessary map_single path (every command is
>         either zero transfer or scatter/gather)
>      2. use accessors to manipulate the SG lists (mainly so that we can
>         alter the implementation without affecting the drivers)
> 
> It strikes me that in all of this, we could also consider doing the DMA
> mapping inside the mid layer (instead of in every driver).  This is
> essentially what libata is already doing ... leading to confusion in
> SCSI drivers that use libata for SATA.

Several drivers don't take account of the mapping failure and handling
the failure for some is tricky. So it would be nice if the mid layer
takes care about the mapping.

Re: Proposals to change the way all drivers work with SCSI commands

[Jeff Garzik]

James Bottomley wrote:
> Right at the moment, we're planning to clean up the way SCSI drivers
> process commands.  The proposals are essentially:
> 
>      1. Get rid of the now unnecessary map_single path (every command is
>         either zero transfer or scatter/gather)
>      2. use accessors to manipulate the SG lists (mainly so that we can
>         alter the implementation without affecting the drivers)
> 
> It strikes me that in all of this, we could also consider doing the DMA
> mapping inside the mid layer (instead of in every driver).  This is
> essentially what libata is already doing ... leading to confusion in
> SCSI drivers that use libata for SATA.

My eyes are crossed :)  How does doing DMA mapping in the mid layer lead 
to confusion in SCSI drivers that use libata for SATA?  Are you talking 
about aic94xx and ipr only, or other drivers?

	Jeff

Re: Proposals to change the way all drivers work with SCSI commands

[James Bottomley]

On Fri, 2007-05-11 at 15:15 -0400, Jeff Garzik wrote:
> James Bottomley wrote:
> > Right at the moment, we're planning to clean up the way SCSI drivers
> > process commands.  The proposals are essentially:
> > 
> >      1. Get rid of the now unnecessary map_single path (every command is
> >         either zero transfer or scatter/gather)
> >      2. use accessors to manipulate the SG lists (mainly so that we can
> >         alter the implementation without affecting the drivers)
> > 
> > It strikes me that in all of this, we could also consider doing the DMA
> > mapping inside the mid layer (instead of in every driver).  This is
> > essentially what libata is already doing ... leading to confusion in
> > SCSI drivers that use libata for SATA.
> 
> My eyes are crossed :)  How does doing DMA mapping in the mid layer lead 
> to confusion in SCSI drivers that use libata for SATA?  Are you talking 
> about aic94xx and ipr only, or other drivers?

Those are the only two that use libata for SATA, yes.

aic94xx is horrible in this regard ... it has to try to distinguish STP
commands that come from libsas (and thus aren't mapped) from ones that
come from libata which are ... and it still doesn't get it entirely
right.

James

Re: Proposals to change the way all drivers work with SCSI commands

[Jeff Garzik]

James Bottomley wrote:
> On Fri, 2007-05-11 at 15:15 -0400, Jeff Garzik wrote:
>> James Bottomley wrote:
>>> Right at the moment, we're planning to clean up the way SCSI drivers
>>> process commands.  The proposals are essentially:
>>>
>>>      1. Get rid of the now unnecessary map_single path (every command is
>>>         either zero transfer or scatter/gather)
>>>      2. use accessors to manipulate the SG lists (mainly so that we can
>>>         alter the implementation without affecting the drivers)
>>>
>>> It strikes me that in all of this, we could also consider doing the DMA
>>> mapping inside the mid layer (instead of in every driver).  This is
>>> essentially what libata is already doing ... leading to confusion in
>>> SCSI drivers that use libata for SATA.
>> My eyes are crossed :)  How does doing DMA mapping in the mid layer lead 
>> to confusion in SCSI drivers that use libata for SATA?  Are you talking 
>> about aic94xx and ipr only, or other drivers?
> 
> Those are the only two that use libata for SATA, yes.
> 
> aic94xx is horrible in this regard ... it has to try to distinguish STP
> commands that come from libsas (and thus aren't mapped) from ones that
> come from libata which are ... and it still doesn't get it entirely
> right.

I would lean towards fixing libata to -not- DMA-map for those drivers. 
Splitting up DMA mapping is ugly.

	Jeff

Re: Proposals to change the way all drivers work with SCSI commands

[David Miller]

From: James Bottomley <James.Bottomley@SteelEye.com>
Date: Fri, 11 May 2007 13:33:41 -0500

> Right at the moment, we're planning to clean up the way SCSI drivers
> process commands.  The proposals are essentially:
> 
>      1. Get rid of the now unnecessary map_single path (every command is
>         either zero transfer or scatter/gather)
>      2. use accessors to manipulate the SG lists (mainly so that we can
>         alter the implementation without affecting the drivers)
> 
> It strikes me that in all of this, we could also consider doing the DMA
> mapping inside the mid layer (instead of in every driver).  This is
> essentially what libata is already doing ... leading to confusion in
> SCSI drivers that use libata for SATA.
> 
> So what do people think about this?

This would require platforms to handle all of their bus types
behind the generic dma_*() and that's isn't true everywhere yet.

For example, SBUS still requires explicitly using sbus_map_sg() etc.

I plan to fix that of course, but we're not there now.

Re: Proposals to change the way all drivers work with SCSI commands

[James Bottomley]

On Fri, 2007-05-11 at 13:00 -0700, David Miller wrote:
> From: James Bottomley <James.Bottomley@SteelEye.com>
> Date: Fri, 11 May 2007 13:33:41 -0500
> 
> > Right at the moment, we're planning to clean up the way SCSI drivers
> > process commands.  The proposals are essentially:
> > 
> >      1. Get rid of the now unnecessary map_single path (every command is
> >         either zero transfer or scatter/gather)
> >      2. use accessors to manipulate the SG lists (mainly so that we can
> >         alter the implementation without affecting the drivers)
> > 
> > It strikes me that in all of this, we could also consider doing the DMA
> > mapping inside the mid layer (instead of in every driver).  This is
> > essentially what libata is already doing ... leading to confusion in
> > SCSI drivers that use libata for SATA.
> > 
> > So what do people think about this?
> 
> This would require platforms to handle all of their bus types
> behind the generic dma_*() and that's isn't true everywhere yet.
> 
> For example, SBUS still requires explicitly using sbus_map_sg() etc.
> 
> I plan to fix that of course, but we're not there now.

I could probably cook up a patch for you, if you like? ... I've already
done it for several other architectures.

James

Re: Proposals to change the way all drivers work with SCSI commands

[David Miller]

From: James Bottomley <James.Bottomley@SteelEye.com>
Date: Fri, 11 May 2007 16:12:47 -0500

> I could probably cook up a patch for you, if you like? ... I've already
> done it for several other architectures.

I don't want to use that "if (bus == &sbus_bus_type)" scheme so
if that's what your patch will do don't bother :-)

I'm consolidating all of the IOMMU handling code on sparc64
since they all do exactly the same thing, so at least on
sparc64 there will be essentially direct calls to the IOMMU
code agnostic of bus type.

Sparc32 will be a bit different, and that platform has no
active maintainer so it'll have to wait until I have a
spare weekend to do nothing but sparc32 stuff :-)

Re: Proposals to change the way all drivers work with SCSI commands

[James Bottomley]

On Fri, 2007-05-11 at 14:21 -0700, David Miller wrote:
> From: James Bottomley <James.Bottomley@SteelEye.com>
> Date: Fri, 11 May 2007 16:12:47 -0500
> 
> > I could probably cook up a patch for you, if you like? ... I've already
> > done it for several other architectures.
> 
> I don't want to use that "if (bus == &sbus_bus_type)" scheme so
> if that's what your patch will do don't bother :-)

That's sort of what you already have, if you're still using the generic
dma-mapping.h helpers in asm-sparc64.  They already have a 

BUG_ON(dev->bus != &pci_bus_type)

> I'm consolidating all of the IOMMU handling code on sparc64
> since they all do exactly the same thing, so at least on
> sparc64 there will be essentially direct calls to the IOMMU
> code agnostic of bus type.

OK ... I'll let you fix it.

On PARISC, we actually use a table of function pointers, but then we
also have several other oddities including having to walk up the bus
tree to find our IOMMU (having several) which can actually be on a
different bus type for some of the older systems (i.e. the PCI iommu is
in the GSC bus etc).

> Sparc32 will be a bit different, and that platform has no
> active maintainer so it'll have to wait until I have a
> spare weekend to do nothing but sparc32 stuff :-)

James

Re: Proposals to change the way all drivers work with SCSI commands

[David Miller]

From: James Bottomley <James.Bottomley@SteelEye.com>
Date: Fri, 11 May 2007 16:36:56 -0500

> On PARISC, we actually use a table of function pointers, but then we
> also have several other oddities including having to walk up the bus
> tree to find our IOMMU (having several) which can actually be on a
> different bus type for some of the older systems (i.e. the PCI iommu is
> in the GSC bus etc).

Yes and you can store such things in asm/device.h's dev_archdata,
that's another approach I considered.

Re: Proposals to change the way all drivers work with SCSI commands

[James Bottomley]

On Fri, 2007-05-11 at 14:47 -0700, David Miller wrote:
> From: James Bottomley <James.Bottomley@SteelEye.com>
> Date: Fri, 11 May 2007 16:36:56 -0500
> 
> > On PARISC, we actually use a table of function pointers, but then we
> > also have several other oddities including having to walk up the bus
> > tree to find our IOMMU (having several) which can actually be on a
> > different bus type for some of the older systems (i.e. the PCI iommu is
> > in the GSC bus etc).
> 
> Yes and you can store such things in asm/device.h's dev_archdata,
> that's another approach I considered.

yes ... that's sort of why it exists ... arm wanted more than just the
void *platform_data which is where we stuff the iommu information on
parisc.  Really, I suppose it's time we eliminated platform_data and
just used dev_archdata instead.

James

Re: Proposals to change the way all drivers work with SCSI commands

[Stefan Richter]

James Bottomley wrote:
> Right at the moment, we're planning to clean up the way SCSI drivers
> process commands.  The proposals are essentially:
> 
>      1. Get rid of the now unnecessary map_single path (every command is
>         either zero transfer or scatter/gather)
>      2. use accessors to manipulate the SG lists (mainly so that we can
>         alter the implementation without affecting the drivers)

Should be fine with the old and the new SBP-2 driver.

> It strikes me that in all of this, we could also consider doing the DMA
> mapping inside the mid layer (instead of in every driver).  This is
> essentially what libata is already doing ... leading to confusion in
> SCSI drivers that use libata for SATA.
> 
> So what do people think about this?

The old SBP-2 driver needs the virtual addresses of S/G elements if
compiled with a certain config option.  This option is meant for use
with FireWire controllers which do not implement OHCI-1394, or to run
the IEEE 1394 stack in a more secure mode.   However, sbp2 seems to be
broken in multiple ways with this option at the moment, and I don't plan
to fix it anytime soon.

So in short, moving the DMA mapping into mid layer should also be fine
with the old and the new SBP-2 driver.
-- 
Stefan Richter
-=====-=-=== -=-= -=-==
http://arcgraph.de/sr/

Re: Proposals to change the way all drivers work with SCSI commands

[Kristian Høgsberg]

On 5/11/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> James Bottomley wrote:
...
> > It strikes me that in all of this, we could also consider doing the DMA
> > mapping inside the mid layer (instead of in every driver).  This is
> > essentially what libata is already doing ... leading to confusion in
> > SCSI drivers that use libata for SATA.
> >
> > So what do people think about this?
>
> The old SBP-2 driver needs the virtual addresses of S/G elements if
> compiled with a certain config option.  This option is meant for use
> with FireWire controllers which do not implement OHCI-1394, or to run
> the IEEE 1394 stack in a more secure mode.   However, sbp2 seems to be
> broken in multiple ways with this option at the moment, and I don't plan
> to fix it anytime soon.
>
> So in short, moving the DMA mapping into mid layer should also be fine
> with the old and the new SBP-2 driver.

Well, if we want to make a more secure mode for fw-sbp2.c where it,
like the old driver, manually copies the payload data from the general
async receive buffer to the destination, we'll still need the virtual
address.  How does the usb storage driver handle this?

Kristian

Re: Proposals to change the way all drivers work with SCSI commands

[Stefan Richter]

Kristian Høgsberg wrote:
> On 5/11/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> James Bottomley wrote:
>> > It strikes me that in all of this, we could also consider doing the DMA
>> > mapping inside the mid layer (instead of in every driver).
...
>> The old SBP-2 driver needs the virtual addresses of S/G elements if
>> compiled with a certain config option.  This option is meant for use
>> with FireWire controllers which do not implement OHCI-1394, or to run
>> the IEEE 1394 stack in a more secure mode.
...
> Well, if we want to make a more secure mode for fw-sbp2.c where it,
> like the old driver, manually copies the payload data from the general
> async receive buffer to the destination, we'll still need the virtual
> address.

Yes, that's right. /If/ we are going to implement such a mode, then (a)
the virtual addresses have to be passed down to ->queuecommand, and (b)
DMA mapping would be unnecessary and wasteful.

(For the list:  By "secure" we mean that we wouldn't allow the FireWire
controller to act as a bus bridge, and would be able to do boundary
checks on local bus addresses which, in the SBP-2 protocol, are
generally coming from the _target_.  Without such a mode, the lower 4G
of the initiator's memory are at the mercy of the target's firmware.)

> How does the usb storage driver handle this?
> 
> Kristian

-- 
Stefan Richter
-=====-=-=== -=-= -=-==
http://arcgraph.de/sr/
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Proposals to change the way all drivers work with SCSI commands

[Muli Ben-Yehuda]

On Fri, May 11, 2007 at 01:33:41PM -0500, James Bottomley wrote:
> Right at the moment, we're planning to clean up the way SCSI drivers
> process commands.  The proposals are essentially:
> 
>      1. Get rid of the now unnecessary map_single path (every command is
>         either zero transfer or scatter/gather)
>      2. use accessors to manipulate the SG lists (mainly so that we can
>         alter the implementation without affecting the drivers)
> 
> It strikes me that in all of this, we could also consider doing the DMA
> mapping inside the mid layer (instead of in every driver).  This is
> essentially what libata is already doing ... leading to confusion in
> SCSI drivers that use libata for SATA.
> 
> So what do people think about this?

Yes please. It will be easier to modify the DMA API interface to
better support hypervisors and other "DMA mapping heavy" users if it's
localized to the mid-layer rather than spread out in drivers.

Cheers,
Muli

Re: Proposals to change the way all drivers work with SCSI commands

[Heiko Carstens]

On Fri, May 11, 2007 at 01:33:41PM -0500, James Bottomley wrote:
> It strikes me that in all of this, we could also consider doing the DMA
> mapping inside the mid layer (instead of in every driver).  This is
> essentially what libata is already doing ... leading to confusion in
> SCSI drivers that use libata for SATA.
> 
> So what do people think about this?

I hope that doesn't mean that each architecture is required to
implement all the DMA mapping stuff? s390 doesn't have any support
for DMA at all. The current workaround seems to be to sprinkle
a lot of BUG() statements in code that simply won't work on such
architectures but still compile it in.
Lately I introduced CONFIG_HAS_DMA and made building
drivers/base/dma-mapping.o and dmapool.o depend on that,
so we don't compile all the non working stuff on s390. Hope
all that can be sorted out if you go that direction.

Re: Proposals to change the way all drivers work with SCSI commands

[James Bottomley]

On Sat, 2007-05-12 at 11:12 +0200, Heiko Carstens wrote:
> I hope that doesn't mean that each architecture is required to
> implement all the DMA mapping stuff?

Yes, it will require this ... or at least dma_map_sg() and
dma_unmap_sg().

>  s390 doesn't have any support
> for DMA at all.

Well ... it does, you just do it differently.  You seem to construct
this fsf thing from sbals that are programmed directly from the kernel
vaddr.

This would all work in the new scenario if you simply defined
dma_map_sg() et al. to be a nop, wouldn't it?

>  The current workaround seems to be to sprinkle
> a lot of BUG() statements in code that simply won't work on such
> architectures but still compile it in.
> Lately I introduced CONFIG_HAS_DMA and made building
> drivers/base/dma-mapping.o and dmapool.o depend on that,
> so we don't compile all the non working stuff on s390. Hope
> all that can be sorted out if you go that direction.

James

Re: Proposals to change the way all drivers work with SCSI commands

[Christoph Hellwig]

On Fri, May 11, 2007 at 01:33:41PM -0500, James Bottomley wrote:
> Right at the moment, we're planning to clean up the way SCSI drivers
> process commands.  The proposals are essentially:
> 
>      1. Get rid of the now unnecessary map_single path (every command is
>         either zero transfer or scatter/gather)
>      2. use accessors to manipulate the SG lists (mainly so that we can
>         alter the implementation without affecting the drivers)
> 
> It strikes me that in all of this, we could also consider doing the DMA
> mapping inside the mid layer (instead of in every driver).  This is
> essentially what libata is already doing ... leading to confusion in
> SCSI drivers that use libata for SATA.
> 
> So what do people think about this?

I don't like it at all.  It means we tange up dma mapping bits into
a layer were they don't belong at all.  We have hbas doing pio, doing
channel programs, mangling dma list before or after the mapping or just
dealing with the commands in kernelspace.  We also have architectures
without dma mapping support.

I'd rather give the drivers proper helpers as in tomo's proposal and
let them handle what should be in their layer.

Btw, Adam Richter already sent a patch doing the dma mapping in
scsi.c back in 2002 and he got shot down for the same reasons, although
I can't find the arguments in the list archives anymore.

Re: Proposals to change the way all drivers work with SCSI commands

[James Bottomley]

On Sat, 2007-05-12 at 16:29 +0100, Christoph Hellwig wrote:
> On Fri, May 11, 2007 at 01:33:41PM -0500, James Bottomley wrote:
> > Right at the moment, we're planning to clean up the way SCSI drivers
> > process commands.  The proposals are essentially:
> > 
> >      1. Get rid of the now unnecessary map_single path (every command is
> >         either zero transfer or scatter/gather)
> >      2. use accessors to manipulate the SG lists (mainly so that we can
> >         alter the implementation without affecting the drivers)
> > 
> > It strikes me that in all of this, we could also consider doing the DMA
> > mapping inside the mid layer (instead of in every driver).  This is
> > essentially what libata is already doing ... leading to confusion in
> > SCSI drivers that use libata for SATA.
> > 
> > So what do people think about this?
> 
> I don't like it at all.  It means we tange up dma mapping bits into
> a layer were they don't belong at all.

Well ... libata already does this ... I don't remember you complaining
about it being a layering violation in libata.

>   We have hbas doing pio, doing
> channel programs, mangling dma list before or after the mapping or just
> dealing with the commands in kernelspace.  We also have architectures
> without dma mapping support.

I agree we have a lot of special cases which need handling ... could we
not get around all of these with a template flag specifying whether the
driver wants the mid-layer to dma_map or not?

> I'd rather give the drivers proper helpers as in tomo's proposal and
> let them handle what should be in their layer.
> 
> Btw, Adam Richter already sent a patch doing the dma mapping in
> scsi.c back in 2002 and he got shot down for the same reasons, although
> I can't find the arguments in the list archives anymore.

In 2002 it really wansn't possible because each bus type had its own dma
mapping functions (or it was possible, but would be extremely ugly).
The generic dma mapping functions take care of that problem.

James

Re: Proposals to change the way all drivers work with SCSI commands

[Christoph Hellwig]

On Sat, May 12, 2007 at 10:50:16AM -0500, James Bottomley wrote:
> > I don't like it at all.  It means we tange up dma mapping bits into
> > a layer were they don't belong at all.
> 
> Well ... libata already does this ... I don't remember you complaining
> about it being a layering violation in libata.

I have in fact never reviewed libata fully, and no I don't like this
in libata either.

> >   We have hbas doing pio, doing
> > channel programs, mangling dma list before or after the mapping or just
> > dealing with the commands in kernelspace.  We also have architectures
> > without dma mapping support.
> 
> I agree we have a lot of special cases which need handling ... could we
> not get around all of these with a template flag specifying whether the
> driver wants the mid-layer to dma_map or not?

We probably could.  But if would be hacky as hell, and we'd have to hack
the core code for every new piece of whacky hardware.  I much prefer
the current clean abstraction where the responsibily is clearly divided.

Just look at the mess the infinibad people created because they put
the dma mapping functionality outside of the drivers.