2002-09-10 05:25:44

by Samium Gromoff

[permalink] [raw]
Subject: [2.5] DAC960

Hello folks, i`m looking at the DAC960 driver and i have
realised its implemented at the block layer, bypassing SCSI.

So given i have some motivation to have a working 2.5 DAC960
driver (i have one, being my only controller)
i`m kinda pondering the matter.

Questions:
1. Whether we need the thing to be ported to SCSI
layer, as opposed to leaving it being a generic block device? (i suppose yes)
2. Which 2.5 SCSI driver should i use as a start of learning?
3. Whether the SCSI driver API would change during 2.5?

---
regards,
Samium Gromoff
____________
________________________________



2002-09-10 06:15:54

by Jens Axboe

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Tue, Sep 10 2002, Samium Gromoff wrote:
> Hello folks, i`m looking at the DAC960 driver and i have
> realised its implemented at the block layer, bypassing SCSI.
>
> So given i have some motivation to have a working 2.5 DAC960
> driver (i have one, being my only controller)
> i`m kinda pondering the matter.
>
> Questions:
> 1. Whether we need the thing to be ported to SCSI
> layer, as opposed to leaving it being a generic block device? (i suppose yes)

No

> 2. Which 2.5 SCSI driver should i use as a start of learning?

Don't bother

> 3. Whether the SCSI driver API would change during 2.5?

Possibly

The DAC960 mainly needs updating to the pci dma api, and to be adjusted
for the bio changes. Please coordinate with Daniel Philips (and check
the list archives, we had a talk about this very driver some weeks ago),
since he's working on making it work in 2.5 again as well.

--
Jens Axboe

2002-09-10 16:11:07

by Dave Olien

[permalink] [raw]
Subject: Re: [2.5] DAC960


I've also been mucking about with the DAC960 driver.
We've got several DAC960 devices in our lab, of
two different versions, I believe.

I've fixed a few minor typos in the version that's in
the kernel.org source. For example, the way the
code references the controller lock doesn't quite work.
The driver uses that lock before the request queue for the
controller has been allocated. So, the controller locking
functions can't reference that lock through the request queue.
I've changed it to reference the lock through the controller
structure directly.

I've been working on the DMA mapping changes. I've been
doing a pretty straight-forward set of changes. As a second
pass, I'm thinking there might be some better ways to manage
the DMA mappings.

I'll post some initial changes at the endof the week, and perhaps
you can give it some review?


On Tue, Sep 10, 2002 at 09:30:28AM +0400, Samium Gromoff wrote:
> Hello folks, i`m looking at the DAC960 driver and i have
> realised its implemented at the block layer, bypassing SCSI.
>
> So given i have some motivation to have a working 2.5 DAC960
> driver (i have one, being my only controller)
> i`m kinda pondering the matter.
>
> Questions:
> 1. Whether we need the thing to be ported to SCSI
> layer, as opposed to leaving it being a generic block device? (i suppose yes)
> 2. Which 2.5 SCSI driver should i use as a start of learning?
> 3. Whether the SCSI driver API would change during 2.5?
>
> ---
> regards,
> Samium Gromoff
> ____________
> ________________________________
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/



On Tue, Sep 10, 2002 at 08:20:30AM +0200, Jens Axboe wrote:
> On Tue, Sep 10 2002, Samium Gromoff wrote:
> > Hello folks, i`m looking at the DAC960 driver and i have
> > realised its implemented at the block layer, bypassing SCSI.
> >
> > So given i have some motivation to have a working 2.5 DAC960
> > driver (i have one, being my only controller)
> > i`m kinda pondering the matter.
> >
> > Questions:
> > 1. Whether we need the thing to be ported to SCSI
> > layer, as opposed to leaving it being a generic block device? (i suppose yes)
>
> No
>
> > 2. Which 2.5 SCSI driver should i use as a start of learning?
>
> Don't bother
>
> > 3. Whether the SCSI driver API would change during 2.5?
>
> Possibly
>
> The DAC960 mainly needs updating to the pci dma api, and to be adjusted
> for the bio changes. Please coordinate with Daniel Philips (and check
> the list archives, we had a talk about this very driver some weeks ago),
> since he's working on making it work in 2.5 again as well.
>
> --
> Jens Axboe
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2002-09-15 04:13:58

by Daniel Phillips

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Tuesday 10 September 2002 08:20, Jens Axboe wrote:
> On Tue, Sep 10 2002, Samium Gromoff wrote:
> > Hello folks, i`m looking at the DAC960 driver and i have
> > realised its implemented at the block layer, bypassing SCSI.
> >
> > So given i have some motivation to have a working 2.5 DAC960
> > driver (i have one, being my only controller)
> > i`m kinda pondering the matter.
> >
> > Questions:
> > 1. Whether we need the thing to be ported to SCSI
> > layer, as opposed to leaving it being a generic block device? (i suppose yes)
>
> No

A somewhat curt reply, it could be seen as a brush-off. I believe the
whole story goes something like this: the scsi system is a festering
sore on the whole and eventually needs to be rationalized. But until
that happens, we should basically just keep nursing along the various
drivers that should be using a generic interface, until there really
is a generic interface around worth putting in the effort to port to.

Linus indicated at the Kernel Summit that he'd like to see a
cleaned-up scsi midlayer used as framework for *all* disk IO,
including IDE. Obviously, what with IDE transitions and whatnot, we
are far from being ready to attempt that, so see "nursing along"
above. There's no longer any chance that a generic disk midlayer is
going to happen in this cycle, as far as I can see. Still, anybody
who is interested would do well by studing the issues, and fixing
broken drivers certainly qualifies as a way to come up to speed.

> > 2. Which 2.5 SCSI driver should i use as a start of learning?
>
> Don't bother

Ah, a little harsh. I'd say: study the DAC960 driver, study the
scsi midlayer, and study the new bio interface. That's what I'm
doing.

> > 3. Whether the SCSI driver API would change during 2.5?
>
> Possibly
>
> The DAC960 mainly needs updating to the pci dma api, and to be adjusted
> for the bio changes. Please coordinate with Daniel Philips (and check
> the list archives, we had a talk about this very driver some weeks ago),
> since he's working on making it work in 2.5 again as well.

Yep, starting with reconfiguring the datacentre over here, so I can
run a serial cable from my wife's machine to the to the closet the
machine with the DAC960 is in, because I'll be dammed if I'm going
to do this without kgdb. Then the serial traffic goes from her
machine over the wireless network to my laptop in the living room,
where I hack in relative comfort. While she surfs and emails. Try
that with Windows :-)

That part took a long time because it certain aging RPM distribution
needed to be replaced by a shiny new Debian Sid, and it had to be
done without taking the machine offline, except to reboot. All I
could wish for now is power cycle over the net, and this will be
civilized.

I suppose my serious work on this will start on Monday.

--
Daniel

2002-09-15 13:14:48

by Jens Axboe

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Sun, Sep 15 2002, Daniel Phillips wrote:
> On Tuesday 10 September 2002 08:20, Jens Axboe wrote:
> > On Tue, Sep 10 2002, Samium Gromoff wrote:
> > > Hello folks, i`m looking at the DAC960 driver and i have
> > > realised its implemented at the block layer, bypassing SCSI.
> > >
> > > So given i have some motivation to have a working 2.5 DAC960
> > > driver (i have one, being my only controller)
> > > i`m kinda pondering the matter.
> > >
> > > Questions:
> > > 1. Whether we need the thing to be ported to SCSI
> > > layer, as opposed to leaving it being a generic block device? (i suppose yes)
> >
> > No
>
> A somewhat curt reply, it could be seen as a brush-off. I believe the
> whole story goes something like this: the scsi system is a festering
> sore on the whole and eventually needs to be rationalized. But until
> that happens, we should basically just keep nursing along the various
> drivers that should be using a generic interface, until there really
> is a generic interface around worth putting in the effort to port to.

Please, the scsi sub system is not a 'festering sore'. Have you even
taken a decent look at it, or just spreading the usual "I heard SCSI
dizzed somwhere" fud? Sure there's room for improvement, 2.5 has in fact
already gotten quite a lot. It's not perfect and there's still stuff
that can be cleaned up, but that doesn't mean it's crap.

> Linus indicated at the Kernel Summit that he'd like to see a
> cleaned-up scsi midlayer used as framework for *all* disk IO,
> including IDE. Obviously, what with IDE transitions and whatnot, we
> are far from being ready to attempt that, so see "nursing along"
> above. There's no longer any chance that a generic disk midlayer is
> going to happen in this cycle, as far as I can see. Still, anybody
> who is interested would do well by studing the issues, and fixing
> broken drivers certainly qualifies as a way to come up to speed.

First of all, I believe Linus' plan is to push more functionality into
the block layer. Generic functionality. And in fact a lot of has already
happened there, but one does need to pay attention to that sort of thing
instead of just assuming spouting fud. Examples:

- queue merge and dma mappings. this is all generic block/bio
functionality now. please compare scsi_merge.c between 2.4 and
2.5, if you care to.

- highmem bounceless operation also added the possibilty to do
isa bouncing generically in 2.5. this is also gone from scsi.

- request tagging. 2.5 has a generic implementation, scsi
transition is not complete though.

that's just off the top of my head.

So where am I going with this? I said "don't bother" to the question of
studying SCSI code, and I stand by that 100%. It would be an absolute
_waste of time_ if the goal is to make dac960 work in 2.5. I believe why
should be pretty obvious now.

Daniel, your (seen before) 'clarification' posts are not.

> > > 2. Which 2.5 SCSI driver should i use as a start of learning?
> >
> > Don't bother
>
> Ah, a little harsh. I'd say: study the DAC960 driver, study the
> scsi midlayer, and study the new bio interface. That's what I'm
> doing.

My advise is: take a look at the transition of other drivers, forget
scsi. Study of bio goes hand in hand with learning from transition of
other drivers. And note that a reasonable update of dac960 should remove
3x as much code as it adds, at least.

I can only note that so far there has been a lot of talk about dac960
and updating it, and that's about it. Talk/code ratio is very very low,
I'm tempted to just do the update myself. Might even safe some time.

--
Jens Axboe

2002-09-15 15:08:09

