Re: Subject: [PATCH 1/3] Let scsi_cmnd->cmnd use request->cmd buffer

2008-02-12 Thread Boaz Harrosh
On Tue, Feb 12 2008 at 19:45 +0200, Christoph Hellwig <[EMAIL PROTECTED]> wrote:
> On Sun, Feb 10, 2008 at 09:05:17PM +0200, Boaz Harrosh wrote:
>> - Lots of drivers still use MAX_COMMAND_SIZE. So I have left
>>   that #define but equate it to BLK_MAX_CDB. The way I see it
>>   and is reflected in the patch below is.
>>   MAX_COMMAND_SIZE - means: The longest fixed-length (*) SCSI CDB
>>  as per the SCSI standard and is not related
>>  to the implementation.
>>   BLK_MAX_CDB. - The allocated space at the request level
>>
>> (*)fixed-length here means commands that their size can be determined
>>by their opcode and the CDB does not carry a length specifier, like
>>the VARIABLE_LENGTH_CMD(0x7f) command. This is actually not exactly
>>true and the SCSI standard also defines extended commands and
>>vendor specific commands that can be bigger than 16 bytes. The kernel
>>will support these using the same infrastructure used for VARLEN CDB's.
>>So in effect MAX_COMMAND_SIZE means the maximum size command
>>scsi-ml supports without specifying a cmd_len by ULD's
> 
> A comment like this should be near the declaration of MAX_COMMAND_SIZE
> 
>> +#define MAX_COMMAND_SIZE 16
>> +#if (MAX_COMMAND_SIZE > BLK_MAX_CDB)
>> +#   error MAX_COMMAND_SIZE can not be smaller than BLK_MAX_CDB
>> +#endif
> 
> No tabs between the # and the rest of the cpp command, please.  Either
> nothing or a single space as indentation instead.
> 
> Except for those two small nitpicks this looks very good to me.  Nice
> memory saving aswel.
Agree with both comments. Thanks for the review, will fix in the next
submission.

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


[PATCH 0/3] iscsi iser limits

2008-02-12 Thread Pete Wyckoff
These three patches remove various block device limits to
the iSER iSCSI transport.  I guess in theory they should go
in via the IB tree, although ACKs from the iSCSI side would
be welcome.

The first two look quite similar to some TCP iSCSI patches
that Mike applied a while back:

d1d81c01f4bdd50577d9f89aa4a8e6344f63aa70
[SCSI] iscsi_tcp: remove DMA alignment restriction

8231f0eddbe425cc3b54f2d723bb03531925272e
[SCSI] iscsi_tcp: increase max_sectors

The third fixes a problem with mapping an unaligned 512 kB
buffer and takes the opportunity to increase the FMR mapping
limit to 1 MB, as is done in SRP.

-- 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


[PATCH 2/3] iscsi iser: increase max_sectors

2008-02-12 Thread Pete Wyckoff
iser has no limit on max sectors.  This lets iscsi iser support
large pass through commands just like iscsi TCP.

Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 1b272a6..78f3242 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -557,7 +557,7 @@ static struct scsi_host_template iscsi_iser_sht = {
.change_queue_depth = iscsi_change_queue_depth,
.can_queue  = ISCSI_DEF_XMIT_CMDS_MAX - 1,
.sg_tablesize   = ISCSI_ISER_SG_TABLESIZE,
-   .max_sectors= 1024,
+   .max_sectors= 0x,
.cmd_per_lun= ISCSI_MAX_CMD_PER_LUN,
.eh_abort_handler   = iscsi_eh_abort,
.eh_device_reset_handler= iscsi_eh_device_reset,
-- 
1.5.3.8

-
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: arcmsr + archttp64 calls dma_free_coherent() with irqs disabled - dmesg filled with warnings

2008-02-12 Thread Joshua Hoblitt
I tried doing a build from that commit id but I get an oops from an
unrelated subsystem.  I'll try again from the head of the tree.

-J

--
On Sat, Feb 09, 2008 at 01:43:39PM -0600, James Bottomley wrote:
> 
> On Sat, 2008-02-09 at 09:35 -1000, Joshua Hoblitt wrote:
> > Hi James,
> > 
> > I can give it a try.  What commit id do you think fixed it?
> 
> commit 76d78300a6eb8b7f08e47703b7e68a659ffc2053
> Author: Nick Cheng <[EMAIL PROTECTED]>
> Date:   Mon Feb 4 23:53:24 2008 -0800
> 
> [SCSI] arcmsr: updates (1.20.00.15)
> 
> 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: [Bug 9775] HOST_MSG_LOOP invalid SCB ff

2008-02-12 Thread James Bottomley
On Fri, 2008-02-08 at 18:52 -0800, [EMAIL PROTECTED]
wrote:
> Ok, I've spent some time trying different combinations of devices.
> 
> Against kernel 2.6.24
> T0 is Quantum DLT8000 ID0
> T1 is Quantum DLT8000 ID1
> MTX is STK L80  ID 15
> Terminators A, B
> 
> Channel A   B
> T0,T1,MTX,B Nil   
>  
> Crash
> Nil T0,T1,MTX,B   
>  
> Parity Error in Data-in Phase
> Nil T0,MTX,B  
>  
> Ok, Tar test ok, MTX ok
> Nil T1,MTX,B  
>  
> Ok, Tar test ok, MTX ok 
> -- Both drives work ok  
> T1,MTX,BNil   
>  
> Ok   Skipped Tests
> T1,MTX,ANil   
>  
> Ok   Skipped Tests
> T0,MTX,BNil   
>  
> Crash
> T0,MTX,ANil   
>  
> Crash
> -- Not the terminator
> 
> 
> --Test on two channels
> T0,MTX,AT1,B  
>  
> Crash
> T1,BT0,MTX,A  
>  
> Parity Error in Data-in Phase   
> 
> It really doesn't like three devices, on two busses or one.

Well, I still think you have some type of bus instability, but that said
we need to get rid of the panic.

I'm afraid this is going to be a long process.  For the first attempt,
let's see if this is an unsolicited msgin ... it looks like the driver
handling for those is wrong.  Can you try this patch?

Thanks,

James

---

diff --git a/drivers/scsi/aic7xxx/aic7xxx_core.c 
b/drivers/scsi/aic7xxx/aic7xxx_core.c
index 6d2ae64..64e62ce 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_core.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_core.c
@@ -695,15 +695,16 @@ ahc_handle_seqint(struct ahc_softc *ahc, u_int intstat)
scb_index = ahc_inb(ahc, SCB_TAG);
scb = ahc_lookup_scb(ahc, scb_index);
if (devinfo.role == ROLE_INITIATOR) {
-   if (scb == NULL)
-   panic("HOST_MSG_LOOP with "
- "invalid SCB %x\n", scb_index);
+   if (bus_phase == P_MESGOUT) {
+   if (scb == NULL)
+   panic("HOST_MSG_LOOP with "
+ "invalid SCB %x\n",
+ scb_index);
 
-   if (bus_phase == P_MESGOUT)
ahc_setup_initiator_msgout(ahc,
   &devinfo,
   scb);
-   else {
+   } else {
ahc->msg_type =
MSG_TYPE_INITIATOR_MSGIN;
ahc->msgin_index = 0;


-
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] bsg: bidi bio map failure fix

2008-02-12 Thread James Bottomley
On Tue, 2008-02-12 at 15:40 -0500, Pete Wyckoff wrote:
> If blk_rq_map_user requires more than one bio, and fails mapping
> somewhere after the first bio, it will return with rq->bio set to
> non-NULL, but it will have already unmapped the partial bio.  The
> "out:" error exit section will see the non-null bio and try to unmap
> it again, triggering a mapcount bug via bad_page().
> 
> Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
> ---
>  block/bsg.c |4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/block/bsg.c b/block/bsg.c
> index 3337125..bba7154 100644
> --- a/block/bsg.c
> +++ b/block/bsg.c
> @@ -295,8 +295,10 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr)
>  
>   dxferp = (void*)(unsigned long)hdr->din_xferp;
>   ret =  blk_rq_map_user(q, next_rq, dxferp, hdr->din_xfer_len);
> - if (ret)
> + if (ret) {
> + next_rq->bio = NULL;  /* do not unmap twice */

Nice ... that's a nasty asymmetry of the blk_rq_map_user API.  The map
takes a request gets a ref and fills in the bio.  The unmap has to be
called on the bio leaving you to clear the now removed bio reference
manually.

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: arcmsr + archttp64 calls dma_free_coherent() with irqs disabled - dmesg filled with warnings

2008-02-12 Thread James Bottomley

On Tue, 2008-02-12 at 12:21 -1000, Joshua Hoblitt wrote:
> I've got a few compilation errors on head.  Before I try it, is it
> possible to backport the 76d78300a6eb8b7f08e47703b7e68a659ffc2053 change
> to 2.6.24.y?

I don't see any reason why not but I'd have to cherry pick it to see ...
so you could save me the bother by trying it yourself ...

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: aic94xx: wrong order of SATA disks

2008-02-12 Thread James Bottomley
On Tue, 2008-02-12 at 23:22 +0200, Sergey Kononenko wrote:
> Hello,
> 
> I have server with motherboard Supermicro X7DBR-3 (AIC-9410 onboard).
> And there are three SATA disks in following order:
> 1st drive bay: WD800AAJS-60
> 2nd drive bay: WD2500YS-01S
> 3rd drive bay: WD2500YS-01S
> in BIOS they listed in the same order.
> 
> But after I've loaded linux (2.6.23.14 and 2.6.24 tested) I had
> WD800AAJS-60 named as sdc, and two WD2500YS-01S as sda and sdb.
> Moreover then i've booted second time with same kernel and settings
> names for WD2500YS-01S swapped. Disk which had name sda become sdb and
> vice versa. I've noticed that through disk's serial numbers, because
> models are same.

I'm afraid this reordering of sdX is pretty much a fact of life on
hotplug storage busses, which SAS/SATA is.  The way around it is to use
udev and mount by id or uuid to keep stable device names.

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.6.25-rc1-git2: GDT SCSI: change drivers/scsi/gdth.c into using pci_get device

2008-02-12 Thread James Bottomley
On Tue, 2008-02-12 at 20:48 -0300, Sergio Luis wrote:
> reposting an updated version of it. Please check if it's ok.

Looks fine, thanks!  You added an extra space at the end of 

while ((pdev = pci_get_device(vendor, device, pdev)) 

Which I fixed.  Unfortunately checkpatch isn't very helpful for this
driver since it uses spaces not tabs everywhere, which checkpatch really
hates.

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: [Scst-devel] Integration of SCST in the mainstream Linux kernel

2008-02-12 Thread Nicholas A. Bellinger
Greetings all,

On Tue, 2008-02-12 at 17:05 +0100, Bart Van Assche wrote:
> On Feb 6, 2008 1:11 AM, Nicholas A. Bellinger <[EMAIL PROTECTED]> wrote:
> > I have always observed the case with LIO SE/iSCSI target mode ...
> 
> Hello Nicholas,
> 
> Are you sure that the LIO-SE kernel module source code is ready for
> inclusion in the mainstream Linux kernel ? As you know I tried to test
> the LIO-SE iSCSI target. Already while configuring the target I
> encountered a kernel crash that froze the whole system. I can
> reproduce this kernel crash easily, and I reported it 11 days ago on
> the LIO-SE mailing list (February 4, 2008). One of the call stacks I
> posted shows a crash in mempool_alloc() called from jbd. Or: the crash
> is most likely the result of memory corruption caused by LIO-SE.
> 

So I was able to FINALLY track this down to:

-# CONFIG_SLUB_DEBUG is not set
-# CONFIG_SLAB is not set
-CONFIG_SLUB=y
+CONFIG_SLAB=y

in both your and Chris Weiss's configs that was causing the
reproduceable general protection faults.  I also disabled
CONFIG_RELOCATABLE and crash dump because I was debugging using kdb in
x86_64 VM on 2.6.24 with your config.  I am pretty sure you can leave
this (crash dump) in your config for testing.

This can take a while to compile and take up alot of space, esp. with
all of the kernel debug options enabled, which on 2.6.24, really amounts
to alot of CPU time when building.  Also with your original config, I
was seeing some strange undefined module objects after Stage 2 Link with
iscsi_target_mod with modpost with the SLUB the lockups (which are not
random btw, and are tracked back to __kmalloc())..  Also, at module load
time with the original config, there where some warning about symbol
objects (I believe it was SCSI related, same as the ones with modpost).

In any event, the dozen 1000 loop discovery test is now working fine (as
well as IPoIB) with the above config change, and you should be ready to
go for your testing.

Tomo, Vlad, Andrew and Co:

Do you have any ideas why this would be the case with LIO-Target..?  Is
anyone else seeing something similar to this with their target mode
(mabye its all out of tree code..?) that is having an issue..? I am
using Debian x86_64 and Bart and Chris are using Ubuntu x86_64 and we
both have this problem with CONFIG_SLUB on >= 2.6.22 kernel.org
kernels. 

Also, I will recompile some of my non x86 machines with the above
enabled and see if I can reproduce..  Here the Bart's config again:

http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/30835aede1028188


> Because I was curious to know why it took so long to fix such a severe
> crash, I started browsing through the LIO-SE source code. Analysis of
> the LIO-SE kernel module source code learned me that this crash is not
> a coincidence. Dynamic memory allocation (kmalloc()/kfree()) in the
> LIO-SE kernel module is complex and hard to verify.

What the LIO-SE Target module does is complex. :P  Sorry for taking so
long, I had to start tracking this down by CONFIG_ option with your
config on an x86_64 VM. 

>  There are 412
> memory allocation/deallocation calls in the current version of the
> LIO-SE kernel module source code, which is a lot. Additionally,
> because of the complexity of the memory handling in LIO-SE, it is not
> possible to verify the correctness of the memory handling by analyzing
> a single function at a time. In my opinion this makes the LIO-SE
> source code hard to maintain.
> Furthermore, the LIO-SE kernel module source code does not follow
> conventions that have proven their value in the past like grouping all
> error handling at the end of a function. As could be expected, the
> consequence is that error handling is not correct in several
> functions, resulting in memory leaks in case of an error.

I would be more than happy to point the release paths for iSCSI Target
and LIO-SE to show they are not actual memory leaks (as I mentioned,
this code has been stable for a number of years) for some particular SE
or iSCSI Target logic if you are interested..

Also, if we are talking about target mode storage engine that should be
going upstream, the API to the current stable and future storage
systems, and of course the Mem->SG and SG->Mem that handles all possible
cases of max_sectors and sector_size to past, present, and future.  I
really glad that you have been taking a look at this, because some of
the code (as you mention) can get very complex to make this a reality as
it has been with LIO-Target since v2.2.  

>  Some
> examples of functions in which error handling is clearly incorrect:
> * transport_allocate_passthrough().
> * iscsi_do_build_list().
> 

You did find the one in transport_allocate_passthrough() and the
strncpy() + strlen() in userspace.  Also, thanks for pointing me to the
missing sg_init_table() and sg_mark_end() usage for 2.6.24.  I will post
an update to my thread about how to do this for other drivers..

I will have a look at 

RE: arcmsr + archttp64 calls dma_free_coherent() with irqsdisabled - dmesg filled with warnings

2008-02-12 Thread nickcheng
Please keep me in the loop.

-Original Message-
From: James Bottomley [mailto:[EMAIL PROTECTED] 
Sent: Wednesday, February 13, 2008 6:31 AM
To: Joshua Hoblitt
Cc: Kim H?jgaard-Hansen; [EMAIL PROTECTED]; linux-scsi@vger.kernel.org;
[EMAIL PROTECTED]; [EMAIL PROTECTED]
Subject: Re: arcmsr + archttp64 calls dma_free_coherent() with irqsdisabled
- dmesg filled with warnings


On Tue, 2008-02-12 at 12:21 -1000, Joshua Hoblitt wrote:
> I've got a few compilation errors on head.  Before I try it, is it
> possible to backport the 76d78300a6eb8b7f08e47703b7e68a659ffc2053 change
> to 2.6.24.y?

I don't see any reason why not but I'd have to cherry pick it to see ...
so you could save me the bother by trying it yourself ...

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] gdth: convert to PCI hotplug API

2008-02-12 Thread Jeff Garzik

Signed-off-by: Jeff Garzik <[EMAIL PROTECTED]>
---
 drivers/scsi/gdth.c |  143 +++-
 1 file changed, 86 insertions(+), 57 deletions(-)

06196f50915da97bb897495863f9f084d785c1e4
diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index c825239..1b53e92 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -595,85 +595,107 @@ static int __init gdth_search_isa(ulong32 bios_adr)
 #endif /* CONFIG_ISA */
 
 #ifdef CONFIG_PCI
-static void gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
-ushort vendor, ushort dev);
+static gdth_pci_str gdth_pcistr[MAXHA];
+static int gdth_pci_cnt;
+static bool gdth_pci_registered;
 
-static int __init gdth_search_pci(gdth_pci_str *pcistr)
+static bool __init gdth_search_vortex(ushort device)
 {
-ushort device, cnt;
-
-TRACE(("gdth_search_pci()\n"));
-
-cnt = 0;
-for (device = 0; device <= PCI_DEVICE_ID_VORTEX_GDT6555; ++device)
-gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX, device);
-for (device = PCI_DEVICE_ID_VORTEX_GDT6x17RP; 
- device <= PCI_DEVICE_ID_VORTEX_GDTMAXRP; ++device)
-gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX, device);
-gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX, 
-PCI_DEVICE_ID_VORTEX_GDTNEWRX);
-gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_VORTEX, 
-PCI_DEVICE_ID_VORTEX_GDTNEWRX2);
-gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_INTEL,
-PCI_DEVICE_ID_INTEL_SRC);
-gdth_search_dev(pcistr, &cnt, PCI_VENDOR_ID_INTEL,
-PCI_DEVICE_ID_INTEL_SRC_XSCALE);
-return cnt;
+   if (device <= PCI_DEVICE_ID_VORTEX_GDT6555)
+   return true;
+   if (device >= PCI_DEVICE_ID_VORTEX_GDT6x17RP &&
+   device <= PCI_DEVICE_ID_VORTEX_GDTMAXRP)
+   return true;
+   if (device == PCI_DEVICE_ID_VORTEX_GDTNEWRX ||
+   device == PCI_DEVICE_ID_VORTEX_GDTNEWRX2)
+   return true;
+   return false;
 }
 
+static int gdth_pci_init_one(struct pci_dev *pdev,
+const struct pci_device_id *ent);
+static void gdth_pci_remove_one(struct pci_dev *pdev);
+static void gdth_remove_one(gdth_ha_str *ha);
+
 /* Vortex only makes RAID controllers.
  * We do not really want to specify all 550 ids here, so wildcard match.
  */
-static struct pci_device_id gdthtable[] __maybe_unused = {
-{PCI_VENDOR_ID_VORTEX,PCI_ANY_ID,PCI_ANY_ID, PCI_ANY_ID},
-{PCI_VENDOR_ID_INTEL,PCI_DEVICE_ID_INTEL_SRC,PCI_ANY_ID,PCI_ANY_ID}, 
-
{PCI_VENDOR_ID_INTEL,PCI_DEVICE_ID_INTEL_SRC_XSCALE,PCI_ANY_ID,PCI_ANY_ID}, 
-{0}
+static struct pci_device_id gdthtable[] __devinitdata = {
+   { PCI_VDEVICE(VORTEX, PCI_ANY_ID) },
+   { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SRC) },
+   { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SRC_XSCALE) },
+   { } /* terminate list */
+};
+MODULE_DEVICE_TABLE(pci, gdthtable);
+
+static struct pci_driver gdth_pci_driver = {
+   .name   = "gdth",
+   .id_table   = gdthtable,
+   .probe  = gdth_pci_init_one,
+   .remove = gdth_pci_remove_one,
 };
-MODULE_DEVICE_TABLE(pci,gdthtable);
 
