RE: [PATCH] mpt fusion: Power Management fixes for MPT SAS PCI-Econtrollers
Thanks James, for your feeback, I will add your comments and repost the patch again. -Sathya -Original Message- From: James Bottomley [mailto:[EMAIL PROTECTED] Sent: Tuesday, February 12, 2008 4:24 AM To: Prakash, Sathya Cc: linux-scsi@vger.kernel.org; Moore, Eric; [EMAIL PROTECTED] Subject: Re: [PATCH] mpt fusion: Power Management fixes for MPT SAS PCI-Econtrollers On Mon, 2008-02-11 at 18:36 +0530, Prakash, Sathya wrote: > This patch was submitted as [PATCH 2/3] yesterday, since it did not > reach the list due to CC errors sending this again. > It is regenerated against new git tree. > > The system power state changes like hibernation and standby are not > happening properly with 106XE controllers, this patch modifies the > driver to free resources and allocate resources in power management > entry points and to enable MSI interrupts for SAS controllers I really wish you hadn't done this. Both MSI and suspend/resume are somewhat bug inducing at the moment. If you combine both the msi default change and suspend/resume changes in a single patch, you'll make it very hard for us to tell which is causing the problem by simple bisection. > Signed-off-by: Sathya Prakash <[EMAIL PROTECTED]> > --- > > diff --git a/drivers/message/fusion/mptbase.c > b/drivers/message/fusion/mptbase.c > index bfda731..b3b80bf 100644 > --- a/drivers/message/fusion/mptbase.c > +++ b/drivers/message/fusion/mptbase.c > @@ -1429,6 +1429,100 @@ mpt_get_product_name(u16 vendor, u16 device, u8 revision, char *prod_name) > sprintf(prod_name, "%s", product_str); } > > +/** > + * mpt_mapresources - map in memory mapped io > + * @ioc: Pointer to pointer to IOC adapter > + * > + **/ > +static int > +mpt_mapresources(MPT_ADAPTER *ioc) > +{ > + u8 __iomem *mem; > + int ii; > + unsigned longmem_phys; > + unsigned longport; > + u32 msize; > + u32 psize; > + u8 revision; > + int r = -ENODEV; > + struct pci_dev *pdev; > + > + pdev = ioc->pcidev; > + ioc->bars = pci_select_bars(pdev, IORESOURCE_MEM); > + if (pci_enable_device_mem(pdev)) { > + printk(MYIOC_s_ERR_FMT "pci_enable_device_mem() " > + "failed\n", ioc->name); > + return r; > + } > + if (pci_request_selected_regions(pdev, ioc->bars, "mpt")) { > + printk(MYIOC_s_ERR_FMT "pci_request_selected_regions() with " > + "MEM failed\n", ioc->name); > + return r; > + } I can't help noticing that any failure after this point leaves the driver with the selected regions requested, meaning it can never be attached again. > + pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision); > + > + if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK) > + && !pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK)) { > + dinitprintk(ioc, printk(MYIOC_s_INFO_FMT > + ": 64 BIT PCI BUS DMA ADDRESSING SUPPORTED\n", > + ioc->name)); > + } else if (!pci_set_dma_mask(pdev, DMA_32BIT_MASK) > + && !pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK)) { > + dinitprintk(ioc, printk(MYIOC_s_INFO_FMT > + ": 32 BIT PCI BUS DMA ADDRESSING SUPPORTED\n", > + ioc->name)); > + } else { > + printk(MYIOC_s_WARN_FMT "no suitable DMA mask for %s\n", > + ioc->name, pci_name(pdev)); > + return r; > + } > + > + mem_phys = msize = 0; > + port = psize = 0; > + for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) { > + if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) { > + if (psize) > + continue; > + /* Get I/O space! */ > + port = pci_resource_start(pdev, ii); > + psize = pci_resource_len(pdev, ii); > + } else { > + if (msize) > + continue; > + /* Get memmap */ > + mem_phys = pci_resource_start(pdev, ii); > + msize = pci_resource_len(pdev, ii); > + } > + } > + ioc->mem_size = msize; > + > + mem = NULL; > + /* Get logical ptr for PciMem0 space */ > + /*mem = ioremap(mem_phys, msize);*/ > + mem = ioremap(mem_phys, msize); > + if (mem == NULL) { > + printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter" > + " memory!\n", ioc->name); > + return -EINVAL; > + } > + ioc->memmap = mem; > + dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n", > + ioc->name, mem, mem_phys)); > + > + ioc->mem_phys = mem_phys; > + ioc->chip = (SYSIF_REGS __iomem *)mem; > + > + /* Save Port IO values in case we need to do downloadboot */ > + { > + u8 *pmem
Drivers 'sd' & 'sr' need updating: please use bus_type methods...
I turned on the option to flag deprecated code, and these two popped up: Driver 'sd' needs updating - please use bus_type methods Driver 'sr' needs updating - please use bus_type methods I'm assuming these are referring to the SCSI devices? I'm assuming they are already "known about"? Just checking... -l - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] block: clear drain buffer if draining for write command
On Sat, 2008-02-09 at 10:40 +0900, Tejun Heo wrote: > Clear drain buffer before chaining if the command in question is a > write. > > Signed-off-by: Tejun Heo <[EMAIL PROTECTED]> > --- > block/blk-merge.c |3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/block/blk-merge.c b/block/blk-merge.c > index d50cfc8..d0b9031 100644 > --- a/block/blk-merge.c > +++ b/block/blk-merge.c > @@ -221,6 +221,9 @@ new_segment: > } /* segments in rq */ > > if (q->dma_drain_size && q->dma_drain_needed(rq)) { > + if (rq->cmd_flags & REQ_RW) > + memset(q->dma_drain_buffer, 0, q->dma_drain_size); > + This is a bit performance impacting, isn't it? If you're worried about data security from a read overrun, then you could do a lazy clear on read completion if the overrun was used (i.e. if there's any residual). James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mpt fusion: Power Management fixes for MPT SAS PCI-E controllers
On Mon, 2008-02-11 at 18:36 +0530, Prakash, Sathya wrote: > This patch was submitted as [PATCH 2/3] yesterday, since it did not reach the > list due to CC errors sending this again. > It is regenerated against new git tree. > > The system power state changes like hibernation and standby are not happening > properly with 106XE controllers, this patch modifies the driver to free > resources and allocate resources in power management entry points and to > enable > MSI interrupts for SAS controllers I really wish you hadn't done this. Both MSI and suspend/resume are somewhat bug inducing at the moment. If you combine both the msi default change and suspend/resume changes in a single patch, you'll make it very hard for us to tell which is causing the problem by simple bisection. > Signed-off-by: Sathya Prakash <[EMAIL PROTECTED]> > --- > > diff --git a/drivers/message/fusion/mptbase.c > b/drivers/message/fusion/mptbase.c > index bfda731..b3b80bf 100644 > --- a/drivers/message/fusion/mptbase.c > +++ b/drivers/message/fusion/mptbase.c > @@ -1429,6 +1429,100 @@ mpt_get_product_name(u16 vendor, u16 device, u8 > revision, char *prod_name) > sprintf(prod_name, "%s", product_str); > } > > +/** > + * mpt_mapresources - map in memory mapped io > + * @ioc: Pointer to pointer to IOC adapter > + * > + **/ > +static int > +mpt_mapresources(MPT_ADAPTER *ioc) > +{ > + u8 __iomem *mem; > + int ii; > + unsigned longmem_phys; > + unsigned longport; > + u32 msize; > + u32 psize; > + u8 revision; > + int r = -ENODEV; > + struct pci_dev *pdev; > + > + pdev = ioc->pcidev; > + ioc->bars = pci_select_bars(pdev, IORESOURCE_MEM); > + if (pci_enable_device_mem(pdev)) { > + printk(MYIOC_s_ERR_FMT "pci_enable_device_mem() " > + "failed\n", ioc->name); > + return r; > + } > + if (pci_request_selected_regions(pdev, ioc->bars, "mpt")) { > + printk(MYIOC_s_ERR_FMT "pci_request_selected_regions() with " > + "MEM failed\n", ioc->name); > + return r; > + } I can't help noticing that any failure after this point leaves the driver with the selected regions requested, meaning it can never be attached again. > + pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision); > + > + if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK) > + && !pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK)) { > + dinitprintk(ioc, printk(MYIOC_s_INFO_FMT > + ": 64 BIT PCI BUS DMA ADDRESSING SUPPORTED\n", > + ioc->name)); > + } else if (!pci_set_dma_mask(pdev, DMA_32BIT_MASK) > + && !pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK)) { > + dinitprintk(ioc, printk(MYIOC_s_INFO_FMT > + ": 32 BIT PCI BUS DMA ADDRESSING SUPPORTED\n", > + ioc->name)); > + } else { > + printk(MYIOC_s_WARN_FMT "no suitable DMA mask for %s\n", > + ioc->name, pci_name(pdev)); > + return r; > + } > + > + mem_phys = msize = 0; > + port = psize = 0; > + for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) { > + if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) { > + if (psize) > + continue; > + /* Get I/O space! */ > + port = pci_resource_start(pdev, ii); > + psize = pci_resource_len(pdev, ii); > + } else { > + if (msize) > + continue; > + /* Get memmap */ > + mem_phys = pci_resource_start(pdev, ii); > + msize = pci_resource_len(pdev, ii); > + } > + } > + ioc->mem_size = msize; > + > + mem = NULL; > + /* Get logical ptr for PciMem0 space */ > + /*mem = ioremap(mem_phys, msize);*/ > + mem = ioremap(mem_phys, msize); > + if (mem == NULL) { > + printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter" > + " memory!\n", ioc->name); > + return -EINVAL; > + } > + ioc->memmap = mem; > + dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n", > + ioc->name, mem, mem_phys)); > + > + ioc->mem_phys = mem_phys; > + ioc->chip = (SYSIF_REGS __iomem *)mem; > + > + /* Save Port IO values in case we need to do downloadboot */ > + { > + u8 *pmem = (u8 *)port; > + ioc->pio_mem_phys = port; > + ioc->pio_chip = (SYSIF_REGS __iomem *)pmem; > + } Why on earth are you doing this; just do ioc->pio_chip = SYSIF_REGS __iomem *)port; port is an unsigned long, so it can be promoted to a pointer. > + return 0; > +} > + > > /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
[PATCH] sym53c416: fix module parameters
It looks like there's been a bug in the module parameter setup forever. The upshot doesn't really matter, because even if no parameters are ever set, we just call sym53c416_setup() three times, but the zero values in the arrays eventually cause nothing to happen. Unfortunately gcc has started to notice this now too: drivers/scsi/sym53c416.c: In function 'sym53c416_detect': drivers/scsi/sym53c416.c:624: warning: the address of 'sym53c416' will always evaluate as 'true' drivers/scsi/sym53c416.c:630: warning: the address of 'sym53c416_1' will always evaluate as 'true' drivers/scsi/sym53c416.c:636: warning: the address of 'sym53c416_2' will always evaluate as 'true' drivers/scsi/sym53c416.c:642: warning: the address of 'sym53c416_3' will always evaluate as 'true' So fix this longstanding bug to keep gcc quiet. James --- diff --git a/drivers/scsi/sym53c416.c b/drivers/scsi/sym53c416.c index 6325901..f7d2795 100644 --- a/drivers/scsi/sym53c416.c +++ b/drivers/scsi/sym53c416.c @@ -187,10 +187,10 @@ #define sym53c416_base_2 sym53c416_2 #define sym53c416_base_3 sym53c416_3 -static unsigned int sym53c416_base[2] = {0,0}; -static unsigned int sym53c416_base_1[2] = {0,0}; -static unsigned int sym53c416_base_2[2] = {0,0}; -static unsigned int sym53c416_base_3[2] = {0,0}; +static unsigned int sym53c416_base[2]; +static unsigned int sym53c416_base_1[2]; +static unsigned int sym53c416_base_2[2]; +static unsigned int sym53c416_base_3[2]; #endif @@ -621,25 +621,25 @@ int __init sym53c416_detect(struct scsi_host_template *tpnt) int ints[3]; ints[0] = 2; - if(sym53c416_base) + if(sym53c416_base[0]) { ints[1] = sym53c416_base[0]; ints[2] = sym53c416_base[1]; sym53c416_setup(NULL, ints); } - if(sym53c416_base_1) + if(sym53c416_base_1[0]) { ints[1] = sym53c416_base_1[0]; ints[2] = sym53c416_base_1[1]; sym53c416_setup(NULL, ints); } - if(sym53c416_base_2) + if(sym53c416_base_2[0]) { ints[1] = sym53c416_base_2[0]; ints[2] = sym53c416_base_2[1]; sym53c416_setup(NULL, ints); } - if(sym53c416_base_3) + if(sym53c416_base_3[0]) { ints[1] = sym53c416_base_3[0]; ints[2] = sym53c416_base_3[1]; - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] cciss: procfs updates to display info about many volumes
Patch 1 of 1 This patch allows us to display information about all of the logical volumes configured on a particular without stepping on memory even when there are many volumes (128 or more) configured. This patch replaces the one submitted on 20071214. See http://groups.google.com/group/linux.kernel/browse_thread/thread/49a50244b19f8855/ba3dc95b23391521?hl=en&lnk=gst&q=cciss#ba3dc95b23391521 which has not been merged. That patch displayed information about only the first logical volume on each controller and had negative side effects for some installers. Please consider this for inclusion. Signed-off-by: Mike Miller <[EMAIL PROTECTED]> diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c index 9715be3..b7cda85 100644 --- a/drivers/block/cciss.c +++ b/drivers/block/cciss.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -174,8 +175,6 @@ static int sendcmd_withirq(__u8 cmd, int ctlr, void *buff, size_t size, static void fail_all_cmds(unsigned long ctlr); #ifdef CONFIG_PROC_FS -static int cciss_proc_get_info(char *buffer, char **start, off_t offset, - int length, int *eof, void *data); static void cciss_procinit(int i); #else static void cciss_procinit(int i) @@ -240,24 +239,44 @@ static inline CommandList_struct *removeQ(CommandList_struct **Qptr, */ #define ENG_GIG 10 #define ENG_GIG_FACTOR (ENG_GIG/512) +#define ENGAGE_SCSI"engage scsi" static const char *raid_label[] = { "0", "4", "1(1+0)", "5", "5+1", "ADG", "UNKNOWN" }; static struct proc_dir_entry *proc_cciss; -static int cciss_proc_get_info(char *buffer, char **start, off_t offset, - int length, int *eof, void *data) +static void cciss_seq_show_header(struct seq_file *seq) { - off_t pos = 0; - off_t len = 0; - int size, i, ctlr; - ctlr_info_t *h = (ctlr_info_t *) data; - drive_info_struct *drv; - unsigned long flags; - sector_t vol_sz, vol_sz_frac; + ctlr_info_t *h = seq->private; + + seq_printf(seq, "%s: HP %s Controller\n" + "Board ID: 0x%08lx\n" + "Firmware Version: %c%c%c%c\n" + "IRQ: %d\n" + "Logical drives: %d\n" + "Current Q depth: %d\n" + "Current # commands on controller: %d\n" + "Max Q depth since init: %d\n" + "Max # commands on controller since init: %d\n" + "Max SG entries since init: %d\n", + h->devname, + h->product_name, + (unsigned long)h->board_id, + h->firm_ver[0], h->firm_ver[1], h->firm_ver[2], + h->firm_ver[3], (unsigned int)h->intr[SIMPLE_MODE_INT], + h->num_luns, + h->Qdepth, h->commands_outstanding, + h->maxQsinceinit, h->max_outstanding, h->maxSG); + + cciss_seq_tape_report(seq, h->ctlr); +} - ctlr = h->ctlr; +static void *cciss_seq_start(struct seq_file *seq, loff_t *pos) +{ + ctlr_info_t *h = seq->private; + unsigned ctlr = h->ctlr; + unsigned long flags; /* prevent displaying bogus info during configuration * or deconfiguration of a logical volume @@ -265,115 +284,150 @@ static int cciss_proc_get_info(char *buffer, char **start, off_t offset, spin_lock_irqsave(CCISS_LOCK(ctlr), flags); if (h->busy_configuring) { spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags); - return -EBUSY; + return ERR_PTR(-EBUSY); } h->busy_configuring = 1; spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags); - size = sprintf(buffer, "%s: HP %s Controller\n" - "Board ID: 0x%08lx\n" - "Firmware Version: %c%c%c%c\n" - "IRQ: %d\n" - "Logical drives: %d\n" - "Max sectors: %d\n" - "Current Q depth: %d\n" - "Current # commands on controller: %d\n" - "Max Q depth since init: %d\n" - "Max # commands on controller since init: %d\n" - "Max SG entries since init: %d\n\n", - h->devname, - h->product_name, - (unsigned long)h->board_id, - h->firm_ver[0], h->firm_ver[1], h->firm_ver[2], - h->firm_ver[3], (unsigned int)h->intr[SIMPLE_MODE_INT], - h->num_luns, - h->cciss_max_sectors, - h->Qdepth, h->commands_outstanding, - h->maxQsinceinit, h->max_outstanding, h->maxSG); - - pos += size; - len += size; - cciss_proc_tape_report(ctlr, buffer, &pos, &len); - for (i = 0; i <= h->highest_lun; i++) {
Re: [GIT PATCH] final SCSI updates for 2.6.24 merge window
On Mon, 2008-02-11 at 13:13 -0800, Harvey Harrison wrote: > On Fri, 2008-02-08 at 10:37 -0600, James Bottomley wrote: > > On Fri, 2008-02-08 at 10:03 +0100, Geert Uytterhoeven wrote: > > > On Thu, 7 Feb 2008, James Bottomley wrote: > > > > On Thu, 2008-02-07 at 17:04 -0800, Harvey Harrison wrote: > > > > > I'm going to guess that this is the entry in feature-removal.txt > > > > > that need an update then: > > > > > > > > > > --- > > > > > > > > > > What: old NCR53C9x driver > > > > > When: October 2007 > > > > > Why: Replaced by the much better esp_scsi driver. Actual low-level > > > > > driver can be ported over almost trivially. > > > > > Who: David Miller <[EMAIL PROTECTED]> > > > > > Christoph Hellwig <[EMAIL PROTECTED]> > > > > > > > > Not immediately ... I anticipate a few "where'd my driver go?" type > > > > questions from m68k for which this provides a useful reference to point > > > > to ... > > > > > > Don't bother, we're fully aware of this. > > > > > > The shortest feature removal notice in Linux's history (is it?) didn't go > > > unnoticed ;-) > > Sowould a patch be welcome to remove this entry then? Not at the moment for the reasons I already gave. Ping me in about three months if it's not gone by then. James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PATCH] final SCSI updates for 2.6.24 merge window
On Fri, 2008-02-08 at 10:37 -0600, James Bottomley wrote: > On Fri, 2008-02-08 at 10:03 +0100, Geert Uytterhoeven wrote: > > On Thu, 7 Feb 2008, James Bottomley wrote: > > > On Thu, 2008-02-07 at 17:04 -0800, Harvey Harrison wrote: > > > > I'm going to guess that this is the entry in feature-removal.txt > > > > that need an update then: > > > > > > > > --- > > > > > > > > What: old NCR53C9x driver > > > > When: October 2007 > > > > Why:Replaced by the much better esp_scsi driver. Actual low-level > > > > driver can be ported over almost trivially. > > > > Who:David Miller <[EMAIL PROTECTED]> > > > > Christoph Hellwig <[EMAIL PROTECTED]> > > > > > > Not immediately ... I anticipate a few "where'd my driver go?" type > > > questions from m68k for which this provides a useful reference to point > > > to ... > > > > Don't bother, we're fully aware of this. > > > > The shortest feature removal notice in Linux's history (is it?) didn't go > > unnoticed ;-) Sowould a patch be welcome to remove this entry then? Harvey - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SCSI] ses: fix memory leaks
On Monday 11 February 2008 09:02:06 am James Bottomley wrote: > > On Mon, 2008-02-11 at 10:23 -0600, James Bottomley wrote: > > On Sun, 2008-02-10 at 23:25 -0800, Yinghai Lu wrote: > > > please check it... > > > > This one looks perfect, thanks! > > Well, nearly perfect. I corrected this typo: > > + if (!buf) > + goto simple_polulate; > > > Which a compile check before you submitted the patch would have picked > up ... sorry for that. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/9] scsi_dh: Add support for SDEV_PASSIVE
On Mon, Feb 11, 2008 at 10:27:46AM -0800, Chandra Seetharaman wrote: > On Sat, 2008-02-09 at 05:45 -0700, Matthew Wilcox wrote: > > On Mon, Feb 04, 2008 at 01:19:30PM -0800, Chandra Seetharaman wrote: > > > The device does send these error messages currently, but it takes some > > > time to get the check condition back, which adds up the time to boot > > > especially when the # of LUNS is huge. > > > > > > For example, in my test configuration, I had 40 luns, and the time > > > difference (with this patch and without it) to boot is 171 seconds and > > > 1426 seconds. > > > > Was that with sync or async SCSI bus scanning? > > I didn't change anything, IOW, i did default scanning, which I would > guess sync ?! That would depend on your CONFIG_SCSI_SCAN_ASYNC setting. Try booting with 'scsi_mod.scan=async' and without this patch, and see how long it takes. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/9] scsi_dh: Add support for SDEV_PASSIVE
On Sat, 2008-02-09 at 05:45 -0700, Matthew Wilcox wrote: > On Mon, Feb 04, 2008 at 01:19:30PM -0800, Chandra Seetharaman wrote: > > The device does send these error messages currently, but it takes some > > time to get the check condition back, which adds up the time to boot > > especially when the # of LUNS is huge. > > > > For example, in my test configuration, I had 40 luns, and the time > > difference (with this patch and without it) to boot is 171 seconds and > > 1426 seconds. > > Was that with sync or async SCSI bus scanning? I didn't change anything, IOW, i did default scanning, which I would guess sync ?! > -- -- Chandra Seetharaman | Be careful what you choose - [EMAIL PROTECTED] | ...you may get it. -- - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/scsi/aic7xxx/aic7xxx_osm_pci.c - remove pointer comparison to 0
On Mon, 2008-02-11 at 09:17 -0800, Randy Dunlap wrote: > On Mon, 11 Feb 2008 09:08:05 -0800 Joe Perches wrote: > > > On Mon, 2008-02-11 at 09:30 -0600, James Bottomley wrote: > > > On Sun, 2008-02-10 at 21:47 -0800, Joe Perches wrote: > > > > - if (request_region(*base, 256, "aic7xxx") == 0) > > > > + if (!request_region(*base, 256, "aic7xxx")) > > > > > > This patch is completely pointless. > > > > It removes a sparse warning. > > I try to say that in the patch description. Andrew also tries > to enforce such errors/warnings in patch descriptions Well, the aic7xxx subdirectory is a nightmare of CodingStyle non conformities ... you can see the return (ENOMEM) just in this patch. That's two problems: the brackets and non negative error returns which are later converted to negative ones thus inviting sign problems. The driver is also about 3x bigger than it should be because of the vestiges of the BSD glue layer. However, I think my life is too short to apply the 32,554 patches it would take to correct this an issue at a time. This is one of those drivers we tolerate because we must and we fix up around the regions we have to touch. James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/scsi/aic7xxx/aic7xxx_osm_pci.c - remove pointer comparison to 0
On Mon, 11 Feb 2008 09:08:05 -0800 Joe Perches wrote: > On Mon, 2008-02-11 at 09:30 -0600, James Bottomley wrote: > > On Sun, 2008-02-10 at 21:47 -0800, Joe Perches wrote: > > > - if (request_region(*base, 256, "aic7xxx") == 0) > > > + if (!request_region(*base, 256, "aic7xxx")) > > > > This patch is completely pointless. > > It removes a sparse warning. I try to say that in the patch description. Andrew also tries to enforce such errors/warnings in patch descriptions > > There's a marginal preference for if (!x) over if (x == NULL) > > for pointers, but it's still up to a driver writer if they wish to use > > the marginally unpreferred form. > > Use == NULL then if you care to. --- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi_error: Fix language abuse.
Seriously, can't you just add a disclaimer to the README file? In http://lkml.org/lkml/2008/2/9/29, Luben Tuikov made an interesting point that in many cases "illegal" refers to a valid value that violates the specification, so the term "invalid" may be technically incorrect. Benny On Feb. 11, 2008, 18:07 +0200, "linux-os (Dick Johnson)" <[EMAIL PROTECTED]> wrote: > On Fri, 8 Feb 2008, Mark Hounschell wrote: > >> linux-os (Dick Johnson) wrote: >>> The correct word should be "invalid," in spite of >>> the fact that the SCSI committee used invalid syntax. >>> >>> Alan is right. There is nothing illegal in the kernel >>> and if there is, it must be removed as soon as it >>> is discovered! >>> >> il·le·gal (-lgl) >> adj. >> 1. Prohibited by law. >> *2. Prohibited by official rules: an illegal pass in football. >> *3. Unacceptable to or not performable by a computer: an illegal operation. >> >> Mark > > Many early computer programmers including myself, while writing > error messages in early software, did not understand that computer > programmers do not make law so we called bad operations "illegal." > > Once you are called to testify in a court of law, about some > message your code wrote to the screen just before a factory > burned down, you start to be much more careful about the syntax. > > I advise that, regardless of the rewrite of dictionaries and, > in fact, the rewrite of history, whenever possible we use > the correct message syntax. Dictionaries pick up "common usage" > in spite of the fact that it is wrong. See "irregardless" and > other abortions which now exist in the dictionary. > > > Cheers, > Dick Johnson > Penguin : Linux version 2.6.22.1 on an i686 machine (5588.28 BogoMips). > My book : http://www.AbominableFirebug.com/ > _ > > > The information transmitted in this message is confidential and may be > privileged. Any review, retransmission, dissemination, or other use of this > information by persons or entities other than the intended recipient is > prohibited. If you are not the intended recipient, please notify Analogic > Corporation immediately - by replying to this message or by sending an email > to [EMAIL PROTECTED] - and destroy all copies of this information, including > any attachments, without reading or disclosing them. > > Thank you. > - > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/scsi/aic7xxx/aic7xxx_osm_pci.c - remove pointer comparison to 0
On Mon, 2008-02-11 at 09:30 -0600, James Bottomley wrote: > On Sun, 2008-02-10 at 21:47 -0800, Joe Perches wrote: > > - if (request_region(*base, 256, "aic7xxx") == 0) > > + if (!request_region(*base, 256, "aic7xxx")) > > This patch is completely pointless. It removes a sparse warning. > There's a marginal preference for if (!x) over if (x == NULL) > for pointers, but it's still up to a driver writer if they wish to use > the marginally unpreferred form. Use == NULL then if you care to. cheers, Joe - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SCSI] ses: fix memory leaks
On Mon, 2008-02-11 at 10:23 -0600, James Bottomley wrote: > On Sun, 2008-02-10 at 23:25 -0800, Yinghai Lu wrote: > > please check it... > > This one looks perfect, thanks! Well, nearly perfect. I corrected this typo: + if (!buf) + goto simple_polulate; Which a compile check before you submitted the patch would have picked up ... James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] iscsi: bidi support - libiscsi
[EMAIL PROTECTED] wrote on Mon, 11 Feb 2008 18:05 +0200: > You are most probably right I will investigate what happened. It looks > like I went back to some old version right? or a merge fallout > Thanks for reviewing. > > Please also test latest head-of-line code if possible + iscsi patches > + last varlen I sent. > > Is there any new patches I need for 2.6.24 or head-of-line for my > osd-dev tree? Testing now. My patch set is actually shrinking (!) thanks to some merges. Some rebasing was required to apply your three varlen patches and three bidi patches, but I'm sure you'll update your tree and push those upstream soon enough. I'll take a look at iser bidi then update my patch list and let you know soon. -- Pete - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SCSI] ses: fix memory leaks
On Sun, 2008-02-10 at 23:25 -0800, Yinghai Lu wrote: > please check it... This one looks perfect, thanks! James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] iscsi: bidi support - libiscsi
On Mon, Feb 11 2008 at 17:43 +0200, Pete Wyckoff <[EMAIL PROTECTED]> wrote: > [EMAIL PROTECTED] wrote on Thu, 31 Jan 2008 22:29 +0200: >> iscsi bidi support at the generic libiscsi level >> - prepare the additional bidi_read rlength header. >> - access the right scsi_in() and/or scsi_out() side of things. >> also for resid. >> - Handle BIDI underflow overflow from target >> >> Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]> > > I see you do this a bit differently than in your previous patch set. > In particular, the residual handling in libiscsi.c. (I'm editing in > a bit more context to the patch below.) > >> +if (scsi_bidi_cmnd(sc) && >> +(rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW | >> +ISCSI_FLAG_CMD_BIDI_OVERFLOW))) { >> +int res_count = be32_to_cpu(rhdr->bi_residual_count); >> + >> +if (res_count > 0 && >> +(rhdr->flags & ISCSI_FLAG_CMD_BIDI_OVERFLOW || >> +res_count <= scsi_in(sc)->length)) >> +scsi_in(sc)->resid = res_count; >> +else >> +sc->result = >> +(DID_BAD_TARGET << 16) | rhdr->cmd_status; >> +} >> + >> if (rhdr->flags & (ISCSI_FLAG_CMD_UNDERFLOW | >> ISCSI_FLAG_CMD_OVERFLOW)) { >> int res_count = be32_to_cpu(rhdr->residual_count); >> >> if (res_count > 0 && >> (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW || >> res_count <= scsi_bufflen(sc))) >> +/* write side for bidi or uni-io set_resid */ >> scsi_set_resid(sc, res_count); >> else >> sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status; >> } else if (rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW | >> ISCSI_FLAG_CMD_BIDI_OVERFLOW)) >> sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status; > > I haven't tested this, but, consider a bidi command that results in > an overflow on the read part, and no overflow on the write part. > E.g., the user supplied a data-in buffer that wasn't big enough to > hold the returned data from the target, but the data-out buffer was > just right. > > Then this code will set scsi_in(sc)->resid properly, informing the > caller that there were extra bytes that were not transferred. But > the "else" clause at the bottom will also set sc->result to be bad. > I don't think we want this. > > Your earlier patch got rid of the second bidi_overflow handler and > just did all the logic for both bidi and non-bidi cases in a single > if clause. Seemed better. > > -- Pete You are most probably right I will investigate what happened. It looks like I went back to some old version right? or a merge fallout Thanks for reviewing. Please also test latest head-of-line code if possible + iscsi patches + last varlen I sent. Is there any new patches I need for 2.6.24 or head-of-line for my osd-dev tree? Boaz - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi_error: Fix language abuse.
On Fri, 8 Feb 2008, Mark Hounschell wrote: > linux-os (Dick Johnson) wrote: >> >> The correct word should be "invalid," in spite of >> the fact that the SCSI committee used invalid syntax. >> >> Alan is right. There is nothing illegal in the kernel >> and if there is, it must be removed as soon as it >> is discovered! >> > > il·le·gal (-lgl) > adj. > 1. Prohibited by law. > *2. Prohibited by official rules: an illegal pass in football. > *3. Unacceptable to or not performable by a computer: an illegal operation. > > Mark Many early computer programmers including myself, while writing error messages in early software, did not understand that computer programmers do not make law so we called bad operations "illegal." Once you are called to testify in a court of law, about some message your code wrote to the screen just before a factory burned down, you start to be much more careful about the syntax. I advise that, regardless of the rewrite of dictionaries and, in fact, the rewrite of history, whenever possible we use the correct message syntax. Dictionaries pick up "common usage" in spite of the fact that it is wrong. See "irregardless" and other abortions which now exist in the dictionary. Cheers, Dick Johnson Penguin : Linux version 2.6.22.1 on an i686 machine (5588.28 BogoMips). My book : http://www.AbominableFirebug.com/ _ The information transmitted in this message is confidential and may be privileged. Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited. If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to [EMAIL PROTECTED] - and destroy all copies of this information, including any attachments, without reading or disclosing them. Thank you. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] iscsi: bidi support - libiscsi
[EMAIL PROTECTED] wrote on Thu, 31 Jan 2008 22:29 +0200: > iscsi bidi support at the generic libiscsi level > - prepare the additional bidi_read rlength header. > - access the right scsi_in() and/or scsi_out() side of things. > also for resid. > - Handle BIDI underflow overflow from target > > Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]> I see you do this a bit differently than in your previous patch set. In particular, the residual handling in libiscsi.c. (I'm editing in a bit more context to the patch below.) > + if (scsi_bidi_cmnd(sc) && > + (rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW | > + ISCSI_FLAG_CMD_BIDI_OVERFLOW))) { > + int res_count = be32_to_cpu(rhdr->bi_residual_count); > + > + if (res_count > 0 && > + (rhdr->flags & ISCSI_FLAG_CMD_BIDI_OVERFLOW || > + res_count <= scsi_in(sc)->length)) > + scsi_in(sc)->resid = res_count; > + else > + sc->result = > + (DID_BAD_TARGET << 16) | rhdr->cmd_status; > + } > + > if (rhdr->flags & (ISCSI_FLAG_CMD_UNDERFLOW | > ISCSI_FLAG_CMD_OVERFLOW)) { > int res_count = be32_to_cpu(rhdr->residual_count); > > if (res_count > 0 && > (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW || >res_count <= scsi_bufflen(sc))) > + /* write side for bidi or uni-io set_resid */ > scsi_set_resid(sc, res_count); > else > sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status; > } else if (rhdr->flags & (ISCSI_FLAG_CMD_BIDI_UNDERFLOW | > ISCSI_FLAG_CMD_BIDI_OVERFLOW)) > sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status; I haven't tested this, but, consider a bidi command that results in an overflow on the read part, and no overflow on the write part. E.g., the user supplied a data-in buffer that wasn't big enough to hold the returned data from the target, but the data-out buffer was just right. Then this code will set scsi_in(sc)->resid properly, informing the caller that there were extra bytes that were not transferred. But the "else" clause at the bottom will also set sc->result to be bad. I don't think we want this. Your earlier patch got rid of the second bidi_overflow handler and just did all the logic for both bidi and non-bidi cases in a single if clause. Seemed better. -- Pete - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/scsi/aic7xxx/aic7xxx_osm_pci.c - remove pointer comparison to 0
On Sun, 2008-02-10 at 21:47 -0800, Joe Perches wrote: > - if (request_region(*base, 256, "aic7xxx") == 0) > + if (!request_region(*base, 256, "aic7xxx")) This patch is completely pointless. if (x == 0) and if (!x) mean identical things and there's no style standard preferring one form over another. There's a marginal preference for if (!x) over if (x == NULL) for pointers, but it's still up to a driver writer if they wish to use the marginally unpreferred form. James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Patches not reaching the list
On Mon, 2008-02-11 at 10:24 +0530, Prakash, Sathya wrote: > I tried to send few patches on last friday, out of which only one > reached the list I resent the rest two and again only on reached the list > Again I have resent the third patch two times, but they are still to reach > the list. I am using mutt as E-mail client. I am sending this E-mail also > from the same client, can any one please help me in sorting this out. The people who might be able to help are the owners of the mailing list <[EMAIL PROTECTED]> However, please don't bother them unless you've checked out the faq here (also listed in the footer of every email): http://vger.kernel.org/majordomo-info.html and tried the autoanswer thing. If it turns out to be your fault, they will be pretty unfriendly. James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SCSI] sun3x_esp: convert to esp_scsi
On Mon, 2008-02-11 at 12:24 +0100, Thomas Bogendoerfer wrote: > On Sun, Feb 10, 2008 at 10:38:15AM +0100, Kars de Jong wrote: > > Thomas, can't you use ioreadxx() and friends instead of rolling your own > > memory mapped I/O handlers? > > well, at least ioread32be/iowrite32be are looking promising, but a quick > grep didn't show them for m68k. Someone could submit a patch ... > > readxx() and friends are only to be used on PCI-like buses. > > hmm, afaik readxx/writexx is not directly related to PCI. It's > like ioread/iowrite to access iomapped address space. The difference > to ioread/iowrite is that it doesn't support PCI IO space. That's correct ... at least that's the meaning we've ascribed on other architectures I've worked on. Any ioremap'd memory area can be accessed by readX/writeX. The current list of Buses I know it works on is MCA, EISA, GSC, Runway ... in addition to PCI. James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mpt fusion: Power Management fixes for MPT SAS PCI-E controllers
This patch was submitted as [PATCH 2/3] yesterday, since it did not reach the list due to CC errors sending this again. It is regenerated against new git tree. The system power state changes like hibernation and standby are not happening properly with 106XE controllers, this patch modifies the driver to free resources and allocate resources in power management entry points and to enable MSI interrupts for SAS controllers Signed-off-by: Sathya Prakash <[EMAIL PROTECTED]> --- diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c index bfda731..b3b80bf 100644 --- a/drivers/message/fusion/mptbase.c +++ b/drivers/message/fusion/mptbase.c @@ -1429,6 +1429,100 @@ mpt_get_product_name(u16 vendor, u16 device, u8 revision, char *prod_name) sprintf(prod_name, "%s", product_str); } +/** + * mpt_mapresources - map in memory mapped io + * @ioc: Pointer to pointer to IOC adapter + * + **/ +static int +mpt_mapresources(MPT_ADAPTER *ioc) +{ + u8 __iomem *mem; + int ii; + unsigned longmem_phys; + unsigned longport; + u32 msize; + u32 psize; + u8 revision; + int r = -ENODEV; + struct pci_dev *pdev; + + pdev = ioc->pcidev; + ioc->bars = pci_select_bars(pdev, IORESOURCE_MEM); + if (pci_enable_device_mem(pdev)) { + printk(MYIOC_s_ERR_FMT "pci_enable_device_mem() " + "failed\n", ioc->name); + return r; + } + if (pci_request_selected_regions(pdev, ioc->bars, "mpt")) { + printk(MYIOC_s_ERR_FMT "pci_request_selected_regions() with " + "MEM failed\n", ioc->name); + return r; + } + + pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision); + + if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK) + && !pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK)) { + dinitprintk(ioc, printk(MYIOC_s_INFO_FMT + ": 64 BIT PCI BUS DMA ADDRESSING SUPPORTED\n", + ioc->name)); + } else if (!pci_set_dma_mask(pdev, DMA_32BIT_MASK) + && !pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK)) { + dinitprintk(ioc, printk(MYIOC_s_INFO_FMT + ": 32 BIT PCI BUS DMA ADDRESSING SUPPORTED\n", + ioc->name)); + } else { + printk(MYIOC_s_WARN_FMT "no suitable DMA mask for %s\n", + ioc->name, pci_name(pdev)); + return r; + } + + mem_phys = msize = 0; + port = psize = 0; + for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) { + if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) { + if (psize) + continue; + /* Get I/O space! */ + port = pci_resource_start(pdev, ii); + psize = pci_resource_len(pdev, ii); + } else { + if (msize) + continue; + /* Get memmap */ + mem_phys = pci_resource_start(pdev, ii); + msize = pci_resource_len(pdev, ii); + } + } + ioc->mem_size = msize; + + mem = NULL; + /* Get logical ptr for PciMem0 space */ + /*mem = ioremap(mem_phys, msize);*/ + mem = ioremap(mem_phys, msize); + if (mem == NULL) { + printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter" + " memory!\n", ioc->name); + return -EINVAL; + } + ioc->memmap = mem; + dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n", + ioc->name, mem, mem_phys)); + + ioc->mem_phys = mem_phys; + ioc->chip = (SYSIF_REGS __iomem *)mem; + + /* Save Port IO values in case we need to do downloadboot */ + { + u8 *pmem = (u8 *)port; + ioc->pio_mem_phys = port; + ioc->pio_chip = (SYSIF_REGS __iomem *)pmem; + } + + return 0; +} + /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/ /** * mpt_attach - Install a PCI intelligent MPT adapter. @@ -1451,13 +1545,6 @@ int mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id) { MPT_ADAPTER *ioc; - u8 __iomem *mem; - u8 __iomem *pmem; - unsigned longmem_phys; - unsigned longport; - u32 msize; - u32 psize; - int ii; u8 cb_idx; int r = -ENODEV; u8 revision; @@ -1467,52 +1554,33 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id) struct proc_dir_entry *dent, *ent; #endif - if (mpt_debug_level)
Re: [SCSI] sun3x_esp: convert to esp_scsi
On Sun, Feb 10, 2008 at 10:38:15AM +0100, Kars de Jong wrote: > Thomas, can't you use ioreadxx() and friends instead of rolling your own > memory mapped I/O handlers? well, at least ioread32be/iowrite32be are looking promising, but a quick grep didn't show them for m68k. > readxx() and friends are only to be used on PCI-like buses. hmm, afaik readxx/writexx is not directly related to PCI. It's like ioread/iowrite to access iomapped address space. The difference to ioread/iowrite is that it doesn't support PCI IO space. Thomas. -- Crap can work. Given enough thrust pigs will fly, but it's not necessary a good idea.[ RFC1925, 2.3 ] - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi/arm/fas216.c compile error
On Mon, Feb 11 2008 at 12:02 +0200, Russell King <[EMAIL PROTECTED]> wrote: > On Mon, Feb 11, 2008 at 11:47:57AM +0200, Boaz Harrosh wrote: >> On Mon, Feb 11 2008 at 0:44 +0200, James Bottomley <[EMAIL PROTECTED]> wrote: >> Andrew this patch was in -mm for two month or so. I was under the impression >> that you have an arm cross compiler that tries to build every -mm kernel. >> Is it possible that for some reason this portion did not get compiled? >> Is there a place that one can inspect the output of -mm compilations, >> Specially >> for cross compiled ARCHs? > > Having a patch sit in -mm for ARM doesn't mean a lot since there's no > guarantee that it'll get built, and that is because the ARM architecture > is very diverse; it's not possible to build a single kernel to support > everything. > > So, when akpm builds a kernel for ARM, it's normally centered around one > particular ARM defconfig (maybe with allyconfig or allmodconfig afterwards) > but even that won't build all ARM specific code. > > This is why we now have kautobuild - http://armlinux.simtec.co.uk/kautobuild/ > That's the only way we can get decent compilation coverage. > > That system isn't publically accessible (it's not even accessible to me) > and it only builds the mainline kernels. Adding -mm to it might be > possible, but as I understand the situation, even though it uses things > like ccache, it can take about 10 or so hours to complete a set of builds. > Thanks Russell. That explains it. I wish they would include an -mm kernel once in a while. like 2-3 every kernel cycle. It is much nicer to find the problems before they are already in mainline. I would certainly sleep better. You have my vote. Boaz - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi/arm/fas216.c compile error
On Mon, Feb 11, 2008 at 11:47:57AM +0200, Boaz Harrosh wrote: > On Mon, Feb 11 2008 at 0:44 +0200, James Bottomley <[EMAIL PROTECTED]> wrote: > Andrew this patch was in -mm for two month or so. I was under the impression > that you have an arm cross compiler that tries to build every -mm kernel. > Is it possible that for some reason this portion did not get compiled? > Is there a place that one can inspect the output of -mm compilations, > Specially > for cross compiled ARCHs? Having a patch sit in -mm for ARM doesn't mean a lot since there's no guarantee that it'll get built, and that is because the ARM architecture is very diverse; it's not possible to build a single kernel to support everything. So, when akpm builds a kernel for ARM, it's normally centered around one particular ARM defconfig (maybe with allyconfig or allmodconfig afterwards) but even that won't build all ARM specific code. This is why we now have kautobuild - http://armlinux.simtec.co.uk/kautobuild/ That's the only way we can get decent compilation coverage. That system isn't publically accessible (it's not even accessible to me) and it only builds the mainline kernels. Adding -mm to it might be possible, but as I understand the situation, even though it uses things like ccache, it can take about 10 or so hours to complete a set of builds. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Integration of SCST in the mainstream Linux kernel
Luben Tuikov wrote: Is there an open iSCSI Target implementation which does NOT issue commands to sub-target devices via the SCSI mid-layer, but bypasses it completely? What do you mean? To call directly low level backstorage SCSI drivers queuecommand() routine? What are advantages of it? Yes, that's what I meant. Just curious. What's advantage of it? Thanks, Luben - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi/arm/fas216.c compile error
On Mon, Feb 11 2008 at 0:44 +0200, James Bottomley <[EMAIL PROTECTED]> wrote: > On Sun, 2008-02-10 at 22:02 +, Russell King wrote: >> On Sun, Feb 10, 2008 at 08:20:24AM -0600, James Bottomley wrote: >>> On Sun, 2008-02-10 at 13:58 +, Russell King wrote: On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote: > It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310 > [SCSI] arm: convert to accessors and !use_sg cleanup > > Thanks for checking. This patch was in scsi-pending tree since forever, > And we were unable > to get a responsive maintainer to ACK on them. until the breakage cause > went into mainline > we finally managed a Tested-by:. > > I guess sometimes people are so busy, you need a bulldozer to shove 20 > minutes into they're > schedule. Oh, I was ill for most of December, particularly at the time that you sent the patch, and by the time I recovered, it was buried in my mailbox. Suggest you have some consideration for others who might not be able to do your beg and call at the immediate moment that you want it, and consider that their email management skills may not be as l33t as yours. >>> OK, sorry about this, it's a bit of a cockup all around. The patch that >>> fixes this problem is still in SCSI pending largely because it's patch >>> description: >>> >>> [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation >>> >>> Doesn't lead one to think it might be build critical, so I concentrated >>> on getting the other arm patch out. >>> >>> Russell, could you give it a quick test, and I'll put it in with a >>> tested-by tag? >> It's not looking good: >> >> CC drivers/scsi/arm/fas216.o >> drivers/scsi/arm/fas216.c: In function `fas216_rq_sns_done': >> drivers/scsi/arm/fas216.c:2021: warning: passing arg 2 of >> `scsi_eh_restore_cmnd' from incompatible pointer type >> drivers/scsi/arm/fas216.c: In function `fas216_std_done': >> drivers/scsi/arm/fas216.c:2107: warning: passing arg 2 of >> `scsi_eh_prep_cmnd' from incompatible pointer type >> >> Since the second argument of scsi_eh_prep_cmnd is 'struct scsi_eh_save *ses' >> this patch is most definitely bad. Not even booted it. > > Yes, there looks to be a fatal screw up in the definition in > FAS216_Info. Could you try this ... I think I've corrected it. > > James > > --- > >>From 35be7297dc581b42c05ea7751ee595b0ce78f669 Mon Sep 17 00:00:00 2001 > From: Boaz Harrosh <[EMAIL PROTECTED]> > Date: Mon, 10 Sep 2007 22:39:11 +0300 > Subject: [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation > > - Use new scsi_eh_prep/restor_cmnd() for synchronous > REQUEST_SENSE invocation. > > Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]> > Cc: Russell King <[EMAIL PROTECTED]> > Signed-off-by: James Bottomley <[EMAIL PROTECTED]> > --- > drivers/scsi/arm/fas216.c | 16 +++- > drivers/scsi/arm/fas216.h |3 +++ > 2 files changed, 6 insertions(+), 13 deletions(-) > > diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c > index fb5f202..a715632 100644 > --- a/drivers/scsi/arm/fas216.c > +++ b/drivers/scsi/arm/fas216.c > @@ -2018,6 +2018,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, > struct scsi_cmnd *SCpnt, >* the upper layers to process. This would have been set >* correctly by fas216_std_done. >*/ > + scsi_eh_restore_cmnd(SCpnt, &info->ses); > SCpnt->scsi_done(SCpnt); > } > > @@ -2103,23 +2104,12 @@ request_sense: > if (SCpnt->cmnd[0] == REQUEST_SENSE) > goto done; > > + scsi_eh_prep_cmnd(SCpnt, &info->ses, NULL, 0, ~0); > fas216_log_target(info, LOG_CONNECT, SCpnt->device->id, > "requesting sense"); > - memset(SCpnt->cmnd, 0, sizeof (SCpnt->cmnd)); > - SCpnt->cmnd[0] = REQUEST_SENSE; > - SCpnt->cmnd[1] = SCpnt->device->lun << 5; > - SCpnt->cmnd[4] = sizeof(SCpnt->sense_buffer); > - SCpnt->cmd_len = COMMAND_SIZE(SCpnt->cmnd[0]); > - SCpnt->SCp.buffer = NULL; > - SCpnt->SCp.buffers_residual = 0; > - SCpnt->SCp.ptr = (char *)SCpnt->sense_buffer; > - SCpnt->SCp.this_residual = sizeof(SCpnt->sense_buffer); > - SCpnt->SCp.phase = sizeof(SCpnt->sense_buffer); > + init_SCp(SCpnt); > SCpnt->SCp.Message = 0; > SCpnt->SCp.Status = 0; > - SCpnt->request_bufflen = sizeof(SCpnt->sense_buffer); > - SCpnt->sc_data_direction = DMA_FROM_DEVICE; > - SCpnt->use_sg = 0; > SCpnt->tag = 0; > SCpnt->host_scribble = (void *)fas216_rq_sns_done; > > diff --git a/drivers/scsi/arm/fas216.h b/drivers/scsi/arm/fas216.h > index 00e5f05..b65f4cf 100644 > --- a/drivers/scsi/arm/fas216.h > +++ b/drivers/scsi/arm/fas216.h > @@ -16,6 +16,8 @@ > #define NO_IRQ 255 > #endif > > +#include > + > #include "queue.h" > #include "msgqueue.h" > > @@ -311,6 +313,7 @@ typedef struct { > > /* miscellaneous */ > int