by Alan

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Sun, 2002-09-15 at 05:21, Daniel Phillips wrote:
> A somewhat curt reply, it could be seen as a brush-off. I believe the
> whole story goes something like this: the scsi system is a festering
> sore on the whole and eventually needs to be rationalized. But until
> that happens, we should basically just keep nursing along the various
> drivers that should be using a generic interface, until there really
> is a generic interface around worth putting in the effort to port to.

DAC960 doesn't present a scsi interface to the higher levels. Its
abstraction truely is block based, like i2o_block, like aacraid, like
many other cards.

2002-09-15 15:15:16

by Daniel Phillips

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Sunday 15 September 2002 15:19, Jens Axboe wrote:
> Please, the scsi sub system is not a 'festering sore'. Have you even
> taken a decent look at it, or just spreading the usual "I heard SCSI
> dizzed somwhere" fud?

You got me. First I haven't looked at it yet (this week though)
and second, I have heard that the scsi mid layer is actually pretty
nice. The "festering sore" part is really the fact we don't use it
for IDE as well.

> Sure there's room for improvement, 2.5 has in fact
> already gotten quite a lot. It's not perfect and there's still stuff
> that can be cleaned up, but that doesn't mean it's crap.

Yup, I know, retracted, back to our regular programming.

> > Linus indicated at the Kernel Summit that he'd like to see a
> > cleaned-up scsi midlayer used as framework for *all* disk IO,
> > including IDE. Obviously, what with IDE transitions and whatnot, we
> > are far from being ready to attempt that, so see "nursing along"
> > above. There's no longer any chance that a generic disk midlayer is
> > going to happen in this cycle, as far as I can see. Still, anybody
> > who is interested would do well by studing the issues, and fixing
> > broken drivers certainly qualifies as a way to come up to speed.
>
> First of all, I believe Linus' plan is to push more functionality into
> the block layer.

I distinctly heard him say he wanted the scsi mid layer repurposed as
an interface for all disks. Maybe he changed his mind?

> Generic functionality. And in fact a lot of has already
> happened there, but one does need to pay attention to that sort of thing
> instead of just assuming spouting fud. Examples:
>
> - queue merge and dma mappings. this is all generic block/bio
> functionality now. please compare scsi_merge.c between 2.4 and
> 2.5, if you care to.
>
> - highmem bounceless operation also added the possibilty to do
> isa bouncing generically in 2.5. this is also gone from scsi.
>
> - request tagging. 2.5 has a generic implementation, scsi
> transition is not complete though.
>
> that's just off the top of my head.

Ah, we could use more summaries like that.

Well, you're looking at the situation from the block IO maintainer's
point of view. We also need certain scsi-style functionality available
across all disk systems, such as barriers in a form usable by
filesystems. Maybe you *can* suck all that functionality into the
block layer, but maybe it needs higher level support as well. I'll
bet on the latter, and I won't have to speculate, pretty soon.

> So where am I going with this? I said "don't bother" to the question of
> studying SCSI code, and I stand by that 100%. It would be an absolute
> _waste of time_ if the goal is to make dac960 work in 2.5. I believe why
> should be pretty obvious now.

My own goal is certainly not limited to making the thing work. I need
to *understand* these systems, because they are important to what I'm
doing and right now they are a big, black hole somewhere over in the
south end of the kernel.

> Daniel, your (seen before) 'clarification' posts are not.

Jens, we just got a whole lot clearer than your "no, don't bother,
possibly" post. Such posts may feel good to write, but they're not
very helpful.

> I can only note that so far there has been a lot of talk about dac960
> and updating it, and that's about it. Talk/code ratio is very very low,
> I'm tempted to just do the update myself. Might even safe some time.

Counts as more talk ;-)

--
Daniel

2002-09-15 16:10:31

by Daniel Phillips

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Sunday 15 September 2002 17:23, Daniel Phillips wrote:
> On Sunday 15 September 2002 15:19, Jens Axboe wrote:
> Well, you're looking at the situation from the block IO maintainer's
> point of view. We also need certain scsi-style functionality available
> across all disk systems, such as barriers in a form usable by
> filesystems. Maybe you *can* suck all that functionality into the
> block layer, but maybe it needs higher level support as well. I'll
> bet on the latter, and I won't have to speculate, pretty soon.

Oops, sorry, this is inverted, the block layer is the higher of the
two. Anyway, that's what you get for making a VM hacker fix the
driver you broke ;-)

The Q of this signal will start increasing pretty soon (got to move
some cables and boxes around next. Got to fix up Theo's PAM breakage
so ssh works again. Got to point LXR at bio.)

Anybody interested in working through this learning curve along with
me:

http://lxr.linux.no/source/drivers/block/DAC960.c?v=2.5.31
(The DAC960 driver in all 6921 lines of funky glory)

http://lxr.linux.no/source/include/linux/bio.h?v=2.5.31#L60
(the bio struct)