-static void __init gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
-   ushort vendor, ushort device)
+static void gdth_pci_remove_one(struct pci_dev *pdev)
+{
+   gdth_ha_str *ha = pci_get_drvdata(pdev);
+
+   pci_set_drvdata(pdev, NULL);
+
+   list_del(&ha->list);
+   gdth_remove_one(ha);
+
+   pci_disable_device(pdev);
+}
+
+static int __devinit gdth_pci_init_one(struct pci_dev *pdev,
+  const struct pci_device_id *ent)
 {
-ulong base0, base1, base2;
-struct pci_dev *pdev;
+   ushort vendor = pdev->vendor;
+   ushort device = pdev->device;
+   ulong base0, base1, base2;
+   int rc;
 
-TRACE(("gdth_search_dev() cnt %d vendor %x device %x\n",
-  *cnt, vendor, device));
+   TRACE(("gdth_search_dev() cnt %d vendor %x device %x\n",
+  gdth_pci_cnt, vendor, device));
+
+   if (vendor == PCI_VENDOR_ID_VORTEX && !gdth_search_vortex(device))
+   return -ENODEV;
+
+   rc = pci_enable_device(pdev);
+   if (rc)
+   return rc;
+
+   if (gdth_pci_cnt >= MAXHA)
+   return -EBUSY;
 
-pdev = NULL;
-while ((pdev = pci_find_device(vendor, device, pdev)) 
-   != NULL) {
-if (pci_enable_device(pdev))
-continue;
-if (*cnt >= MAXHA)
-return;
 /* GDT PCI controller found, resources are already in pdev */
-pcistr[*cnt].pdev = pdev;
-pcistr[*cnt].irq = pdev->irq;
+   gdth_pcistr[gdth_pci_cnt].pdev = pdev;
+   gdth_pcistr[gdth_pci_cnt].irq = pdev->irq;
 base0 = pci_resource_flags(pdev, 0);
 base1 = pci_resource_flags(pdev, 1);
 base2

[PATCH] scsi: le*_add_cpu conversion

2008-02-12 Thread marcin . slusarz
From: Marcin Slusarz <[EMAIL PROTECTED]>

replace all:
little_endian_variable = cpu_to_leX(leX_to_cpu(little_endian_variable) +
expression_in_cpu_byteorder);
with:
leX_add_cpu(&little_endian_variable, expression_in_cpu_byteorder);
generated with semantic patch

Signed-off-by: Marcin Slusarz <[EMAIL PROTECTED]>
Cc: linux-scsi@vger.kernel.org
Cc: [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
---
 drivers/scsi/aacraid/commsup.c |2 +-
 drivers/scsi/ips.c |8 ++--
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 81b3692..3ac8c82 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -594,7 +594,7 @@ void aac_consumer_free(struct aac_dev * dev, struct 
aac_queue *q, u32 qid)
if (le32_to_cpu(*q->headers.consumer) >= q->entries)
*q->headers.consumer = cpu_to_le32(1);
else
-   *q->headers.consumer = 
cpu_to_le32(le32_to_cpu(*q->headers.consumer)+1);
+   le32_add_cpu(q->headers.consumer, 1);
 
if (wasfull) {
switch (qid) {
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index bb152fb..f45770a 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -3696,9 +3696,7 @@ ips_send_cmd(ips_ha_t * ha, ips_scb_t * scb)
scb->cmd.basic_io.sg_count = scb->sg_len;
 
if (scb->cmd.basic_io.lba)
-   scb->cmd.basic_io.lba =
-   cpu_to_le32(le32_to_cpu
-   (scb->cmd.basic_io.lba) +
+   le32_add_cpu(&scb->cmd.basic_io.lba,
le16_to_cpu(scb->cmd.basic_io.
sector_count));
else
@@ -3744,9 +3742,7 @@ ips_send_cmd(ips_ha_t * ha, ips_scb_t * scb)
scb->cmd.basic_io.sg_count = scb->sg_len;
 
if (scb->cmd.basic_io.lba)
-   scb->cmd.basic_io.lba =
-   cpu_to_le32(le32_to_cpu
-   (scb->cmd.basic_io.lba) +
+   le32_add_cpu(&scb->cmd.basic_io.lba,
le16_to_cpu(scb->cmd.basic_io.
sector_count));
else
-- 
1.5.3.7

-
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: arcmsr + archttp64 calls dma_free_coherent() with irqs disabled - dmesg filled with warnings

2008-02-12 Thread Joshua Hoblitt
I've got a few compilation errors on head.  Before I try it, is it
possible to backport the 76d78300a6eb8b7f08e47703b7e68a659ffc2053 change
to 2.6.24.y?

-J

--
On Tue, Feb 12, 2008 at 10:53:45AM -1000, Joshua Hoblitt wrote:
> I tried doing a build from that commit id but I get an oops from an
> unrelated subsystem.  I'll try again from the head of the tree.
> 
> -J
> 
> --
> On Sat, Feb 09, 2008 at 01:43:39PM -0600, James Bottomley wrote:
> > 
> > On Sat, 2008-02-09 at 09:35 -1000, Joshua Hoblitt wrote:
> > > Hi James,
> > > 
> > > I can give it a try.  What commit id do you think fixed it?
> > 
> > commit 76d78300a6eb8b7f08e47703b7e68a659ffc2053
> > Author: Nick Cheng <[EMAIL PROTECTED]>
> > Date:   Mon Feb 4 23:53:24 2008 -0800
> > 
> > [SCSI] arcmsr: updates (1.20.00.15)
> > 
> > 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


[Bug 9775] HOST_MSG_LOOP invalid SCB ff

2008-02-12 Thread bugme-daemon
http://bugzilla.kernel.org/show_bug.cgi?id=9775





--- Comment #10 from [EMAIL PROTECTED]  2008-02-12 13:56 ---
Reply-To: [EMAIL PROTECTED]

On Fri, 2008-02-08 at 18:52 -0800, [EMAIL PROTECTED]
wrote:
> Ok, I've spent some time trying different combinations of devices.
> 
> Against kernel 2.6.24
> T0 is Quantum DLT8000 ID0
> T1 is Quantum DLT8000 ID1
> MTX is STK L80  ID 15
> Terminators A, B
> 
> Channel A   B
> T0,T1,MTX,B Nil   
>  
> Crash
> Nil T0,T1,MTX,B   
>  
> Parity Error in Data-in Phase
> Nil T0,MTX,B  
>  
> Ok, Tar test ok, MTX ok
> Nil T1,MTX,B  
>  
> Ok, Tar test ok, MTX ok 
> -- Both drives work ok  
> T1,MTX,BNil   
>  
> Ok   Skipped Tests
> T1,MTX,ANil   
>  
> Ok   Skipped Tests
> T0,MTX,BNil   
>  
> Crash
> T0,MTX,ANil   
>  
> Crash
> -- Not the terminator
> 
> 
> --Test on two channels
> T0,MTX,AT1,B  
>  
> Crash
> T1,BT0,MTX,A  
>  
> Parity Error in Data-in Phase   
> 
> It really doesn't like three devices, on two busses or one.

Well, I still think you have some type of bus instability, but that said
we need to get rid of the panic.

I'm afraid this is going to be a long process.  For the first attempt,
let's see if this is an unsolicited msgin ... it looks like the driver
handling for those is wrong.  Can you try this patch?

Thanks,

James

---

diff --git a/drivers/scsi/aic7xxx/aic7xxx_core.c
b/drivers/scsi/aic7xxx/aic7xxx_core.c
index 6d2ae64..64e62ce 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_core.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_core.c
@@ -695,15 +695,16 @@ ahc_handle_seqint(struct ahc_softc *ahc, u_int intstat)
scb_index = ahc_inb(ahc, SCB_TAG);
scb = ahc_lookup_scb(ahc, scb_index);
if (devinfo.role == ROLE_INITIATOR) {
-   if (scb == NULL)
-   panic("HOST_MSG_LOOP with "
- "invalid SCB %x\n", scb_index);
+   if (bus_phase == P_MESGOUT) {
+   if (scb == NULL)
+   panic("HOST_MSG_LOOP with "
+ "invalid SCB %x\n",
+ scb_index);

-   if (bus_phase == P_MESGOUT)
ahc_setup_initiator_msgout(ahc,
   &devinfo,
   scb);
-   else {
+   } else {
ahc->msg_type =
MSG_TYPE_INITIATOR_MSGIN;
ahc->msgin_index = 0;


-- 
Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.
-
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


aic94xx: wrong order of SATA disks

2008-02-12 Thread Sergey Kononenko
Hello,

I have server with motherboard Supermicro X7DBR-3 (AIC-9410 onboard).
And there are three SATA disks in following order:
1st drive bay: WD800AAJS-60
2nd drive bay: WD2500YS-01S
3rd drive bay: WD2500YS-01S
in BIOS they listed in the same order.

But after I've loaded linux (2.6.23.14 and 2.6.24 tested) I had
WD800AAJS-60 named as sdc, and two WD2500YS-01S as sda and sdb.
Moreover then i've booted second time with same kernel and settings
names for WD2500YS-01S swapped. Disk which had name sda become sdb and
vice versa. I've noticed that through disk's serial numbers, because
models are same.

When I checked all in dmesg related to aic94xx (compiled with debug
option) i've found this messages:

sas: phy1 added to port0, phy_mask:0x2
...
sas: phy2 added to port1, phy_mask:0x4
sas: phy0 added to port2, phy_mask:0x1

and for second booting:

sas: phy2 added to port0, phy_mask:0x4
...
sas: phy1 added to port1, phy_mask:0x2
sas: phy0 added to port2, phy_mask:0x1

I've repeated reboot several times. But have noticed only two
variants of disks order.
So phys assigned to ports not in right order. Also this order can be
changed randomly for disks with same model.
I've disabled and enabled HostRAID in controlled BIOS. There wasn't
effect from that. Also I tried to assign different SAS addresses to
phys, also this didn't help. CONFIG_SCSI_SCAN_ASYNC is unset in kernel
config.

I have another server with same motherboard there are two SAS disks
(Fujitsu MAX3147RC) and one SATA (WDC WD1600YS-01S). And there phys
assigned in right order:
sas: phy0 added to port0, phy_mask:0x1

sas: phy1 added to port1, phy_mask:0x2
sas: phy2 added to port2, phy_mask:0x4

I think it only related to cases there are more then one SATA disk
connected to controller. Probably this happened because SATA disks
replay to escb requests in wrong order.
I've included below fragments of dmesg that related to aic94xx from
first and second boot of my server. I think this may help to figure out
why disks assigned in such order.

first:
aic94xx: Adaptec aic94xx SAS/SATA driver version 1.0.3 loaded
ACPI: PCI Interrupt :09:02.0[A] -> GSI 18 (level, low) -> IRQ 17
aic94xx: found Adaptec AIC-9410W SAS/SATA Host Adapter, device :09:02.0
scsi2 : aic94xx
aic94xx: BIOS present (1,1), 1822
aic94xx: ue num:3, ue size:88
aic94xx: manuf sect SAS_ADDR 500304800022cd30
aic94xx: manuf sect PCBA SN ORG
aic94xx: ms: num_phy_desc: 8
aic94xx: ms: phy0: ENABLED
aic94xx: ms: phy1: ENABLED
aic94xx: ms: phy2: ENABLED
aic94xx: ms: phy3: ENABLED
aic94xx: ms: phy4: ENABLED
aic94xx: ms: phy5: ENABLED
aic94xx: ms: phy6: ENABLED
aic94xx: ms: phy7: ENABLED
aic94xx: ms: max_phys:0x8, num_phys:0x8
aic94xx: ms: enabled_phys:0xff
aic94xx: ctrla: phy0: sas_addr: 500304800022cd30, sas rate:0x9-0x8, sata 
rate:0x0-0x0, flags:0x0
aic94xx: ctrla: phy1: sas_addr: 500304800022cd31, sas rate:0x9-0x8, sata 
rate:0x0-0x0, flags:0x0
aic94xx: ctrla: phy2: sas_addr: 500304800022cd32, sas rate:0x9-0x8, sata 
rate:0x0-0x0, flags:0x0
aic94xx: ctrla: phy3: sas_addr: 500304800022cd30, sas rate:0x9-0x8, sata 
rate:0x0-0x0, flags:0x0
aic94xx: ctrla: phy4: sas_addr: 500304800022cd30, sas rate:0x9-0x8, sata 
rate:0x0-0x0, flags:0x0
aic94xx: ctrla: phy5: sas_addr: 500304800022cd30, sas rate:0x9-0x8, sata 
rate:0x0-0x0, flags:0x0
aic94xx: ctrla: phy6: sas_addr: 500304800022cd30, sas rate:0x9-0x8, sata 
rate:0x0-0x0, flags:0x0
aic94xx: ctrla: phy7: sas_addr: 500304800022cd30, sas rate:0x9-0x8, sata 
rate:0x0-0x0, flags:0x0
aic94xx: max_scbs:512, max_ddbs:128
aic94xx: setting phy0 addr to 500304800022cd30
aic94xx: setting phy1 addr to 500304800022cd30
aic94xx: setting phy2 addr to 500304800022cd30
aic94xx: setting phy3 addr to 500304800022cd30
aic94xx: setting phy4 addr to 500304800022cd30
aic94xx: setting phy5 addr to 500304800022cd30
aic94xx: setting phy6 addr to 500304800022cd30
aic94xx: setting phy7 addr to 500304800022cd30
aic94xx: num_edbs:21
aic94xx: num_escbs:3
aic94xx: Found sequencer Firmware version 1.1 (V30)
aic94xx: downloading CSEQ...
aic94xx: dma-ing 8192 bytes
aic94xx: verified 8192 bytes, passed
aic94xx: downloading LSEQs...
aic94xx: dma-ing 14336 bytes
aic94xx: LSEQ0 verified 14336 bytes, passed
aic94xx: LSEQ1 verified 14336 bytes, passed
aic94xx: LSEQ2 verified 14336 bytes, passed
aic94xx: LSEQ3 verified 14336 bytes, passed
aic94xx: LSEQ4 verified 14336 bytes, passed
aic94xx: LSEQ5 verified 14336 bytes, passed
aic94xx: LSEQ6 verified 14336 bytes, passed
aic94xx: LSEQ7 verified 14336 bytes, passed
aic94xx: max_scbs:446
aic94xx: first_scb_site_no:0x20
aic94xx: last_scb_site_no:0x1fe
aic94xx: First SCB dma_handle: 0x37e92000
aic94xx: device :09:02.0: SAS addr 500304800022cd30, PCBA SN ORG, 8 phys, 8 
enabled phys, flash present, BIOS build 1822
aic94xx: posting 3 escbs
aic94xx: escbs posted
aic94xx: posting 8 control phy scbs
aic94xx: control_phy_tasklet_complete: phy0, lrate:0x8, proto:0xe
aic94xx: control_phy_tasklet_complete: phy1, lrate:0x8, proto

Re: [PATCH 1/3] iscsi iser: remove DMA restrictions

2008-02-12 Thread Pete Wyckoff
[EMAIL PROTECTED] wrote on Tue, 12 Feb 2008 15:10 -0600:
> On Tue, 2008-02-12 at 15:54 -0500, Pete Wyckoff wrote:
> > iscsi_iser does not have any hardware DMA restrictions.  Add a
> > slave_configure function to remove any DMA alignment restriction,
> > allowing the use of direct IO from arbitrary offsets within a page.
> > Also disable page bouncing; iser has no restrictions on which pages it
> > can address.
> > 
> > Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
> > ---
> >  drivers/infiniband/ulp/iser/iscsi_iser.c |8 
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
> > b/drivers/infiniband/ulp/iser/iscsi_iser.c
> > index be1b9fb..1b272a6 100644
> > --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> > +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> > @@ -543,6 +543,13 @@ iscsi_iser_ep_disconnect(__u64 ep_handle)
> > iser_conn_terminate(ib_conn);
> >  }
> >  
> > +static int iscsi_iser_slave_configure(struct scsi_device *sdev)
> > +{
> > +   blk_queue_bounce_limit(sdev->request_queue, BLK_BOUNCE_ANY);
> 
> You really don't want to do this.  That signals to the block layer that
> we have an iommu, although it's practically the same thing as a 64 bit
> DMA mask ... but I'd just leave it to the DMA mask to set this up
> correctly.  Anything else is asking for a subtle bug to turn up years
> from now when something causes the mask and the limit to be mismatched.

Oh.  I decided to add that line for symmetry with TCP, and was
convinced by the arguments here:

commit b6d44fe9582b9d90a0b16f508ac08a90d899bf56
Author: Mike Christie <[EMAIL PROTECTED]>
Date:   Thu Jul 26 12:46:47 2007 -0500

[SCSI] iscsi_tcp: Turn off bounce buffers

It was found by LSI that on setups with large amounts of memory
we were bouncing buffers when we did not need to. If the iscsi tcp
code touches the data buffer (or a helper does),
it will kmap the buffer. iscsi_tcp also does not interact with hardware,
so it does not have any hw dma restrictions. This patch sets the bounce
buffer settings for our device queue so buffers should not be bounced
because of a driver limit.

I don't see a convenient place to callback into particular iscsi
devices to set the DMA mask per-host.  It has to go on the
shost_gendev, right?, but only for TCP and iSER, not qla4xxx, which
handles its DMA mask during device probe.

-- 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 1/3] iscsi iser: remove DMA restrictions

2008-02-12 Thread James Bottomley
On Tue, 2008-02-12 at 16:46 -0500, Pete Wyckoff wrote:
> [EMAIL PROTECTED] wrote on Tue, 12 Feb 2008 15:10 -0600:
> > On Tue, 2008-02-12 at 15:54 -0500, Pete Wyckoff wrote:
> > > iscsi_iser does not have any hardware DMA restrictions.  Add a
> > > slave_configure function to remove any DMA alignment restriction,
> > > allowing the use of direct IO from arbitrary offsets within a page.
> > > Also disable page bouncing; iser has no restrictions on which pages it
> > > can address.
> > > 
> > > Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
> > > ---
> > >  drivers/infiniband/ulp/iser/iscsi_iser.c |8 
> > >  1 files changed, 8 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
> > > b/drivers/infiniband/ulp/iser/iscsi_iser.c
> > > index be1b9fb..1b272a6 100644
> > > --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> > > +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> > > @@ -543,6 +543,13 @@ iscsi_iser_ep_disconnect(__u64 ep_handle)
> > >   iser_conn_terminate(ib_conn);
> > >  }
> > >  
> > > +static int iscsi_iser_slave_configure(struct scsi_device *sdev)
> > > +{
> > > + blk_queue_bounce_limit(sdev->request_queue, BLK_BOUNCE_ANY);
> > 
> > You really don't want to do this.  That signals to the block layer that
> > we have an iommu, although it's practically the same thing as a 64 bit
> > DMA mask ... but I'd just leave it to the DMA mask to set this up
> > correctly.  Anything else is asking for a subtle bug to turn up years
> > from now when something causes the mask and the limit to be mismatched.
> 
> Oh.  I decided to add that line for symmetry with TCP, and was
> convinced by the arguments here:
> 
> commit b6d44fe9582b9d90a0b16f508ac08a90d899bf56
> Author: Mike Christie <[EMAIL PROTECTED]>
> Date:   Thu Jul 26 12:46:47 2007 -0500
> 
> [SCSI] iscsi_tcp: Turn off bounce buffers
> 
> It was found by LSI that on setups with large amounts of memory
> we were bouncing buffers when we did not need to. If the iscsi tcp
> code touches the data buffer (or a helper does),
> it will kmap the buffer. iscsi_tcp also does not interact with hardware,
> so it does not have any hw dma restrictions. This patch sets the bounce
> buffer settings for our device queue so buffers should not be bounced
> because of a driver limit.
> 
> I don't see a convenient place to callback into particular iscsi
> devices to set the DMA mask per-host.  It has to go on the
> shost_gendev, right?, but only for TCP and iSER, not qla4xxx, which
> handles its DMA mask during device probe.

You should be taking your mask from the underlying infiniband device as
part of the setup, shouldn't you?

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 1/3] iscsi iser: remove DMA restrictions

2008-02-12 Thread James Bottomley
On Tue, 2008-02-12 at 15:54 -0500, Pete Wyckoff wrote:
> iscsi_iser does not have any hardware DMA restrictions.  Add a
> slave_configure function to remove any DMA alignment restriction,
> allowing the use of direct IO from arbitrary offsets within a page.
> Also disable page bouncing; iser has no restrictions on which pages it
> can address.
> 
> Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
> ---
>  drivers/infiniband/ulp/iser/iscsi_iser.c |8 
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
> b/drivers/infiniband/ulp/iser/iscsi_iser.c
> index be1b9fb..1b272a6 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> @@ -543,6 +543,13 @@ iscsi_iser_ep_disconnect(__u64 ep_handle)
>   iser_conn_terminate(ib_conn);
>  }
>  
> +static int iscsi_iser_slave_configure(struct scsi_device *sdev)
> +{
> + blk_queue_bounce_limit(sdev->request_queue, BLK_BOUNCE_ANY);

You really don't want to do this.  That signals to the block layer that
we have an iommu, although it's practically the same thing as a 64 bit
DMA mask ... but I'd just leave it to the DMA mask to set this up
correctly.  Anything else is asking for a subtle bug to turn up years
from now when something causes the mask and the limit to be mismatched.

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 3/3] iscsi iser: increase sg_tablesize

2008-02-12 Thread Pete Wyckoff
Increase FMR limits from 512 kB to 1 MB.  This matches the limit
used by SRP which also uses FMRs for memory mapping.  Also
correct a bug where the sg_tablesize was 1 smaller than what was
allocated, by folding the "+ 1" used everywhere into the
definition of the constant.

Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h |3 ++-
 drivers/infiniband/ulp/iser/iser_verbs.c |6 ++
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h 
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 1ee867b..db8f81a 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -87,7 +87,8 @@
 #define MASK_4K(~(SIZE_4K-1))
 
/* support upto 512KB in one RDMA */
-#define ISCSI_ISER_SG_TABLESIZE (0x8 >> SHIFT_4K)
+/* FMR space for 1 MB of 4k-page transfers, plus 1 if not page aligned */
+#define ISCSI_ISER_SG_TABLESIZE (((1<<20) >> SHIFT_4K) + 1)
 #define ISCSI_ISER_MAX_LUN 256
 #define ISCSI_ISER_MAX_CMD_LEN 16
 
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c 
b/drivers/infiniband/ulp/iser/iser_verbs.c
index 714b8db..c5b374f 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -140,7 +140,7 @@ static int iser_create_ib_conn_res(struct iser_conn 
*ib_conn)
device = ib_conn->device;
 
ib_conn->page_vec = kmalloc(sizeof(struct iser_page_vec) +
-   (sizeof(u64) * (ISCSI_ISER_SG_TABLESIZE 
+1)),
+   sizeof(u64) * ISCSI_ISER_SG_TABLESIZE,
GFP_KERNEL);
if (!ib_conn->page_vec) {
ret = -ENOMEM;
@@ -149,9 +149,7 @@ static int iser_create_ib_conn_res(struct iser_conn 
*ib_conn)
ib_conn->page_vec->pages = (u64 *) (ib_conn->page_vec + 1);
 
params.page_shift= SHIFT_4K;
-   /* when the first/last SG element are not start/end *
-* page aligned, the map whould be of N+1 pages */
-   params.max_pages_per_fmr = ISCSI_ISER_SG_TABLESIZE + 1;
+   params.max_pages_per_fmr = ISCSI_ISER_SG_TABLESIZE;
/* make the pool size twice the max number of SCSI commands *
 * the ML is expected to queue, watermark for unmap at 50%  */
params.pool_size = ISCSI_DEF_XMIT_CMDS_MAX * 2;
-- 
1.5.3.8

-
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 0/3] iscsi bidi & varlen support

2008-02-12 Thread Pete Wyckoff
[EMAIL PROTECTED] wrote on Thu, 31 Jan 2008 20:08 +0200:
> Cheers after 1.3 years these can go in.
> 
> [PATCH 1/3] iscsi: extended cdb support
>The varlen support is not yet in mainline for
>   block and scsi-ml. But the API for drivers will
>   not change. All LLD need to do is max_command to
>   the it's maximum and be ready for bigger commands.
>   This is what's done here. Once these commands start
>   coming iscsi will be ready for them.
> 
> [PATCH 2/3] iscsi: bidi support - libiscsi
> [PATCH 3/3] iscsi: bidi support - iscsi_tcp
>   bidirectional commands support in iscsi.
>   iSER is not yet ready, but it will not break.
>   There is already a mechanism in libiscsi that will
>   return error if bidi commands are sent iSER way.
> 
> Pete please send me the iSER bits so we can port them
> to this latest version.
> 
> Mike these patches are ontop of iscs branch of the iscsi
> git tree, they will apply but for compilation you will need
> to sync with Linus mainline. The patches are for the in-tree
> iscsi code. I own you the compat patch for the out-off-tree
> code, but this I will only be Sunday.
> 
> If we do it fast it might get accepted to 2.6.25 merge window
> 
> Everybody is invited to a party at Shila ben-yhuda 52 Tel-Aviv
> 9:45 pm. Drinks and wonderful see-food on us :)

Here's the patch to add bidi support to iSER too.  It works
with my setup, but could use more testing.  Note that this does
rely on the 3 patches quoted above.

-- Pete


From: Pete Wyckoff <[EMAIL PROTECTED]>

Support bidirectional SCSI commands in iSCSI iSER.  The handling of
data buffers for each direction was already mostly separated.  It was
necessary to separate the unmapping of data bufs when one was found
to be unaligned.  This was done by adding a "direction" parameter to
iser_dma_unmap_task_data() and calling it appropriately.  Also
iser_ctask_rdma_finalize() used local variables in such a way that
it assumed a unidirectional command; that was split up to make it
cleaner and order the operations correctly for bidi.

Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h |3 +-
 drivers/infiniband/ulp/iser/iser_initiator.c |   79 +++---
 drivers/infiniband/ulp/iser/iser_memory.c|9 ++-
 3 files changed, 41 insertions(+), 50 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h 
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 66905df..f5497b0 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -356,5 +356,6 @@ int iser_dma_map_task_data(struct iscsi_iser_cmd_task 
*iser_ctask,
enum   iser_data_dir   iser_dir,
enum   dma_data_direction  dma_dir);
 
-void iser_dma_unmap_task_data(struct iscsi_iser_cmd_task *iser_ctask);
+void iser_dma_unmap_task_data(struct iscsi_iser_cmd_task *iser_ctask,
+ enum iser_data_dir cmd_dir);
 #endif
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c 
b/drivers/infiniband/ulp/iser/iser_initiator.c
index ea3f5dc..491ffab 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -325,9 +325,8 @@ int iser_send_command(struct iscsi_conn *conn,
 {
struct iscsi_iser_conn *iser_conn = conn->dd_data;
struct iscsi_iser_cmd_task *iser_ctask = ctask->dd_data;
-   struct iser_dto *send_dto = NULL;
-   unsigned long edtl;
-   int err = 0;
+   struct iser_dto *send_dto;
+   int err;
struct iser_data_buf *data_buf;
 
struct iscsi_cmd *hdr =  ctask->hdr;
@@ -340,37 +339,32 @@ int iser_send_command(struct iscsi_conn *conn,
if (iser_check_xmit(conn, ctask))
return -ENOBUFS;
 
-   edtl = ntohl(hdr->data_length);
-
/* build the tx desc regd header and add it to the tx desc dto */
iser_ctask->desc.type = ISCSI_TX_SCSI_COMMAND;
send_dto = &iser_ctask->desc.dto;
send_dto->ctask = iser_ctask;
iser_create_send_desc(iser_conn, &iser_ctask->desc, ctask->hdr_len);
 
-   if (hdr->flags & ISCSI_FLAG_CMD_READ)
-   data_buf = &iser_ctask->data[ISER_DIR_IN];
-   else
-   data_buf = &iser_ctask->data[ISER_DIR_OUT];
-
-   if (scsi_sg_count(sc)) { /* using a scatter list */
-   data_buf->buf  = scsi_sglist(sc);
-   data_buf->size = scsi_sg_count(sc);
-   }
-
-   data_buf->data_len = scsi_bufflen(sc);
-
if (hdr->flags & ISCSI_FLAG_CMD_READ) {
-   err = iser_prepare_read_cmd(ctask, edtl);
+   data_buf = &iser_ctask->data[ISER_DIR_IN];
+   data_buf->buf  = scsi_in(sc)->table.sgl;
+   data_buf->size = scsi_in(sc)->table.nents;
+   data_buf->data_len = scsi_in(sc)->length;
+   err = iser_prepare_read_cmd(ctask, data_buf->data_len);
if (err)
   

Re: [patch 02/13] git-scsi-misc gdth fix

2008-02-12 Thread Andrew Morton
On Tue, 12 Feb 2008 17:27:33 +0200
Boaz Harrosh <[EMAIL PROTECTED]> wrote:

> On Tue, Feb 05 2008 at 9:53 +0200, [EMAIL PROTECTED] wrote:
> > From: James Bottomley <[EMAIL PROTECTED]>
> > 
> > On Sun, 2007-10-14 at 12:21 -0700, Andrew Morton wrote:
> >> On Sun, 14 Oct 2007 22:45:47 +0400 "Dave Milter" <[EMAIL PROTECTED]> wrote:
> >>
> >>> I build linux-2.6.23-mm1 and try to boot it using qemu,
> >>> and it crashed with trace like this:
> >>> do_page_fault
> >>> error_code
> >>> lock_acquire
> >>> _spin_lock_irqsave
> >>> gdth_timeout
> >>> run_timer_softirq
> >>> __do_softirq
> >>> do_softirq
> >>>
> >>> I have screenshot, but have no idea, is it legal to include it, if I
> >>> sent copy to lkml.
> >>> config of kernel in attachment,
> >>> I apply all three patches from hot-fixes.
> >>>
> >> The screenshot is here:  http://userweb.kernel.org/~akpm/crash.png
> >>
> >> It would appear that gdth_timeout() is passing a bad pointer into
> >> spin_lock_irqsave().
> > 
> > There's a bug in the gdth rework in that the instance can be deleted
> > from the list before the actual timer is stopped.  This can be worked
> > around I think by the following patch; although we really should be
> > stopping the timer from firing when the list goes empty.
> > 
> > 
> > James said:
> > 
> > This is almost certainly the wrong fix for real hardware.  Although it
> > kills the timer when the list goes empty, nothing will ever restart it
> > when the list fills again.
> > 
> > Boaz, since you touched all of this, you get to fix it.  The correct fix
> > will be to control the timer along with the actual list instead of at
> > entry/exit time.  If you're not going to add this empty check to the
> > timer routine, make sure you use del_timer_sync() before removing the
> > last element from the list.
> > 
> > 
> > Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
> > ---
> > 
> >  drivers/scsi/gdth.c |3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff -puN drivers/scsi/gdth.c~git-scsi-misc-gdth-fix drivers/scsi/gdth.c
> > --- a/drivers/scsi/gdth.c~git-scsi-misc-gdth-fix
> > +++ a/drivers/scsi/gdth.c
> > @@ -3791,6 +3791,9 @@ static void gdth_timeout(ulong data)
> >  gdth_ha_str *ha;
> >  ulong flags;
> >  
> > +if (list_empty(&gdth_instances))
> > +   return;
> > +
> >  ha = list_first_entry(&gdth_instances, gdth_ha_str, list);
> >  spin_lock_irqsave(&ha->smp_lock, flags);
> >  
> > _
> Hello dear Andrew
> 
> Do you perhaps remember who as reported this problem, and if he can
> test patches?
> 

It was Dave Milter, who has been cc'ed on all of this.

> and if he can test patches?

Don't know.  Dave, would it be a possibility?

Thanks.

> 
> ---
> gdth: Try to fix the Timer at exit problem
> 
> Remove_sync the timer before we delete the cards.
> 
> Testing-patches: Boaz Harrosh <[EMAIL PROTECTED]>
> 
> ---
> git-diff --stat -p v2.6.24
>  drivers/scsi/gdth.c |   19 ---
>  1 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
> index b253b8c..57fa756 100644
> --- a/drivers/scsi/gdth.c
> +++ b/drivers/scsi/gdth.c
> @@ -5102,6 +5105,9 @@ static int __init gdth_pci_probe_one(gdth_pci_str 
> *pcistr, int ctr)
>   if (error)
>   goto out_free_coal_stat;
>   list_add_tail(&ha->list, &gdth_instances);
> +
> + scsi_scan_host(shp);
> +
>   return 0;
>  
>   out_free_coal_stat:
> @@ -5137,8 +5143,6 @@ static void gdth_remove_one(gdth_ha_str *ha)
>   ha->sdev = NULL;
>   }
>  
> - gdth_flush(ha);
> -
>   if (shp->irq)
>   free_irq(shp->irq,ha);
>  
> @@ -5236,14 +5240,15 @@ static void __exit gdth_exit(void)
>  {
>   gdth_ha_str *ha;
>  
> - list_for_each_entry(ha, &gdth_instances, list)
> - gdth_remove_one(ha);
> + unregister_chrdev(major,"gdth");
> + unregister_reboot_notifier(&gdth_notifier);
>  
>  #ifdef GDTH_STATISTICS
> - del_timer(&gdth_timer);
> + del_timer_sync(&gdth_timer);
>  #endif
> - unregister_chrdev(major,"gdth");
> - unregister_reboot_notifier(&gdth_notifier);
> +
> + list_for_each_entry(ha, &gdth_instances, list)
> + gdth_remove_one(ha);
>  }
>  
>  module_init(gdth_init);
> 
> 
> 
> 
-
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: Subject: [PATCH 1/3] Let scsi_cmnd->cmnd use request->cmd buffer

2008-02-12 Thread James Bottomley
On Sun, 2008-02-10 at 21:05 +0200, Boaz Harrosh wrote:
> - struct scsi_cmnd had a 16 bytes command buffer of its own.
>   This is an unnecessary duplication and copy of request's
>   cmd. It is probably left overs from the time that scsi_cmnd
>   could function without a request attached. So clean that up.
> 
> - Once above is done, few places, apart from scsi-ml, needed
>   adjustments due to changing the data type of scsi_cmnd->cmnd.
> 
> - Lots of drivers still use MAX_COMMAND_SIZE. So I have left
>   that #define but equate it to BLK_MAX_CDB. The way I see it
>   and is reflected in the patch below is.
>   MAX_COMMAND_SIZE - means: The longest fixed-length (*) SCSI CDB
>  as per the SCSI standard and is not related
>  to the implementation.
>   BLK_MAX_CDB. - The allocated space at the request level
> 
> (*)fixed-length here means commands that their size can be determined
>by their opcode and the CDB does not carry a length specifier, like
>the VARIABLE_LENGTH_CMD(0x7f) command. This is actually not exactly
>true and the SCSI standard also defines extended commands and
>vendor specific commands that can be bigger than 16 bytes. The kernel
>will support these using the same infrastructure used for VARLEN CDB's.
>So in effect MAX_COMMAND_SIZE means the maximum size command
>scsi-ml supports without specifying a cmd_len by ULD's

When we do this, what happens to the minority of drivers that need the
command in DMAable memory ... or have you audited them all and we can
now dump the DMA pool allocation for SCSI commands?

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] enclosure: add support for enclosure services

2008-02-12 Thread James Bottomley
On Tue, 2008-02-12 at 11:07 -0800, Kristen Carlson Accardi wrote:
> I understand what you are trying to do - I guess I just doubt the value
> you've added by doing this.  I think that there's going to be so much
> customization that system vendors will want to add, that they are going
> to wind up adding a custom library regardless, so standardising those
> few things won't buy us anything.

It depends ... if you actually have a use for the customisations, yes.
If you just want the basics of who (what's in the enclousure), what
(activity) and where (locate) then I think it solves your problem almost
entirely.

So, entirely as a straw horse, tell me what else your enclosures provide
that I haven't listed in the four points.  The SES standards too provide
a huge range of things that no-one ever seems to implement (temperature,
power, fan speeds etc).

I think the users of enclosures fall int these categories

85% just want to know where their device actually is (i.e. that sdc is
in enclosure slot 5)
50% like watching the activity lights
30% want to be able to have a visual locate function
20% want a visual failure indication (the other 80% rely on some OS
notification instead)

When you add up the overlapping needs, you get about 90% of people happy
with the basics that the enclosure services provide.  Could there be
more ... sure; should there be more ... I don't think so ... that's what
value add the user libraries can provide.

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] enclosure: add support for enclosure services

2008-02-12 Thread Kristen Carlson Accardi
On Tue, 12 Feb 2008 12:45:35 -0600
James Bottomley <[EMAIL PROTECTED]> wrote:

> On Tue, 2008-02-12 at 10:22 -0800, Kristen Carlson Accardi wrote:
> > I apologize for taking so long to review this patch.  I obviously
> > agree wholeheartedly with Luben.  The problem I ran into while
> > trying to design an enclosure management interface for the SATA
> > devices is that there is all this vendor defined stuff.  For
> > example, for the AHCI LED protocol, the only "defined" LED is
> > 'activity'.  For LED2 and LED3 it is up to hardware vendors to
> > define these.  For SGPIO there's all kinds of ways for hw vendors
> > to customize.  I felt that it was going to be a maintainance
> > nightmare to have to keep track of various vendors enclosure
> > implementations in the ahci driver, and that it'd be better to just
> > have user space libraries take care of that.  Plus, that way a
> > vendor doesn't have to get a patch into the kernel to get their new
> > spiffy wizzy bang blinky lights working (think of how long it takes
> > something to even get into a vendor kernel, which is what these
> > guys care about...).  So I'm still not sold on having an enclosure
> > abstraction in the kernel - at least for the SATA controllers.
> 
> Correct me if I'm wrong, but didn't the original AHCI enclosure patch
> expose activity LEDs via sysfs?

You are sort of wrong.  we exposed a sysfs entry to enable sofware
controlled activity LED, then the driver was responsible for turning it
on and off. (blech, I know, but some vendors want this feature).

> 
> I'm not saying there aren't a lot of non standard pieces that need to
> be activated by direct commands or other user activated protocol.  I
> am saying there are a lot of standard pieces that we could do with
> showing in a uniform manner.
> 
> The pieces I think are absolutely standard are
> 
> 1. Actual enclosure presence (is this device in an enclosure)
> 2. Activity LED, this seems to be a feature of every enclosure.
> 
> I also think the following are reasonably standard (based on the fact
> that most enclosure standards recommend but don't require this):
> 
> 3. Locate LED (for locating the device).  Even if you only have an
> activity LED, this is usually done by flashing the activity LED in a
> well defined pattern.
> 4. Fault.  this is the least standardised of the lot, but does seem to
> be present in about every enclosure implementation.
> 
> All I've done is standardise these four pieces ... the services
> actually take into account that it might not be possible to do
> certain of these (like fault).
> 
> James
> 
> 

I understand what you are trying to do - I guess I just doubt the value
you've added by doing this.  I think that there's going to be so much
customization that system vendors will want to add, that they are going
to wind up adding a custom library regardless, so standardising those
few things won't buy us anything.

-
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


[Bug 8908] 3ware 9650SE -8LPML not recognized by 3w-9xxx driver

2008-02-12 Thread bugme-daemon
http://bugzilla.kernel.org/show_bug.cgi?id=8908


[EMAIL PROTECTED] changed:

   What|Removed |Added

 Status|NEW |REJECTED
 Resolution||DOCUMENTED




--- Comment #7 from [EMAIL PROTECTED]  2008-02-12 11:32 ---
Thanks much, closing the bug.


-- 
Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.
-
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


[Bug 8908] 3ware 9650SE -8LPML not recognized by 3w-9xxx driver

2008-02-12 Thread bugme-daemon
http://bugzilla.kernel.org/show_bug.cgi?id=8908





--- Comment #6 from [EMAIL PROTECTED]  2008-02-12 11:30 ---
The 9650SE (vendor:0x13c1, device id: 0x1004) controller series has been 
supported by the in-kernel 3w-9xxx driver since 2.6.19 kernel.

The user did not properly configure 'single disk' support through the 3ware
BIOS, tw_cli or 3dm2 tool.

Please close this bug.

-Adam Radford


-- 
Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.
-
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: Open bugs

2008-02-12 Thread Natalie Protasevich
On Feb 12, 2008 9:57 AM, James Bottomley
<[EMAIL PROTECTED]> wrote:
> Added linux-scsi for the SCSI ones
>
> On Tue, 2008-02-12 at 00:18 -0800, Natalie Protasevich wrote:
> > Hello,
> >
> > The bugs listed are over a month old, and haven't been addressed yet.
> > It would be appreciated if corresponding maintainers identify whether
> > the bugs have been fixed, or need to be worked on, and take
> > appropriate action.
> >
> > In most cases, reporters are standing by and ready to provide
> > information and necessary testing.
> > Thanks!
> >
> > SCSI==
> >
> > Problems on booting
> > http://bugzilla.kernel.org/show_bug.cgi?id=9621
> > Date: 12/22/2007
> > Regression
> > Summary: The boot stops / hangs on hardware detection of SCSI.  I have
> > an InitioINI-9X00U/UW
> > When I have an usb key sticked in /dev/sba, and run lilo then, then it
> > dont boot but give L99 99 99 99 ... error
>
> I think this was fixed by commit
> e2d435ea4084022ab88efa74214accb45b1f9e92
>
> Apparently bugzilla email is on the fritz again because this bug report
> didn't come across linux-scsi.

I just fixed that, thanks. It was incorrect "assign-to" party.

>
> > Resetting RAID attached to a FC Switch causes kernel panic and crash
> > http://bugzilla.kernel.org/show_bug.cgi?id=9598
> > 12/18/2007
> > Hardware Environment:SunFire X4200 - 2 x dual core AMD Opteron CPUs,
> > 8GB Ram, Qlogic FC adapter.
> > Summary: Resetting the RAID box causes the X4200 to crash.
>
> This one looks like the usual problem of remove re-add with the SCSI
> device model.
>
> > 3ware 9650SE -8LPML not recognized by 3w-9xxx driver
> > http://bugzilla.kernel.org/show_bug.cgi?id=8908
> > 08/19/2007
> > Problem Description: The 3w-9xxx kernel driver for 3ware 9xxx SATA
> > RAID Controller series did not recognize the 3ware 9650SE-8LPML SATA
> > RAID Controller.
>
> Since this one never apparently worked it's not a regression but an
> enhancement request, isn't it?

No this is not a regression. The bug list that I post is just any bugs
(mostly unattended or stalled). Sometimes those are regressions off of
the Raphael's list, when they don't get resolved for a while, or just
random regressions that haven't showed up on the "hot list" by
Raphael.

>
> However, adding this PCI ID to the driver should be fairly
> straightforward.  Does anyone know what the actual PCI IDs are?

I just asked the reporter to provide this information, 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


[Bug 8908] 3ware 9650SE -8LPML not recognized by 3w-9xxx driver

2008-02-12 Thread bugme-daemon
http://bugzilla.kernel.org/show_bug.cgi?id=8908


[EMAIL PROTECTED] changed:

   What|Removed |Added

 CC|linux-scsi@vger.kernel.org  |
 AssignedTo|[EMAIL PROTECTED]|[EMAIL PROTECTED]
   ||bugs.osdl.org




--- Comment #5 from [EMAIL PROTECTED]  2008-02-12 10:49 ---
(Reassigning to the [EMAIL PROTECTED], instead of CC-ing)


-- 
Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug, or are watching someone who is.
You are on the CC list for the bug, or are watching someone who is.
-
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] enclosure: add support for enclosure services

2008-02-12 Thread James Bottomley
On Tue, 2008-02-12 at 10:22 -0800, Kristen Carlson Accardi wrote:
> I apologize for taking so long to review this patch.  I obviously agree
> wholeheartedly with Luben.  The problem I ran into while trying to
> design an enclosure management interface for the SATA devices is that
> there is all this vendor defined stuff.  For example, for the AHCI LED
> protocol, the only "defined" LED is 'activity'.  For LED2 and LED3 it
> is up to hardware vendors to define these.  For SGPIO there's all kinds
> of ways for hw vendors to customize.  I felt that it was going to be a
> maintainance nightmare to have to keep track of various vendors
> enclosure implementations in the ahci driver, and that it'd be better
> to just have user space libraries take care of that.  Plus, that way a
> vendor doesn't have to get a patch into the kernel to get their new
> spiffy wizzy bang blinky lights working (think of how long it takes
> something to even get into a vendor kernel, which is what these guys
> care about...).  So I'm still not sold on having an enclosure
> abstraction in the kernel - at least for the SATA controllers.

Correct me if I'm wrong, but didn't the original AHCI enclosure patch
expose activity LEDs via sysfs?

I'm not saying there aren't a lot of non standard pieces that need to be
activated by direct commands or other user activated protocol.  I am
saying there are a lot of standard pieces that we could do with showing
in a uniform manner.

The pieces I think are absolutely standard are

1. Actual enclosure presence (is this device in an enclosure)
2. Activity LED, this seems to be a feature of every enclosure.

I also think the following are reasonably standard (based on the fact
that most enclosure standards recommend but don't require this):

3. Locate LED (for locating the device).  Even if you only have an
activity LED, this is usually done by flashing the activity LED in a
well defined pattern.
4. Fault.  this is the least standardised of the lot, but does seem to
be present in about every enclosure implementation.

All I've done is standardise these four pieces ... the services actually
take into account that it might not be possible to do certain of these
(like fault).

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


[Bug 8908] 3ware 9650SE -8LPML not recognized by 3w-9xxx driver

2008-02-12 Thread bugme-daemon
http://bugzilla.kernel.org/show_bug.cgi?id=8908


[EMAIL PROTECTED] changed:

   What|Removed |Added

 CC||linux-scsi@vger.kernel.org




-- 
Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug, or are watching someone who is.
-
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 3/3] scsi: varlen extended and vendor-specific cdbs

2008-02-12 Thread Boaz Harrosh
On Tue, Feb 12 2008 at 19:51 +0200, Christoph Hellwig <[EMAIL PROTECTED]> wrote:
> On Sun, Feb 10, 2008 at 09:12:02PM +0200, Boaz Harrosh wrote:
>> @@ -525,6 +516,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
>>  unsigned long flags = 0;
>>  unsigned long timeout;
>>  int rtn = 0;
>> +unsigned cmd_len;
>>  
>>  /* check if the device is still usable */
>>  if (unlikely(cmd->device->sdev_state == SDEV_DEL)) {
>> @@ -606,9 +598,17 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
>>   * Before we queue this command, check if the command
>>   * length exceeds what the host adapter can handle.
>>   */
>> -if (CDB_SIZE(cmd) > cmd->device->host->max_cmd_len) {
>> +cmd_len = cmd->cmd_len;
>> +if (!cmd_len) {
>> +BUG_ON(cmd->cmnd[0] == VARIABLE_LENGTH_CMD);
>> +cmd_len = COMMAND_SIZE((cmd)->cmnd[0]);
>> +}
> 
> This looks odd to me.  Shouldn't we make sure cmd_len is always
> initialized in a single place for either varlen or fixed length
> commands and not have things like this?
> 
I used to have a BUG_ON(!cmd_len) here at around the 2.6.20 kernels
And it would trigger. I'm not sure exactly how. Through a retransmit
or something.

Reinspecting all command submission paths, I agree with you that it
should not happen anymore. I will look at it some more and run tests.

Perhaps if this code will sit in -mm tree for a while I can put the
BUG_ON back and see if it triggers again. What do you recommend?

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] enclosure: add support for enclosure services

2008-02-12 Thread Kristen Carlson Accardi
On Mon, 4 Feb 2008 18:01:36 -0800 (PST)
Luben Tuikov <[EMAIL PROTECTED]> wrote:

> --- On Mon, 2/4/08, James Bottomley
> <[EMAIL PROTECTED]> wrote:
> > > > The enclosure misc device is really just a
> > library providing
> > > > sysfs
> > > > support for physical enclosure devices and their
> > > > components.
> > > 
> > > Who is the target audience/user of those facilities?
> > > a) The kernel itself needing to read/write SES pages?
> > 
> > That depends on the enclosure integration, but right at the
> > moment, it
> > doesn't
> 
> Yes, I didn't suspect so.
> 
> > 
> > > b) A user space application using sysfs to read/write
> > >SES pages?
> > 
> > Not an application so much as a user.  The idea of sysfs is
> > to allow
> > users to get and set the information in addition to
> > applications.
> 
> Exactly the same argument stands for a user-space
> application with a user-space library.
> 
> This is the classical case of where it is better to
> do this in user-space as opposed to the kernel.
> 
> The kernel provides capability to access the SES
> device.  The user space application and library
> provide interpretation and control.  Thus if the
> enclosure were upgraded, one doesn't need to
> upgrade their kernel in order to utilize the new
> capabilities of the SES device.  Plus upgrading
> a user-space application is a lot easier than
> the kernel (and no reboot necessary).
> 
> Consider another thing: vendors would really like
> unprecedented access to the SES device in the enclosure
> so as your ses/enclosure code keeps state it would
> get out of sync when vendor user-space enclosure
> applications access (and modify) the SES device's
> pages.
> 
> You can test this yourself: submit a patch
> that removes SES /dev/sgX support; advertise your
> ses/class solution and watch the fun.
> 
> > > At the moment SES device management is done via
> > > an application (user-space) and a user-space library
> > > used by the application and /dev/sgX to send SCSI
> > > commands to the SES device.
> > 
> > I must have missed that when I was looking for
> > implementations; what's
> > the URL?
> 
> I'm not aware of any GPLed ones.  That doesn't
> necessarily mean that the best course of action is
> to bloat the kernel.  You can move your ses/enclosure
> stuff to a user space application library
> and thus start a GPLed one.
> 
> > But, if we have non-scsi enclosures to integrate, that
> > makes it harder
> > for a user application because it has to know all the
> > implementations.
> 
> So does the kernel.  And as I pointed out above, it
> is a lot easier to upgrade a user-space application and
> library than it is to upgrade a new kernel and having
> to reboot the computer to run the new kernel.
> 
> > A sysfs framework on the other hand is a universal known
> > thing for the
> > user applications.
> 
> So would a user-space ses library, a la libses.so.
> 
> > > One could have a very good argument to not bloat
> > > the kernel with this but leave it to a user-space
> > > application and a library to do all this and
> > > communicate with the SES device via the kernel's
> > /dev/sgX.
> > 
> > The same thing goes for other esoteric SCSI infrastructure
> > pieces like
> > cd changers.  On the whole, given that ATA is asking for
> > enclosure
> > management in kernel, it makes sense to consolidate the
> > infrastructure
> > and a ses ULD is a very good test bed.
> 
> What is wrong with exporting the SES device as /dev/sgX
> and having a user-space application and library to
> do all this?
> 
> Luben
> 

Hi,
I apologize for taking so long to review this patch.  I obviously agree
wholeheartedly with Luben.  The problem I ran into while trying to
design an enclosure management interface for the SATA devices is that
there is all this vendor defined stuff.  For example, for the AHCI LED
protocol, the only "defined" LED is 'activity'.  For LED2 and LED3 it
is up to hardware vendors to define these.  For SGPIO there's all kinds
of ways for hw vendors to customize.  I felt that it was going to be a
maintainance nightmare to have to keep track of various vendors
enclosure implementations in the ahci driver, and that it'd be better
to just have user space libraries take care of that.  Plus, that way a
vendor doesn't have to get a patch into the kernel to get their new
spiffy wizzy bang blinky lights working (think of how long it takes
something to even get into a vendor kernel, which is what these guys
care about...).  So I'm still not sold on having an enclosure
abstraction in the kernel - at least for the SATA controllers.

Kristen
-
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] block layer varlen-cdb

2008-02-12 Thread Boaz Harrosh
On Tue, Feb 12 2008 at 19:54 +0200, Boaz Harrosh <[EMAIL PROTECTED]> wrote:
> On Tue, Feb 12 2008 at 19:48 +0200, Christoph Hellwig <[EMAIL PROTECTED]> 
> wrote:
>> On Sun, Feb 10, 2008 at 09:09:41PM +0200, Boaz Harrosh wrote:
>>> - add varlen_cdb and varlen_cdb_len to hold a large user cdb
>>>   if needed. They start as empty. Allocation of buffer must
>>>   be done by user and held until request execution is done.
>>> - Since there can be either a fix_length command up to 16 bytes
>>>   or a variable_length, larger then 16 bytes, commands but never
>>>   both, we hold the two types in a union to save space. The
>>>   presence of varlen_cdb_len and cmd_len==0 signals a varlen_cdb
>>>   mode.
>> this one I'm a bit confused by, why can't we just set the length
>> of the variable length command in cmd_len aswell, and if cmd_len >
>> the length of the cmd array it's a variable length command?
>>
>> Note that this is both to keep the logic simpler and not to grow
>> struct request further.  Especially for the rather rare case
>> of a bidi command.
> 
> Because this will be dangerous for the Legacy block devices.
> Unlike scsi drivers block drivers do not have a .max_cmnd_len
> and upper layer will not check to make sure that the device supports
> the longer command. If such a command goes through, lets say bsg
> the drivers do blindly memcpy(,,rq->cmd_len) and will crash.
> Better safe then sorry, at no cost.
> 
> Boaz
> 
> -
PS: the struct request does not grow. Because before cmd_len was
unsigned but now both cmd_len and varlen_cdb_len are short so
it is the same as before.

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: [BUGFIX 1/2] gdth: scan for scsi devices

2008-02-12 Thread Christoph Hellwig
On Tue, Feb 12, 2008 at 07:35:22PM +0200, Boaz Harrosh wrote:
> 
> The patch: "gdth: switch to modern scsi host registration"
> 
> missed one simple fact when moving a way from scsi_module.c.
> That is to call scsi_scan_host() on the probed host.
> With this the gdth driver from 2.6.24 is again able to
> see drives and boot.

Doh, someone please hand me a brown paper bag.  My first series
of patches had this but it got dropped when I rebased it over various
janitor cleanups.

The patch looks obviously correct, thanks.

-
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: Open bugs

2008-02-12 Thread James Bottomley
Added linux-scsi for the SCSI ones

On Tue, 2008-02-12 at 00:18 -0800, Natalie Protasevich wrote:
> Hello,
> 
> The bugs listed are over a month old, and haven't been addressed yet.
> It would be appreciated if corresponding maintainers identify whether
> the bugs have been fixed, or need to be worked on, and take
> appropriate action.
> 
> In most cases, reporters are standing by and ready to provide
> information and necessary testing.
> Thanks!
> 
> SCSI==
> 
> Problems on booting
> http://bugzilla.kernel.org/show_bug.cgi?id=9621
> Date: 12/22/2007
> Regression
> Summary: The boot stops / hangs on hardware detection of SCSI.  I have
> an InitioINI-9X00U/UW
> When I have an usb key sticked in /dev/sba, and run lilo then, then it
> dont boot but give L99 99 99 99 ... error

I think this was fixed by commit
e2d435ea4084022ab88efa74214accb45b1f9e92

Apparently bugzilla email is on the fritz again because this bug report
didn't come across linux-scsi.

> Resetting RAID attached to a FC Switch causes kernel panic and crash
> http://bugzilla.kernel.org/show_bug.cgi?id=9598
> 12/18/2007
> Hardware Environment:SunFire X4200 - 2 x dual core AMD Opteron CPUs,
> 8GB Ram, Qlogic FC adapter.
> Summary: Resetting the RAID box causes the X4200 to crash.

This one looks like the usual problem of remove re-add with the SCSI
device model.

> 3ware 9650SE -8LPML not recognized by 3w-9xxx driver
> http://bugzilla.kernel.org/show_bug.cgi?id=8908
> 08/19/2007
> Problem Description: The 3w-9xxx kernel driver for 3ware 9xxx SATA
> RAID Controller series did not recognize the 3ware 9650SE-8LPML SATA
> RAID Controller.

Since this one never apparently worked it's not a regression but an
enhancement request, isn't it?

However, adding this PCI ID to the driver should be fairly
straightforward.  Does anyone know what the actual PCI IDs are?

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] block layer varlen-cdb

2008-02-12 Thread Boaz Harrosh
On Tue, Feb 12 2008 at 19:48 +0200, Christoph Hellwig <[EMAIL PROTECTED]> wrote:
> On Sun, Feb 10, 2008 at 09:09:41PM +0200, Boaz Harrosh wrote:
>> - add varlen_cdb and varlen_cdb_len to hold a large user cdb
>>   if needed. They start as empty. Allocation of buffer must
>>   be done by user and held until request execution is done.
>> - Since there can be either a fix_length command up to 16 bytes
>>   or a variable_length, larger then 16 bytes, commands but never
>>   both, we hold the two types in a union to save space. The
>>   presence of varlen_cdb_len and cmd_len==0 signals a varlen_cdb
>>   mode.
> 
> this one I'm a bit confused by, why can't we just set the length
> of the variable length command in cmd_len aswell, and if cmd_len >
> the length of the cmd array it's a variable length command?
> 
> Note that this is both to keep the logic simpler and not to grow
> struct request further.  Especially for the rather rare case
> of a bidi command.

Because this will be dangerous for the Legacy block devices.
Unlike scsi drivers block drivers do not have a .max_cmnd_len
and upper layer will not check to make sure that the device supports
the longer command. If such a command goes through, lets say bsg
the drivers do blindly memcpy(,,rq->cmd_len) and will crash.
Better safe then sorry, at no cost.

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 3/3] scsi: varlen extended and vendor-specific cdbs

2008-02-12 Thread Christoph Hellwig
On Sun, Feb 10, 2008 at 09:12:02PM +0200, Boaz Harrosh wrote:
> @@ -525,6 +516,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
>   unsigned long flags = 0;
>   unsigned long timeout;
>   int rtn = 0;
> + unsigned cmd_len;
>  
>   /* check if the device is still usable */
>   if (unlikely(cmd->device->sdev_state == SDEV_DEL)) {
> @@ -606,9 +598,17 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
>* Before we queue this command, check if the command
>* length exceeds what the host adapter can handle.
>*/
> - if (CDB_SIZE(cmd) > cmd->device->host->max_cmd_len) {
> + cmd_len = cmd->cmd_len;
> + if (!cmd_len) {
> + BUG_ON(cmd->cmnd[0] == VARIABLE_LENGTH_CMD);
> + cmd_len = COMMAND_SIZE((cmd)->cmnd[0]);
> + }

This looks odd to me.  Shouldn't we make sure cmd_len is always
initialized in a single place for either varlen or fixed length
commands and not have things like this?

-
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: Subject: [PATCH 1/3] Let scsi_cmnd->cmnd use request->cmd buffer

2008-02-12 Thread Christoph Hellwig
On Sun, Feb 10, 2008 at 09:05:17PM +0200, Boaz Harrosh wrote:
> - Lots of drivers still use MAX_COMMAND_SIZE. So I have left
>   that #define but equate it to BLK_MAX_CDB. The way I see it
>   and is reflected in the patch below is.
>   MAX_COMMAND_SIZE - means: The longest fixed-length (*) SCSI CDB
>  as per the SCSI standard and is not related
>  to the implementation.
>   BLK_MAX_CDB. - The allocated space at the request level
> 
> (*)fixed-length here means commands that their size can be determined
>by their opcode and the CDB does not carry a length specifier, like
>the VARIABLE_LENGTH_CMD(0x7f) command. This is actually not exactly
>true and the SCSI standard also defines extended commands and
>vendor specific commands that can be bigger than 16 bytes. The kernel
>will support these using the same infrastructure used for VARLEN CDB's.
>So in effect MAX_COMMAND_SIZE means the maximum size command
>scsi-ml supports without specifying a cmd_len by ULD's

A comment like this should be near the declaration of MAX_COMMAND_SIZE

> +#define MAX_COMMAND_SIZE 16
> +#if (MAX_COMMAND_SIZE > BLK_MAX_CDB)
> +#error MAX_COMMAND_SIZE can not be smaller than BLK_MAX_CDB
> +#endif

No tabs between the # and the rest of the cpp command, please.  Either
nothing or a single space as indentation instead.

Except for those two small nitpicks this looks very good to me.  Nice
memory saving aswel.
-
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


[BUGFIX 2/2] gdth: bugfix for the Timer at exit crash

2008-02-12 Thread Boaz Harrosh

gdth _exit would first remove all cards then stop the timer
and would not sync with the timer function. This caused a crash
in gdth_timer() when module was unloaded.

del_timer_sync the timer before we delete the cards.

NOT YET TESTED

Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
 drivers/scsi/gdth.c |   15 ---
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 8eb78be..103280e 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -3793,6 +3793,8 @@ static void gdth_timeout(ulong data)
 gdth_ha_str *ha;
 ulong flags;
 
+BUG_ON(list_empty(&gdth_instances));
+
 ha = list_first_entry(&gdth_instances, gdth_ha_str, list);
 spin_lock_irqsave(&ha->smp_lock, flags);
 
@@ -5146,8 +5148,6 @@ static void gdth_remove_one(gdth_ha_str *ha)
ha->sdev = NULL;
}
 
-   gdth_flush(ha);
-
if (shp->irq)
free_irq(shp->irq,ha);
 
@@ -5245,14 +5245,15 @@ static void __exit gdth_exit(void)
 {
gdth_ha_str *ha;
 
-   list_for_each_entry(ha, &gdth_instances, list)
-   gdth_remove_one(ha);
+   unregister_chrdev(major,"gdth");
+   unregister_reboot_notifier(&gdth_notifier);
 
 #ifdef GDTH_STATISTICS
-   del_timer(&gdth_timer);
+   del_timer_sync(&gdth_timer);
 #endif
-   unregister_chrdev(major,"gdth");
-   unregister_reboot_notifier(&gdth_notifier);
+
+   list_for_each_entry(ha, &gdth_instances, list)
+   gdth_remove_one(ha);
 }
 
 module_init(gdth_init);
-- 
1.5.3.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


[BUGFIX 1/2] gdth: scan for scsi devices

2008-02-12 Thread Boaz Harrosh

The patch: "gdth: switch to modern scsi host registration"

missed one simple fact when moving a way from scsi_module.c.
That is to call scsi_scan_host() on the probed host.
With this the gdth driver from 2.6.24 is again able to
see drives and boot.

Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
Tested-by: Joerg Dorchain: <[EMAIL PROTECTED]>
Tested-by: Stefan Priebe <[EMAIL PROTECTED]>
Tested-by: Jon Chelton <[EMAIL PROTECTED]>
---
 drivers/scsi/gdth.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index b253b8c..8eb78be 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -4838,6 +4838,9 @@ static int __init gdth_isa_probe_one(ulong32 isa_bios)
if (error)
goto out_free_coal_stat;
list_add_tail(&ha->list, &gdth_instances);
+
+   scsi_scan_host(shp);
+
return 0;
 
  out_free_coal_stat:
@@ -4965,6 +4968,9 @@ static int __init gdth_eisa_probe_one(ushort eisa_slot)
if (error)
goto out_free_coal_stat;
list_add_tail(&ha->list, &gdth_instances);
+
+   scsi_scan_host(shp);
+
return 0;
 
  out_free_ccb_phys:
@@ -5102,6 +5108,9 @@ static int __init gdth_pci_probe_one(gdth_pci_str 
*pcistr, int ctr)
if (error)
goto out_free_coal_stat;
list_add_tail(&ha->list, &gdth_instances);
+
+   scsi_scan_host(shp);
+
return 0;
 
  out_free_coal_stat:
-- 
1.5.3.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


[PATCH] bsg: bidi bio map failure fix

2008-02-12 Thread Pete Wyckoff
If blk_rq_map_user requires more than one bio, and fails mapping
somewhere after the first bio, it will return with rq->bio set to
non-NULL, but it will have already unmapped the partial bio.  The
"out:" error exit section will see the non-null bio and try to unmap
it again, triggering a mapcount bug via bad_page().

Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
---
 block/bsg.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/block/bsg.c b/block/bsg.c
index 3337125..bba7154 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -295,8 +295,10 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr)
 
dxferp = (void*)(unsigned long)hdr->din_xferp;
ret =  blk_rq_map_user(q, next_rq, dxferp, hdr->din_xfer_len);
-   if (ret)
+   if (ret) {
+   next_rq->bio = NULL;  /* do not unmap twice */
goto out;
+   }
}
 
if (hdr->dout_xfer_len) {
-- 
1.5.3.8

-
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 0/3] iscsi bidi & varlen support

2008-02-12 Thread Pete Wyckoff
[EMAIL PROTECTED] wrote on Tue, 12 Feb 2008 15:12 -0500:
> [EMAIL PROTECTED] wrote on Thu, 31 Jan 2008 20:08 +0200:
> > Cheers after 1.3 years these can go in.
> > 
> > [PATCH 1/3] iscsi: extended cdb support
> >The varlen support is not yet in mainline for
> >   block and scsi-ml. But the API for drivers will
> >   not change. All LLD need to do is max_command to
> >   the it's maximum and be ready for bigger commands.
> >   This is what's done here. Once these commands start
> >   coming iscsi will be ready for them.
> > 
> > [PATCH 2/3] iscsi: bidi support - libiscsi
> > [PATCH 3/3] iscsi: bidi support - iscsi_tcp
> >   bidirectional commands support in iscsi.
> >   iSER is not yet ready, but it will not break.
> >   There is already a mechanism in libiscsi that will
> >   return error if bidi commands are sent iSER way.
> > 
> > Pete please send me the iSER bits so we can port them
> > to this latest version.
> > 
> > Mike these patches are ontop of iscs branch of the iscsi
> > git tree, they will apply but for compilation you will need
> > to sync with Linus mainline. The patches are for the in-tree
> > iscsi code. I own you the compat patch for the out-off-tree
> > code, but this I will only be Sunday.
> 
> Here's the patch to add bidi support to iSER too.  It works
> with my setup, but could use more testing.  Note that this does
> rely on the 3 patches quoted above.

Similar, for varlen support to iSER.  Probably apply this one before
the bidi one I just sent.

-- Pete


From: Pete Wyckoff <[EMAIL PROTECTED]>
Subject: [PATCH] iscsi iser: varlen

Handle variable-length CDBs in iSER.

Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |5 +++--
 drivers/infiniband/ulp/iser/iscsi_iser.h |2 +-
 drivers/infiniband/ulp/iser/iser_initiator.c |   16 ++--
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 5f2284d..9dfc310 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -401,7 +401,8 @@ iscsi_iser_session_create(struct iscsi_transport *iscsit,
ctask  = session->cmds[i];
iser_ctask = ctask->dd_data;
ctask->hdr = (struct iscsi_cmd *)&iser_ctask->desc.iscsi_header;
-   ctask->hdr_max = sizeof(iser_ctask->desc.iscsi_header);
+   ctask->hdr_max = sizeof(iser_ctask->desc.iscsi_header) +
+sizeof(iser_ctask->desc.hdrextbuf);
}
 
for (i = 0; i < session->mgmtpool_max; i++) {
@@ -604,7 +605,7 @@ static struct iscsi_transport iscsi_iser_transport = {
.host_template  = &iscsi_iser_sht,
.conndata_size  = sizeof(struct iscsi_conn),
.max_lun= ISCSI_ISER_MAX_LUN,
-   .max_cmd_len= ISCSI_ISER_MAX_CMD_LEN,
+   .max_cmd_len= 260,
/* session management */
.create_session = iscsi_iser_session_create,
.destroy_session= iscsi_session_teardown,
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h 
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index db8f81a..66905df 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -90,7 +90,6 @@
 /* FMR space for 1 MB of 4k-page transfers, plus 1 if not page aligned */
 #define ISCSI_ISER_SG_TABLESIZE (((1<<20) >> SHIFT_4K) + 1)
 #define ISCSI_ISER_MAX_LUN 256
-#define ISCSI_ISER_MAX_CMD_LEN 16
 
 /* QP settings */
 /* Maximal bounds on received asynchronous PDUs */
@@ -217,6 +216,7 @@ enum iser_desc_type {
 struct iser_desc {
struct iser_hdr  iser_header;
struct iscsi_hdr iscsi_header;
+   char hdrextbuf[ISCSI_MAX_AHS_SIZE];
struct iser_regd_buf hdr_regd_buf;
void *data; /* used by RX & TX_CONTROL 
*/
struct iser_regd_buf data_regd_buf; /* used by RX & TX_CONTROL 
*/
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c 
b/drivers/infiniband/ulp/iser/iser_initiator.c
index 83247f1..ea3f5dc 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -246,9 +246,13 @@ post_rx_kmalloc_failure:
return err;
 }
 
-/* creates a new tx descriptor and adds header regd buffer */
+/**
+ * iser_create_send_desc - creates a new tx descriptor and adds header regd 
buffer
+ * @iscsi_hdr_len: length of &struct iscsi_hdr and any AHSes in hdrextbuf
+ */
 static void iser_create_send_desc(struct iscsi_iser_conn *iser_conn,
- struct iser_desc   *tx_desc)
+ struct iser_desc   *tx_desc,
+ int iscsi_hdr_len)
 {
struct iser_regd_buf *regd_

Re: [PATCH] 2.6.25-rc1-git2: GDT SCSI: change drivers/scsi/gdth.c into using pci_get device

2008-02-12 Thread Boaz Harrosh
On Tue, Feb 12 2008 at 18:22 +0200, James Bottomley <[EMAIL PROTECTED]> wrote:
> On Tue, 2008-02-12 at 11:31 -0300, Sergio Luis wrote:
>> Fix compilation warning in drivers/scsi/gdth.c, using deprecated 
>> pci_find_device. 
>> Change it into using pci_get_device instead:
>> drivers/scsi/gdth.c:645: warning: 'pci_find_device' is deprecated (declared 
>> at include/linux/pci.h:495)
>>
>> Signed-off-by: Sergio Luis <[EMAIL PROTECTED]>
>>
>> gdth.c |2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>
>> diff -urN linux-2.6.25-rc1-git2.orig/drivers/scsi/gdth.c 
>> linux-2.6.25-rc1-git2/drivers/scsi/gdth.c
>> --- linux-2.6.25-rc1-git2.orig/drivers/scsi/gdth.c   2008-02-12 
>> 09:26:14.0 -0300
>> +++ linux-2.6.25-rc1-git2/drivers/scsi/gdth.c2008-02-12 
>> 10:33:08.0 -0300
>> @@ -642,7 +642,7 @@
>>*cnt, vendor, device));
>>  
>>  pdev = NULL;
>> -while ((pdev = pci_find_device(vendor, device, pdev)) 
>> +while ((pdev = pci_get_device(vendor, device, pdev)) 
>> != NULL) {
>>  if (pci_enable_device(pdev))
>>  continue;
> 
> This can't be correct without a matching put in the error leg.
> 
> The difference between the two APIs is that pci_get_device returns a
> pci_device with a reference taken and pci_find_device doesn't.  However,
> pci_get_device does drop the reference again so as long as you never
> exit the loop until it returns NULL, it is OK ... it's the exits before
> pci_get_device() returns NULL that need the put.
> 
> James
> 
Yes you are right I have just realized that. Reinspecting pci_get_device()
Sergio will you do it? or should I have a try?
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


CONFIG_SLUB and reproducable general protection faults on 2.6.2x

2008-02-12 Thread Nicholas A. Bellinger
On Tue, 2008-02-12 at 19:57 -0800, Nicholas A. Bellinger wrote:
> Greetings all,
> 
> On Tue, 2008-02-12 at 17:05 +0100, Bart Van Assche wrote:
> > On Feb 6, 2008 1:11 AM, Nicholas A. Bellinger <[EMAIL PROTECTED]> wrote:
> > > I have always observed the case with LIO SE/iSCSI target mode ...
> > 
> > Hello Nicholas,
> > 
> > Are you sure that the LIO-SE kernel module source code is ready for
> > inclusion in the mainstream Linux kernel ? As you know I tried to test
> > the LIO-SE iSCSI target. Already while configuring the target I
> > encountered a kernel crash that froze the whole system. I can
> > reproduce this kernel crash easily, and I reported it 11 days ago on
> > the LIO-SE mailing list (February 4, 2008). One of the call stacks I
> > posted shows a crash in mempool_alloc() called from jbd. Or: the crash
> > is most likely the result of memory corruption caused by LIO-SE.
> > 
> 
> So I was able to FINALLY track this down to:
> 
> -# CONFIG_SLUB_DEBUG is not set
> -# CONFIG_SLAB is not set
> -CONFIG_SLUB=y
> +CONFIG_SLAB=y
> 
> in both your and Chris Weiss's configs that was causing the
> reproduceable general protection faults.  I also disabled
> CONFIG_RELOCATABLE and crash dump because I was debugging using kdb in
> x86_64 VM on 2.6.24 with your config.  I am pretty sure you can leave
> this (crash dump) in your config for testing.
> 
> This can take a while to compile and take up alot of space, esp. with
> all of the kernel debug options enabled, which on 2.6.24, really amounts
> to alot of CPU time when building.  Also with your original config, I
> was seeing some strange undefined module objects after Stage 2 Link with
> iscsi_target_mod with modpost with the SLUB the lockups (which are not
> random btw, and are tracked back to __kmalloc())..  Also, at module load
> time with the original config, there where some warning about symbol
> objects (I believe it was SCSI related, same as the ones with modpost).
> 
> In any event, the dozen 1000 loop discovery test is now working fine (as
> well as IPoIB) with the above config change, and you should be ready to
> go for your testing.
> 
> Tomo, Vlad, Andrew and Co:
> 
> Do you have any ideas why this would be the case with LIO-Target..?  Is
> anyone else seeing something similar to this with their target mode
> (mabye its all out of tree code..?) that is having an issue..? I am
> using Debian x86_64 and Bart and Chris are using Ubuntu x86_64 and we
> both have this problem with CONFIG_SLUB on >= 2.6.22 kernel.org
> kernels. 
> 
> Also, I will recompile some of my non x86 machines with the above
> enabled and see if I can reproduce..  Here the Bart's config again:
> 
> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/30835aede1028188
> 

This is also failing on CONFIG_SLUB on 2.6.24 ppc64.  Since the rest of
the system seems to work fine, my only guess it may be related to the
fact that the module is being compiled out of tree.  I took a quick
glance at what kbuild was using for compiler and linker parameters, but
nothing looked out of the ordinary.

I will take a look with kdb and SLUB re-enabled on x86_64 and see if this
helps shed any light on the issue.  Is anyone else seeing an issue with 
CONFIG_SLUB..?

Also, I wonder who else aside from Ubuntu is using this by default in
their .config..?


--nab

-
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/3] iscsi iser: remove DMA restrictions

2008-02-12 Thread Pete Wyckoff
iscsi_iser does not have any hardware DMA restrictions.  Add a
slave_configure function to remove any DMA alignment restriction,
allowing the use of direct IO from arbitrary offsets within a page.
Also disable page bouncing; iser has no restrictions on which pages it
can address.

Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index be1b9fb..1b272a6 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -543,6 +543,13 @@ iscsi_iser_ep_disconnect(__u64 ep_handle)
iser_conn_terminate(ib_conn);
 }
 
+static int iscsi_iser_slave_configure(struct scsi_device *sdev)
+{
+   blk_queue_bounce_limit(sdev->request_queue, BLK_BOUNCE_ANY);
+   blk_queue_dma_alignment(sdev->request_queue, 0);
+   return 0;
+}
+
 static struct scsi_host_template iscsi_iser_sht = {
.module = THIS_MODULE,
.name   = "iSCSI Initiator over iSER, v." DRV_VER,
@@ -556,6 +563,7 @@ static struct scsi_host_template iscsi_iser_sht = {
.eh_device_reset_handler= iscsi_eh_device_reset,
.eh_host_reset_handler  = iscsi_eh_host_reset,
.use_clustering = DISABLE_CLUSTERING,
+   .slave_configure= iscsi_iser_slave_configure,
.proc_name  = "iscsi_iser",
.this_id= -1,
 };
-- 
1.5.3.8

-
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] enclosure: add support for enclosure services

2008-02-12 Thread Luben Tuikov
--- On Tue, 2/12/08, Kristen Carlson Accardi <[EMAIL PROTECTED]> wrote:
> Hi,
> I apologize for taking so long to review this patch.  I
> obviously agree
> wholeheartedly with Luben.  The problem I ran into while
> trying to
> design an enclosure management interface for the SATA
> devices is that
> there is all this vendor defined stuff.  For example, for
> the AHCI LED
> protocol, the only "defined" LED is
> 'activity'.  For LED2 and LED3 it
> is up to hardware vendors to define these.  For SGPIO
> there's all kinds
> of ways for hw vendors to customize.  I felt that it was
> going to be a
> maintainance nightmare to have to keep track of various
> vendors
> enclosure implementations in the ahci driver, and that
> it'd be better
> to just have user space libraries take care of that.  Plus,
> that way a
> vendor doesn't have to get a patch into the kernel to
> get their new
> spiffy wizzy bang blinky lights working (think of how long
> it takes
> something to even get into a vendor kernel, which is what
> these guys
> care about...).  So I'm still not sold on having an
> enclosure
> abstraction in the kernel - at least for the SATA
> controllers.

And I agree wholeheartedly with Kristen.

   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


[BUGFIXES 0/2] gdth: fix 2.6.24 driver breakage

2008-02-12 Thread Boaz Harrosh
On Thu, Jan 31 2008 at 12:08 +0200, Boaz Harrosh <[EMAIL PROTECTED]> wrote:
> On Wed, Jan 30 2008 at 21:47 +0200, Sven Köhler <[EMAIL PROTECTED]> wrote:
>> Hi,
>>
>> so i have upgraded a system to kernel 2.6.24. After that, it failed to 
>> boot with the usual message telling, that the rootfs on device /dev/sda1 
>>   cannot be mounted (a raid1 run by the controller below).
>>
>> With 2.6.23.12, everything is working fine.
>>
>> # lspci -v:
>>
>> 03:01.0 RAID bus controller: Intel Corporation RAID Controller
>>  Subsystem: Intel Corporation Unknown device 01db
>>  Flags: bus master, 66MHz, slow devsel, latency 64, IRQ 17
>>  Memory at ddffc000 (32-bit, prefetchable) [size=16K]
>>  [virtual] Expansion ROM at deef [disabled] [size=32K]
>>  Capabilities: [80] Power Management version 2
>>
>> # GDT-related dmesg output (2.6.23.12):
>>
>> GDT-HA: Storage RAID Controller Driver. Version: 3.05
>> ACPI: PCI Interrupt :03:01.0[A] -> GSI 24 (level, low) -> IRQ 17
>> GDT-HA: Found 1 PCI Storage RAID Controllers
>> Configuring GDT-PCI HA at 3/1 IRQ 17
>> GDT-HA 0: Name: SRCU42L
>> scsi0 : SRCU42L
>> scsi 0:0:0:0: Direct-Access IntelHost Drive  #00   PQ: 0 ANSI: 2
>> scsi 0:2:6:0: Processor ESG-SHV  SCA HSBP M29 1.06 PQ: 0 ANSI: 2
>> sd 0:0:0:0: [sda] 143299800 512-byte hardware sectors (73369 MB)
>> sd 0:0:0:0: [sda] Assuming Write Enabled
>> sd 0:0:0:0: [sda] Assuming drive cache: write through
>> sd 0:0:0:0: [sda] 143299800 512-byte hardware sectors (73369 MB)
>> sd 0:0:0:0: [sda] Assuming Write Enabled
>> sd 0:0:0:0: [sda] Assuming drive cache: write through
>>   sda: sda1 sda2 < sda5 >
>> sd 0:0:0:0: [sda] Attached SCSI disk
>>
>> # cat /boot/config-2.6.24 |grep GDT
>>
>> CONFIG_SCSI_GDTH=y
>>
>>
>>
>>
>> Any ideas?
>>
>> http://www.kernel.org/diff/diffview.cgi?file=%2Fpub%2Flinux%2Fkernel%2Fv2.6%2Fpatch-2.6.24.bz2
>>  
>> show huge drivers/scsi/gdth* related changes.
>>
>> Can't test at the moment. System went production.
>>
>>
>> Regards,
>>Sven
>>
> 
> Hi Sven!


With the kind help of:
Joerg Dorchain: <[EMAIL PROTECTED]>
Jon Chelton <[EMAIL PROTECTED]>
Stefan Priebe <[EMAIL PROTECTED]>

Which let me take up their machines their effort and their time
I hope I'm able to fix the gdth driver for the 2.6.24 kernel and forward.
Actually it was a simple miss by Christoph, but with my inexperience
it took me a bisection and a while to get it.

Both Joerg, and Stefan where able to boot with these patches and work
on their machine. Jon Chelton's disks array should also work, we're testing.

Submitted 2 patches. They should also be included after some testing
into the 2.6.24.x stable releases. (Will be posted after some more testing)

[PATCH 1/2] gdth: scan for scsi devices
 simple but must fatal.

[PATCH 2/2] gdth: bugfix for the Timer at exit crash
  James please inspect and comment on this patch. It was not yet tested
  by the original bug submitter.

In the original gdth series Christoph has forgotten to add the call to
scsi_scan_host(). Jeff alternative patches did do the scan. After everything
was probed, the code would loop on all found cards and scan. However I like
to individually scan at each probe, because I think this way it is more ready
for the hot-plug API where the discovery is done outside of the driver, and the
probe is called on single host at a time. Is that right? please comment.

Test away.

Meany thanks to Joerg, Jon && Stefan, Cheers.

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] 2.6.25-rc1-git2: GDT SCSI: change drivers/scsi/gdth.c into using pci_get device

2008-02-12 Thread James Bottomley
On Tue, 2008-02-12 at 11:31 -0300, Sergio Luis wrote:
> Fix compilation warning in drivers/scsi/gdth.c, using deprecated 
> pci_find_device. 
> Change it into using pci_get_device instead:
> drivers/scsi/gdth.c:645: warning: 'pci_find_device' is deprecated (declared 
> at include/linux/pci.h:495)
> 
> Signed-off-by: Sergio Luis <[EMAIL PROTECTED]>
> 
> gdth.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff -urN linux-2.6.25-rc1-git2.orig/drivers/scsi/gdth.c 
> linux-2.6.25-rc1-git2/drivers/scsi/gdth.c
> --- linux-2.6.25-rc1-git2.orig/drivers/scsi/gdth.c2008-02-12 
> 09:26:14.0 -0300
> +++ linux-2.6.25-rc1-git2/drivers/scsi/gdth.c 2008-02-12 10:33:08.0 
> -0300
> @@ -642,7 +642,7 @@
>*cnt, vendor, device));
>  
>  pdev = NULL;
> -while ((pdev = pci_find_device(vendor, device, pdev)) 
> +while ((pdev = pci_get_device(vendor, device, pdev)) 
> != NULL) {
>  if (pci_enable_device(pdev))
>  continue;

This can't be correct without a matching put in the error leg.

The difference between the two APIs is that pci_get_device returns a
pci_device with a reference taken and pci_find_device doesn't.  However,
pci_get_device does drop the reference again so as long as you never
exit the loop until it returns NULL, it is OK ... it's the exits before
pci_get_device() returns NULL that need the put.

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: [Scst-devel] Integration of SCST in the mainstream Linux kernel

2008-02-12 Thread Bart Van Assche
On Feb 6, 2008 1:11 AM, Nicholas A. Bellinger <[EMAIL PROTECTED]> wrote:
> I have always observed the case with LIO SE/iSCSI target mode ...

Hello Nicholas,

Are you sure that the LIO-SE kernel module source code is ready for
inclusion in the mainstream Linux kernel ? As you know I tried to test
the LIO-SE iSCSI target. Already while configuring the target I
encountered a kernel crash that froze the whole system. I can
reproduce this kernel crash easily, and I reported it 11 days ago on
the LIO-SE mailing list (February 4, 2008). One of the call stacks I
posted shows a crash in mempool_alloc() called from jbd. Or: the crash
is most likely the result of memory corruption caused by LIO-SE.

Because I was curious to know why it took so long to fix such a severe
crash, I started browsing through the LIO-SE source code. Analysis of
the LIO-SE kernel module source code learned me that this crash is not
a coincidence. Dynamic memory allocation (kmalloc()/kfree()) in the
LIO-SE kernel module is complex and hard to verify. There are 412
memory allocation/deallocation calls in the current version of the
LIO-SE kernel module source code, which is a lot. Additionally,
because of the complexity of the memory handling in LIO-SE, it is not
possible to verify the correctness of the memory handling by analyzing
a single function at a time. In my opinion this makes the LIO-SE
source code hard to maintain.
Furthermore, the LIO-SE kernel module source code does not follow
conventions that have proven their value in the past like grouping all
error handling at the end of a function. As could be expected, the
consequence is that error handling is not correct in several
functions, resulting in memory leaks in case of an error. Some
examples of functions in which error handling is clearly incorrect:
* transport_allocate_passthrough().
* iscsi_do_build_list().

Bart Van Assche.
-
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 02/13] git-scsi-misc gdth fix

2008-02-12 Thread Boaz Harrosh
On Tue, Feb 05 2008 at 9:53 +0200, [EMAIL PROTECTED] wrote:
> From: James Bottomley <[EMAIL PROTECTED]>
> 
> On Sun, 2007-10-14 at 12:21 -0700, Andrew Morton wrote:
>> On Sun, 14 Oct 2007 22:45:47 +0400 "Dave Milter" <[EMAIL PROTECTED]> wrote:
>>
>>> I build linux-2.6.23-mm1 and try to boot it using qemu,
>>> and it crashed with trace like this:
>>> do_page_fault
>>> error_code
>>> lock_acquire
>>> _spin_lock_irqsave
>>> gdth_timeout
>>> run_timer_softirq
>>> __do_softirq
>>> do_softirq
>>>
>>> I have screenshot, but have no idea, is it legal to include it, if I
>>> sent copy to lkml.
>>> config of kernel in attachment,
>>> I apply all three patches from hot-fixes.
>>>
>> The screenshot is here:  http://userweb.kernel.org/~akpm/crash.png
>>
>> It would appear that gdth_timeout() is passing a bad pointer into
>> spin_lock_irqsave().
> 
> There's a bug in the gdth rework in that the instance can be deleted
> from the list before the actual timer is stopped.  This can be worked
> around I think by the following patch; although we really should be
> stopping the timer from firing when the list goes empty.
> 
> 
> James said:
> 
> This is almost certainly the wrong fix for real hardware.  Although it
> kills the timer when the list goes empty, nothing will ever restart it
> when the list fills again.
> 
> Boaz, since you touched all of this, you get to fix it.  The correct fix
> will be to control the timer along with the actual list instead of at
> entry/exit time.  If you're not going to add this empty check to the
> timer routine, make sure you use del_timer_sync() before removing the
> last element from the list.
> 
> 
> Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
> ---
> 
>  drivers/scsi/gdth.c |3 +++
>  1 file changed, 3 insertions(+)
> 
> diff -puN drivers/scsi/gdth.c~git-scsi-misc-gdth-fix drivers/scsi/gdth.c
> --- a/drivers/scsi/gdth.c~git-scsi-misc-gdth-fix
> +++ a/drivers/scsi/gdth.c
> @@ -3791,6 +3791,9 @@ static void gdth_timeout(ulong data)
>  gdth_ha_str *ha;
>  ulong flags;
>  
> +if (list_empty(&gdth_instances))
> + return;
> +
>  ha = list_first_entry(&gdth_instances, gdth_ha_str, list);
>  spin_lock_irqsave(&ha->smp_lock, flags);
>  
> _
Hello dear Andrew

Do you perhaps remember who as reported this problem, and if he can
test patches?

Boaz

---
gdth: Try to fix the Timer at exit problem

Remove_sync the timer before we delete the cards.

Testing-patches: Boaz Harrosh <[EMAIL PROTECTED]>

---
git-diff --stat -p v2.6.24
 drivers/scsi/gdth.c |   19 ---
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index b253b8c..57fa756 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -5102,6 +5105,9 @@ static int __init gdth_pci_probe_one(gdth_pci_str 
*pcistr, int ctr)
if (error)
goto out_free_coal_stat;
list_add_tail(&ha->list, &gdth_instances);
+
+   scsi_scan_host(shp);
+
return 0;
 
  out_free_coal_stat:
@@ -5137,8 +5143,6 @@ static void gdth_remove_one(gdth_ha_str *ha)
ha->sdev = NULL;
}
 
-   gdth_flush(ha);
-
if (shp->irq)
free_irq(shp->irq,ha);
 
@@ -5236,14 +5240,15 @@ static void __exit gdth_exit(void)
 {
gdth_ha_str *ha;
 
-   list_for_each_entry(ha, &gdth_instances, list)
-   gdth_remove_one(ha);
+   unregister_chrdev(major,"gdth");
+   unregister_reboot_notifier(&gdth_notifier);
 
 #ifdef GDTH_STATISTICS
-   del_timer(&gdth_timer);
+   del_timer_sync(&gdth_timer);
 #endif
-   unregister_chrdev(major,"gdth");
-   unregister_reboot_notifier(&gdth_notifier);
+
+   list_for_each_entry(ha, &gdth_instances, list)
+   gdth_remove_one(ha);
 }
 
 module_init(gdth_init);





-
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] 2.6.25-rc1-git2: GDT SCSI: change drivers/scsi/gdth.c into using pci_get device

