* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 [not found] <41471163.10709@rtr.ca> @ 2004-09-14 17:00 ` Jeff Garzik 2004-09-14 17:27 ` Mark Lord ` (2 more replies) 0 siblings, 3 replies; 26+ messages in thread From: Jeff Garzik @ 2004-09-14 17:00 UTC (permalink / raw) To: Mark Lord Cc: linux-kernel, SCSI Mailing List, linux-ide@vger.kernel.org, Alan Cox Mark Lord wrote: > My first attempt at posting this seems to have gone AWOL, > so here it comes again. Also being posted to linux-scsi. There is that CC feature in your mailer, you know... :) Repeating what I posted to linux-scsi: > Here is the first public release of the 2.6.xx driver > source code for the Pacific Digital Corporation QStor SATA/RAID chip. > > This 4-channel chip has hardware-assisted RAID0/RAID1/RAID10, > host-queuing, per-request TCQ/NCQ support, support for hot insertion > and removal of drives, etc.. The 64-bit/66Mhz chip shows throughput > in excess of 200MByte/sec on my ancient P3-1GHz test system, > and can do much better when installed in a PCI-X slot. How much of the RAID is actually hardware-assisted? This looks pretty much like an ATA driver to me. Linus vetoed future SCSI->ATA translators. He only allowed libata because I promised to remove the translation and make it a native block driver in the future, which I have been working towards (see struct ata_queued_cmd, etc.) > The driver (attached) supports most of the chip features, > including host, native and legacy tagged queuing, > but does not yet include boot-from-raid support (coming soon). > > Both hdparm and smartmontools are fully supported by this driver. Actual comments on the code: 0) so far, this driver looks like fake RAID to me... if so that's a big veto. if it's real RAID, only the following grumbles apply :) 1) not endian safe at all 2) I am dubious about including Yet Another set of event logging functions 3) in qstor_read_events you unlock the spinlock without first locking it, in one path (wait_event_interruptible rc==0) 4) qstor_extract_id_string appears to be generic across IDE/libata/qstor 5) new procfs stuff discouraged 6) style: ditch braces on single statements, e.g., + if (drive != NULL) { + qstor_destroy_device(drive); + } 7) double spin_unlock_irq possible in qstor_create_device 8) use msleep() rather than schedule_timeout() 9) use of do_sleep paradigm is dubious: you should instead try to keep your locked code regions as small as possible. in general, this code has far too many unlock-doit-lock sections. Experience has shown that too much unlock-doit-lock leads to bugs and increases the pain when analyzing your locking. In particular, releasing the lock and sleeping would be very wrong in the ->queuecommand and error handling paths (alas... I would love to sleep in the fine-grained eh hooks) 10) in qstor_scsi_done, when is cmd->scsi_done ever NULL? 11) do you properly keep track of the 'done' function passed to you in ->queuecommand? or do you mistakenly assume that cmd->scsi_done is the same as the ->queuecommand argument? 12) fix the sd.c code, don't add silly driver-specific workarounds: + buf[0] = TYPE_DISK; // Cannot use TYPE_RAID -- sd.c rejects it 13) doh! check for pci_map_xxx failure 14) use sg_dma_len() macro, not sg->length 15) Bart and I are slowly moving over to using linux/ata.h for ATA-generic constants and enums. Please use ATA_CMD_xxx (and add constants to that header as required). 16) There are WAY too many magic numbers in this driver :( 17) Are you 100% certain of your queued error handling? The reason why libata doesn't do NCQ is purely because error handling is so complicated. Potential problems I don't see you handling (but I could be missing something!): a) on a bus error (not device error), one has _no idea_ which commands complete etc. b) if SERVICE interrupt is enabled, you may not get back the correct D2H Register FIS showing the errored device in question c) even if you receive the correct D2H Register FIS, you may need to manually abort the queue with a NOP Queueing is easy. Picking up the pieces when it fails isn't. libata's error handling is dumb, but also easy to review and verify. 18) return -EFOO values from your PCI probe function 19) propagate return value from pci_enable_device 20) check (and return) pci_set_dma_mask retval 21) use the proper ULL suffix for pci_set_dma_mask argument 22) use pci_set_consistent_dma_mask also 23) use pci_request_regions to reserve resources 24) when qstor_probe fails, don't just return! undo the stuff that occurred before the error (such as calling pci_disable_device or pci_release_regions or iounmap) 25) propagate return value from request_irq 26) check scsi_add_host return value 27) the following is highly silly. you are locking a function, just so you can unlock a function so you can sleep. + spin_lock_irqsave(uhba->lock, flags); + qstor_reset(uhba); + spin_unlock_irqrestore(uhba->lock, flags); 28) "SECTOR_BYTES" -- how many more definitions of the 512-byte sector do we need? :) 29) You use QSTOR_PACKED_STRUCTURE when not needed, which causes gcc to generate horribly sub-optimal code 30) style: use u32/u64 as kernel standard. + unsigned pLEN :32; // Byte count + unsigned spare32 :32; // 0 31) none of your bitfield structures are endian-safe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 17:00 ` [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 Jeff Garzik @ 2004-09-14 17:27 ` Mark Lord 2004-09-14 17:33 ` Jeff Garzik 2004-09-14 17:51 ` Mark Lord 2004-09-14 18:25 ` James Bottomley 2 siblings, 1 reply; 26+ messages in thread From: Mark Lord @ 2004-09-14 17:27 UTC (permalink / raw) To: Jeff Garzik Cc: linux-kernel, SCSI Mailing List, linux-ide@vger.kernel.org, Alan Cox Thanks Jeff, I'll look into most of your points and give responses and changes where required. But first, a few overall notes on the approach. This is a hardware RAID device, but it requires driver knowledge of the RAID features. It does not map to libata at all, unfortunately, because all of the queuing features are completely non-SATA standard, and the RAID stuff is (as normal) peculiar to the chip. Here's a question for you: like all of the other RAID drivers, this one needs an interface to a userland RAID management GUI. The usual method for this is to create a fake character device driver, and use that as the interface to userland. This is commonly done, but is it the best way to handle such? A /proc/ or /sys/ interface could achieve similar goals, but without the need of a fake device. We can go either way with this one, so lets hear some opinions on it. For the rest, this driver has been around (vendor driver) since before libata became usable, and certainly before libata existed in 2.4.xx. The driver will eventuall need to compile and run in 2.4.20, for customers using old Redhat kernels. It's not there yet, but if it were to lean more heavily on 2.6.xx stuff, then that will be more difficult to achieve. Cheers -- Mark Lord (hdparm keeper & the original "Linux IDE Guy") ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 17:27 ` Mark Lord @ 2004-09-14 17:33 ` Jeff Garzik 0 siblings, 0 replies; 26+ messages in thread From: Jeff Garzik @ 2004-09-14 17:33 UTC (permalink / raw) To: Mark Lord Cc: linux-kernel, SCSI Mailing List, linux-ide@vger.kernel.org, Alan Cox On Tue, Sep 14, 2004 at 01:27:54PM -0400, Mark Lord wrote: > Here's a question for you: like all of the other RAID drivers, > this one needs an interface to a userland RAID management GUI. > > The usual method for this is to create a fake character device driver, > and use that as the interface to userland. This is commonly done, > but is it the best way to handle such? A /proc/ or /sys/ interface > could achieve similar goals, but without the need of a fake device. > > We can go either way with this one, so lets hear some opinions on it. Well, * if the userland interface is 100% sending cdbs or taskfiles, then I would prefer that Jens Axboe's "bsg" be used. Its a chardev interface for sending/receiving commands to a request queue. * otherwise, I would pick either chrdev or sysfs. if you gotta support 2.4, I guess that means chrdev. > For the rest, this driver has been around (vendor driver) since before > libata became usable, and certainly before libata existed in 2.4.xx. > The driver will eventuall need to compile and run in 2.4.20, > for customers using old Redhat kernels. It's not there yet, > but if it were to lean more heavily on 2.6.xx stuff, > then that will be more difficult to achieve. libata and all its drivers work on RHEL2.1 (2.4.9), and someone is even crazy enough to be porting libata to 2.2.x ;-) Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 17:00 ` [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 Jeff Garzik 2004-09-14 17:27 ` Mark Lord @ 2004-09-14 17:51 ` Mark Lord 2004-09-14 17:56 ` Jeff Garzik 2004-09-14 18:25 ` James Bottomley 2 siblings, 1 reply; 26+ messages in thread From: Mark Lord @ 2004-09-14 17:51 UTC (permalink / raw) To: Jeff Garzik Cc: linux-kernel, SCSI Mailing List, linux-ide@vger.kernel.org, Alan Cox >In particular, releasing the lock and sleeping would be very wrong >in the ->queuecommand and error handling paths >(alas... I would love to sleep in the fine-grained eh hooks) Mmm.. definitely no sleeps in queuecommand(), but sleeping seems necessary in host_reset_handler() -- the alternative is to just busywait inline.. which would really not be good. Isn't the protocol for the eh host_reset_handler() basically just "do the reset, and return whether it worked or not?". If so, the driver really has to hang around until the reset completes so that correct status can be returned. This generally takes a couple of milliseconds in practice (measured it). Is there a better way to do that? I really would prefer never to have to reset the drives, but when they have a queuing error, many of them simply won't talk to us again without a reset. The driver avoids the reset as much as it can for other situations, though. Cheers -- Mark Lord (hdparm keeper & the original "Linux IDE Guy") ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 17:51 ` Mark Lord @ 2004-09-14 17:56 ` Jeff Garzik 2004-09-14 18:03 ` Mark Lord 0 siblings, 1 reply; 26+ messages in thread From: Jeff Garzik @ 2004-09-14 17:56 UTC (permalink / raw) To: Mark Lord; +Cc: linux-kernel, SCSI Mailing List, James Bottomley Mark Lord wrote: > >In particular, releasing the lock and sleeping would be very wrong > >in the ->queuecommand and error handling paths > >(alas... I would love to sleep in the fine-grained eh hooks) > > Mmm.. definitely no sleeps in queuecommand(), but sleeping seems > necessary in host_reset_handler() -- the alternative is to just > busywait inline.. which would really not be good. > > Isn't the protocol for the eh host_reset_handler() basically > just "do the reset, and return whether it worked or not?". > If so, the driver really has to hang around until the reset > completes so that correct status can be returned. This generally > takes a couple of milliseconds in practice (measured it). > > Is there a better way to do that? > > I really would prefer never to have to reset the drives, > but when they have a queuing error, many of them simply > won't talk to us again without a reset. The driver avoids > the reset as much as it can for other situations, though. James and I occasionally talk about this. This is a _big_ reason why I use the ->eh_strategy_handler() rather than the more fine-grained ->eh hooks: you get unlocked, unfettered sleep priveleges inside the scsi EH thread. The SCSI LLD API really needs to -not- spinlock on the EH hooks, and instead simply guarantee that ->queuecommand and other hooks will not be called while the driver is in EH. ISTR James didn't disagree, so maybe a patch can be worked out... Of course, you could always just use ->eh_strategy_handler and do 100% of the error handling yourself. That's the route libata chose. Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 17:56 ` Jeff Garzik @ 2004-09-14 18:03 ` Mark Lord 2004-09-14 18:07 ` Jeff Garzik 2004-09-14 18:08 ` Jeff Garzik 0 siblings, 2 replies; 26+ messages in thread From: Mark Lord @ 2004-09-14 18:03 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-kernel, SCSI Mailing List, James Bottomley >The SCSI LLD API really needs to -not- spinlock on the EH hooks, >and instead simply guarantee that ->queuecommand and other hooks >will not be called while the driver is in EH. > >ISTR James didn't disagree, so maybe a patch can be worked out... It looks to me as if the eh code prevents further queuecommand() calls while the LLD *_reset_handler() code is running. I wonder if it also does so for the eh_strategy_handler() ? Have to look at it, I guess. >Of course, you could always just use ->eh_strategy_handler >and do 100% of the error handling yourself. Mmmm.. yes, that may be better, perhaps. Whatever this driver does, it has to be reasonably portable back to early 2.4.xx kernels, so it cannot depend too much upon newly (or to-be) implemented semantics in 2.6.xx. Cheers -- Mark Lord (hdparm keeper & the original "Linux IDE Guy") ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 18:03 ` Mark Lord @ 2004-09-14 18:07 ` Jeff Garzik 2004-09-14 18:08 ` Jeff Garzik 1 sibling, 0 replies; 26+ messages in thread From: Jeff Garzik @ 2004-09-14 18:07 UTC (permalink / raw) To: Mark Lord; +Cc: linux-kernel, SCSI Mailing List, James Bottomley On Tue, Sep 14, 2004 at 02:03:28PM -0400, Mark Lord wrote: > Whatever this driver does, it has to be reasonably portable > back to early 2.4.xx kernels, so it cannot depend too much > upon newly (or to-be) implemented semantics in 2.6.xx. Feel free to examine libata's use of ->eh_strategy_handler in 2.4.x ;-) Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 18:03 ` Mark Lord 2004-09-14 18:07 ` Jeff Garzik @ 2004-09-14 18:08 ` Jeff Garzik 1 sibling, 0 replies; 26+ messages in thread From: Jeff Garzik @ 2004-09-14 18:08 UTC (permalink / raw) To: Mark Lord; +Cc: linux-kernel, SCSI Mailing List, James Bottomley On Tue, Sep 14, 2004 at 02:03:28PM -0400, Mark Lord wrote: > It looks to me as if the eh code prevents further queuecommand() > calls while the LLD *_reset_handler() code is running. > I wonder if it also does so for the eh_strategy_handler() ? Yes, it definitely does, by definition: SCSI's fine-grained eh hooks are called inside scsi_unjam_host(), which is the default ->eh_strategy_handler. Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 17:00 ` [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 Jeff Garzik 2004-09-14 17:27 ` Mark Lord 2004-09-14 17:51 ` Mark Lord @ 2004-09-14 18:25 ` James Bottomley 2004-09-14 18:35 ` Jeff Garzik 2004-09-15 2:39 ` Mark Lord 2 siblings, 2 replies; 26+ messages in thread From: James Bottomley @ 2004-09-14 18:25 UTC (permalink / raw) To: Jeff Garzik Cc: Mark Lord, Linux Kernel, SCSI Mailing List, linux-ide@vger.kernel.org, Alan Cox On Tue, 2004-09-14 at 13:00, Jeff Garzik wrote: > 9) use of do_sleep paradigm is dubious: you should instead try to keep > your locked code regions as small as possible. in general, this code > has far too many unlock-doit-lock sections. > > Experience has shown that too much unlock-doit-lock leads to bugs and > increases the pain when analyzing your locking. > > In particular, releasing the lock and sleeping would be very wrong in > the ->queuecommand and error handling paths (alas... I would love to > sleep in the fine-grained eh hooks) Actually, its only wrong in queuecommand because that can be called in softirq context. Sleeping in the eh paths is fine (as long as you drop the locks that the EH thread has uselessly taken for you). Indeed it's often required since the return is supposed to tell the eh thread whether the action was successful or not. James ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 18:25 ` James Bottomley @ 2004-09-14 18:35 ` Jeff Garzik 2004-09-14 18:51 ` James Bottomley 2004-09-15 2:39 ` Mark Lord 1 sibling, 1 reply; 26+ messages in thread From: Jeff Garzik @ 2004-09-14 18:35 UTC (permalink / raw) To: James Bottomley Cc: Mark Lord, Linux Kernel, SCSI Mailing List, linux-ide@vger.kernel.org, Alan Cox On Tue, Sep 14, 2004 at 02:25:35PM -0400, James Bottomley wrote: > Sleeping in the eh paths is fine (as long as you drop the locks that the > EH thread has uselessly taken for you). Indeed it's often required > since the return is supposed to tell the eh thread whether the action > was successful or not. I'm not sure this true for all arches? The lock is taken in the SCSI layer with spin_lock_irqsave(), but the low-level driver cannot perform the exact opposite, spin_unlock_irqrestore(). The best they can do is spin_lock_irq(), which isnt 100% the same. Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 18:35 ` Jeff Garzik @ 2004-09-14 18:51 ` James Bottomley 0 siblings, 0 replies; 26+ messages in thread From: James Bottomley @ 2004-09-14 18:51 UTC (permalink / raw) To: Jeff Garzik Cc: Mark Lord, Linux Kernel, SCSI Mailing List, linux-ide@vger.kernel.org, Alan Cox On Tue, 2004-09-14 at 14:35, Jeff Garzik wrote: > The lock is taken in the SCSI layer with spin_lock_irqsave(), but the > low-level driver cannot perform the exact opposite, > spin_unlock_irqrestore(). The best they can do is spin_lock_irq(), > which isnt 100% the same. That's what they do if you look. The eh_ stubs are only called from the eh_ thread, so it's safe to enable interrupts as well. The business of the mid-layer taking the locks is an annoying holdover from the "drivers don't need to do locking" mentality. Unfortunately most drivers now simply drop the locks immediately they begin an eh_ entry point and reacquire them just prior to returning ... which makes all the eh code look messy. James ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 18:25 ` James Bottomley 2004-09-14 18:35 ` Jeff Garzik @ 2004-09-15 2:39 ` Mark Lord 2004-09-15 2:47 ` Jeff Garzik 1 sibling, 1 reply; 26+ messages in thread From: Mark Lord @ 2004-09-15 2:39 UTC (permalink / raw) To: James Bottomley Cc: Jeff Garzik, Mark Lord, Linux Kernel, SCSI Mailing List, linux-ide@vger.kernel.org, Alan Cox >Actually, its only wrong in queuecommand because that can be called in >softirq context. > >Sleeping in the eh paths is fine (as long as you drop the locks that the >EH thread has uselessly taken for you). Good, that's how I understood it as well. But the locking is certainly a mess as-is in the QStor driver. Sure, it is actually all technically correct, but hard to follow. I believe I can remove nearly all of it and really tidy things up as a result. Thanks guys, this has been really helpful so far. -- Mark Lord (hdparm keeper & the original "Linux IDE Guy") ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-15 2:39 ` Mark Lord @ 2004-09-15 2:47 ` Jeff Garzik 2004-09-15 12:35 ` Mark Lord 0 siblings, 1 reply; 26+ messages in thread From: Jeff Garzik @ 2004-09-15 2:47 UTC (permalink / raw) To: Mark Lord Cc: James Bottomley, Mark Lord, Linux Kernel, SCSI Mailing List, linux-ide@vger.kernel.org, Alan Cox Groovy. FWIW (if it wasn't obvious from context) my objection in general to the driver is withdrawn, since you explained it is RAID and not an ATA driver. I would really like to work on consolidating the ATA code in libata, though. As the name implies, it's a library -- don't feel that your driver must conform to the libata driver API in order to make use of all its functions. And feel free to add to it. Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-15 2:47 ` Jeff Garzik @ 2004-09-15 12:35 ` Mark Lord 0 siblings, 0 replies; 26+ messages in thread From: Mark Lord @ 2004-09-15 12:35 UTC (permalink / raw) To: Jeff Garzik Cc: Mark Lord, James Bottomley, Linux Kernel, SCSI Mailing List, linux-ide@vger.kernel.org, Alan Cox I would really like to work on consolidating the ATA code in libata, though. As the name implies, it's a library -- don't feel that your driver must conform to the libata driver API in order to make use of all its functions. And feel free to add to it. Yes, there are definite code sharing possibilities there to be explored. Right now, my first priority is to get support for this hardware into the kernel. This same driver source will also be backported to mid-2.4.xx series, both Redhat and generic. After that, we can modify some interfaces to reduce the small overlaps that may present. Next revision is due out later today. It may still have a few warts to work out, but I think it is looking much better than before. Better to have a decent hardware driver within the tree, than an unknown vendor-only binary driver outside the tree. Cheers -- Mark Lord (hdparm keeper & the original "Linux IDE Guy") ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 @ 2004-09-14 15:43 Mark Lord 2004-09-14 16:21 ` Jeff Garzik 2004-09-15 7:02 ` Christoph Hellwig 0 siblings, 2 replies; 26+ messages in thread From: Mark Lord @ 2004-09-14 15:43 UTC (permalink / raw) To: linux-scsi [-- Attachment #1: Type: text/plain, Size: 1030 bytes --] My first attempt at posting this seems to have gone AWOL, so here it comes again. Also being posted to linux-scsi. Here is the first public release of the 2.6.xx driver source code for the Pacific Digital Corporation QStor SATA/RAID chip. This 4-channel chip has hardware-assisted RAID0/RAID1/RAID10, host-queuing, per-request TCQ/NCQ support, support for hot insertion and removal of drives, etc.. The 64-bit/66Mhz chip shows throughput in excess of 200MByte/sec on my ancient P3-1GHz test system, and can do much better when installed in a PCI-X slot. The driver (attached) supports most of the chip features, including host, native and legacy tagged queuing, but does not yet include boot-from-raid support (coming soon). Both hdparm and smartmontools are fully supported by this driver. This patch is against linux-2.6.9-rc2. Please email me any errors or corrections you may deem necessary for kernel inclusion. Signed-off-by: Mark Lord <mlord@pobox.com> -- Mark Lord (hdparm keeper & the original "Linux IDE Guy") [-- Attachment #2: qstor.patch-2.6.9-rc2.gz --] [-- Type: application/x-gzip, Size: 35858 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 15:43 Mark Lord @ 2004-09-14 16:21 ` Jeff Garzik 2004-09-14 16:23 ` Jeff Garzik ` (2 more replies) 2004-09-15 7:02 ` Christoph Hellwig 1 sibling, 3 replies; 26+ messages in thread From: Jeff Garzik @ 2004-09-14 16:21 UTC (permalink / raw) To: Mark Lord; +Cc: linux-scsi, James Bottomley Mark Lord wrote: > My first attempt at posting this seems to have gone AWOL, > so here it comes again. Also being posted to linux-scsi. > > Here is the first public release of the 2.6.xx driver > source code for the Pacific Digital Corporation QStor SATA/RAID chip. > > This 4-channel chip has hardware-assisted RAID0/RAID1/RAID10, > host-queuing, per-request TCQ/NCQ support, support for hot insertion > and removal of drives, etc.. The 64-bit/66Mhz chip shows throughput > in excess of 200MByte/sec on my ancient P3-1GHz test system, > and can do much better when installed in a PCI-X slot. How much of the RAID is actually hardware-assisted? > The driver (attached) supports most of the chip features, > including host, native and legacy tagged queuing, > but does not yet include boot-from-raid support (coming soon). > > Both hdparm and smartmontools are fully supported by this driver. Linus vetoed future SCSI->ATA translators. He only allowed libata because I promised to remove the translation and make it a native block driver in the future, which I have been working towards. Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 16:21 ` Jeff Garzik @ 2004-09-14 16:23 ` Jeff Garzik 2004-09-14 16:39 ` Nathan Bryant 2004-09-15 4:22 ` Douglas Gilbert 2 siblings, 0 replies; 26+ messages in thread From: Jeff Garzik @ 2004-09-14 16:23 UTC (permalink / raw) To: Mark Lord; +Cc: linux-scsi, James Bottomley Jeff Garzik wrote: > Linus vetoed future SCSI->ATA translators. He only allowed libata > because I promised to remove the translation and make it a native block > driver in the future, which I have been working towards. This is why the Promise SX8 driver is a block driver, for example... Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 16:21 ` Jeff Garzik 2004-09-14 16:23 ` Jeff Garzik @ 2004-09-14 16:39 ` Nathan Bryant 2004-09-14 17:02 ` Jeff Garzik 2004-09-15 4:22 ` Douglas Gilbert 2 siblings, 1 reply; 26+ messages in thread From: Nathan Bryant @ 2004-09-14 16:39 UTC (permalink / raw) To: Jeff Garzik; +Cc: Mark Lord, linux-scsi, James Bottomley Jeff Garzik wrote: > Linus vetoed future SCSI->ATA translators. He only allowed libata > because I promised to remove the translation and make it a native block > driver in the future, which I have been working towards. Oh, this is incredibly good to hear. It definitely clarifies the path towards implementing new features such as power management... Is this written up in a FAQ somewhere? Nathan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 16:39 ` Nathan Bryant @ 2004-09-14 17:02 ` Jeff Garzik 0 siblings, 0 replies; 26+ messages in thread From: Jeff Garzik @ 2004-09-14 17:02 UTC (permalink / raw) To: Nathan Bryant; +Cc: Mark Lord, linux-scsi, James Bottomley Nathan Bryant wrote: > Jeff Garzik wrote: > >> Linus vetoed future SCSI->ATA translators. He only allowed libata >> because I promised to remove the translation and make it a native >> block driver in the future, which I have been working towards. > > > Oh, this is incredibly good to hear. It definitely clarifies the path > towards implementing new features such as power management... > > Is this written up in a FAQ somewhere? Not AFAIK. The basic rule is "don't translate the native protocol into something else." and it has been in place for quite a while. I was allowed a one-time exception for libata, which translates the native ATA into SCSI. The SCSI portions are separated into libata-scsi.c so that they may be easily excised or made optional (likely). Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 16:21 ` Jeff Garzik 2004-09-14 16:23 ` Jeff Garzik 2004-09-14 16:39 ` Nathan Bryant @ 2004-09-15 4:22 ` Douglas Gilbert 2004-09-15 4:30 ` Jeff Garzik 2004-09-15 12:47 ` Mark Lord 2 siblings, 2 replies; 26+ messages in thread From: Douglas Gilbert @ 2004-09-15 4:22 UTC (permalink / raw) To: Jeff Garzik, Mark Lord; +Cc: linux-scsi Jeff Garzik wrote: > Mark Lord wrote: > >> My first attempt at posting this seems to have gone AWOL, >> so here it comes again. Also being posted to linux-scsi. >> >> Here is the first public release of the 2.6.xx driver >> source code for the Pacific Digital Corporation QStor SATA/RAID chip. >> >> This 4-channel chip has hardware-assisted RAID0/RAID1/RAID10, >> host-queuing, per-request TCQ/NCQ support, support for hot insertion >> and removal of drives, etc.. The 64-bit/66Mhz chip shows throughput >> in excess of 200MByte/sec on my ancient P3-1GHz test system, >> and can do much better when installed in a PCI-X slot. > > > How much of the RAID is actually hardware-assisted? > > >> The driver (attached) supports most of the chip features, >> including host, native and legacy tagged queuing, >> but does not yet include boot-from-raid support (coming soon). >> >> Both hdparm and smartmontools are fully supported by this driver. Mark, I'm curious how smartmontools is supported. Does the driver support SCSI LOG SENSE commands on the physical units or does it take the 3ware/Marvell route? There is no sign of QStor specific code in smartmontools's CVS. > Linus vetoed future SCSI->ATA translators. He only allowed libata > because I promised to remove the translation and make it a native block > driver in the future, which I have been working towards. As Jeff is aware, SCSI->ATA translation (SAT) is in the process of being standardized at t10.org . The model being used outlines a SAT layer going into any of three places: - in the host OS above a SATA/ATA HBA driver (i.e. what libata does and Linus frowns upon) - in the host OS above a SAS HBA driver which, amongst other protocols, has the SAS Tunnelling Protocol (STP) which conveys ATA/ATAPI7 commands through SAS infrastructure - somewhere in the service delivery subsystem, specifically in SAS expanders which have phys connected to SATA disks (Linus vetoes have no influence here) Consider this pathological situation: Start with one SATA II disk and connect it to a port selector (a SATA II device) which effectively makes the SATA disk dual ported. Connect one of those ports to a SATA HBA that lives in the Linux ATA subsystem (where no SAT is allowed). Connect the other port to a SAS expander which includes a SAT layer and then connect that expander to a SAS HBA in the Linux SCSI subsytem. That should confuse the !@#$ out of any application client trying to get a handle on what is happening. I assume that OS-based ATA->SCSI translation is also out of the question. Doug Gilbert ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-15 4:22 ` Douglas Gilbert @ 2004-09-15 4:30 ` Jeff Garzik 2004-09-15 12:47 ` Mark Lord 1 sibling, 0 replies; 26+ messages in thread From: Jeff Garzik @ 2004-09-15 4:30 UTC (permalink / raw) To: Douglas Gilbert; +Cc: Mark Lord, linux-scsi On Wed, Sep 15, 2004 at 02:22:36PM +1000, Douglas Gilbert wrote: > I assume that OS-based ATA->SCSI translation is also out of > the question. In order to keep compatibility with existing libata users after libata becomes a block driver, the ATA->SCSI translation will live on as a completely separate, independent, optional kernel module. If we are really lucky we can use it as a library for some of the other drivers that do some amount of ATA->SCSI translation, or ide-scsi... Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-15 4:22 ` Douglas Gilbert 2004-09-15 4:30 ` Jeff Garzik @ 2004-09-15 12:47 ` Mark Lord 2004-09-15 12:55 ` Jens Axboe 1 sibling, 1 reply; 26+ messages in thread From: Mark Lord @ 2004-09-15 12:47 UTC (permalink / raw) To: dougg; +Cc: Jeff Garzik, linux-scsi Douglas Gilbert wrote: >>> >>> Both hdparm and smartmontools are fully supported by this driver. > > I'm curious how smartmontools is supported. Does the driver > support SCSI LOG SENSE commands on the physical units or does > it take the 3ware/Marvell route? > There is no sign of QStor specific code in smartmontools's CVS. This driver simply implements the HDIO_DRIVE_CMD/HDIO_DRIVE_TASK ioctl call (from Linux IDE). By doing so, it automatically gains compatibility with smartmontools (-d ata), and with hdparm. Currently, the implementation of those is native to the controller. But eventually, I'd like to see it implement Curtis's ATA Passthrough spec (to which I've been contributing). Port selectors aside --> a nightmare for any sysadmin or driver writer --> Linux has a huge unresolved issue for block devices. It is currently next-to-impossible for a vendor to support a new drive controller card under Linux unless it uses either SCSI or libata. Doing one as a new block driver is not an option, because of the installation issues for the end-user during the first couple of years of deployment --> until the distros begin to include the required /dev/ entries everywhere. To solve this requires a nice generic /dev/ interface for disks, common to all drivers which talk to such devices. Currently on Linux, that interface is called "SCSI". I think it might not be unreasonable to gradually evolve the SCSI host interface to include, say, a non-translating queuecommand() method, and associated pals. This would get rid of much (not all) of the double-translation objection, and provide a smooth path for supporting Firewire, USB, parallel-port, SATA, ATA, and of course SCSI, disks all under one naming subsystem. We practically have that already today. The SCSI mid-layer is a nice generic block device glue system. We just need perhaps to make it less SCSI-specific. Cheers -- Mark Lord (hdparm keeper & the original "Linux IDE Guy") ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-15 12:47 ` Mark Lord @ 2004-09-15 12:55 ` Jens Axboe 2004-09-15 13:13 ` Matthew Wilcox 0 siblings, 1 reply; 26+ messages in thread From: Jens Axboe @ 2004-09-15 12:55 UTC (permalink / raw) To: Mark Lord; +Cc: dougg, Jeff Garzik, linux-scsi On Wed, Sep 15 2004, Mark Lord wrote: > It is currently next-to-impossible for a vendor to support a > new drive controller card under Linux unless it uses either SCSI > or libata. Doing one as a new block driver is not an option, > because of the installation issues for the end-user during the > first couple of years of deployment --> until the distros begin > to include the required /dev/ entries everywhere. > > To solve this requires a nice generic /dev/ interface for disks, > common to all drivers which talk to such devices. > > Currently on Linux, that interface is called "SCSI". > I think it might not be unreasonable to gradually evolve > the SCSI host interface to include, say, a non-translating > queuecommand() method, and associated pals. > > This would get rid of much (not all) of the double-translation objection, > and provide a smooth path for supporting Firewire, USB, parallel-port, > SATA, ATA, and of course SCSI, disks all under one naming subsystem. > > We practically have that already today. > The SCSI mid-layer is a nice generic block device glue system. > We just need perhaps to make it less SCSI-specific. This is what we have been doing for quite some time. If you look, a lot of the helper functions have actually been moved _out_ of SCSI and into the block layer. Most of it is already there. The SCSI mid layer is getting smaller, not bigger. The remaining piece (well the biggest one at least) is a nice /dev/disk abstraction. That's currently the biggest obstacle to nice generic block drivers, not the infrastructure. -- Jens Axboe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-15 12:55 ` Jens Axboe @ 2004-09-15 13:13 ` Matthew Wilcox 2004-09-15 16:14 ` Jeff Garzik 0 siblings, 1 reply; 26+ messages in thread From: Matthew Wilcox @ 2004-09-15 13:13 UTC (permalink / raw) To: Jens Axboe; +Cc: Mark Lord, dougg, Jeff Garzik, linux-scsi On Wed, Sep 15, 2004 at 02:55:46PM +0200, Jens Axboe wrote: > The remaining piece (well the biggest one at least) is a nice /dev/disk > abstraction. That's currently the biggest obstacle to nice generic block > drivers, not the infrastructure. I'll just take this opportunity to plug /dev/drive over /dev/disk. Not only does it avoid the disc/disk dialect problem, it is also more accurate in the case of removable media. -- "Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception." -- Mark Twain ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-15 13:13 ` Matthew Wilcox @ 2004-09-15 16:14 ` Jeff Garzik 0 siblings, 0 replies; 26+ messages in thread From: Jeff Garzik @ 2004-09-15 16:14 UTC (permalink / raw) To: Matthew Wilcox; +Cc: Jens Axboe, Mark Lord, dougg, linux-scsi Matthew Wilcox wrote: > On Wed, Sep 15, 2004 at 02:55:46PM +0200, Jens Axboe wrote: > >>The remaining piece (well the biggest one at least) is a nice /dev/disk >>abstraction. That's currently the biggest obstacle to nice generic block >>drivers, not the infrastructure. > > > I'll just take this opportunity to plug /dev/drive over /dev/disk. I'll take this opportunity to recommend that posters avoid following this sub-thread, for fear of distracting from the real technical issues.... Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 2004-09-14 15:43 Mark Lord 2004-09-14 16:21 ` Jeff Garzik @ 2004-09-15 7:02 ` Christoph Hellwig 1 sibling, 0 replies; 26+ messages in thread From: Christoph Hellwig @ 2004-09-15 7:02 UTC (permalink / raw) To: Mark Lord; +Cc: linux-scsi Please fix all the style issues in here, aka - linebreaks after 80 characters - don't typedef structs unless they're used opaqueuely - please avoid C++-style comments even if C99 allows them - don't use "scsi.h", only headers from <scsi/*.h>, that also means getting rid of the Scsi_Foo typedefs - please don't include headers in other headers unless nessecary I also don't want another SCSI->ATA translator in drivers/scsi, please find some way to share codce with an existing one. I'll do a real review once the driver is readable. ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2004-09-15 16:14 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <41471163.10709@rtr.ca>
2004-09-14 17:00 ` [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2 Jeff Garzik
2004-09-14 17:27 ` Mark Lord
2004-09-14 17:33 ` Jeff Garzik
2004-09-14 17:51 ` Mark Lord
2004-09-14 17:56 ` Jeff Garzik
2004-09-14 18:03 ` Mark Lord
2004-09-14 18:07 ` Jeff Garzik
2004-09-14 18:08 ` Jeff Garzik
2004-09-14 18:25 ` James Bottomley
2004-09-14 18:35 ` Jeff Garzik
2004-09-14 18:51 ` James Bottomley
2004-09-15 2:39 ` Mark Lord
2004-09-15 2:47 ` Jeff Garzik
2004-09-15 12:35 ` Mark Lord
2004-09-14 15:43 Mark Lord
2004-09-14 16:21 ` Jeff Garzik
2004-09-14 16:23 ` Jeff Garzik
2004-09-14 16:39 ` Nathan Bryant
2004-09-14 17:02 ` Jeff Garzik
2004-09-15 4:22 ` Douglas Gilbert
2004-09-15 4:30 ` Jeff Garzik
2004-09-15 12:47 ` Mark Lord
2004-09-15 12:55 ` Jens Axboe
2004-09-15 13:13 ` Matthew Wilcox
2004-09-15 16:14 ` Jeff Garzik
2004-09-15 7:02 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox