Re: [PATCH 3/3] libata: support host-aware ZAC devices
On Tue, 31 Mar 2015 10:52:43 +0200 Hannes Reinecke wrote: > Byte 69 bits 0:1 in the IDENTIFY DEVICE data are proposed > to indicate a host-aware ZAC device. > And whenever we detect a ZAC-compatible device we should > be displaying the zoned block characteristics VPD page. I don't think this belongs upstream as is - because this is a "proposed" - it might change and then we have Linux boxes in the field randomly misinterpreting bits if instead other bits are used or the definition changes. Until it's officially in the spec there ought IMHO to be a boot option to enable the test. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: fdomain: add missing CONFIG_ to PCMCIA
On Tue, 9 Feb 2016 14:16:49 -0800 Yang Shi wrote: > There are some "#ifdef PCMCIA" in the driver code, but all of them missed > CONFIG_ prefix. Correct PCMCIA to CONFIG_PCMCIA. > > Signed-off-by: Yang Shi NAK. This breaks the driver completely Read drivers/scsi/pcmcia/fdomain* Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] [SCSI] dpt_i2o: use proper pci driver
On Wed, 17 Feb 2016 17:50:14 +0530 Sudip Mukherjee wrote: > This is a pci device but was not done in the usual way a pci driver is > done. Convert the driver into a proper pci driver. This looks completely wrong. Please read the I2O 1.5 specification document before playing with that code. You can't simply convert it to a standard PCI driver as all the IOPs are supposed to be detected and then the correct initialization sequence executed across the set of IOPs. IOPs are allowed to talk to one another and the system table binds it all together. If you do hot plug then you need to follow the specification and do all the extra notifications. Unless you've got multiple FC909/FC920 fibechannel cards or similar to test with I would just leave this well alone. Your original simple patch is *MUCH* safer in this specific case. So NAK Alan > > Signed-off-by: Sudip Mukherjee > --- > > v1: only build warning related to "dptids defined but not used" was > fixed using #ifdef > > drivers/scsi/dpt_i2o.c | 269 > ++--- > drivers/scsi/dpti.h| 5 +- > 2 files changed, 120 insertions(+), 154 deletions(-) > > diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c > index d4cda5e..9f50d1ae 100644 > --- a/drivers/scsi/dpt_i2o.c > +++ b/drivers/scsi/dpt_i2o.c > @@ -111,6 +111,7 @@ static int sys_tbl_len; > > static adpt_hba* hba_chain = NULL; > static int hba_count = 0; > +static bool control_reg; > > static struct class *adpt_sysfs_class; > > @@ -187,118 +188,6 @@ static struct pci_device_id dptids[] = { > }; > MODULE_DEVICE_TABLE(pci,dptids); > > -static int adpt_detect(struct scsi_host_template* sht) > -{ > - struct pci_dev *pDev = NULL; > - adpt_hba *pHba; > - adpt_hba *next; > - > - PINFO("Detecting Adaptec I2O RAID controllers...\n"); > - > -/* search for all Adatpec I2O RAID cards */ > - while ((pDev = pci_get_device( PCI_DPT_VENDOR_ID, PCI_ANY_ID, pDev))) { > - if(pDev->device == PCI_DPT_DEVICE_ID || > -pDev->device == PCI_DPT_RAPTOR_DEVICE_ID){ > - if(adpt_install_hba(sht, pDev) ){ > - PERROR("Could not Init an I2O RAID device\n"); > - PERROR("Will not try to detect others.\n"); > - return hba_count-1; > - } > - pci_dev_get(pDev); > - } > - } > - > - /* In INIT state, Activate IOPs */ > - for (pHba = hba_chain; pHba; pHba = next) { > - next = pHba->next; > - // Activate does get status , init outbound, and get hrt > - if (adpt_i2o_activate_hba(pHba) < 0) { > - adpt_i2o_delete_hba(pHba); > - } > - } > - > - > - /* Active IOPs in HOLD state */ > - > -rebuild_sys_tab: > - if (hba_chain == NULL) > - return 0; > - > - /* > - * If build_sys_table fails, we kill everything and bail > - * as we can't init the IOPs w/o a system table > - */ > - if (adpt_i2o_build_sys_table() < 0) { > - adpt_i2o_sys_shutdown(); > - return 0; > - } > - > - PDEBUG("HBA's in HOLD state\n"); > - > - /* If IOP don't get online, we need to rebuild the System table */ > - for (pHba = hba_chain; pHba; pHba = pHba->next) { > - if (adpt_i2o_online_hba(pHba) < 0) { > - adpt_i2o_delete_hba(pHba); > - goto rebuild_sys_tab; > - } > - } > - > - /* Active IOPs now in OPERATIONAL state */ > - PDEBUG("HBA's in OPERATIONAL state\n"); > - > - printk("dpti: If you have a lot of devices this could take a few > minutes.\n"); > - for (pHba = hba_chain; pHba; pHba = next) { > - next = pHba->next; > - printk(KERN_INFO"%s: Reading the hardware resource table.\n", > pHba->name); > - if (adpt_i2o_lct_get(pHba) < 0){ > - adpt_i2o_delete_hba(pHba); > - continue; > - } > - > - if (adpt_i2o_parse_lct(pHba) < 0){ > - adpt_i2o_delete_hba(pHba); > - continue; > - } > - adpt_inquiry(pHba); > - } > - > - adpt_sysfs_class = class_create(THIS_MODULE, "dpt_i2o"); > - if (IS_ERR(adpt_sysfs_class)) { > - printk(KERN_WARNING"dpti: unable to create dpt_i2o class\n"); > - adpt_sysfs_class = NULL; > - } > - > - for (pHba = hba_chain; pHba; pHba = next) { > - next = pHba->next; > - if (adpt_scsi_host_alloc(pHba, sht) < 0){ > - adpt_i2o_delete_hba(pHba); > - continue; > - } > - pHba->initialized = TRUE; > - pHba->state &= ~DPTI_STATE_RESET; > - if (adpt_sysfs_class) { > - struct device *dev = device_create(adpt_
Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
On Wed, 9 Dec 2015 13:24:53 +0300 Dan Carpenter wrote: > On 64 bit CPUs there is a memory corruption bug on probe(). It should > be a u32 pointer instead of an unsigned long pointer or we write past > the end of the setupdata[] array. > > Signed-off-by: Dan Carpenter > Reviewed-by: Hannes Reinecke > --- > Resending because we have shuffled the code around so the patch needed > to be refreshed against linux-next. Although I do wonder why we are > still working on this code since it has never worked on 64 bit systems > so probably all the users gave up a decade ago. So this is untested ? If so please make it very clear in the commit message because the kernel is IMHO getting too full of polished, neat, recently modified, never tested, never used code. I agree it would be better if the driver was simply deleted. I've not even seen an ATP870 bug report in years. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
On Wed, 9 Dec 2015 16:45:12 +0300 Dan Carpenter wrote: > Everyone knows I didn't test it but it's an obvious one line fix for > memory corruption. If no one uses the code, at least this is harmless > and silences a static checker warning. > > In olden times we used to say, "Oh this bounds checking is crap but it's > root only so let's leave it alone." But these days we just fix it. > It's easier to just fix everything instead of trying to decide which > bugs are critical. Unfortunately it's all too easy to look down 50 commit messages to an apaprently active file all "fixing small bugs" or "correcting indenting" without realising that every single one of them should have been tagged "[UNTESTED]: " so that anyone looking at the code can see immediately its historical hazardous waste. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
> You should just delete that code along with the Kconfig and Makefile > entries. Don't bother moving it to staging. The move to staging is > supposed to give people one last chance to yell if they absolutely need > the code. No one has compiled this code for years so no one will miss > it. And for stuff which might be worth saving (eg something that looks rather broken but has possibly got users) the driver goes into staging in its own directory and the Makefile and Kconfig entry for it move into the staging directory with the hope that someone screams and maintains it. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 66/77] ncr5380: Fix soft lockups
On Tue, 22 Dec 2015 12:18:44 +1100 Finn Thain wrote: > Because of the rudimentary design of the chip, it is necessary to poll the > SCSI bus signals during PIO and this tends to hog the CPU. The driver will > accept new commands while others execute, and this causes a soft lockup > because the workqueue item will not terminate until the issue queue is > emptied. > > When exercising dmx3191d using sequential IO from dd, the driver is sent > 512 KiB WRITE commands and 128 KiB READs. For a PIO transfer, the rate is > is only about 300 KiB/s, so these are long-running commands. And although > PDMA may run at several MiB/s, interrupts are disabled for the duration > of the transfer. > > Fix the unresponsiveness and soft lockup issues by calling cond_resched() > after each command is completed and by limiting max_sectors for drivers > that don't implement real DMA. Is there a reason for not doing some limiting in the DMA case too. A 512K write command even with DMA on a low end 68K box introduces a second of latency before another I/O can be scheduled ? Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 69/78] ncr5380: Fix whitespace in comments using regexp
On Sat, 02 Jan 2016 23:54:28 -0800 Joe Perches wrote: > On Sun, 2016-01-03 at 16:06 +1100, Finn Thain wrote: > > Hanging indentation was a poor choice for the text inside comments. It > > has been used in the wrong places and done badly elsewhere. There is > > little consistency within any file. One fork of the core driver uses > > tabs for this indentation while the other uses spaces. Better to use > > flush-left alignment throughout. > > > > This patch is the result of the following substitution. It replaces tabs > > and spaces at the start of a comment line with a single space. > > > > perl -i -pe 's,^(\t*[/ ]\*)[ \t]+,$1 ,' drivers/scsi/{atari_,}NCR5380.c > > > > This removes some unimportant discrepancies between the two core driver > > forks so that the important ones become obvious, to facilitate > > reunification. > > I still think this patch is poor at best and > overall not useful. I would beg to differ. As a tool for understanding the differences as you step through the versions it's invaluable. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 69/78] ncr5380: Fix whitespace in comments using regexp
> > I would beg to differ. As a tool for understanding the differences as you > > step through the versions it's invaluable. > > This removes intentional formatting that > makes reading comments easier. And this matters at the end of the patch series because ? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RESEND v2 1/2] Hard disk resume time optimization, asynchronous ata_port_resume
> thus allowing the UI to come online sooner. There may be a short period after > resume where the disks are still spinning up in the background, but the user > shouldn't notice since the OS can function with the data left in RAM. I wonder how many marginal power supplies this will find 8) I still think it's the right thing to do. In the SCSI world a bit more caution was needed however. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/1] drivers/scsi/bfa/bfad_debugfs.c: replace 2 kzalloc/copy_from_user to memdup_user
On Fri, 30 May 2014 19:07:36 +0200 Fabian Frederick wrote: > (memdup_user can be used to replace kmalloc/copy_from_user. Not sure if it's > ok with kzalloc ...) > + kern_buf = memdup_user((void __user *)buf, nbytes); > + if (IS_ERR(kern_buf)) { > + printk(KERN_INFO "bfad[%d]: Failed to copy buffer\n", > +bfad->inst_no); While you are tidying up lose the printk KERN_INFO stuff here, it really isn't needed. Looks good to me and it also fixes the incorrect return of -ENOMEM the old code had on a copy_from_user failing. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [regression] fix 32-bit breakage in block device read(2) (was Re: 32-bit bug in iovec iterator changes)
On Mon, 23 Jun 2014 11:43:02 -0400 "Theodore Ts'o" wrote: > On Mon, Jun 23, 2014 at 08:44:40AM +0100, Al Viro wrote: > > > > OK, here it is, hopefully with sufficient comments: > > The comments look really good. I assume you'll get this to > Linus in time for 3.16-rc3? Fixes the 32GB 'can't partition' bug I reported too. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] libata: Whitelist SSDs that are known to properly return zeroes after TRIM
On Mon, 8 Dec 2014 10:15:59 -0500 "Theodore Ts'o" wrote: > On Fri, Dec 05, 2014 at 10:58:09PM +, Elliott, Robert (Server Storage) > wrote: > > > I have a Crucial_CT256MX1 (i.e. MX100) and it does reliably zero. > > > > make me concerned about this whitelist approach. > > > > I think you need a manufacturer assertion that this is indeed > > the design intent; you cannot be certain based on observation > > from outside. > > How is this different from a manufacturer assertion that they follow a > SCSI or ATA standard? There have been cases in the distant past > (fortunately) of disk manufacturers that ignored a CACHE FLUSH command > just to get higher Winbench scores. Does that mean we can't trust > them to do anything right? At the time they never promised to honour cache flush. The reason it was became mandatory in the specification was in part so that the vendors could all force each other to play fair. If its "optional" then it's tough..., if they say they meet the standard it's class action 8) If this is a promise then it ought to be good James: "USB devices tend to supply their own driver" has not been true for some years now. Microsoft provide an in-box driver and vendors have the choice of using that or certifying their own via WHQL, which is a bit like choosing between free ice cream and banging your head against a plank cover in nails. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] libata-core: do not set dev->max_sectors for LBA48 devices
> Tom> is totally irrational for a general default. I mean, given that it > Tom> was 1024 (512k), try to double it? Fine. Try to quadruple it? > Tom> Alright. We'll need to deal with some alignment / boundary issue > Tom> (like the typical 65535 vs 65536 case)? Okay let's do it. But > Tom> what's the sense in picking a random RAID configuartion as the base > Tom> to decide the default? > > I agree that the new default is completely arbitrary. And I think there > was a bit of controversy at the time. However, the numbers were > compelling so the change went in. For older SCSI and especially ATA drives (and it wouldn't surprise me if it is true of modern ones) there are also huge latency tradeoffs. Before you jump up and down about numbers what are the latency numbers like on classic ATA drives with that sized block I/O. You could easily up your RAID numbers while wrecking realtime and desktop performance. Without NCQ you certainly don't want huge I/O's blocking up the controller, with NCQ it depends if the firmware these days has improved and understands any kind of fairness or like the early ones and older SCSI drives will happily starve requests. Conceptually it also seems wrong - if you want to avoid partial stripe writes on a consumer grade disk subsystem then use smaller stripes. At the ATA level we can detect both the presence of command queueing ability, and also whether the device is spinnning rust or not, so it may be smarter defaults could be done based upon whether the device is an SSD or not. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html