2008-02-12 Thread Sergio Luis
Fix compilation warning in drivers/scsi/gdth.c, using deprecated 
pci_find_device. 
Change it into using pci_get_device instead:
drivers/scsi/gdth.c:645: warning: 'pci_find_device' is deprecated (declared at 
include/linux/pci.h:495)

Signed-off-by: Sergio Luis <[EMAIL PROTECTED]>

gdth.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


diff -urN linux-2.6.25-rc1-git2.orig/drivers/scsi/gdth.c 
linux-2.6.25-rc1-git2/drivers/scsi/gdth.c
--- linux-2.6.25-rc1-git2.orig/drivers/scsi/gdth.c  2008-02-12 
09:26:14.0 -0300
+++ linux-2.6.25-rc1-git2/drivers/scsi/gdth.c   2008-02-12 10:33:08.0 
-0300
@@ -642,7 +642,7 @@
   *cnt, vendor, device));
 
 pdev = NULL;
-while ((pdev = pci_find_device(vendor, device, pdev)) 
+while ((pdev = pci_get_device(vendor, device, pdev)) 
!= NULL) {
 if (pci_enable_device(pdev))
 continue;

-
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


2.6.24.2: MPT Fusion LTO-3 tape troubles

2008-02-12 Thread Gerhard Schneider

After using 2.6.11.12 for long time (with success) I tried to update to
2.6.24.2

Hardware:   LSI53C1030 B2
Overland NEO Changer
HP Ultrium 3-SCSI tape drive

Kernel messages when booting:

Feb 12 14:28:03 ilfb03 kernel: Fusion MPT base driver 3.04.06
Feb 12 14:28:03 ilfb03 kernel: Copyright (c) 1999-2007 LSI Corporation
Feb 12 14:28:03 ilfb03 kernel: Fusion MPT SPI Host driver 3.04.06
Feb 12 14:28:03 ilfb03 kernel: mptbase: ioc0: Initiating bringup
Feb 12 14:28:03 ilfb03 kernel: ioc0: LSI53C1030 B2: Capabilities={Initiator}
Feb 12 14:28:03 ilfb03 kernel: scsi10 : ioc0: LSI53C1030 B2,
FwRev=0100h, Ports=1, MaxQ=255, IRQ=10
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:3:0: Medium ChangerOVERLAND
NEO Series   0513 PQ: 0 ANSI: 2
Feb 12 14:28:03 ilfb03 kernel:  target10:0:3: Beginning Domain Validation
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: attempting task abort!
(sc=f7fb7dc0)
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:3:0: CDB: Inquiry: 12 00 00 00
3a 00
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: Issue of TaskMgmt failed!
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: task abort: FAILED
(sc=f7fb7dc0)
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: attempting target reset!
(sc=f7fb7dc0)
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:3:0: CDB: Inquiry: 12 00 00 00
3a 00
Feb 12 14:28:03 ilfb03 kernel: mptbase: ioc0: Initiating recovery
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:3:0: mptscsih: ioc0: completing
cmds: fw_channel 0, fw_id 3, sc=f7fb7dc0, mf = f7fc3400, idx=20
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: Issue of TaskMgmt failed!
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: target reset: FAILED
(sc=f7fb7dc0)
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: attempting bus reset!
(sc=f7fb7dc0)
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:3:0: CDB: Inquiry: 12 00 00 00
3a 00
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: bus reset: SUCCESS
(sc=f7fb7dc0)
Feb 12 14:28:03 ilfb03 kernel:  target10:0:3: Wide Transfers Fail
Feb 12 14:28:03 ilfb03 kernel:  target10:0:3: Domain Validation skipping
write tests
Feb 12 14:28:03 ilfb03 kernel:  target10:0:3: Ending Domain Validation
Feb 12 14:28:03 ilfb03 kernel:  target10:0:3: FAST-10 SCSI 10.0 MB/s ST
(100 ns, offset 15)
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:4:0: Sequential-Access HP
 Ultrium 3-SCSI   G24H PQ: 0 ANSI: 3
Feb 12 14:28:03 ilfb03 kernel:  target10:0:4: Beginning Domain Validation
Feb 12 14:28:03 ilfb03 kernel:  target10:0:4: Domain Validation Disabing
Information Units
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:4:0: Write Buffer failure 802
Feb 12 14:28:03 ilfb03 kernel:  target10:0:4: Domain Validation detected
failure, dropping back
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:4:0: Write Buffer failure 802
Feb 12 14:28:03 ilfb03 kernel:  target10:0:4: Domain Validation detected
failure, dropping back
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:4:0: Write Buffer failure 802
Feb 12 14:28:03 ilfb03 kernel:  target10:0:4: Domain Validation detected
failure, dropping back
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:4:0: Write Buffer failure 802
Feb 12 14:28:03 ilfb03 kernel:  target10:0:4: Domain Validation detected
failure, dropping back
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:4:0: Write Buffer failure 802
Feb 12 14:28:03 ilfb03 kernel:  target10:0:4: Domain Validation detected
failure, dropping back
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:4:0: Write Buffer failure 802
Feb 12 14:28:03 ilfb03 kernel:  target10:0:4: Domain Validation detected
failure, dropping back
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:4:0: Write Buffer failure 802
Feb 12 14:28:03 ilfb03 kernel:  target10:0:4: Domain Validation detected
failure, dropping back
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: attempting task abort!
(sc=f7fb7dc0)
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:4:0: CDB: Write Buffer: 3b 0a
00 00 00 00 00 10 00 00
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: WARNING - TM Handler for
type=1: IOC Not operational (0x400e)!
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: WARNING -  Issuing
HardReset!!
Feb 12 14:28:03 ilfb03 kernel: mptbase: ioc0: Initiating recovery
Feb 12 14:28:03 ilfb03 kernel: mptbase: ioc0: WARNING - IOC is in FAULT
state!!!
Feb 12 14:28:03 ilfb03 kernel: mptbase: ioc0: WARNING -FAULT
code = 000eh
Feb 12 14:28:03 ilfb03 kernel: mptbase: ioc0: ERROR - Doorbell ACK
timeout (count=4999), IntStatus=8001!
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:4:0: mptscsih: ioc0: completing
cmds: fw_channel 0, fw_id 4, sc=f7fb7dc0, mf = f7fc5680, idx=7c
Feb 12 14:28:03 ilfb03 kernel: mptbase: ioc0: Recovered from IOC FAULT
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: task abort: FAILED
(sc=f7fb7dc0)
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: attempting target reset!
(sc=f7fb7dc0)
Feb 12 14:28:03 ilfb03 kernel: scsi 10:0:4:0: CDB: Write Buffer: 3b 0a
00 00 00 00 00 10 00 00
Feb 12 14:28:03 ilfb03 kernel: mptbase: ioc0: Initiating recovery
Feb 12 14:28:03 ilfb03 kernel: mptscsih: ioc0: Iss

[PATCH] SCSI: fix data corruption caused by ses

2008-02-12 Thread Yinghai Lu

one system: initrd get courrupted:

RAMDISK: Compressed image found at block 0
RAMDISK: incomplete write (-28 != 2048) 134217728
crc error
VFS: Mounted root (ext2 filesystem).
Freeing unused kernel memory: 388k freed
init_special_inode: bogus i_mode (17)
Warning: unable to open an initial console.
init_special_inode: bogus i_mode (17)
init_special_inode: bogus i_mode (17)
Kernel panic - not syncing: No init found.  Try passing init= option to kernel.

bisected to
commit 9927c68864e9c39cc317b4f559309ba29e642168
Author: James Bottomley <[EMAIL PROTECTED]>
Date:   Sun Feb 3 15:48:56 2008 -0600

[SCSI] ses: add new Enclosure ULD

changes:
1. change char to unsigned char to avoid type change later.
2. preserve len for page1
3. need to move desc_ptr even the entry is not enclosure_component_device/raid.
   so keep desc_ptr on right position
4. also record page7 len, and double check if desc_ptr out of boundary before 
touch.

Signed-off-by: Yinghai Lu <[EMAIL PROTECTED]>

Index: linux-2.6/drivers/scsi/ses.c
===
--- linux-2.6.orig/drivers/scsi/ses.c
+++ linux-2.6/drivers/scsi/ses.c
@@ -33,9 +33,9 @@
 #include 
 
 struct ses_device {
-   char *page1;
-   char *page2;
-   char *page10;
+   unsigned char *page1;
+   unsigned char *page2;
+   unsigned char *page10;
short page1_len;
short page2_len;
short page10_len;
@@ -67,7 +67,7 @@ static int ses_probe(struct device *dev)
 static int ses_recv_diag(struct scsi_device *sdev, int page_code,
 void *buf, int bufflen)
 {
-   char cmd[] = {
+   unsigned char cmd[] = {
RECEIVE_DIAGNOSTIC,
1,  /* Set PCV bit */
page_code,
@@ -85,7 +85,7 @@ static int ses_send_diag(struct scsi_dev
 {
u32 result;
 
-   char cmd[] = {
+   unsigned char cmd[] = {
SEND_DIAGNOSTIC,
0x10,   /* Set PF bit */
0,
@@ -104,13 +104,13 @@ static int ses_send_diag(struct scsi_dev
 
 static int ses_set_page2_descriptor(struct enclosure_device *edev,
  struct enclosure_component *ecomp,
- char *desc)
+ unsigned char *desc)
 {
int i, j, count = 0, descriptor = ecomp->number;
struct scsi_device *sdev = to_scsi_device(edev->cdev.dev);
struct ses_device *ses_dev = edev->scratch;
-   char *type_ptr = ses_dev->page1 + 12 + ses_dev->page1[11];
-   char *desc_ptr = ses_dev->page2 + 8;
+   unsigned char *type_ptr = ses_dev->page1 + 12 + ses_dev->page1[11];
+   unsigned char *desc_ptr = ses_dev->page2 + 8;
 
/* Clear everything */
memset(desc_ptr, 0, ses_dev->page2_len - 8);
@@ -133,14 +133,14 @@ static int ses_set_page2_descriptor(stru
return ses_send_diag(sdev, 2, ses_dev->page2, ses_dev->page2_len);
 }
 
-static char *ses_get_page2_descriptor(struct enclosure_device *edev,
+static unsigned char *ses_get_page2_descriptor(struct enclosure_device *edev,
  struct enclosure_component *ecomp)
 {
int i, j, count = 0, descriptor = ecomp->number;
struct scsi_device *sdev = to_scsi_device(edev->cdev.dev);
struct ses_device *ses_dev = edev->scratch;
-   char *type_ptr = ses_dev->page1 + 12 + ses_dev->page1[11];
-   char *desc_ptr = ses_dev->page2 + 8;
+   unsigned char *type_ptr = ses_dev->page1 + 12 + ses_dev->page1[11];
+   unsigned char *desc_ptr = ses_dev->page2 + 8;
 
ses_recv_diag(sdev, 2, ses_dev->page2, ses_dev->page2_len);
 
@@ -160,17 +160,18 @@ static char *ses_get_page2_descriptor(st
 static void ses_get_fault(struct enclosure_device *edev,
  struct enclosure_component *ecomp)
 {
-   char *desc;
+   unsigned char *desc;
 
desc = ses_get_page2_descriptor(edev, ecomp);
-   ecomp->fault = (desc[3] & 0x60) >> 4;
+   if (desc)
+   ecomp->fault = (desc[3] & 0x60) >> 4;
 }
 
 static int ses_set_fault(struct enclosure_device *edev,
  struct enclosure_component *ecomp,
 enum enclosure_component_setting val)
 {
-   char desc[4] = {0 };
+   unsigned char desc[4] = {0 };
 
switch (val) {
case ENCLOSURE_SETTING_DISABLED:
@@ -190,26 +191,28 @@ static int ses_set_fault(struct enclosur
 static void ses_get_status(struct enclosure_device *edev,
   struct enclosure_component *ecomp)
 {
-   char *desc;
+   unsigned char *desc;
 
desc = ses_get_page2_descriptor(edev, ecomp);
-   ecomp->status = (desc[0] & 0x0f);
+   if (desc)
+   ecomp->status = (desc[0] & 0x0f);
 }
 
 static void ses_get_locate(struct enclosure_device *edev,
   struct enclosure_component *ecomp)
 {
-   char *de

Re: [BUGFIX 2/2] gdth: bugfix for the Timer at exit crash

2008-02-12 Thread Stefan Priebe - allied internet ag

Hello!

I've tested this patch now - and it works fine. Now rmmod, halt and 
reboot also works.


Stefan Priebe


Boaz Harrosh schrieb:

gdth _exit would first remove all cards then stop the timer
and would not sync with the timer function. This caused a crash
in gdth_timer() when module was unloaded.

del_timer_sync the timer before we delete the cards.

NOT YET TESTED

Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
 drivers/scsi/gdth.c |   15 ---
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 8eb78be..103280e 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -3793,6 +3793,8 @@ static void gdth_timeout(ulong data)
 gdth_ha_str *ha;
 ulong flags;
 
+BUG_ON(list_empty(&gdth_instances));

+
 ha = list_first_entry(&gdth_instances, gdth_ha_str, list);
 spin_lock_irqsave(&ha->smp_lock, flags);
 
@@ -5146,8 +5148,6 @@ static void gdth_remove_one(gdth_ha_str *ha)

ha->sdev = NULL;
}
 
-	gdth_flush(ha);

-
if (shp->irq)
free_irq(shp->irq,ha);
 
@@ -5245,14 +5245,15 @@ static void __exit gdth_exit(void)

 {
gdth_ha_str *ha;
 
-	list_for_each_entry(ha, &gdth_instances, list)

-   gdth_remove_one(ha);
+   unregister_chrdev(major,"gdth");
+   unregister_reboot_notifier(&gdth_notifier);
 
 #ifdef GDTH_STATISTICS

-   del_timer(&gdth_timer);
+   del_timer_sync(&gdth_timer);
 #endif
-   unregister_chrdev(major,"gdth");
-   unregister_reboot_notifier(&gdth_notifier);
+
+   list_for_each_entry(ha, &gdth_instances, list)
+   gdth_remove_one(ha);
 }
 
 module_init(gdth_init);

-
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] block layer varlen-cdb

2008-02-12 Thread Christoph Hellwig
On Sun, Feb 10, 2008 at 09:09:41PM +0200, Boaz Harrosh wrote:
> - add varlen_cdb and varlen_cdb_len to hold a large user cdb
>   if needed. They start as empty. Allocation of buffer must
>   be done by user and held until request execution is done.
> - Since there can be either a fix_length command up to 16 bytes
>   or a variable_length, larger then 16 bytes, commands but never
>   both, we hold the two types in a union to save space. The
>   presence of varlen_cdb_len and cmd_len==0 signals a varlen_cdb
>   mode.

this one I'm a bit confused by, why can't we just set the length
of the variable length command in cmd_len aswell, and if cmd_len >
the length of the cmd array it's a variable length command?

Note that this is both to keep the logic simpler and not to grow
struct request further.  Especially for the rather rare case
of a bidi command.
-
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