http://lxr.linux.no/ident?v=2.5.31;i=bio
(everwhere it's used)

http://lxr.linux.no/source/Documentation/block/biodoc.txt?v=2.5.31
(Suparna's excellent bio writeup)

By the way, the gentlemanly thing to do would be to put Suparna first in
the list of authors of the writeup since 90% of the text is hers. On
the other hand, you're being a little too humble calling this a "bio
rewrite" - it's not, it's a new animal, or at least, it's a wholesale
block layer replacement.

--
Daniel

2002-09-15 16:13:08

by Daniel Phillips

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Sunday 15 September 2002 17:14, Alan Cox wrote:
> On Sun, 2002-09-15 at 05:21, Daniel Phillips wrote:
> > A somewhat curt reply, it could be seen as a brush-off. I believe the
> > whole story goes something like this: the scsi system is a festering
> > sore on the whole and eventually needs to be rationalized. But until
> > that happens, we should basically just keep nursing along the various
> > drivers that should be using a generic interface, until there really
> > is a generic interface around worth putting in the effort to port to.
>
> DAC960 doesn't present a scsi interface to the higher levels. Its
> abstraction truely is block based, like i2o_block, like aacraid, like
> many other cards.

Yup, brainlock, I knew that. Just bracketing the target...

--
Daniel

2002-09-16 20:08:47

by Dave Olien

[permalink] [raw]
Subject: Re: [2.5] DAC960

>
> I can only note that so far there has been a lot of talk about dac960
> and updating it, and that's about it. Talk/code ratio is very very low,
> I'm tempted to just do the update myself. Might even safe some time.
>
> --
> Jens Axboe
>
> -

I have the DAC960 driver working in 2.5.34. The work isn't
complete yet. But, I'm able to boot, and do mke2fs
on partitions on logical drives, and then do e2fsck
on those partitions. It seems to work, although more
testing is required. Is there any interest in reviewing
the code so far, or should I continue testing and complete
the remaining issues first?

Here's what I've done:

1. I've removed all references to struct bio
(or BufferHeader_T) from the code. I'm using the
blk_rq_map_sg() function to create scatter lists
from I/O request structures. Then, using pci_map_sg
to produce DMA maps for those scatter/gather lists.

I'm using "end_that_request_first" and
"end_that_request_last" to do I/O request completions.


2. I've placed the DAC960's ScatterGatherList arrays into
dma-mapped memory. I'm currently using pci_pool_create()
for esablishing these maps. These are "consisent" maps.
Should these be "streaming maps"? For x86, it doesn't make
any difference, but...

3. I've placed the DAC960 RequestSense structures into dma-mapped
memory. Again, I'm using pci_pool_create() to do this.

At the moment, I think I have some kind of problem writing
new partitions to a logical disk. It seems if the system
comes up with all the logical disks already partitioned, then
I can work with them OK. If I try to repartition a logical
disk with the new driver, the logical drive seems to be
unpartitioned afterwards. I haven't completely understood this
problem yet. This is what I'm working on now.

Before going on with the next steps, I'll probably do more
vigorous testing.


Work to do:

I've temporarliy put back the virt_to_bus and bus_to_virt
calls. The remaining steps will get rid of those.

1. Establish dma maps for the DAC960 command mailbox.
I'll establish consistent maps for them.

The existing code here is a little odd.
The function DAC960_V1_EnableMemoryMailboxInterface()
should call any of the RestoreMemoryMailboxInfo()
functions. It seems to be trying to retrieve
from the controller old pointers to memory
mailboxes. But I can't imagine cirumstances
where this would work in the context of the current
driver.


2. Establish dma maps for the sizeable collection of
status and monitoring operations.

3. I have an issue with the read/write retry commands.
I think I know how to do it, but just need to code
it and test it. Since I don't have any
disks with media errors, I'll probably use some kind
of fault insertion to test this.

4. I need to add calls to pci_dma_sync*() in the approriate
places. This function doesn't do anything on the current
x86 platform. So, I'll put these in place after getting
things to work.

2002-09-16 20:21:34

by Daniel Phillips

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Monday 16 September 2002 22:13, Dave Olien wrote:
> >
> > I can only note that so far there has been a lot of talk about dac960
> > and updating it, and that's about it. Talk/code ratio is very very low,
> > I'm tempted to just do the update myself. Might even safe some time.
> >
> > --
> > Jens Axboe
> >
> > -
>
> I have the DAC960 driver working in 2.5.34. The work isn't
> complete yet. But, I'm able to boot, and do mke2fs
> on partitions on logical drives, and then do e2fsck
> on those partitions. It seems to work, although more
> testing is required. Is there any interest in reviewing
> the code so far, or should I continue testing and complete
> the remaining issues first?

Please post the patch, I'll try it right away.

--
Daniel

2002-09-16 22:03:24

by Dave Olien

[permalink] [raw]
Subject: Re: [2.5] DAC960

>
> Please post the patch, I'll try it right away.
>
> --
> Daniel

Here's what I have so far. Just don't try
to create a new disk partition using this patch.
Just use the partitions that already exist on the
logical drives.


diff -ur linux-2.5.34_original/drivers/block/DAC960.c linux-2.5.34_patch/drivers/block/DAC960.c
--- linux-2.5.34_original/drivers/block/DAC960.c Mon Sep 16 14:34:12 2002
+++ linux-2.5.34_patch/drivers/block/DAC960.c Mon Sep 16 14:59:14 2002
@@ -156,15 +156,42 @@
int CommandAllocationLength, CommandAllocationGroupSize;
int CommandsRemaining = 0, CommandIdentifier, CommandGroupByteCount;
void *AllocationPointer = NULL;
+ void *ScatterGatherCPU = NULL;
+ dma_addr_t ScatterGatherDMA;
+ struct pci_pool *ScatterGatherPool;
+ void *RequestSenseCPU = NULL;
+ dma_addr_t RequestSenseDMA;
+ struct pci_pool *RequestSensePool = NULL;
+
if (Controller->FirmwareType == DAC960_V1_Controller)
{
CommandAllocationLength = offsetof(DAC960_Command_T, V1.EndMarker);
CommandAllocationGroupSize = DAC960_V1_CommandAllocationGroupSize;
+ ScatterGatherPool = pci_pool_create("DAC960_V1_ScatterGather",
+ Controller->PCIDevice,
+ DAC960_V1_ScatterGatherLimit * sizeof(DAC960_V1_ScatterGatherSegment_T),
+ sizeof(DAC960_V1_ScatterGatherSegment_T), 0, SLAB_ATOMIC);
+ if (ScatterGatherPool == NULL)
+ return DAC960_Failure(Controller,
+ "AUXILIARY STRUCTURE CREATION (SG)");
+ Controller->ScatterGatherPool = ScatterGatherPool;
}
else
{
CommandAllocationLength = offsetof(DAC960_Command_T, V2.EndMarker);
CommandAllocationGroupSize = DAC960_V2_CommandAllocationGroupSize;
+ ScatterGatherPool = pci_pool_create("DAC960_V2_ScatterGather",
+ Controller->PCIDevice,
+ DAC960_V2_ScatterGatherLimit * sizeof(DAC960_V2_ScatterGatherSegment_T),
+ sizeof(DAC960_V2_ScatterGatherSegment_T), 0, SLAB_ATOMIC);
+ RequestSensePool = pci_pool_create("DAC960_V2_RequestSense",
+ Controller->PCIDevice, sizeof(DAC960_SCSI_RequestSense_T),
+ sizeof(int), 0, SLAB_ATOMIC);
+ if (ScatterGatherPool == NULL || RequestSensePool == NULL)
+ return DAC960_Failure(Controller,
+ "AUXILIARY STRUCTURE CREATION (SG)");
+ Controller->ScatterGatherPool = ScatterGatherPool;
+ Controller->V2.RequestSensePool = RequestSensePool;
}
Controller->CommandAllocationGroupSize = CommandAllocationGroupSize;
Controller->FreeCommands = NULL;
@@ -176,16 +203,17 @@
if (--CommandsRemaining <= 0)
{
CommandsRemaining =
- Controller->DriverQueueDepth - CommandIdentifier + 1;
+ Controller->DriverQueueDepth - CommandIdentifier + 1;
if (CommandsRemaining > CommandAllocationGroupSize)
- CommandsRemaining = CommandAllocationGroupSize;
+ CommandsRemaining = CommandAllocationGroupSize;
CommandGroupByteCount =
- CommandsRemaining * CommandAllocationLength;
+ CommandsRemaining * CommandAllocationLength;
AllocationPointer = kmalloc(CommandGroupByteCount, GFP_ATOMIC);
if (AllocationPointer == NULL)
- return DAC960_Failure(Controller, "AUXILIARY STRUCTURE CREATION");
+ return DAC960_Failure(Controller,
+ "AUXILIARY STRUCTURE CREATION");
memset(AllocationPointer, 0, CommandGroupByteCount);
- }
+ }
Command = (DAC960_Command_T *) AllocationPointer;
AllocationPointer += CommandAllocationLength;
Command->CommandIdentifier = CommandIdentifier;
@@ -193,6 +221,33 @@
Command->Next = Controller->FreeCommands;
Controller->FreeCommands = Command;
Controller->Commands[CommandIdentifier-1] = Command;
+ ScatterGatherCPU = pci_pool_alloc(ScatterGatherPool, SLAB_ATOMIC,
+ &ScatterGatherDMA);
+ if (ScatterGatherCPU == NULL)
+ return DAC960_Failure(Controller, "AUXILIARY STRUCTURE CREATION");
+
+ if (RequestSensePool != NULL) {
+ RequestSenseCPU = pci_pool_alloc(RequestSensePool, SLAB_ATOMIC,
+ &RequestSenseDMA);
+ if (RequestSenseCPU == NULL) {
+ pci_pool_free(ScatterGatherPool, ScatterGatherCPU,
+ ScatterGatherDMA);
+ return DAC960_Failure(Controller,
+ "AUXILIARY STRUCTURE CREATION");
+ }
+ }
+ if (Controller->FirmwareType == DAC960_V1_Controller) {
+ Command->V1.ScatterGatherList =
+ (DAC960_V1_ScatterGatherSegment_T *)ScatterGatherCPU;
+ Command->V1.ScatterGatherListDMA = ScatterGatherDMA;
+ } else {
+ Command->V2.ScatterGatherList =
+ (DAC960_V2_ScatterGatherSegment_T *)ScatterGatherCPU;
+ Command->V2.ScatterGatherListDMA = ScatterGatherDMA;
+ Command->V2.RequestSense =
+ (DAC960_SCSI_RequestSense_T *)RequestSenseCPU;
+ Command->V2.RequestSenseDMA = RequestSenseDMA;
+ }
}
return true;
}
@@ -206,16 +261,57 @@
static void DAC960_DestroyAuxiliaryStructures(DAC960_Controller_T *Controller)
{
int i;
+ struct pci_pool *ScatterGatherPool;
+ struct pci_pool *RequestSensePool;
+ void *ScatterGatherCPU;
+ dma_addr_t ScatterGatherDMA;
+ void *RequestSenseCPU;
+ dma_addr_t RequestSenseDMA = 0;
+ DAC960_Command_T *CommandGroup = NULL;
+
+ ScatterGatherPool = Controller->ScatterGatherPool;
+ if (Controller->FirmwareType == DAC960_V1_Controller)
+ RequestSensePool = Controller->V2.RequestSensePool;
+
Controller->FreeCommands = NULL;
+ RequestSenseCPU = NULL;
for (i = 0; i < Controller->DriverQueueDepth; i++)
{
DAC960_Command_T *Command = Controller->Commands[i];
- if (Command != NULL &&
- (Command->CommandIdentifier
- % Controller->CommandAllocationGroupSize) == 1)
- kfree(Command);
+
+ if (Command == NULL)
+ continue;
+
+ if (Controller->FirmwareType == DAC960_V1_Controller) {
+ ScatterGatherCPU = (void *)Command->V1.ScatterGatherList;
+ ScatterGatherDMA = Command->V1.ScatterGatherListDMA;
+ } else {
+ ScatterGatherCPU = (void *)Command->V2.ScatterGatherList;
+ ScatterGatherDMA = Command->V2.ScatterGatherListDMA;
+ RequestSenseCPU = (void *)Command->V2.RequestSense;
+ RequestSenseDMA = Command->V2.RequestSenseDMA;
+ }
+ if (ScatterGatherCPU != NULL)
+ pci_pool_free(ScatterGatherPool, ScatterGatherCPU, ScatterGatherDMA);
+ if (RequestSenseCPU != NULL)
+ pci_pool_free(ScatterGatherPool, RequestSenseCPU, RequestSenseDMA);
+
+ if ((Command->CommandIdentifier
+ % Controller->CommandAllocationGroupSize) == 1) {
+ /*
+ * We can't free the group of commands until all of the
+ * request sense and scatter gather dma structures are free.
+ * Remember the beginning of the group, but don't free it
+ * until we've reached the beginning of the next group.
+ */
+ if (CommandGroup != NULL)
+ kfree(CommandGroup);
+ CommandGroup = Command;
+ }
Controller->Commands[i] = NULL;
}
+ if (CommandGroup != NULL)
+ kfree(CommandGroup);
if (Controller->CombinedStatusBuffer != NULL)
{
kfree(Controller->CombinedStatusBuffer);
@@ -297,6 +393,8 @@
static inline void DAC960_DeallocateCommand(DAC960_Command_T *Command)
{
DAC960_Controller_T *Controller = Command->Controller;
+
+ Command->Request = NULL;
Command->Next = Controller->FreeCommands;
Controller->FreeCommands = Command;
}
@@ -308,9 +406,9 @@

static void DAC960_WaitForCommand(DAC960_Controller_T *Controller)
{
- spin_unlock_irq(Controller->RequestQueue->queue_lock);
+ spin_unlock_irq(&Controller->queue_lock);
__wait_event(Controller->CommandWaitQueue, Controller->FreeCommands);
- spin_lock_irq(Controller->RequestQueue->queue_lock);
+ spin_lock_irq(&Controller->queue_lock);
}


@@ -1932,7 +2030,6 @@
int MajorNumber = DAC960_MAJOR + Controller->ControllerNumber;
char *names;
RequestQueue_T *RequestQueue;
- int MinorNumber;
int n;

names = kmalloc(9 * DAC960_MaxLogicalDrives, GFP_KERNEL);
@@ -1955,7 +2052,6 @@
Initialize the I/O Request Queue.
*/
RequestQueue = BLK_DEFAULT_QUEUE(MajorNumber);
- Controller->queue_lock = SPIN_LOCK_UNLOCKED;
blk_init_queue(RequestQueue, DAC960_RequestFunction, &Controller->queue_lock);
RequestQueue->queuedata = Controller;
blk_queue_max_hw_segments(RequestQueue,
@@ -1991,10 +2087,11 @@
{
int MajorNumber = DAC960_MAJOR + Controller->ControllerNumber;
int disk;
- for (disk = 0; disk < DAC960_MaxLogicalDrives; disk++) {
+
+ for (disk = 0; disk < DAC960_MaxLogicalDrives; disk++)
del_gendisk(&Controller->disks[disk]);
- kfree(Controller->disks[0].major_name);
- }
+
+ kfree(Controller->disks[0].major_name);
/*
Unregister the Block Device Major Number for this DAC960 Controller.
*/
@@ -2218,6 +2315,7 @@
Controller->ControllerNumber = DAC960_ControllerCount;
init_waitqueue_head(&Controller->CommandWaitQueue);
init_waitqueue_head(&Controller->HealthStatusWaitQueue);
+ Controller->queue_lock = SPIN_LOCK_UNLOCKED;
DAC960_Controllers[DAC960_ControllerCount++] = Controller;
DAC960_AnnounceDriver(Controller);
Controller->FirmwareType = FirmwareType;
@@ -2676,57 +2774,60 @@
{
DAC960_Controller_T *Controller = Command->Controller;
DAC960_V1_CommandMailbox_T *CommandMailbox = &Command->V1.CommandMailbox;
+ DAC960_V1_ScatterGatherSegment_T *ScatterGatherList =
+ Command->V1.ScatterGatherList;
+ struct scatterlist *ScatterList = Command->V1.ScatterList;
+ int DmaDirection, SegCount;
+
DAC960_V1_ClearCommand(Command);
- if (Command->SegmentCount == 1)
+
+ if (Command->CommandType == DAC960_ReadCommand)
+ DmaDirection = PCI_DMA_FROMDEVICE;
+ else
+ DmaDirection = PCI_DMA_TODEVICE;
+
+ SegCount = blk_rq_map_sg(Controller->RequestQueue, Command->Request,
+ ScatterList);
+ /* pci_map_sg MAY change the value of SegCount */
+ SegCount = pci_map_sg(Command->PciDevice, ScatterList, SegCount,
+ DmaDirection);
+ Command->SegmentCount = SegCount;
+
+ if (SegCount == 1)
{
if (Command->CommandType == DAC960_ReadCommand)
CommandMailbox->Type5.CommandOpcode = DAC960_V1_Read;
- else CommandMailbox->Type5.CommandOpcode = DAC960_V1_Write;
+ else
+ CommandMailbox->Type5.CommandOpcode = DAC960_V1_Write;
+
CommandMailbox->Type5.LD.TransferLength = Command->BlockCount;
CommandMailbox->Type5.LD.LogicalDriveNumber = Command->LogicalDriveNumber;
CommandMailbox->Type5.LogicalBlockAddress = Command->BlockNumber;
CommandMailbox->Type5.BusAddress =
- Virtual_to_Bus32(Command->RequestBuffer);
+ (DAC960_BusAddress32_T)sg_dma_address(ScatterList);
}
else
{
- DAC960_V1_ScatterGatherSegment_T
- *ScatterGatherList = Command->V1.ScatterGatherList;
- BufferHeader_T *BufferHeader = Command->BufferHeader;
- char *LastDataEndPointer = NULL;
- int SegmentNumber = 0;
+ int i;
+
if (Command->CommandType == DAC960_ReadCommand)
CommandMailbox->Type5.CommandOpcode = DAC960_V1_ReadWithScatterGather;
else
CommandMailbox->Type5.CommandOpcode = DAC960_V1_WriteWithScatterGather;
+
CommandMailbox->Type5.LD.TransferLength = Command->BlockCount;
CommandMailbox->Type5.LD.LogicalDriveNumber = Command->LogicalDriveNumber;
CommandMailbox->Type5.LogicalBlockAddress = Command->BlockNumber;
- CommandMailbox->Type5.BusAddress = Virtual_to_Bus32(ScatterGatherList);
- CommandMailbox->Type5.ScatterGatherCount = Command->SegmentCount;
- while (BufferHeader != NULL)
- {
- if (bio_data(BufferHeader) == LastDataEndPointer)
- {
- ScatterGatherList[SegmentNumber-1].SegmentByteCount +=
- BufferHeader->bi_size;
- LastDataEndPointer += BufferHeader->bi_size;
- }
- else
- {
- ScatterGatherList[SegmentNumber].SegmentDataPointer =
- Virtual_to_Bus32(bio_data(BufferHeader));
- ScatterGatherList[SegmentNumber].SegmentByteCount =
- BufferHeader->bi_size;
- LastDataEndPointer = bio_data(BufferHeader) +
- BufferHeader->bi_size;
- if (SegmentNumber++ > Controller->DriverScatterGatherLimit)
- panic("DAC960: Scatter/Gather Segment Overflow\n");
- }
- BufferHeader = BufferHeader->bi_next;
- }
- if (SegmentNumber != Command->SegmentCount)
- panic("DAC960: SegmentNumber != SegmentCount\n");
+ CommandMailbox->Type5.BusAddress = Command->V1.ScatterGatherListDMA;
+
+ CommandMailbox->Type5.ScatterGatherCount = SegCount;
+
+ for (i = 0; i < SegCount; i++, ScatterList++, ScatterGatherList++) {
+ ScatterGatherList->SegmentDataPointer =
+ (DAC960_BusAddress32_T)sg_dma_address(ScatterList);
+ ScatterGatherList->SegmentByteCount =
+ (DAC960_ByteCount32_T)sg_dma_len(ScatterList);
+ }
}
DAC960_QueueCommand(Command);
}
@@ -2741,18 +2842,32 @@
{
DAC960_Controller_T *Controller = Command->Controller;
DAC960_V2_CommandMailbox_T *CommandMailbox = &Command->V2.CommandMailbox;
+ struct scatterlist *ScatterList = Command->V2.ScatterList;
+ int DmaDirection, SegCount;
+
DAC960_V2_ClearCommand(Command);
+
+ if (Command->CommandType == DAC960_ReadCommand)
+ DmaDirection = PCI_DMA_FROMDEVICE;
+ else
+ DmaDirection = PCI_DMA_TODEVICE;
+
+ SegCount = blk_rq_map_sg(Controller->RequestQueue, Command->Request,
+ ScatterList);
+ /* pci_map_sg MAY change the value of SegCount */
+ SegCount = pci_map_sg(Command->PciDevice, ScatterList, SegCount,
+ DmaDirection);
+ Command->SegmentCount = SegCount;
+
CommandMailbox->SCSI_10.CommandOpcode = DAC960_V2_SCSI_10;
CommandMailbox->SCSI_10.CommandControlBits.DataTransferControllerToHost =
(Command->CommandType == DAC960_ReadCommand);
CommandMailbox->SCSI_10.DataTransferSize =
Command->BlockCount << DAC960_BlockSizeBits;
- CommandMailbox->SCSI_10.RequestSenseBusAddress =
- Virtual_to_Bus64(&Command->V2.RequestSense);
+ CommandMailbox->SCSI_10.RequestSenseBusAddress = Command->V2.RequestSenseDMA;
CommandMailbox->SCSI_10.PhysicalDevice =
Controller->V2.LogicalDriveToVirtualDevice[Command->LogicalDriveNumber];
- CommandMailbox->SCSI_10.RequestSenseSize =
- sizeof(DAC960_SCSI_RequestSense_T);
+ CommandMailbox->SCSI_10.RequestSenseSize = sizeof(DAC960_SCSI_RequestSense_T);
CommandMailbox->SCSI_10.CDBLength = 10;
CommandMailbox->SCSI_10.SCSI_CDB[0] =
(Command->CommandType == DAC960_ReadCommand ? 0x28 : 0x2A);
@@ -2762,12 +2877,13 @@
CommandMailbox->SCSI_10.SCSI_CDB[5] = Command->BlockNumber;
CommandMailbox->SCSI_10.SCSI_CDB[7] = Command->BlockCount >> 8;
CommandMailbox->SCSI_10.SCSI_CDB[8] = Command->BlockCount;
- if (Command->SegmentCount == 1)
+
+ if (SegCount == 1)
{
CommandMailbox->SCSI_10.DataTransferMemoryAddress
.ScatterGatherSegments[0]
.SegmentDataPointer =
- Virtual_to_Bus64(Command->RequestBuffer);
+ (DAC960_BusAddress64_T)sg_dma_address(ScatterList);
CommandMailbox->SCSI_10.DataTransferMemoryAddress
.ScatterGatherSegments[0]
.SegmentByteCount =
@@ -2775,49 +2891,30 @@
}
else
{
- DAC960_V2_ScatterGatherSegment_T
- *ScatterGatherList = Command->V2.ScatterGatherList;
- BufferHeader_T *BufferHeader = Command->BufferHeader;
- char *LastDataEndPointer = NULL;
- int SegmentNumber = 0;
- if (Command->SegmentCount > 2)
+ DAC960_V2_ScatterGatherSegment_T *ScatterGatherList;
+ int i;
+
+ if (SegCount > 2)
{
+ ScatterGatherList = Command->V2.ScatterGatherList;
CommandMailbox->SCSI_10.CommandControlBits
.AdditionalScatterGatherListMemory = true;
CommandMailbox->SCSI_10.DataTransferMemoryAddress
- .ExtendedScatterGather.ScatterGatherList0Length =
- Command->SegmentCount;
+ .ExtendedScatterGather.ScatterGatherList0Length = SegCount;
CommandMailbox->SCSI_10.DataTransferMemoryAddress
.ExtendedScatterGather.ScatterGatherList0Address =
- Virtual_to_Bus64(ScatterGatherList);
+ Command->V2.ScatterGatherListDMA;
}
else
- ScatterGatherList =
- CommandMailbox->SCSI_10.DataTransferMemoryAddress
+ ScatterGatherList = CommandMailbox->SCSI_10.DataTransferMemoryAddress
.ScatterGatherSegments;
- while (BufferHeader != NULL)
- {
- if (bio_data(BufferHeader) == LastDataEndPointer)
- {
- ScatterGatherList[SegmentNumber-1].SegmentByteCount +=
- BufferHeader->bi_size;
- LastDataEndPointer += BufferHeader->bi_size;
- }
- else
- {
- ScatterGatherList[SegmentNumber].SegmentDataPointer =
- Virtual_to_Bus64(bio_data(BufferHeader));
- ScatterGatherList[SegmentNumber].SegmentByteCount =
- BufferHeader->bi_size;
- LastDataEndPointer = bio_data(BufferHeader) +
- BufferHeader->bi_size;
- if (SegmentNumber++ > Controller->DriverScatterGatherLimit)
- panic("DAC960: Scatter/Gather Segment Overflow\n");
- }
- BufferHeader = BufferHeader->bi_next;
- }
- if (SegmentNumber != Command->SegmentCount)
- panic("DAC960: SegmentNumber != SegmentCount\n");
+
+ for (i = 0; i < SegCount; i++, ScatterList++, ScatterGatherList++) {
+ ScatterGatherList->SegmentDataPointer =
+ (DAC960_BusAddress64_T)sg_dma_address(ScatterList);
+ ScatterGatherList->SegmentByteCount =
+ (DAC960_ByteCount64_T)sg_dma_len(ScatterList);
+ }
}
DAC960_QueueCommand(Command);
}
@@ -2834,32 +2931,36 @@
boolean WaitForCommand)
{
RequestQueue_T *RequestQueue = Controller->RequestQueue;
- ListHead_T *RequestQueueHead;
IO_Request_T *Request;
DAC960_Command_T *Command;
- if (RequestQueue == NULL) return false;
- RequestQueueHead = &RequestQueue->queue_head;
- while (true)
- {
- if (list_empty(RequestQueueHead)) return false;
+
+ if (RequestQueue == NULL)
+ return false;
+
+ while (true) {
+ if (blk_queue_empty(RequestQueue))
+ return false;
+
Request = elv_next_request(RequestQueue);
Command = DAC960_AllocateCommand(Controller);
- if (Command != NULL) break;
- if (!WaitForCommand) return false;
+ if (Command != NULL)
+ break;
+
+ if (!WaitForCommand)
+ return false;
+
DAC960_WaitForCommand(Controller);
- }
- if (Request->cmd == READ)
+ }
+ if (rq_data_dir(Request) == READ)
Command->CommandType = DAC960_ReadCommand;
- else Command->CommandType = DAC960_WriteCommand;
+ else
+ Command->CommandType = DAC960_WriteCommand;
Command->Completion = Request->waiting;
Command->LogicalDriveNumber = DAC960_LogicalDriveNumber(Request->rq_dev);
Command->BlockNumber = Request->sector;
Command->BlockCount = Request->nr_sectors;
- Command->SegmentCount = Request->nr_phys_segments;
- Command->BufferHeader = Request->bio;
- Command->RequestBuffer = Request->buffer;
+ Command->Request = Request;
blkdev_dequeue_request(Request);
- blk_put_request(Request);
DAC960_QueueReadWriteCommand(Command);
return true;
}
@@ -2906,18 +3007,50 @@
individual Buffer.
*/

-static inline void DAC960_ProcessCompletedBuffer(BufferHeader_T *BufferHeader,
+static inline void DAC960_ProcessCompletedRequest(DAC960_Command_T *Command,
boolean SuccessfulIO)
{
- if (SuccessfulIO)
- set_bit(BIO_UPTODATE, &BufferHeader->bi_flags);
- blk_finished_io(bio_sectors(BufferHeader));
- BufferHeader->bi_end_io(BufferHeader);
+ DAC960_CommandType_T CommandType = Command->CommandType;
+ IO_Request_T *Request = Command->Request;
+ int DmaDirection;
+ int UpToDate;
+
+ UpToDate = 0;
+ if (SuccessfulIO == true)
+ UpToDate = 1;
+
+ /*
+ * We could save DmaDirection in the command structure
+ * and just reuse that information here.
+ */
+ if (CommandType == DAC960_ReadCommand ||
+ CommandType == DAC960_ReadRetryCommand)
+ DmaDirection = PCI_DMA_FROMDEVICE;
+ else
+ DmaDirection = PCI_DMA_TODEVICE;
+
+ pci_unmap_sg(Command->PciDevice, Command->V1.ScatterList,
+ Command->SegmentCount, DmaDirection);
+ /*
+ * BlockCount is redundant with nr_sectors in the request
+ * structure. Consider eliminating BlockCount from the
+ * command structure now that Command includes a pointer to
+ * the request.
+ */
+ while (end_that_request_first(Request, UpToDate, Command->BlockCount));
+
+ end_that_request_last(Request);
+
+ if (Command->Completion != NULL)
+ {
+ complete(Command->Completion);
+ Command->Completion = NULL;
+ }
}

static inline int DAC960_PartitionByCommand(DAC960_Command_T *Command)
{
- return DAC960_PartitionNumber(to_kdev_t(Command->BufferHeader->bi_bdev->bd_dev));
+ return DAC960_PartitionNumber(Command->Request->rq_dev);
}

/*
@@ -2975,8 +3108,8 @@
Controller, Controller->ControllerNumber,
Command->LogicalDriveNumber,
DAC960_PartitionByCommand(Command),
- Command->BufferHeader->bi_sector,
- Command->BufferHeader->bi_sector + Command->BlockCount - 1);
+ Command->Request->sector,
+ Command->Request->sector + Command->BlockCount - 1);
}


@@ -2992,106 +3125,50 @@
DAC960_V1_CommandOpcode_T CommandOpcode =
Command->V1.CommandMailbox.Common.CommandOpcode;
DAC960_V1_CommandStatus_T CommandStatus = Command->V1.CommandStatus;
- BufferHeader_T *BufferHeader = Command->BufferHeader;
+
if (CommandType == DAC960_ReadCommand ||
CommandType == DAC960_WriteCommand)
{
if (CommandStatus == DAC960_V1_NormalCompletion)
+
+ DAC960_ProcessCompletedRequest(Command, true);
+
+ else if (CommandStatus == DAC960_V1_IrrecoverableDataError ||
+ CommandStatus == DAC960_V1_BadDataEncountered)
{
+
/*
- Perform completion processing for all buffers in this I/O Request.
- */
- while (BufferHeader != NULL)
- {
- BufferHeader_T *NextBufferHeader = BufferHeader->bi_next;
- BufferHeader->bi_next = NULL;
- DAC960_ProcessCompletedBuffer(BufferHeader, true);
- BufferHeader = NextBufferHeader;
- }
- if (Command->Completion != NULL)
- {
- complete(Command->Completion);
- Command->Completion = NULL;
- }
- add_blkdev_randomness(DAC960_MAJOR + Controller->ControllerNumber);
- }
- else if ((CommandStatus == DAC960_V1_IrrecoverableDataError ||
- CommandStatus == DAC960_V1_BadDataEncountered) &&
- BufferHeader != NULL &&
- BufferHeader->bi_next != NULL)
- {
- DAC960_V1_CommandMailbox_T *CommandMailbox =
- &Command->V1.CommandMailbox;
- if (CommandType == DAC960_ReadCommand)
- {
- Command->CommandType = DAC960_ReadRetryCommand;
- CommandMailbox->Type5.CommandOpcode = DAC960_V1_Read;
- }
- else
- {
- Command->CommandType = DAC960_WriteRetryCommand;
- CommandMailbox->Type5.CommandOpcode = DAC960_V1_Write;
- }
- Command->BlockCount = BufferHeader->bi_size >> DAC960_BlockSizeBits;
- CommandMailbox->Type5.LD.TransferLength = Command->BlockCount;
- CommandMailbox->Type5.BusAddress =
- Virtual_to_Bus32(bio_data(BufferHeader));
- DAC960_QueueCommand(Command);
- return;
+ * Finish this later.
+ *
+ * We should call "complete_that_request_first()"
+ * to remove the first part of the request. Then, if there
+ * is still more I/O to be done, resubmit the request.
+ *
+ * We want to recalculate scatter/gather list,
+ * and requeue the command.
+ *
+ * For now, print a message on the console, and clone
+ * the code for "normal" completion.
+ */
+ printk("V1_ProcessCompletedCommand: I/O error on read/write\n");
+
+ DAC960_ProcessCompletedRequest(Command, false);
}
else
{
if (CommandStatus != DAC960_V1_LogicalDriveNonexistentOrOffline)
DAC960_V1_ReadWriteError(Command);
- /*
- Perform completion processing for all buffers in this I/O Request.
- */
- while (BufferHeader != NULL)
- {
- BufferHeader_T *NextBufferHeader = BufferHeader->bi_next;
- BufferHeader->bi_next = NULL;
- DAC960_ProcessCompletedBuffer(BufferHeader, false);
- BufferHeader = NextBufferHeader;
- }
- if (Command->Completion != NULL)
- {
- complete(Command->Completion);
- Command->Completion = NULL;
- }
+
+ DAC960_ProcessCompletedRequest(Command, false);
}
}
else if (CommandType == DAC960_ReadRetryCommand ||
CommandType == DAC960_WriteRetryCommand)
{
- BufferHeader_T *NextBufferHeader = BufferHeader->bi_next;
- BufferHeader->bi_next = NULL;
- /*
- Perform completion processing for this single buffer.
- */
- if (CommandStatus == DAC960_V1_NormalCompletion)
- DAC960_ProcessCompletedBuffer(BufferHeader, true);
- else
- {
- if (CommandStatus != DAC960_V1_LogicalDriveNonexistentOrOffline)
- DAC960_V1_ReadWriteError(Command);
- DAC960_ProcessCompletedBuffer(BufferHeader, false);
- }
- if (NextBufferHeader != NULL)
- {
- DAC960_V1_CommandMailbox_T *CommandMailbox =
- &Command->V1.CommandMailbox;
- Command->BlockNumber +=
- BufferHeader->bi_size >> DAC960_BlockSizeBits;
- Command->BlockCount =
- NextBufferHeader->bi_size >> DAC960_BlockSizeBits;
- Command->BufferHeader = NextBufferHeader;
- CommandMailbox->Type5.LD.TransferLength = Command->BlockCount;
- CommandMailbox->Type5.LogicalBlockAddress = Command->BlockNumber;
- CommandMailbox->Type5.BusAddress =
- Virtual_to_Bus32(bio_data(NextBufferHeader));
- DAC960_QueueCommand(Command);
- return;
- }
+ /*
+ * We're not doing retry commands yet.
+ */
+ printk("DAC960_ProcessCompletedCommand: RetryCommand not done yet\n");
}
else if (CommandType == DAC960_MonitoringCommand ||
CommandOpcode == DAC960_V1_Enquiry ||
@@ -3829,7 +3906,7 @@
break;
}
DAC960_Error("Error Condition %s on %s:\n", Controller,
- SenseErrors[Command->V2.RequestSense.SenseKey], CommandName);
+ SenseErrors[Command->V2.RequestSense->SenseKey], CommandName);
DAC960_Error(" /dev/rd/c%dd%d: absolute blocks %u..%u\n",
Controller, Controller->ControllerNumber,
Command->LogicalDriveNumber, Command->BlockNumber,
@@ -3839,8 +3916,8 @@
Controller, Controller->ControllerNumber,
Command->LogicalDriveNumber,
DAC960_PartitionByCommand(Command),
- Command->BufferHeader->bi_sector,
- Command->BufferHeader->bi_sector + Command->BlockCount - 1);
+ Command->Request->sector,
+ Command->Request->sector + Command->BlockCount - 1);
}


