On Thu, 2008-05-01 at 21:14 +0300, Boaz Harrosh wrote: > On Thu, May 01 2008 at 20:33 +0300, James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, 2008-05-01 at 20:06 +0300, Boaz Harrosh wrote: > >> On Thu, May 01 2008 at 19:38 +0300, James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > >> <snip> > >>> Er, but > >>> > >>> 1. struct cmdinfo is stack allocated; you can't dma to stack > >>> 2. even if you fix this, the structure is packed and will have the > >>> original ppc coherence issues > >>> 3. Finally, even on the stack, it's not necessarily reachable with > >>> the DMA mask > >>> > >> Ho, thanks for the catch. I really goofed here. > >> > >>> The whole point of managing sense buffer allocations centrally is to > >>> prevent driver writers from falling into all these traps. I really > >>> don't think going back to hand rolled bounce buffering imporves the > >>> situation. > >>> > >> We don't have a choice Andi is removing all this. ISA is becoming > >> ISA's driver private problem. > > > > No he's not .. not if it's going to cause this type of grief. > > > > But I think you misread his patch set. He still has a flag equivalent > > of unchecked_isa_dma it's just called sense_buffer_isa instead. The > > shift in his patch set is > > > > 1. it removes the need to allocate commands in dma'able memory > > 2. it removes the need for the ULD request path to allocate in > > dma'able memory (it uses the block bounce path instead) > > 3. it *can't* remove the need to dma to the sense buffer, hence the > > flag. > > > > So it looks like the re-rolled patch set (when Andi gets around to > > posting it) will use one pool for the cmd buffer, but separate isa and > > non-isa pools for the sense buffer. Thus it should just work for gdth > > with scsi_allocate_command predicated on sense_buffer_isa. > > > > James > > > > I know all about Andi's patches, since I offered to help him as I > have some experience with scsi drivers. And it will be me that will > post the next round of ISA patches. > > I have the complete body of work here: > http://git.open-osd.org/gitweb.cgi?p=open-osd.git;a=shortlog;h=boaz_sense_effort > > I planed to first submit up to the Remove-ISA-dma tag and only then RFC and try > to push the rest of the work. But most of what you're doing in that patch set doesn't work. The whole reason for separating the sense buffer from the scsi command is the DMA problem of cache line interference caused by doing DMA to an area embedded in a structure that has host accessed variables in it. With what you're doing embedding the sense buffer in the internal control areas, you're re-introducing that bug. Additionally, it really doesn't make sense for every driver to do a roll your own allocator. It makes far more sense for them to be allocated centrally (and correctly) so there's only one place it needs fixing if there's yet another problem discovered. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