@@ -4098,116 +4175,42 @@
DAC960_V2_CommandMailbox_T *CommandMailbox = &Command->V2.CommandMailbox;
DAC960_V2_IOCTL_Opcode_T CommandOpcode = CommandMailbox->Common.IOCTL_Opcode;
DAC960_V2_CommandStatus_T CommandStatus = Command->V2.CommandStatus;
- BufferHeader_T *BufferHeader = Command->BufferHeader;
+
if (CommandType == DAC960_ReadCommand ||
CommandType == DAC960_WriteCommand)
{
if (CommandStatus == DAC960_V2_NormalCompletion)
+
+ DAC960_ProcessCompletedRequest(Command, true);
+
+ else if (Command->V2.RequestSense->SenseKey == DAC960_SenseKey_MediumError)
{
+
/*
- Perform completion processing for all buffers in this I/O Request.
- */
- while (BufferHeader != NULL)
- {
- BufferHeader_T *NextBufferHeader = BufferHeader->bi_next;
- BufferHeader->bi_next = NULL;
- DAC960_ProcessCompletedBuffer(BufferHeader, true);
- BufferHeader = NextBufferHeader;
- }
- if (Command->Completion != NULL)
- {
- complete(Command->Completion);
- Command->Completion = NULL;
- }
- add_blkdev_randomness(DAC960_MAJOR + Controller->ControllerNumber);
- }
- else if (Command->V2.RequestSense.SenseKey
- == DAC960_SenseKey_MediumError &&
- BufferHeader != NULL &&
- BufferHeader->bi_next != NULL)
- {
- if (CommandType == DAC960_ReadCommand)
- Command->CommandType = DAC960_ReadRetryCommand;
- else Command->CommandType = DAC960_WriteRetryCommand;
- Command->BlockCount = BufferHeader->bi_size >> DAC960_BlockSizeBits;
- CommandMailbox->SCSI_10.CommandControlBits
- .AdditionalScatterGatherListMemory = false;
- CommandMailbox->SCSI_10.DataTransferSize =
- Command->BlockCount << DAC960_BlockSizeBits;
- CommandMailbox->SCSI_10.DataTransferMemoryAddress
- .ScatterGatherSegments[0].SegmentDataPointer =
- Virtual_to_Bus64(bio_data(BufferHeader));
- CommandMailbox->SCSI_10.DataTransferMemoryAddress
- .ScatterGatherSegments[0].SegmentByteCount =
- CommandMailbox->SCSI_10.DataTransferSize;
- CommandMailbox->SCSI_10.SCSI_CDB[7] = Command->BlockCount >> 8;
- CommandMailbox->SCSI_10.SCSI_CDB[8] = Command->BlockCount;
- DAC960_QueueCommand(Command);
- return;
+ * Don't know yet how to handle this case.
+ * See comments in DAC960_V1_ProcessCompletedCommand()
+ *
+ * For now, print a message on the console, and clone
+ * the code for "normal" completion.
+ */
+ printk("V1_ProcessCompletedCommand: I/O error on read/write\n");
+
+ DAC960_ProcessCompletedRequest(Command, false);
}
else
{
- if (Command->V2.RequestSense.SenseKey != DAC960_SenseKey_NotReady)
+ if (Command->V2.RequestSense->SenseKey != DAC960_SenseKey_NotReady)
DAC960_V2_ReadWriteError(Command);
/*
Perform completion processing for all buffers in this I/O Request.
*/
- while (BufferHeader != NULL)
- {
- BufferHeader_T *NextBufferHeader = BufferHeader->bi_next;
- BufferHeader->bi_next = NULL;
- DAC960_ProcessCompletedBuffer(BufferHeader, false);
- BufferHeader = NextBufferHeader;
- }
- if (Command->Completion != NULL)
- {
- complete(Command->Completion);
- Command->Completion = NULL;
- }
+ DAC960_ProcessCompletedRequest(Command, false);
}
}
else if (CommandType == DAC960_ReadRetryCommand ||
CommandType == DAC960_WriteRetryCommand)
{
- BufferHeader_T *NextBufferHeader = BufferHeader->bi_next;
- BufferHeader->bi_next = NULL;
- /*
- Perform completion processing for this single buffer.
- */
- if (CommandStatus == DAC960_V2_NormalCompletion)
- DAC960_ProcessCompletedBuffer(BufferHeader, true);
- else
- {
- if (Command->V2.RequestSense.SenseKey != DAC960_SenseKey_NotReady)
- DAC960_V2_ReadWriteError(Command);
- DAC960_ProcessCompletedBuffer(BufferHeader, false);
- }
- if (NextBufferHeader != NULL)
- {
- Command->BlockNumber +=
- BufferHeader->bi_size >> DAC960_BlockSizeBits;
- Command->BlockCount =
- NextBufferHeader->bi_size >> DAC960_BlockSizeBits;
- Command->BufferHeader = NextBufferHeader;
- CommandMailbox->SCSI_10.DataTransferSize =
- Command->BlockCount << DAC960_BlockSizeBits;
- CommandMailbox->SCSI_10.DataTransferMemoryAddress
- .ScatterGatherSegments[0]
- .SegmentDataPointer =
- Virtual_to_Bus64(bio_data(NextBufferHeader));
- CommandMailbox->SCSI_10.DataTransferMemoryAddress
- .ScatterGatherSegments[0]
- .SegmentByteCount =
- CommandMailbox->SCSI_10.DataTransferSize;
- CommandMailbox->SCSI_10.SCSI_CDB[2] = Command->BlockNumber >> 24;
- CommandMailbox->SCSI_10.SCSI_CDB[3] = Command->BlockNumber >> 16;
- CommandMailbox->SCSI_10.SCSI_CDB[4] = Command->BlockNumber >> 8;
- CommandMailbox->SCSI_10.SCSI_CDB[5] = Command->BlockNumber;
- CommandMailbox->SCSI_10.SCSI_CDB[7] = Command->BlockCount >> 8;
- CommandMailbox->SCSI_10.SCSI_CDB[8] = Command->BlockCount;
- DAC960_QueueCommand(Command);
- return;
- }
+ printk("DAC960_V2_ProcessCompletedCommand: retries not coded yet\n");
}
else if (CommandType == DAC960_MonitoringCommand)
{
@@ -5319,7 +5322,6 @@
int LogicalDriveNumber = DAC960_LogicalDriveNumber(Inode->i_rdev);
DiskGeometry_T Geometry, *UserGeometry;
DAC960_Controller_T *Controller;
- int res;

if (File != NULL && (File->f_flags & O_NONBLOCK))
return DAC960_UserIOCTL(Inode, File, Request, Argument);
@@ -5497,11 +5499,11 @@
while (Controller->V1.DirectCommandActive[DCDB.Channel]
[DCDB.TargetID])
{
- spin_unlock_irq(Controller->RequestQueue->queue_lock);
+ spin_unlock_irq(&Controller->queue_lock);
__wait_event(Controller->CommandWaitQueue,
!Controller->V1.DirectCommandActive
[DCDB.Channel][DCDB.TargetID]);
- spin_lock_irq(Controller->RequestQueue->queue_lock);
+ spin_lock_irq(&Controller->queue_lock);
}
Controller->V1.DirectCommandActive[DCDB.Channel]
[DCDB.TargetID] = true;
@@ -5536,9 +5538,10 @@
if (DataTransferLength > 0)
{
if (copy_to_user(UserCommand.DataTransferBuffer,
- DataTransferBuffer, DataTransferLength))
+ DataTransferBuffer, DataTransferLength)) {
ErrorCode = -EFAULT;
goto Failure1;
+ }
}
if (CommandOpcode == DAC960_V1_DCDB)
{
diff -ur linux-2.5.34_original/drivers/block/DAC960.h linux-2.5.34_patch/drivers/block/DAC960.h
--- linux-2.5.34_original/drivers/block/DAC960.h Mon Sep 16 14:34:12 2002
+++ linux-2.5.34_patch/drivers/block/DAC960.h Mon Sep 16 14:41:41 2002
@@ -109,6 +109,43 @@

typedef unsigned long long DAC960_ByteCount64_T;

+/******************************************/
+
+/*
+ Virtual_to_Bus32 maps from Kernel Virtual Addresses to 32 Bit PCI Bus
+ Addresses.
+*/
+
+static inline DAC960_BusAddress32_T Virtual_to_Bus32(void *VirtualAddress)
+{
+ return (DAC960_BusAddress32_T) virt_to_bus(VirtualAddress);
+}
+
+
+/*
+ Bus32_to_Virtual maps from 32 Bit PCI Bus Addresses to Kernel Virtual
+ Addresses.
+*/
+
+static inline void *Bus32_to_Virtual(DAC960_BusAddress32_T BusAddress)
+{
+ return (void *) bus_to_virt(BusAddress);
+}
+
+
+/*
+ Virtual_to_Bus64 maps from Kernel Virtual Addresses to 64 Bit PCI Bus
+ Addresses.
+*/
+
+static inline DAC960_BusAddress64_T Virtual_to_Bus64(void *VirtualAddress)
+{
+ return (DAC960_BusAddress64_T) virt_to_bus(VirtualAddress);
+}
+
+
+/******************************************/
+

/*
Define the SCSI INQUIRY Standard Data structure.
@@ -2191,7 +2228,6 @@
of the Linux Kernel and I/O Subsystem.
*/

-typedef struct bio BufferHeader_T;
typedef struct file File_T;
typedef struct block_device_operations BlockDeviceOperations_T;
typedef struct completion Completion_T;
@@ -2278,16 +2314,16 @@
unsigned int BlockNumber;
unsigned int BlockCount;
unsigned int SegmentCount;
- BufferHeader_T *BufferHeader;
- void *RequestBuffer;
+ IO_Request_T *Request;
+ struct pci_dev *PciDevice;
union {
struct {
DAC960_V1_CommandMailbox_T CommandMailbox;
DAC960_V1_KernelCommand_T *KernelCommand;
DAC960_V1_CommandStatus_T CommandStatus;
- DAC960_V1_ScatterGatherSegment_T
- ScatterGatherList[DAC960_V1_ScatterGatherLimit]
- __attribute__ ((aligned (sizeof(DAC960_V1_ScatterGatherSegment_T))));
+ DAC960_V1_ScatterGatherSegment_T *ScatterGatherList;
+ dma_addr_t ScatterGatherListDMA;
+ struct scatterlist ScatterList[DAC960_V1_ScatterGatherLimit];
unsigned int EndMarker[0];
} V1;
struct {
@@ -2296,11 +2332,11 @@
DAC960_V2_CommandStatus_T CommandStatus;
unsigned char RequestSenseLength;
int DataTransferResidue;
- DAC960_V2_ScatterGatherSegment_T
- ScatterGatherList[DAC960_V2_ScatterGatherLimit]
- __attribute__ ((aligned (sizeof(DAC960_V2_ScatterGatherSegment_T))));
- DAC960_SCSI_RequestSense_T RequestSense
- __attribute__ ((aligned (sizeof(int))));
+ DAC960_V2_ScatterGatherSegment_T *ScatterGatherList;
+ dma_addr_t ScatterGatherListDMA;
+ DAC960_SCSI_RequestSense_T *RequestSense;
+ dma_addr_t RequestSenseDMA;
+ struct scatterlist ScatterList[DAC960_V2_ScatterGatherLimit];
unsigned int EndMarker[0];
} V2;
} FW;
@@ -2320,6 +2356,7 @@
DAC960_HardwareType_T HardwareType;
DAC960_IO_Address_T IO_Address;
DAC960_PCI_Address_T PCI_Address;
+ PCI_Device_T *PCIDevice;
unsigned char ControllerNumber;
unsigned char ControllerName[4];
unsigned char ModelName[20];
@@ -2361,6 +2398,7 @@
boolean SuppressEnclosureMessages;
Timer_T MonitoringTimer;
struct gendisk disks[DAC960_MaxLogicalDrives];
+ struct pci_pool *ScatterGatherPool;
DAC960_Command_T *FreeCommands;
unsigned char *CombinedStatusBuffer;
unsigned char *CurrentStatusBuffer;
@@ -2446,6 +2484,7 @@
boolean NeedDeviceSerialNumberInformation;
boolean StartLogicalDeviceInformationScan;
boolean StartPhysicalDeviceInformationScan;
+ struct pci_pool *RequestSensePool;
DAC960_V2_CommandMailbox_T *FirstCommandMailbox;
DAC960_V2_CommandMailbox_T *LastCommandMailbox;
DAC960_V2_CommandMailbox_T *NextCommandMailbox;
@@ -2498,13 +2537,18 @@

/*
DAC960_AcquireControllerLock acquires exclusive access to Controller.
+ Reference the queue_lock through the controller structure,
+ rather than through the request queue. These macros are
+ used to mutex on the controller structure during initialization,
+ BEFORE the request queue is allocated and initialized in
+ DAC960_RegisterBlockDevice().
*/

static inline
void DAC960_AcquireControllerLock(DAC960_Controller_T *Controller,
ProcessorFlags_T *ProcessorFlags)
{
- spin_lock_irqsave(Controller->RequestQueue->queue_lock, *ProcessorFlags);
+ spin_lock_irqsave(&Controller->queue_lock, *ProcessorFlags);
}


@@ -2516,7 +2560,7 @@
void DAC960_ReleaseControllerLock(DAC960_Controller_T *Controller,
ProcessorFlags_T *ProcessorFlags)
{
- spin_unlock_irqrestore(Controller->RequestQueue->queue_lock, *ProcessorFlags);
+ spin_unlock_irqrestore(&Controller->queue_lock, *ProcessorFlags);
}


@@ -2553,7 +2597,7 @@
void DAC960_AcquireControllerLockIH(DAC960_Controller_T *Controller,
ProcessorFlags_T *ProcessorFlags)
{
- spin_lock_irqsave(Controller->RequestQueue->queue_lock, *ProcessorFlags);
+ spin_lock_irqsave(&Controller->queue_lock, *ProcessorFlags);
}


@@ -2566,10 +2610,9 @@
void DAC960_ReleaseControllerLockIH(DAC960_Controller_T *Controller,
ProcessorFlags_T *ProcessorFlags)
{
- spin_unlock_irqrestore(Controller->RequestQueue->queue_lock, *ProcessorFlags);
+ spin_unlock_irqrestore(&Controller->queue_lock, *ProcessorFlags);
}

-#error I am a non-portable driver, please convert me to use the Documentation/DMA-mapping.txt interfaces

/*
Define the DAC960 BA Series Controller Interface Register Offsets.
@@ -4208,7 +4251,7 @@
static void DAC960_FinalizeController(DAC960_Controller_T *);
static int DAC960_Notifier(NotifierBlock_T *, unsigned long, void *);
static void DAC960_V1_QueueReadWriteCommand(DAC960_Command_T *);
-static void DAC960_V2_QueueReadWriteCommand(DAC960_Command_T *);
+static void DAC960_V2_QueueReadWriteCommand(DAC960_Command_T *);
static void DAC960_RequestFunction(RequestQueue_T *);
static void DAC960_BA_InterruptHandler(int, void *, Registers_T *);
static void DAC960_LP_InterruptHandler(int, void *, Registers_T *);

2002-09-16 22:20:25

by Dave Olien

[permalink] [raw]
Subject: Re: [2.5] DAC960


Daniel,

That last patch I sent is currently running on an
eXtremeRAID 2000 Mylex controller.
I haven't tried it yet on other controller
versions. When you give it a try, let me know
what controller type you are using.

I thought I was having difficulties writing
new disk partitions. But it seems to be working
fine now. I don't know what I was seeing earlier.
I can't seem to reproduce it.

So, as far as I know, this driver is completely
functional. I just need to complete the conversion
to using the dma interfaces, and do more rigorous
tests.

Getting test coverage on other controller versions would
be great, too.

Dave

2002-09-18 16:12:52

by James Bottomley

[permalink] [raw]
Subject: Re: [2.5] DAC960

> > > Linus indicated at the Kernel Summit that he'd like to see a
> > > cleaned-up scsi midlayer used as framework for *all* disk IO,
> > > including IDE. Obviously, what with IDE transitions and whatnot, we
> > > are far from being ready to attempt that, so see "nursing along"
> > > above. There's no longer any chance that a generic disk midlayer is
> > > going to happen in this cycle, as far as I can see. Still, anybody
> > > who is interested would do well by studing the issues, and fixing
> > > broken drivers certainly qualifies as a way to come up to speed.
> > > First of all, I believe Linus' plan is to push more functionality into
> > the block layer.
>
> I distinctly heard him say he wanted the scsi mid layer repurposed as
> an interface for all disks. Maybe he changed his mind?

I don't recall hearing this. I remember his agreeing with the idea of
slimming down the SCSI mid layer, which does rather contradict the use SCSI
for everything approach.

After the Kernel Summit, there was quite a long thread on l-k with Joerg
Schilling on exactly this issue. The upshot of which was I clearly said we
weren't going to go the SCSI is everything route. Unless there's any reason
to change course, I think that's the current plan.

James


2002-09-18 16:35:37

by Daniel Phillips

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Wednesday 18 September 2002 18:17, James Bottomley wrote:
> > > > Linus indicated at the Kernel Summit that he'd like to see a
> > > > cleaned-up scsi midlayer used as framework for *all* disk IO,
> > > > including IDE. Obviously, what with IDE transitions and whatnot, we
> > > > are far from being ready to attempt that, so see "nursing along"
> > > > above. There's no longer any chance that a generic disk midlayer is
> > > > going to happen in this cycle, as far as I can see. Still, anybody
> > > > who is interested would do well by studing the issues, and fixing
> > > > broken drivers certainly qualifies as a way to come up to speed.
> > > > First of all, I believe Linus' plan is to push more functionality into
> > > the block layer.
> >
> > I distinctly heard him say he wanted the scsi mid layer repurposed as
> > an interface for all disks. Maybe he changed his mind?
>
> I don't recall hearing this. I remember his agreeing with the idea of
> slimming down the SCSI mid layer, which does rather contradict the use SCSI
> for everything approach.

> After the Kernel Summit, there was quite a long thread on l-k with Joerg
> Schilling on exactly this issue.

Subject line "IDE/ATAPI in 2.5"?

> The upshot of which was I clearly said we
> weren't going to go the SCSI is everything route. Unless there's any reason
> to change course, I think that's the current plan.

Given that Halloween is 6 weeks away, I don't doubt you.

--
Daniel

2002-09-19 18:44:40

by Dave Olien

[permalink] [raw]
Subject: Re: [2.5] DAC960


I've been doing more work on the driver. Wednesday, I was
going crazy because the data I read back from the device
was SOMETIMES NOT the same data I wrote there.

On Thursday, I switched from Linux 2.5.34 to Linux 2.5.36.
Now, the driver reads back the same data it wrote. There must
have been some bio changes in 2.5.36. 2.5.36 also
calls the driver shutdown notifier routine, which 2.5.34 didn't.
This uncovered a coding bug that causes a kernel OOPS during shutdown.
That'll be fixed in the next patch.

I'm about to test changes that put the command and status memory
mailboxes into dma memory regions. Once I've tested that,
I'll send you a new patch (Probably on Monday after week end
testing).

After that, I'll change the status reporting structures to be in dma
memory regions. Expect a patch containing that maybe the end of
next week, or the Monday following ( September 30).



> >
> > I have the DAC960 driver working in 2.5.34. The work isn't
> > complete yet. But, I'm able to boot, and do mke2fs
> > on partitions on logical drives, and then do e2fsck
> > on those partitions. It seems to work, although more
> > testing is required. Is there any interest in reviewing
> > the code so far, or should I continue testing and complete
> > the remaining issues first?
>
> Please post the patch, I'll try it right away.
>
> --
> Daniel

2002-09-19 19:11:58

by Daniel Phillips

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Thursday 19 September 2002 20:49, Dave Olien wrote:
> I've been doing more work on the driver. Wednesday, I was
> going crazy because the data I read back from the device
> was SOMETIMES NOT the same data I wrote there.
>
> On Thursday, I switched from Linux 2.5.34 to Linux 2.5.36.
> Now, the driver reads back the same data it wrote. There must
> have been some bio changes in 2.5.36. 2.5.36 also
> calls the driver shutdown notifier routine, which 2.5.34 didn't.
> This uncovered a coding bug that causes a kernel OOPS during shutdown.
> That'll be fixed in the next patch.
>
> I'm about to test changes that put the command and status memory
> mailboxes into dma memory regions. Once I've tested that,
> I'll send you a new patch (Probably on Monday after week end
> testing).
>
> After that, I'll change the status reporting structures to be in dma
> memory regions. Expect a patch containing that maybe the end of
> next week, or the Monday following ( September 30).

I was in the process of writing to you as this one came in...

I have booted 2.5.34 and 2.5.36, the same controller as yours, on my dual
PIII box and it is apparently functioning well. I have not done any kind
of load testing yet.

Congratulations! I presume you are now the DAC960 maintainer, subject to
approval from on high of course, and assuming you are willing. I'd like
to do some spelling changes, just obvious ones for now, like removing
spelling wrappers from standard kernel interfaces. Want patches?

--
Daniel

2002-09-19 21:21:53

by Dave Olien

[permalink] [raw]
Subject: Re: [2.5] DAC960


Daniel

Here's my latest progress on my changes to the DAC960 driver.

I spent Tuesday banging my head trying to figure out why data blocks
written to disk to SOMETIMES were read back with DIFFERENT data.
On wednesday, I changed from using Linux 2.5.34 to using Linux 2.5.36.
My bad data problem went away with that change. There must have been
an important change in the 2.5.36 BIO code.

However, this change uncovered a yank-put error in my version of the
driver's shutdown notification function. The version I mailed you will
print out messages about "bad dma" when you try to shutdown.
That code is freeing some dma structures to the wrong dma pool.
Apparently Linux 2.5.34 never called that function.

I'm about to test changes to put the command and status mailboxes
into dma memory regions. I'll run some tests on it
over the week end, and mail you a new set of changes on Monday.

Let me know how it goes.

Thanks!


On Mon, Sep 16, 2002 at 10:26:23PM +0200, Daniel Phillips wrote:
> >
> > I have the DAC960 driver working in 2.5.34. The work isn't
> > complete yet. But, I'm able to boot, and do mke2fs
> > on partitions on logical drives, and then do e2fsck
> > on those partitions. It seems to work, although more
> > testing is required. Is there any interest in reviewing
> > the code so far, or should I continue testing and complete
> > the remaining issues first?
>
> Please post the patch, I'll try it right away.
>
> --
> Daniel

2002-09-19 22:04:37

by Dave Olien

[permalink] [raw]
Subject: Re: [2.5] DAC960


Daniel,

My mailer here has been misbehaving. I didn't think THIS mail
had actually made it out. So, you may be seeing another version
of this mail sometime. Just ignore it.

I think some coding style repairs would be great! But I'd like to
hold off on that until we've finished all the functional changes.
That way, anyone doing a code review can easily see only the
changes to make the driver function.

Once functional changes are mostly complete, then cleaning up
some coding style issues would be a good thing.

Regarding being a mainteiner, lets get the code changes done
first ;-)


On Thu, Sep 19, 2002 at 09:16:53PM +0200, Daniel Phillips wrote:
> On Thursday 19 September 2002 20:49, Dave Olien wrote:
> > I've been doing more work on the driver. Wednesday, I was
> > going crazy because the data I read back from the device
> > was SOMETIMES NOT the same data I wrote there.
> >
> > On Thursday, I switched from Linux 2.5.34 to Linux 2.5.36.
> > Now, the driver reads back the same data it wrote. There must
> > have been some bio changes in 2.5.36. 2.5.36 also
> > calls the driver shutdown notifier routine, which 2.5.34 didn't.
> > This uncovered a coding bug that causes a kernel OOPS during shutdown.
> > That'll be fixed in the next patch.
> >
> > I'm about to test changes that put the command and status memory
> > mailboxes into dma memory regions. Once I've tested that,
> > I'll send you a new patch (Probably on Monday after week end
> > testing).
> >
> > After that, I'll change the status reporting structures to be in dma
> > memory regions. Expect a patch containing that maybe the end of
> > next week, or the Monday following ( September 30).
>
> I was in the process of writing to you as this one came in...
>
> I have booted 2.5.34 and 2.5.36, the same controller as yours, on my dual
> PIII box and it is apparently functioning well. I have not done any kind
> of load testing yet.
>
> Congratulations! I presume you are now the DAC960 maintainer, subject to
> approval from on high of course, and assuming you are willing. I'd like
> to do some spelling changes, just obvious ones for now, like removing
> spelling wrappers from standard kernel interfaces. Want patches?
>
> --
> Daniel
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2002-09-19 22:16:54

by Daniel Phillips

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Friday 20 September 2002 00:09, Dave Olien wrote:
> Daniel,
>
> My mailer here has been misbehaving. I didn't think THIS mail
> had actually made it out. So, you may be seeing another version
> of this mail sometime. Just ignore it.
>
> I think some coding style repairs would be great! But I'd like to
> hold off on that until we've finished all the functional changes.
> That way, anyone doing a code review can easily see only the
> changes to make the driver function.
>
> Once functional changes are mostly complete, then cleaning up
> some coding style issues would be a good thing.

Yep. And this is not a halloween deadline kind of thing, or more
accurately, you just did the halloween part of it. The rest of the
job is to try to make it nice. It would be great to find out if the
hardware is really as slow as it seems, or if it's the driver.

> Regarding being a mainteiner, lets get the code changes done
> first ;-)

It's working! I see this in very simple terms ;-)

--
Daniel

2002-09-20 06:16:49

by Jens Axboe

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Fri, Sep 20 2002, Daniel Phillips wrote:
> On Friday 20 September 2002 00:09, Dave Olien wrote:
> > Daniel,
> >
> > My mailer here has been misbehaving. I didn't think THIS mail
> > had actually made it out. So, you may be seeing another version
> > of this mail sometime. Just ignore it.
> >
> > I think some coding style repairs would be great! But I'd like to
> > hold off on that until we've finished all the functional changes.
> > That way, anyone doing a code review can easily see only the
> > changes to make the driver function.
> >
> > Once functional changes are mostly complete, then cleaning up
> > some coding style issues would be a good thing.
>
> Yep. And this is not a halloween deadline kind of thing, or more
> accurately, you just did the halloween part of it. The rest of the
> job is to try to make it nice. It would be great to find out if the
> hardware is really as slow as it seems, or if it's the driver.

Not at all a Halloween thing, this is just a driver update.

> > Regarding being a mainteiner, lets get the code changes done
> > first ;-)
>
> It's working! I see this in very simple terms ;-)

Good that it's working, but I would have to agree with Dave, there's
still lots of work to be done. pci dma mapping is not complete, for one.

--
Jens Axboe

2002-09-20 06:15:34

by Jens Axboe

[permalink] [raw]
Subject: Re: [2.5] DAC960

On Thu, Sep 19 2002, Dave Olien wrote:
>
> Daniel
>
> Here's my latest progress on my changes to the DAC960 driver.
>
> I spent Tuesday banging my head trying to figure out why data blocks
> written to disk to SOMETIMES were read back with DIFFERENT data.
> On wednesday, I changed from using Linux 2.5.34 to using Linux 2.5.36.
> My bad data problem went away with that change. There must have been
> an important change in the 2.5.36 BIO code.

Neither 2.5.35 nor 2.5.36 has any critical bio fixes, so I would look
into this a bit more if I were you. Only if you were using
bio_kmap_irq() would there be something to look for, but DAC960 is not.
That was 2.5.35. 2.5.36 just starts sizing bio_vec pools based on free
memory, no bug fixes. Likewise in the block layer, I'm not seeing
anything.

--
Jens Axboe

2002-09-20 21:27:45

by Daniel Phillips

[permalink] [raw]
Subject: Re: [2.5] DAC960

Hi Dave,

Here's the trivial patch you need, on top of your last-posted patch, to fix
up the one failed hunk and make it work in 2.5.37.

I cleaned up a few stylistic things to bring them in line with
penguin-classic style, things like never writing "== true" in logical
expressions and having line breaks in the right places. Very minor.

Obviously, a lot more trivial cleanup is still needed, especially spelling.
The code is pretty easy to read, though, including your code.

Daniel

--- 2.5.37.clean/drivers/block/DAC960.c 2002-09-20 23:08:09.000000000 +0200
+++ 2.5.37/drivers/block/DAC960.c 2002-09-20 23:19:47.000000000 +0200
@@ -3007,16 +3007,48 @@
individual Buffer.
*/

-static inline void DAC960_ProcessCompletedBuffer(BufferHeader_T *BufferHeader,
- boolean SuccessfulIO)
+static inline void DAC960_ProcessCompletedRequest(DAC960_Command_T *Command,
+ boolean SuccessfulIO)
{
- bio_endio(BufferHeader, BufferHeader->bi_size, SuccessfulIO ? 0 : -EIO);
- blk_finished_io(bio_sectors(BufferHeader));
+ DAC960_CommandType_T CommandType = Command->CommandType;
+ IO_Request_T *Request = Command->Request;
+ int DmaDirection, UpToDate;
+
+ UpToDate = 0;
+ if (SuccessfulIO)
+ UpToDate = 1;
+
+ /*
+ * We could save DmaDirection in the command structure
+ * and just reuse that information here.
+ */
+ if (CommandType == DAC960_ReadCommand || CommandType == DAC960_ReadRetryCommand)
+ DmaDirection = PCI_DMA_FROMDEVICE;
+ else
+ DmaDirection = PCI_DMA_TODEVICE;
+
+ pci_unmap_sg(Command->PciDevice, Command->V1.ScatterList,
+ Command->SegmentCount, DmaDirection);
+
+ /*
+ * BlockCount is redundant with nr_sectors in the request
+ * structure. Consider eliminating BlockCount from the
+ * command structure now that Command includes a pointer to
+ * the request.
+ */
+ while (end_that_request_first(Request, UpToDate, Command->BlockCount))
+ ;
+ end_that_request_last(Request);
+
+ if (Command->Completion) {
+ complete(Command->Completion);
+ Command->Completion = NULL;
+ }
}

static inline int DAC960_PartitionByCommand(DAC960_Command_T *Command)
{
- return DAC960_PartitionNumber(to_kdev_t(Command->BufferHeader->bi_bdev->bd_dev));
+ return DAC960_PartitionNumber(Command->Request->rq_dev);
}

/*

2002-09-23 19:19:35

by Dave Olien

[permalink] [raw]
Subject: Re: [2.5] DAC960


On Fri, Sep 20 2002, Jens Axboe wrote:
>
> Neither 2.5.35 nor 2.5.36 has any critical bio fixes, so I would look
> into this a bit more if I were you. Only if you were using
> bio_kmap_irq() would there be something to look for, but DAC960 is not.
> That was 2.5.35. 2.5.36 just starts sizing bio_vec pools based on free
> memory, no bug fixes. Likewise in the block layer, I'm not seeing
> anything.

I'll spend some time this week looking into this. I have an idea
on how I might track down what's going on. I'll let you know if
I discover anything.

Thanks!

Dave