Re: gdth new set of patches for 2.6.24 stable

2008-02-18 Thread Boaz Harrosh
On Sun, Feb 17 2008 at 19:24 +0200, James Bottomley <[EMAIL PROTECTED]> wrote:
> On Sun, 2008-02-17 at 18:46 +0200, Boaz Harrosh wrote:
>> On Thu, Feb 14 2008 at 20:47 +0200, Boaz Harrosh <[EMAIL PROTECTED]> wrote:
>>> Submitted are a new set of patches, that fix lots of problems
>>> with the gdth driver.
>>>
>>> It fixes the following problems:
>>> - scan for drives on hosts. (Already in mainline)
>>> - truly fixes the exit/reboot problems but does call flush() before
>>>   reboot.
>>> - fix crash when accessing array with icpcon management application.
>>> - fix crash when doing $ cat /proc/sys/gdth/0.
>>>   This one still has the below WARN_ON in messages (see  below)
>>>   So there is one more thing hiding in there.
>>> - use pci_get_device
>>>   One of the testers requested if we can also put the move to 
>>> pci_get_device 
>>>   patch with removal of dependency on PCI_LEGACY, to the stable release.
>>>
>>> The patches are for and based on Linux-2.6.24. here is the list of patches:
>>>   [PATCH 1/5] gdth: update deprecated pci_find_device
>>>   [PATCH 2/5] gdth: scan for scsi devices
>>>   [PATCH 3/5] gdth: bugfix for the at-exit problems
>>>   [PATCH 4/5] gdth: fix to internal commands execution
>>>   [PATCH 5/5] gdth: remove gdth cooked up command accessors
>>>
>>> Please all test and report your findings.
>>>
>>> Thanks in advance
>>> Boaz
>>>
>>> ---
>>> 
>>>   WARNING: at arch/x86/kernel/pci-dma_32.c:66 dma_free_coherent()
>>>   Pid: 5501, comm: cat Not tainted 2.6.24 #43
>>>[] dma_free_coherent+0x93/0x95
>>>[] gdth_ioctl_free+0x4c/0x69
>>>[] gdth_proc_info+0x165f/0x182c
>>>[] update_curr+0xeb/0xf2
>>>[] task_rq_lock+0x29/0x50
>>>[] try_to_wake_up+0x42/0x342
>>>[] try_to_wake_up+0x42/0x342
>>>[] __wake_up_common+0x46/0x6d
>>>[] __wake_up+0x32/0x42
>>>[] n_tty_receive_buf+0x2e8/0xe97
>>>[] n_tty_receive_buf+0x2e8/0xe97
>>>[] update_curr+0x7b/0xf2
>>>[] enqueue_task_fair+0x27/0x30
>>>[] enqueue_task+0xa/0x14
>>>[] proc_scsi_read+0x29/0x3d
>>>[] proc_scsi_read+0x0/0x3d
>>>[] proc_file_read+0x1c6/0x279
>>>[] proc_file_read+0x0/0x279
>>>[] proc_reg_read+0x53/0x71
>>>[] proc_reg_read+0x0/0x71
>>>[] vfs_read+0x85/0x11b
>>>[] sys_read+0x41/0x6a
>>>[] sysenter_past_esp+0x5f/0x85
>>>  
>>> -
>> James hi.
>>
>> All my testers have reported back that with these 5 patches applied they can
>> now run with a 2.6.24 kernel the same way they ran before. However there is
>> that reported issue, with the dma_free_coherent WARN_ON (above). The code 
>> was 
>> like that from day one and it is a very old issue, however it is a 
>> regression 
>> because 2.6.24 introduced that new WARN_ON.
>> (infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba)
>> >From posts on lkml and even recent one in linux-scsi about the arcmsr driver
>> it looks that all a driver can do is work around it with different kernel 
>> mechanisms
>> and driver rewrites. I'm afraid I need your help here. I'm not sure I 
>> understand
>> why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and 
>> what
>> is needed to replace it. Could you please have a look in gdth_proc.c and 
>> also in
>> gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and 
>> advise
>> what can I do in it's place. Please bear in mind that we need it for 2.6.24, 
>> as
>> a bugfix.
>>
>> Apart from the above issue, please accept patches 3,4,5 above they have now
>> been tested and are reported to bring broken system back to production.
>> (Given that you approve off course). And mark them for inclusion to the
>> 2.6.24 stable releases. (Or is there some thing that I should do)
>>
>> ---
>> Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not
>> pose any harm. Some people have reported stability with temporarily disabling
>> it. For testers that want to try, here it is below. At your own risk.
> 
> Isn't this the correct fix?  pscratch is a permanent address (it's
> allocated at boot time and never changes).  All you need the smp lock
> for is mediating the scratch in use flag.
> 
> James
> 
> diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c
> index de57734..ce0228e 100644
> --- a/drivers/scsi/gdth_proc.c
> +++ b/drivers/scsi/gdth_proc.c
> @@ -694,15 +694,13 @@ static void gdth_ioctl_free(gdth_ha_str *ha, int size, 
> char *buf, ulong64 paddr)
>  {
>  ulong flags;
>  
> -spin_lock_irqsave(&ha->smp_lock, flags);
> -
>  if (buf == ha->pscratch) {
> + spin_lock_irqsave(&ha->smp_lock, flags);
>  ha->scratch_busy = FALSE;
> + spin_unlock_irqrestore(&ha->smp_lock, flags);
>  } else {
>  pci_free_consistent(ha->pdev, size, buf, paddr);
>  }
> -
> -spin_unlock_irqrestore(&ha->smp_lock, flags);
>  }
>  
>  #ifdef GDTH_IOCTL_PROC
> 
> 
> -

James
You are bung on the money. It was tested and it works. So simple, I was 
thinking it was accessed by DMA and freed at interrupt

Re: Integration of SCST in the mainstream Linux kernel

2008-02-18 Thread Erez Zilber
Bart Van Assche wrote:
> On Feb 5, 2008 6:01 PM, Erez Zilber <[EMAIL PROTECTED]> wrote:
>   
>> Using such large values for FirstBurstLength will give you poor
>> performance numbers for WRITE commands (with iSER). FirstBurstLength
>> means how much data should you send as unsolicited data (i.e. without
>> RDMA). It means that your WRITE commands were sent without RDMA.
>> 
>
> Sorry, but I'm afraid you got this wrong. When the iSER transport is
> used instead of TCP, all data is sent via RDMA, including unsolicited
> data. If you have look at the iSER implementation in the Linux kernel
> (source files under drivers/infiniband/ulp/iser), you will see that
> all data is transferred via RDMA and not via TCP/IP.
>
>   

When you execute WRITE commands with iSCSI, it works like this:

EDTL (Expected data length) - the data length of your command

FirstBurstLength - the length of data that will be sent as unsolicited
data (i.e. as immediate data with the SCSI command and as unsolicited
data-out PDUs)

If you use a high value for FirstBurstLength, all (or most) of your data
will be sent as unsolicited data-out PDUs. These PDUs don't use the RDMA
engine, so you miss the advantage of IB.

If you use a lower value for FirstBurstLength, EDTL - FirstBurstLength
bytes will be sent as solicited data-out PDUs. With iSER, solicited
data-out PDUs are RDMA operations.

I hope that I'm more clear now.

Erez
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Integration of SCST in the mainstream Linux kernel

2008-02-18 Thread Bart Van Assche
On Feb 18, 2008 10:43 AM, Erez Zilber <[EMAIL PROTECTED]> wrote:
> If you use a high value for FirstBurstLength, all (or most) of your data
> will be sent as unsolicited data-out PDUs. These PDUs don't use the RDMA
> engine, so you miss the advantage of IB.

Hello Erez,

Did you notice the e-mail Roland Dreier wrote on Februari 6, 2008 ?
This is what Roland wrote:
> I think the confusion here is caused by a slight misuse of the term
> "RDMA".  It is true that all data is always transported over an
> InfiniBand connection when iSER is used, but not all such transfers
> are one-sided RDMA operations; some data can be transferred using
> send/receive operations.

Or: data sent during the first burst is not transferred via one-sided
remote memory reads or writes but via two-sided send/receive
operations. At least on my setup, these operations are as fast as
one-sided remote memory reads or writes. As an example, I obtained the
following numbers on my setup (SDR 4x network);
ib_write_bw: 933 MB/s.
ib_read_bw: 905 MB/s.
ib_send_bw: 931 MB/s.

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

2008-02-18 Thread Jens Axboe
On Tue, Feb 12 2008, James Bottomley wrote:
> 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.

It is nasty, how about fixing that instead?

diff --git a/block/blk-map.c b/block/blk-map.c
index 955d75c..bc5ce60 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -143,6 +143,7 @@ int blk_rq_map_user(struct request_queue *q, struct request 
*rq,
return 0;
 unmap_rq:
blk_rq_unmap_user(bio);
+   rq->bio = NULL;
return ret;
 }
 EXPORT_SYMBOL(blk_rq_map_user);

-- 
Jens Axboe

-
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


Device rescan does not find all new devices

2008-02-18 Thread Markus Naeher
Hello people ! 

I have an x86 SLES 10 system that is connected to some disks on two
EnterpriseStorageSystems (ESS) via two QLogic FibreChannel SCSI Adapters. 
Each adapter has one path to each disk, so I have a real multipathing
environment. 

Now, I have the need to re-configure the available disks during the system 
is
up and running. 
Up to now (and hopefully in future also), the only kind of 
re-configuration is
to add more disks. 

To be more exact, the system is started wth only the disks of ESS 1 
connected,
and the disks on ESS 2 need to be added later on. 
The disk layouts on both ESS's are identical. 

To make the new disks known to the system, I followed these instructions: 
http://www-941.ibm.com/collaboration/wiki/display/LinuxP/SCSI+-+Hot+add,+remove,+rescan+of+SCSI+devices
And this is the script I have created, acording to the above mentioned
instructions: 

for HOST in /sys/class/scsi_host/host? 
do 
  echo "- - -" > $HOST/scan 
done 

When I run this script, all new disks except one are detected. 
All new disks (except the missing one of course) are found by both pathes. 


In the syslog, there are many messages about the disks that were found, 
but none
(not even error messages) about the disk that was not found. 
When I then rebot the system, all disks are there, including the one that 
was not
found by the rescan.
Therefore I think my configuration is OK. 

The missing disk is always the first one (LUN 000). I have tested this by 
changing
the order of the disks on ESS 2. 
I have also repeated the test scenario with only one path per disk. 
In this testcase, I have divided the ESS's on the two Adapters (Adapter 1 
accesses
the disks on ESS 1, Adapter 2 those on ESS 2). 
Sadly, with no change in the result. 

I think the first thing I would need help with is how to further analyze 
the problem. 

Thanks and Regards, 
Markus
-
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: gdth new set of patches for 2.6.24 stable

2008-02-18 Thread Andrew Morton
On Sun, 17 Feb 2008 18:46:03 +0200 Boaz Harrosh <[EMAIL PROTECTED]> wrote:

> 
> ...
>
> All my testers have reported back that with these 5 patches applied they can
> now run with a 2.6.24 kernel the same way they ran before. However there is
> that reported issue, with the dma_free_coherent WARN_ON (above). The code was 
> like that from day one and it is a very old issue, however it is a regression 
> because 2.6.24 introduced that new WARN_ON.
> (infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba)
> >From posts on lkml and even recent one in linux-scsi about the arcmsr driver
> it looks that all a driver can do is work around it with different kernel 
> mechanisms
> and driver rewrites. I'm afraid I need your help here. I'm not sure I 
> understand
> why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and 
> what
> is needed to replace it. Could you please have a look in gdth_proc.c and also 
> in
> gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and 
> advise
> what can I do in it's place. Please bear in mind that we need it for 2.6.24, 
> as
> a bugfix.
> 
> Apart from the above issue, please accept patches 3,4,5 above they have now
> been tested and are reported to bring broken system back to production.
> (Given that you approve off course). And mark them for inclusion to the
> 2.6.24 stable releases. (Or is there some thing that I should do)
> 
> ---
> Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not
> pose any harm. Some people have reported stability with temporarily disabling
> it. For testers that want to try, here it is below. At your own risk.
> 
> ---
> >From 50d3657bf6a138ee63ad1ce00052380edc75ace7 Mon Sep 17 00:00:00 2001
> From: Boaz Harrosh <[EMAIL PROTECTED]>
> Date: Sun, 17 Feb 2008 12:49:35 +0200
> Subject: [PATCH] gdth: Hack to remove WARN_ON in arch/x86/kernel/pci-dma_32.c
> 
>   gdth uses dma_free_coherent() with interrupts disabled. Which
>   is not portable, but is safe on the HW that supports gdth.
> 
> NOT Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
> ---
>  arch/x86/kernel/pci-dma_32.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/pci-dma_32.c b/arch/x86/kernel/pci-dma_32.c
> index 5133032..350dcfd 100644
> --- a/arch/x86/kernel/pci-dma_32.c
> +++ b/arch/x86/kernel/pci-dma_32.c
> @@ -63,7 +63,7 @@ void dma_free_coherent(struct device *dev, size_t size,
>   struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
>   int order = get_order(size);
>  
> - WARN_ON(irqs_disabled());   /* for portability */
> +/*   WARN_ON(irqs_disabled());*/ /* for portability */
>   if (mem && vaddr >= mem->virt_base && vaddr < (mem->virt_base + 
> (mem->size << PAGE_SHIFT))) {
>   int page = (vaddr - mem->virt_base) >> PAGE_SHIFT;
>  

Yes.   Let's reprise aa24886e379d2b641c5117e178b15ce1d5d366ba:

: commit aa24886e379d2b641c5117e178b15ce1d5d366ba
: Author: David Brownell <[EMAIL PROTECTED]>
: Date:   Fri Aug 10 13:10:27 2007 -0700
: 
: dma_free_coherent() needs irqs enabled (sigh)
: 
: On at least ARM (and I'm told MIPS too) dma_free_coherent() has a newish
: call context requirement: unlike its dma_alloc_coherent() sibling, it may
: not be called with IRQs disabled.  (This was new behavior on ARM as of 
late
: 2005, caused by ARM SMP updates.) This little surprise can be annoyingly
: driver-visible.
: 
: Since it looks like that restriction won't be removed, this patch changes
: the definition of the API to include that requirement.  Also, to help 
catch
: nonportable drivers, it updates the x86 and swiotlb versions to include 
the
: relevant warnings.  (I already observed that it trips on the
: bus_reset_tasklet of the new firewire_ohci driver.)
: 

In general, all Linux memory-freeing functions can be called from all
contexts.  (vfree is an irritating exception).  This is good, and provides
maximum usefulness to callees, as all utility functions should seek to do. 
It would be best to fix arm and mips.

But arm and mips require enabled local irqs because their
dma_free_coherent() needs to do a cross-cpu IPI call.  Presumably because
of certain unusual TLB protocols.

I'm not sure what we should do about this.  Presumably the gdth-on-arm
usage base is, umm, zero, so we could lamely add
CONFIG_DMA_FREE_COHERENT_WITH_LOCAL_IRQS_DISABLED_IS_OK and then use that
to disable gdth (and similar) on arm amd mips.  But ugh.

Russell, Ralf: is there something we can do here to relax this requirement?

I'm thinking that perhaps we can do some rcu/refcounting tricks: launch the
IPI from within dma_free_coherent(), but don't wait for it to complete. 
When all CPUs have handled the IPI then (and only then) the virtual address
becomes recyclable, or something like that?



Actually I think David might have been wrong about mips.  afaict its
dma_free_coherent() is callable under local_irq_disable

[Bug 9769] CONFIG_SCSI_WAIT_SCAN configure error

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


[EMAIL PROTECTED] changed:

   What|Removed |Added

 Status|REJECTED|REOPENED
 Resolution|INVALID |




--- Comment #4 from [EMAIL PROTECTED]  2008-02-18 05:16 ---
This is NOT an invalid bug!

In all previous kernels, if
# CONFIG_SCSI_SCAN_ASYNC is not set

then
CONFIG_SCSI_WAIT_SCAN=m
does not even appear in the .config file, and the module
scsi_wait_scan.ko is not built.
The module is NOT built, only in 2.6.24 is it getting built
regardless.

For 2.6.23.x and all earlier kernels, the module is not
built. 

Ditto when doing 'make menuconfig', in all previous kernels,
if CONFIG_SCSI_SCAN_ASYNC is turned off then the option to build
the module is not even offered. However in 2.6.24 it continues
to be offered regardless.

So, I repeat, this is a BUG!

Or, if you have made a change that requires the module to
be built regardless, can you please explain why it is needed
in the case of '.CONFIG_SCSI_SCAN_ASYNC is not set' and why
this change from all previous kernels.

Barry Kauler


-- 
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: gdth new set of patches for 2.6.24 stable

2008-02-18 Thread Boaz Harrosh
On Mon, Feb 18 2008 at 14:57 +0200, Andrew Morton <[EMAIL PROTECTED]> wrote:
> On Sun, 17 Feb 2008 18:46:03 +0200 Boaz Harrosh <[EMAIL PROTECTED]> wrote:
> 
>> ...
>>
>> All my testers have reported back that with these 5 patches applied they can
>> now run with a 2.6.24 kernel the same way they ran before. However there is
>> that reported issue, with the dma_free_coherent WARN_ON (above). The code 
>> was 
>> like that from day one and it is a very old issue, however it is a 
>> regression 
>> because 2.6.24 introduced that new WARN_ON.
>> (infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba)
>> >From posts on lkml and even recent one in linux-scsi about the arcmsr driver
>> it looks that all a driver can do is work around it with different kernel 
>> mechanisms
>> and driver rewrites. I'm afraid I need your help here. I'm not sure I 
>> understand
>> why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and 
>> what
>> is needed to replace it. Could you please have a look in gdth_proc.c and 
>> also in
>> gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and 
>> advise
>> what can I do in it's place. Please bear in mind that we need it for 2.6.24, 
>> as
>> a bugfix.
>>
>> Apart from the above issue, please accept patches 3,4,5 above they have now
>> been tested and are reported to bring broken system back to production.
>> (Given that you approve off course). And mark them for inclusion to the
>> 2.6.24 stable releases. (Or is there some thing that I should do)
>>
>> ---
>> Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not
>> pose any harm. Some people have reported stability with temporarily disabling
>> it. For testers that want to try, here it is below. At your own risk.
>>
>> ---
>> >From 50d3657bf6a138ee63ad1ce00052380edc75ace7 Mon Sep 17 00:00:00 2001
>> From: Boaz Harrosh <[EMAIL PROTECTED]>
>> Date: Sun, 17 Feb 2008 12:49:35 +0200
>> Subject: [PATCH] gdth: Hack to remove WARN_ON in arch/x86/kernel/pci-dma_32.c
>>
>>   gdth uses dma_free_coherent() with interrupts disabled. Which
>>   is not portable, but is safe on the HW that supports gdth.
>>
>> NOT Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
>> ---
>>  arch/x86/kernel/pci-dma_32.c |2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/kernel/pci-dma_32.c b/arch/x86/kernel/pci-dma_32.c
>> index 5133032..350dcfd 100644
>> --- a/arch/x86/kernel/pci-dma_32.c
>> +++ b/arch/x86/kernel/pci-dma_32.c
>> @@ -63,7 +63,7 @@ void dma_free_coherent(struct device *dev, size_t size,
>>  struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
>>  int order = get_order(size);
>>  
>> -WARN_ON(irqs_disabled());   /* for portability */
>> +/*  WARN_ON(irqs_disabled());*/ /* for portability */
>>  if (mem && vaddr >= mem->virt_base && vaddr < (mem->virt_base + 
>> (mem->size << PAGE_SHIFT))) {
>>  int page = (vaddr - mem->virt_base) >> PAGE_SHIFT;
>>  
> 
> Yes.   Let's reprise aa24886e379d2b641c5117e178b15ce1d5d366ba:
> 
> : commit aa24886e379d2b641c5117e178b15ce1d5d366ba
> : Author: David Brownell <[EMAIL PROTECTED]>
> : Date:   Fri Aug 10 13:10:27 2007 -0700
> : 
> : dma_free_coherent() needs irqs enabled (sigh)
> : 
> : On at least ARM (and I'm told MIPS too) dma_free_coherent() has a newish
> : call context requirement: unlike its dma_alloc_coherent() sibling, it 
> may
> : not be called with IRQs disabled.  (This was new behavior on ARM as of 
> late
> : 2005, caused by ARM SMP updates.) This little surprise can be annoyingly
> : driver-visible.
> : 
> : Since it looks like that restriction won't be removed, this patch 
> changes
> : the definition of the API to include that requirement.  Also, to help 
> catch
> : nonportable drivers, it updates the x86 and swiotlb versions to include 
> the
> : relevant warnings.  (I already observed that it trips on the
> : bus_reset_tasklet of the new firewire_ohci driver.)
> : 
> 
> In general, all Linux memory-freeing functions can be called from all
> contexts.  (vfree is an irritating exception).  This is good, and provides
> maximum usefulness to callees, as all utility functions should seek to do. 
> It would be best to fix arm and mips.
> 
> But arm and mips require enabled local irqs because their
> dma_free_coherent() needs to do a cross-cpu IPI call.  Presumably because
> of certain unusual TLB protocols.
> 
> I'm not sure what we should do about this.  Presumably the gdth-on-arm
> usage base is, umm, zero, so we could lamely add
> CONFIG_DMA_FREE_COHERENT_WITH_LOCAL_IRQS_DISABLED_IS_OK and then use that
> to disable gdth (and similar) on arm amd mips.  But ugh.
> 
> Russell, Ralf: is there something we can do here to relax this requirement?
> 
> I'm thinking that perhaps we can do some rcu/refcounting tricks: launch the
> IPI from within dma_free_coherent(), but don't wait for it to complete. 
> When all CPUs have

Re: ips.c broken since 2.6.23 on x86_64?

2008-02-18 Thread FUJITA Tomonori
On Sun, 17 Feb 2008 15:37:02 -0800
Tim Pepper <[EMAIL PROTECTED]> wrote:

> On Mon 19 Feb at 07:31:56 +0900 [EMAIL PROTECTED] said:
> > 
> > Can you apply the 0001 and 0002 against 2.6.24 and see how it works?
> > If it works well, then please apply the 0001, 0002 and 0003.
> 
> Fujita-san,
> 
> I've started through the patches in order, cumulatively and after applying
> 0005 things break.  I wont be able to test anything else until tomorrow
> when I can phycisally reset the machine...

Great, thanks a lot!

Can you apply this patch after the 0005 patch and see how it works? If
it works, then please continue to test 0006, 0007 ...


diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index 05bb6ea..39cdd68 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -6906,7 +6906,7 @@ ips_register_scsi(int index)
sh->max_channel = ha->nbus - 1;
sh->can_queue = ha->max_cmds - 1;
 
-   scsi_add_host(sh, NULL);
+   scsi_add_host(sh, &ha->pcidev->dev);
scsi_scan_host(sh);
 
return 0;
-- 
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: [PATCH] ps3rom: disable clustering

2008-02-18 Thread Geert Uytterhoeven
On Sun, 17 Feb 2008, FUJITA Tomonori wrote:
> ps3rom does:
> 
> scsi_for_each_sg(cmd, sgpnt, scsi_sg_count(cmd), k) {
>   kaddr = kmap_atomic(sg_page(sgpnt), KM_IRQ0);
> 
> We cannot do something like that with the clustering enabled (or we
> can use scsi_kmap_atomic_sg).
> 
> Signed-off-by: FUJITA Tomonori <[EMAIL PROTECTED]>
> Cc: Geert Uytterhoeven <[EMAIL PROTECTED]>
> Cc: James Bottomley <[EMAIL PROTECTED]>

Seems to (still) work fine..

James: as you're scripts seem to be waiting for an ack from me, and it's not
really obvious to me whether this patch is needed or not, I'll leave it up to
you.

> ---
>  drivers/scsi/ps3rom.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
> index 0cd614a..90985cd 100644
> --- a/drivers/scsi/ps3rom.c
> +++ b/drivers/scsi/ps3rom.c
> @@ -427,7 +427,7 @@ static struct scsi_host_template ps3rom_host_template = {
>   .cmd_per_lun =  1,
>   .emulated = 1,  /* only sg driver uses this */
>   .max_sectors =  PS3ROM_MAX_SECTORS,
> - .use_clustering =   ENABLE_CLUSTERING,
> + .use_clustering =   DISABLE_CLUSTERING,
>   .module =   THIS_MODULE,
>  };

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:+32 (0)2 700 8453
Fax:  +32 (0)2 700 8622
E-mail:   [EMAIL PROTECTED]
Internet: http://www.sony-europe.com/

Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

[Bug 9769] CONFIG_SCSI_WAIT_SCAN configure error

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


[EMAIL PROTECTED] changed:

   What|Removed |Added

 CC||[EMAIL PROTECTED]
 Status|REOPENED|REJECTED
 Resolution||INVALID




--- Comment #5 from [EMAIL PROTECTED]  2008-02-18 06:19 ---
Sorry, it is an invalid bug.  This has been discussed a few times on
linux-scsi.  If you have any SCSI driver built as a module, you (or the distro
you are running) may need it.  We can't tell in Kconfig -- and you might choose
to build another driver as a module later.  Or out of tree.

If you're sure you don't need it, then don't run 'make modules_install'.  It's
about 2500 bytes ... why waste so much energy being angry?


-- 
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: gdth new set of patches for 2.6.24 stable

2008-02-18 Thread Russell King
On Mon, Feb 18, 2008 at 04:57:36AM -0800, Andrew Morton wrote:
> But arm and mips require enabled local irqs because their
> dma_free_coherent() needs to do a cross-cpu IPI call.  Presumably because
> of certain unusual TLB protocols.

Consider that TLB flushing needs to call a function on another CPU.
Now look at x86's implementation to do cross calls:

/**
 * smp_call_function_mask(): Run a function on a set of other CPUs.
 * @mask: The set of cpus to run on.  Must not include the current cpu.
 * @func: The function to run. This must be fast and non-blocking.
 * @info: An arbitrary pointer to pass to the function.
 * @wait: If true, wait (atomically) until function has completed on other 
CPUs. *
 * Returns 0 on success, else a negative status code.
 *
 * If @wait is true, then returns once @func has returned; otherwise
 * it returns just before the target cpu calls @func.
 *
 * You must not call this function with disabled interrupts or from a
 * hardware interrupt handler or from a bottom half handler.
 */
static int
native_smp_call_function_mask(cpumask_t mask,
  void (*func)(void *), void *info,
  int wait)

You can not call functions on other CPUs without having IRQs enabled,
otherwise this functionality deadlocks.  That restriction is on all
smp_call_function() implementations.

Unfortunately, to flush the TLB on other CPUs on ARM, we need to do a
cross call, which means using smp_call_function(), which introduces the
same sodding restrictions that smp_call_function() has on the functions
which call it.

> Russell, Ralf: is there something we can do here to relax this requirement?

Do what x86 people have so far been unable to resolve and find some
way to allow smp_call_function() to operate with IRQs disabled without
deadlocking?

Another solution jejb suggested was to make dma_free_coherent() lazy,
but (a) I'm unconvinced that this'll work with drivers which constantly
alloc+free in IRQ context since there's generally only 2MB of VM space
for such mappings, and it probably won't take long to eat through that
limited space with such a scheme.

Also, I don't have the facility to really test out these issues...

-- 
Russell King
 Linux kernel2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bsg: bidi bio map failure fix

2008-02-18 Thread FUJITA Tomonori
On Mon, 18 Feb 2008 13:55:08 +0100
Jens Axboe <[EMAIL PROTECTED]> wrote:

> On Tue, Feb 12 2008, James Bottomley wrote:
> > 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.
> 
> It is nasty, how about fixing that instead?

Yeah, looks better for me though the blk_rq_map_user API is still a
bit hacky, as James said.

James, Pete's patch is still in scsi-fixes, so how about dropping it
and sending this patch via the block?


> diff --git a/block/blk-map.c b/block/blk-map.c
> index 955d75c..bc5ce60 100644
> --- a/block/blk-map.c
> +++ b/block/blk-map.c
> @@ -143,6 +143,7 @@ int blk_rq_map_user(struct request_queue *q, struct 
> request *rq,
>   return 0;
>  unmap_rq:
>   blk_rq_unmap_user(bio);
> + rq->bio = NULL;
>   return ret;
>  }
>  EXPORT_SYMBOL(blk_rq_map_user);
> 
> -- 
> Jens Axboe
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: aic94xx: failing on high load (another data point)

2008-02-18 Thread Keith Hopkins
On 02/15/2008 11:28 PM, James Bottomley wrote:
> On Fri, 2008-02-15 at 00:11 +0800, Keith Hopkins wrote:
>> On 01/31/2008 03:29 AM, Darrick J. Wong wrote:
>>> On Wed, Jan 30, 2008 at 06:59:34PM +0800, Keith Hopkins wrote:
>>>> V28.  My controller functions well with a single drive (low-medium load).  
>>>> Unfortunately, all attempts to get the mirrors in sync fail and usually 
>>>> hang the whole box.
>>> Adaptec posted a V30 sequencer on their website; does that fix the
>>> problems?
>>>
>>> http://www.adaptec.com/en-US/speed/scsi/linux/aic94xx-seq-30-1_tar_gz.htm
>>>
>> I lost connectivity to the drive again, and had to reboot to recover
>> the drive, so it seemed a good time to try out the V30 firmware.
>> Unfortunately, it didn't work any better.  Details are in the
>> attachment.
> 
> Well, I can offer some hope.  The errors you report:
> 
>> aic94xx: escb_tasklet_complete: REQ_TASK_ABORT, reason=0x6
>> aic94xx: escb_tasklet_complete: Can't find task (tc=6) to abort!
> 
> Are requests by the sequencer to abort a task because of a protocol
> error.  IBM did some extensive testing with seagate drives and found
> that the protocol errors were genuine and the result of drive firmware
> problems.  IBM released a version of seagate firmware (BA17) to correct
> these.  Unfortunately, your drive identifies its firmware as S513 which
> is likely OEM firmware from another vendor ... however, that vendor may
> have an update which corrects the problem.
> 
> Of course, the other issue is this:
> 
>> aic94xx: escb_tasklet_complete: Can't find task (tc=6) to abort!
> 
> This is a bug in the driver.  It's not finding the task in the
> outstanding list.  The problem seems to be that it's taking the task
> from the escb which, by definition, is always NULL.  It should be taking
> the task from the ascb it finds by looping over the pending queue.
> 
> If you're willing, could you try this patch which may correct the
> problem?  It's sort of like falling off a cliff: if you never go near
> the edge (i.e. you upgrade the drive fw) you never fall off;
> alternatively, it would be nice if you could help me put up guard rails
> just in case.
> 

Well, that made life interesting
  but didn't seem to fix anything.

The behavior is about the same as before, but with more verbose errors.  I 
failed one member of the raid and had it rebuild as a test...which hangs for a 
while and the drive falls off-line.

Please grab the dmesg output in all its gory glory from here: 
http://wiki.hopnet.net/dokuwiki/lib/exe/fetch.php?media=myit:sas:dmesg-20080218-wpatch-fail.txt.gz

The drive is a Dell OEM drive, but it's not in a Dell system.  There is at 
least one firmware (S527) upgrade for it, but the Dell loader refuses to load 
it (because it isn't in a Dell system...)
Does anyone know a generic way to load a new firmware onto a SAS drive?

--Keith
-
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-18 Thread Jens Axboe
On Mon, Feb 18 2008, FUJITA Tomonori wrote:
> On Mon, 18 Feb 2008 13:55:08 +0100
> Jens Axboe <[EMAIL PROTECTED]> wrote:
> 
> > On Tue, Feb 12 2008, James Bottomley wrote:
> > > 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.
> > 
> > It is nasty, how about fixing that instead?
> 
> Yeah, looks better for me though the blk_rq_map_user API is still a
> bit hacky, as James said.

Seems symmetric to me now, either we fail and everything is cleaned up,
or return success. What remains?

-- 
Jens Axboe

-
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: gdth new set of patches for 2.6.24 stable

2008-02-18 Thread James Bottomley
On Mon, 2008-02-18 at 14:02 +, Russell King wrote:
> Another solution jejb suggested was to make dma_free_coherent() lazy,
> but (a) I'm unconvinced that this'll work with drivers which
> constantly
> alloc+free in IRQ context since there's generally only 2MB of VM space
> for such mappings, and it probably won't take long to eat through that
> limited space with such a scheme.

I didn't say make the alloc/free lazy, I said make the mapping setup
teardown lazy.  So on alloc, you first look for existing space, if none,
you map some more.  On free you *don't* teardown the mappings (which is
what requires the IPI) but simply free to the existing space pool ready
for reuse.  You map and free in page size chunks, since the coherent
memory users that want > PAGE_SIZE usually all do boot time allocation
(and don't free until release time).

The reuse of the pages should keep even the drivers that do persistent
alloc/free of dma coherent memory happy, and it should reach a steady
state fairly quickly.

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: ips.c broken since 2.6.23 on x86_64?

2008-02-18 Thread Salyzyn, Mark
The path needs to be triggered, it is the path to handle spoofing of the 
Adapter's inquiry.

You need more printk instrumentation to determine *why* it is not reaching that 
code path. What is the result of scb->scsi_cmd. scb->bus, 
ips_is_passthru(scb->scsi_cmd)?

The sg breakup issue may need to be looked at, but keep in mind the driver is 
going down a path that was not intended.

Sincerely -- Mark Salyzyn

> -Original Message-
> From: Tim Pepper [mailto:[EMAIL PROTECTED]
> Sent: Wednesday, February 13, 2008 7:04 PM
> To: linux-scsi@vger.kernel.org
> Cc: FUJITA Tomonori; Salyzyn, Mark
> Subject: Re: ips.c broken since 2.6.23 on x86_64?
>
> (replaced missing cc's including linux-scsi)
>
> On Wed 13 Feb at 14:39:07 -0800 [EMAIL PROTECTED] said:
> >
> > - Is the command path code even reaching the controller's
> processor inquiry
> >   spoofing section?
> >
> > if (scb->scsi_cmd->cmnd[0] == INQUIRY) {
> > IPS_SCSI_INQ_DATA inquiry;
> >
> > memset(&inquiry, 0,
> >sizeof (IPS_SCSI_INQ_DATA));
> >
> > inquiry.DeviceType =
> > IPS_SCSI_INQ_TYPE_PROCESSOR;
>
> I just added printk's in ips_send_cmd()'s INQUIRY case just before
> the above condition test and just within the conditional block.
> Neither showed on the console.
>
> > The target_id check may be flawed? It is not using the
> > scmd/sdev accessors and could be the wrong value as a
> result. Wild goose
> > since arrays are working correctly (?)
>
> In the original case the arrays appeared to be working
> correctly although
> we were worried.  In the repro case we don't actually have any disk
> attached...Forgot to mention that in the original email.
>
> > - There appears to be a missing 'break' statement for the
> START_STOP command
> >   immediately preceding the TEST_UNIT_READY/INQUIRY cases.
> What are the side
> >   effects of that? It appears to be innocuous.
>
> No change noted with the missing break added.
>
> > - ips_scmd_buf_write is used for array capacity and other
> fiddly bits and
> >   must be functioning correctly (?), so I can NOT harm it's
> functionality
> >   except to question if the kmap_atomic returned a non-null
> value, it's
> >   return value apparently is not checked. It's failure
> could speak of
> >   problem(s) elsewhere.
>
> I've got a printk there and haven't seen any output from it.
> Haven't seen
> anything from any of the printk's I've tried actually before:
>
> In the run above to check if ips_send_cmd() hits the
> condition you were
> curious about...I did capture the following from printk's I added in
> ips_allocatescbs():
>
> pci_alloc_consistent returns ha->scbs@ 0x18446604437762007040
> pci_alloc_consistent returns ips_sg.list @ 0x18446604437762002944
> pci_alloc_consistent returns ha->scbs@ 0x18446604437698871296
> pci_alloc_consistent returns ips_sg.list @ 0x18446604437756837888
>
> I take that as ips_init_phase2() being called and presumable returning
> SUCCESS.
>
> --
> Tim Pepper  <[EMAIL PROTECTED]>
> IBM Linux Technology Center
>
-
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-18 Thread James Bottomley
On Mon, 2008-02-18 at 23:37 +0900, FUJITA Tomonori wrote:
> On Mon, 18 Feb 2008 13:55:08 +0100
> Jens Axboe <[EMAIL PROTECTED]> wrote:
> 
> > On Tue, Feb 12 2008, James Bottomley wrote:
> > > 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.
> > 
> > It is nasty, how about fixing that instead?
> 
> Yeah, looks better for me though the blk_rq_map_user API is still a
> bit hacky, as James said.
> 
> James, Pete's patch is still in scsi-fixes, so how about dropping it
> and sending this patch via the block?

Yes, sure.

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 1/3 ver2] iscsi: extended cdb support

2008-02-18 Thread Boaz Harrosh

Support for extended CDBs in iscsi.
All we need is to check if command spills over 16 bytes then allocate
an iscsi-extended-header for the leftovers.

Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
Reviewed-by: Pete Wyckoff <[EMAIL PROTECTED]>
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
 drivers/scsi/libiscsi.c|   55 
 include/scsi/iscsi_proto.h |6 +++-
 2 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 59f8445..a43b8ee 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -137,6 +137,45 @@ static int iscsi_add_hdr(struct iscsi_cmd_task *ctask, 
unsigned len)
return 0;
 }
 
+/*
+ * make an extended cdb AHS
+ */
+static int iscsi_prep_ecdb_ahs(struct iscsi_cmd_task *ctask)
+{
+   struct scsi_cmnd *cmd = ctask->sc;
+   unsigned rlen, pad_len;
+   unsigned short ahslength;
+   struct iscsi_ecdb_ahdr *ecdb_ahdr;
+   int rc;
+
+   ecdb_ahdr = iscsi_next_hdr(ctask);
+   rlen = cmd->cmd_len - ISCSI_CDB_SIZE;
+
+   BUG_ON(rlen > sizeof(ecdb_ahdr->ecdb));
+   ahslength = rlen + sizeof(ecdb_ahdr->reserved);
+
+   pad_len = iscsi_padding(rlen);
+
+   rc = iscsi_add_hdr(ctask, sizeof(ecdb_ahdr->ahslength) +
+  sizeof(ecdb_ahdr->ahstype) + ahslength + pad_len);
+   if (rc)
+   return rc;
+
+   if (pad_len)
+   memset(&ecdb_ahdr->ecdb[rlen], 0, pad_len);
+
+   ecdb_ahdr->ahslength = cpu_to_be16(ahslength);
+   ecdb_ahdr->ahstype = ISCSI_AHSTYPE_CDB;
+   ecdb_ahdr->reserved = 0;
+   memcpy(ecdb_ahdr->ecdb, cmd->cmnd + ISCSI_CDB_SIZE, rlen);
+
+   debug_scsi("iscsi_prep_ecdb_ahs: varlen_cdb_len %d "
+  "rlen %d pad_len %d ahs_length %d iscsi_headers_size %u\n",
+  cmd->cmd_len, rlen, pad_len, ahslength, ctask->hdr_len);
+
+   return 0;
+}
+
 /**
  * iscsi_prep_scsi_cmd_pdu - prep iscsi scsi cmd pdu
  * @ctask: iscsi cmd task
@@ -150,7 +189,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task 
*ctask)
struct iscsi_session *session = conn->session;
struct iscsi_cmd *hdr = ctask->hdr;
struct scsi_cmnd *sc = ctask->sc;
-   unsigned hdrlength;
+   unsigned hdrlength, cmd_len;
int rc;
 
ctask->hdr_len = 0;
@@ -165,10 +204,16 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task 
*ctask)
hdr->cmdsn = cpu_to_be32(session->cmdsn);
session->cmdsn++;
hdr->exp_statsn = cpu_to_be32(conn->exp_statsn);
-   memcpy(hdr->cdb, sc->cmnd, sc->cmd_len);
-   if (sc->cmd_len < MAX_COMMAND_SIZE)
-   memset(&hdr->cdb[sc->cmd_len], 0,
-   MAX_COMMAND_SIZE - sc->cmd_len);
+   cmd_len = sc->cmd_len;
+   if (cmd_len < ISCSI_CDB_SIZE)
+   memset(&hdr->cdb[cmd_len], 0, ISCSI_CDB_SIZE - cmd_len);
+   else if (cmd_len > ISCSI_CDB_SIZE) {
+   rc = iscsi_prep_ecdb_ahs(ctask);
+   if (rc)
+   return rc;
+   cmd_len = ISCSI_CDB_SIZE;
+   }
+   memcpy(hdr->cdb, sc->cmnd, cmd_len);
 
ctask->imm_count = 0;
if (sc->sc_data_direction == DMA_TO_DEVICE) {
diff --git a/include/scsi/iscsi_proto.h b/include/scsi/iscsi_proto.h
index 5ffec8a..e0593bf 100644
--- a/include/scsi/iscsi_proto.h
+++ b/include/scsi/iscsi_proto.h
@@ -112,6 +112,7 @@ struct iscsi_ahs_hdr {
 
 #define ISCSI_AHSTYPE_CDB  1
 #define ISCSI_AHSTYPE_RLENGTH  2
+#define ISCSI_CDB_SIZE 16
 
 /* iSCSI PDU Header */
 struct iscsi_cmd {
@@ -125,7 +126,7 @@ struct iscsi_cmd {
__be32 data_length;
__be32 cmdsn;
__be32 exp_statsn;
-   uint8_t cdb[16];/* SCSI Command Block */
+   uint8_t cdb[ISCSI_CDB_SIZE];/* SCSI Command Block */
/* Additional Data (Command Dependent) */
 };
 
@@ -154,7 +155,8 @@ struct iscsi_ecdb_ahdr {
__be16 ahslength;   /* CDB length - 15, including reserved byte */
uint8_t ahstype;
uint8_t reserved;
-   uint8_t ecdb[260 - 16]; /* 4-byte aligned extended CDB spillover */
+   /* 4-byte aligned extended CDB spillover */
+   uint8_t ecdb[260 - ISCSI_CDB_SIZE];
 };
 
 /* SCSI Response Header */
-- 
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


Re: [PATCH] bsg: bidi bio map failure fix

2008-02-18 Thread James Bottomley
On Mon, 2008-02-18 at 15:46 +0100, Jens Axboe wrote:
> Seems symmetric to me now, either we fail and everything is cleaned up,
> or return success. What remains?

My main symmetry complaint was the API:  The map takes a request, the
unmap takes a bio.

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

2008-02-18 Thread Boaz Harrosh
On Thu, Jan 31 2008 at 20:08 +0200, Boaz Harrosh <[EMAIL PROTECTED]> wrote:
> 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 :)
> 
> Boaz
>  
> -
Everything the same as before. But working this time. Also
Pete's comment about second patch, was correct and code is now
fixed.

I have got Mike's Signed-off-by, on these they were tested and
approved by him. So they are for scsi-misc. But ... James? is
there any chance these can go into scsi-rc-fixes for the 2.6.25
kernel? The reason they are so late was mainly because of a fallout
in the merge process and a bug that was introduced because of that,
but they were intended to go together with bidi into 2.6.25. Also
as an important client code to the bidi-api that is introduced in
2.6.25 kernel.

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

2008-02-18 Thread Boaz Harrosh
On Tue, Feb 12 2008 at 22:17 +0200, Pete Wyckoff <[EMAIL PROTECTED]> wrote:
> [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,

Same bug I had. .max_cmd_len is still char, before the varlen patch to scsi-ml.
So it must be at most 252, Until that patch is introduced and it can return to
the correct 260 or better yet SCSI_MAX_VARLEN_CDB_SIZE. That also is only 
defined in the scsi-ml varlen patch.

>   /* 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 */
> +/**
> 

[PATCH 3/3 ver2] iscsi: bidi support - iscsi_tcp

2008-02-18 Thread Boaz Harrosh

  bidi support for iscsi_tcp
  - access the right scsi_in() and/or scsi_out() side of things.
also for resid

Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
Reviewed-by: Pete Wyckoff <[EMAIL PROTECTED]>
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
 drivers/scsi/iscsi_tcp.c |   31 +--
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 8a17867..72b9b2a 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -528,6 +528,7 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct 
iscsi_cmd_task *ctask)
struct iscsi_session *session = conn->session;
struct scsi_cmnd *sc = ctask->sc;
int datasn = be32_to_cpu(rhdr->datasn);
+   unsigned total_in_length = scsi_in(sc)->length;
 
iscsi_update_cmdsn(session, (struct iscsi_nopin*)rhdr);
if (tcp_conn->in.datalen == 0)
@@ -542,10 +543,10 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct 
iscsi_cmd_task *ctask)
tcp_ctask->exp_datasn++;
 
tcp_ctask->data_offset = be32_to_cpu(rhdr->offset);
-   if (tcp_ctask->data_offset + tcp_conn->in.datalen > scsi_bufflen(sc)) {
+   if (tcp_ctask->data_offset + tcp_conn->in.datalen > total_in_length) {
debug_tcp("%s: data_offset(%d) + data_len(%d) > 
total_length_in(%d)\n",
  __FUNCTION__, tcp_ctask->data_offset,
- tcp_conn->in.datalen, scsi_bufflen(sc));
+ tcp_conn->in.datalen, total_in_length);
return ISCSI_ERR_DATA_OFFSET;
}
 
@@ -558,8 +559,8 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct 
iscsi_cmd_task *ctask)
 
if (res_count > 0 &&
(rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
-res_count <= scsi_bufflen(sc)))
-   scsi_set_resid(sc, res_count);
+res_count <= total_in_length))
+   scsi_in(sc)->resid = res_count;
else
sc->result = (DID_BAD_TARGET << 16) |
rhdr->cmd_status;
@@ -670,11 +671,11 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, struct 
iscsi_cmd_task *ctask)
r2t->data_length, session->max_burst);
 
r2t->data_offset = be32_to_cpu(rhdr->data_offset);
-   if (r2t->data_offset + r2t->data_length > scsi_bufflen(ctask->sc)) {
+   if (r2t->data_offset + r2t->data_length > scsi_out(ctask->sc)->length) {
iscsi_conn_printk(KERN_ERR, conn,
  "invalid R2T with data len %u at offset %u "
  "and total length %d\n", r2t->data_length,
- r2t->data_offset, scsi_bufflen(ctask->sc));
+ r2t->data_offset, 
scsi_out(ctask->sc)->length);
__kfifo_put(tcp_ctask->r2tpool.queue, (void*)&r2t,
sizeof(void*));
return ISCSI_ERR_DATALEN;
@@ -771,6 +772,7 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct 
iscsi_hdr *hdr)
if (tcp_conn->in.datalen) {
struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
struct hash_desc *rx_hash = NULL;
+   struct scsi_data_buffer *sdb = scsi_in(ctask->sc);
 
/*
 * Setup copy of Data-In into the Scsi_Cmnd
@@ -788,8 +790,8 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct 
iscsi_hdr *hdr)
  tcp_ctask->data_offset,
  tcp_conn->in.datalen);
return iscsi_segment_seek_sg(&tcp_conn->in.segment,
-scsi_sglist(ctask->sc),
-scsi_sg_count(ctask->sc),
+sdb->table.sgl,
+sdb->table.nents,
 tcp_ctask->data_offset,
 tcp_conn->in.datalen,
 iscsi_tcp_process_data_in,
@@ -1332,7 +1334,8 @@ iscsi_tcp_ctask_init(struct iscsi_cmd_task *ctask)
return 0;
 
/* If we have immediate data, attach a payload */
-   err = iscsi_tcp_send_data_prep(conn, scsi_sglist(sc), scsi_sg_count(sc),
+   err = iscsi_tcp_send_data_prep(conn, scsi_out(sc)->table.sgl,
+  scsi_out(sc)->table.nents,
   0, ctask->imm_count);
if (err)
return err;
@@ -1386,6 +1389,7 @@ iscsi_tcp_ctask_xmit(struct iscsi_conn *conn, struct 
iscsi_cmd_task *ctask)
 {
struct iscsi_tcp_

Re: [PATCH] bsg: bidi bio map failure fix

2008-02-18 Thread FUJITA Tomonori
On Mon, 18 Feb 2008 09:09:07 -0600
James Bottomley <[EMAIL PROTECTED]> wrote:

> On Mon, 2008-02-18 at 15:46 +0100, Jens Axboe wrote:
> > Seems symmetric to me now, either we fail and everything is cleaned up,
> > or return success. What remains?
> 
> My main symmetry complaint was the API:  The map takes a request, the
> unmap takes a bio.

Yeah, it would be nice if we avoid such code:

blk_rq_map_user(q, rq, ...)
bio = rq->bio
blk_execute_rq(q, ...
blk_rq_unmap_user(bio);

I think that none of the users of blk_rq_map_user is interested in
bio, the details of how kernel manage I/Os. At least, we can remove
bio stuff in bsg if blk_rq_map_user and blk_rq_unmap_user take
requests.
-
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 ver2] iscsi: bidi support - libiscsi

2008-02-18 Thread Boaz Harrosh

  iscsi bidi support at the generic libiscsi level
  - prepare the additional bidi_read rlength header.
  - access the right scsi_in() and/or scsi_out() side of things.
also for resid.
  - Handle BIDI underflow overflow from target

Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
Reviewed-by: Pete Wyckoff <[EMAIL PROTECTED]>
Signed-off-by: Mike Christie <[EMAIL PROTECTED]>
---
 drivers/scsi/libiscsi.c |   85 ++
 1 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index a43b8ee..9c12915 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -176,6 +176,31 @@ static int iscsi_prep_ecdb_ahs(struct iscsi_cmd_task 
*ctask)
return 0;
 }
 
+static int iscsi_prep_bidi_ahs(struct iscsi_cmd_task *ctask)
+{
+   struct scsi_cmnd *sc = ctask->sc;
+   struct iscsi_rlength_ahdr *rlen_ahdr;
+   int rc;
+
+   rlen_ahdr = iscsi_next_hdr(ctask);
+   rc = iscsi_add_hdr(ctask, sizeof(*rlen_ahdr));
+   if (rc)
+   return rc;
+
+   rlen_ahdr->ahslength =
+   cpu_to_be16(sizeof(rlen_ahdr->read_length) +
+ sizeof(rlen_ahdr->reserved));
+   rlen_ahdr->ahstype = ISCSI_AHSTYPE_RLENGTH;
+   rlen_ahdr->reserved = 0;
+   rlen_ahdr->read_length = cpu_to_be32(scsi_in(sc)->length);
+
+   debug_scsi("bidi-in rlen_ahdr->read_length(%d) "
+  "rlen_ahdr->ahslength(%d)\n",
+  be32_to_cpu(rlen_ahdr->read_length),
+  be16_to_cpu(rlen_ahdr->ahslength));
+   return 0;
+}
+
 /**
  * iscsi_prep_scsi_cmd_pdu - prep iscsi scsi cmd pdu
  * @ctask: iscsi cmd task
@@ -200,7 +225,6 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task 
*ctask)
hdr->flags = ISCSI_ATTR_SIMPLE;
int_to_scsilun(sc->device->lun, (struct scsi_lun *)hdr->lun);
hdr->itt = build_itt(ctask->itt, session->age);
-   hdr->data_length = cpu_to_be32(scsi_bufflen(sc));
hdr->cmdsn = cpu_to_be32(session->cmdsn);
session->cmdsn++;
hdr->exp_statsn = cpu_to_be32(conn->exp_statsn);
@@ -216,7 +240,15 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task 
*ctask)
memcpy(hdr->cdb, sc->cmnd, cmd_len);
 
ctask->imm_count = 0;
+   if (scsi_bidi_cmnd(sc)) {
+   hdr->flags |= ISCSI_FLAG_CMD_READ;
+   rc = iscsi_prep_bidi_ahs(ctask);
+   if (rc)
+   return rc;
+   }
if (sc->sc_data_direction == DMA_TO_DEVICE) {
+   unsigned out_len = scsi_out(sc)->length;
+   hdr->data_length = cpu_to_be32(out_len);
hdr->flags |= ISCSI_FLAG_CMD_WRITE;
/*
 * Write counters:
@@ -237,19 +269,19 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task 
*ctask)
ctask->unsol_datasn = 0;
 
if (session->imm_data_en) {
-   if (scsi_bufflen(sc) >= session->first_burst)
+   if (out_len >= session->first_burst)
ctask->imm_count = min(session->first_burst,
conn->max_xmit_dlength);
else
-   ctask->imm_count = min(scsi_bufflen(sc),
+   ctask->imm_count = min(out_len,
conn->max_xmit_dlength);
hton24(hdr->dlength, ctask->imm_count);
} else
zero_data(hdr->dlength);
 
if (!session->initial_r2t_en) {
-   ctask->unsol_count = min((session->first_burst),
-   (scsi_bufflen(sc))) - ctask->imm_count;
+   ctask->unsol_count = min(session->first_burst, out_len)
+- ctask->imm_count;
ctask->unsol_offset = ctask->imm_count;
}
 
@@ -259,6 +291,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task 
*ctask)
} else {
hdr->flags |= ISCSI_FLAG_CMD_FINAL;
zero_data(hdr->dlength);
+   hdr->data_length = cpu_to_be32(scsi_in(sc)->length);
 
if (sc->sc_data_direction == DMA_FROM_DEVICE)
hdr->flags |= ISCSI_FLAG_CMD_READ;
@@ -277,10 +310,12 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task 
*ctask)
return EIO;
 
conn->scsicmd_pdus_cnt++;
-   debug_scsi("iscsi prep [%s cid %d sc %p cdb 0x%x itt 0x%x len %d "
-   "cmdsn %d win %d]\n",
-   sc->sc_data_direction == DMA_TO_DEVICE ? "write" : "read",
-   conn->id, sc, sc->cmnd[0], ctask->itt, scsi_bufflen(sc),
+   debug_scsi("iscsi prep [%s cid %d sc %p cdb 0x%x itt 0x%x "
+   "

Re: [PATCH 0/3] iscsi bidi & varlen support

2008-02-18 Thread Pete Wyckoff
[EMAIL PROTECTED] wrote on Mon, 18 Feb 2008 17:39 +0200:
> On Tue, Feb 12 2008 at 22:17 +0200, Pete Wyckoff <[EMAIL PROTECTED]> wrote:
> > 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,
> 
> Same bug I had. .max_cmd_len is still char, before the varlen patch to 
> scsi-ml.
> So it must be at most 252, Until that patch is introduced and it can return to
> the correct 260 or better yet SCSI_MAX_VARLEN_CDB_SIZE. That also is only 
> defined in the scsi-ml varlen patch.

Ah, that is unfortunate.

> I'm afraid the varlen patches to block and scsi-ml are waiting because of
> me. There are more things I need to check, before they can get approved.
> 
> Once I do that, and varlen gets accepted, iSER and iscsi_tcp can go up 
> to 260 for the .max_cmd_len as they should.

I will sit on these iser changes until we get core varlen resolved,
then.  Or you can just sequence it all cleverly through the various
maintainers.

-- 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: aic94xx: failing on high load (another data point)

2008-02-18 Thread James Bottomley
On Mon, 2008-02-18 at 22:26 +0800, Keith Hopkins wrote:
> Well, that made life interesting
>   but didn't seem to fix anything.
> 
> The behavior is about the same as before, but with more verbose
> errors.  I failed one member of the raid and had it rebuild as a
> test...which hangs for a while and the drive falls off-line.

Actually, it now finds the task and tries to do error handling for
it ... so we've now uncovered bugs in the error handler.  It may not
look like it, but this is actually progress.  Although, I'm afraid it's
going to be a bit like peeling an onion: every time one error gets
fixed, you just get to the next layer of errors.

> Please grab the dmesg output in all its gory glory from here:
> http://wiki.hopnet.net/dokuwiki/lib/exe/fetch.php?media=myit:sas:dmesg-20080218-wpatch-fail.txt.gz
> 
> The drive is a Dell OEM drive, but it's not in a Dell system.  There
> is at least one firmware (S527) upgrade for it, but the Dell loader
> refuses to load it (because it isn't in a Dell system...)
> Does anyone know a generic way to load a new firmware onto a SAS drive?

The firmware upgrade tools are usually vendor specific, though because
the format of the firmware file is vendor specific.  Could you just put
it in a dell box to upgrade?

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: 2.6.25-rc2-mm1 (cciss build error)

2008-02-18 Thread Randy Dunlap
On Sat, 16 Feb 2008 00:25:22 -0800 Andrew Morton wrote:

> 
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.25-rc2/2.6.25-rc2-mm1/

cciss driver has a bad macro definition:

#else /* no CONFIG_CISS_SCSI_TAPE */

/* If no tape support, then these become defined out of existence */

#define cciss_scsi_setup(cntl_num)
#define cciss_unregister_scsi(ctlr)
#define cciss_register_scsi(ctlr)
#define cciss_seq_tape_report(struct seq_file *seq, int ctlr)

#endif /* CONFIG_CISS_SCSI_TAPE */

which causes this error:

In file included from 
/local/linsrc/linux-2.6.25-rc2-mm1/drivers/block/cciss.c:231:
/local/linsrc/linux-2.6.25-rc2-mm1/drivers/block/cciss_scsi.c:1498:38: error: 
macro parameters must be comma-separated
make[3]: *** [drivers/block/cciss.o] Error 1

---
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3 ver2] iscsi bidi & varlen support

2008-02-18 Thread Boaz Harrosh
On Mon, Feb 18 2008 at 19:22 +0200, James Bottomley <[EMAIL PROTECTED]> wrote:
> On Mon, 2008-02-18 at 17:08 +0200, Boaz Harrosh wrote:
>> But ... James? is
>> there any chance these can go into scsi-rc-fixes for the 2.6.25
>> kernel? The reason they are so late was mainly because of a fallout
>> in the merge process and a bug that was introduced because of that,
>> but they were intended to go together with bidi into 2.6.25. Also
>> as an important client code to the bidi-api that is introduced in
>> 2.6.25 kernel.
> 
> Well, I think you know the answer to that one under Linus' rules for non
> merge window submission.  It's not a bug fix; we haven't even put it
> into -mm for testing and it's a pretty invasive change.
> 
> James
> 
> 

It was extensively tested by all iscsi people. It has the Sign-off-by
of the iscsi maintainer. They are not new patches.

But, yes you are right. I now remember the trouble we had with Linus
last time. So it's 2.6.26 then. :-( . People that need it for 2.6.25
will just get it off the git tree.

Boaz

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3 ver2] iscsi bidi & varlen support

2008-02-18 Thread James Bottomley
On Mon, 2008-02-18 at 17:08 +0200, Boaz Harrosh wrote:
> But ... James? is
> there any chance these can go into scsi-rc-fixes for the 2.6.25
> kernel? The reason they are so late was mainly because of a fallout
> in the merge process and a bug that was introduced because of that,
> but they were intended to go together with bidi into 2.6.25. Also
> as an important client code to the bidi-api that is introduced in
> 2.6.25 kernel.

Well, I think you know the answer to that one under Linus' rules for non
merge window submission.  It's not a bug fix; we haven't even put it
into -mm for testing and it's a pretty invasive change.

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

2008-02-18 Thread Jens Axboe
On Mon, Feb 18 2008, James Bottomley wrote:
> On Mon, 2008-02-18 at 15:46 +0100, Jens Axboe wrote:
> > Seems symmetric to me now, either we fail and everything is cleaned up,
> > or return success. What remains?
> 
> My main symmetry complaint was the API:  The map takes a request, the
> unmap takes a bio.

Oh that, well that was done to make it easier for existing users. Feel
free to send a patch changing that, however it was done that way on
purpose at the time being.

-- 
Jens Axboe

-
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: gdth new set of patches for 2.6.24 stable

2008-02-18 Thread David Brownell
On Monday 18 February 2008, Andrew Morton wrote:
> Russell, Ralf: is there something we can do here to relax this requirement?
> 
> I'm thinking that perhaps we can do some rcu/refcounting tricks: launch the
> IPI from within dma_free_coherent(), but don't wait for it to complete. 
> When all CPUs have handled the IPI then (and only then) the virtual address
> becomes recyclable, or something like that?

Or the trick some drivers had to do:  just defer the actual work of
dma_free_coherent() into a tasklet.  Better have one such tasklet in
arch code than N of them in drivers.

To be clear:  I never thought that API restriction was a good idea.

(Although this discussion now seems moot for the gdth driver.)


> 
> 
> Actually I think David might have been wrong about mips.  afaict its
> dma_free_coherent() is callable under local_irq_disable(), so ARM SMP is
> the sole exception?  

All I recall at this point was getting some arch-specific patches in that
area; I thought it was MIPS, maybe it was PPC.  The arch code may have
changed since then too.

- Dave
-
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: gdth new set of patches for 2.6.24 stable

2008-02-18 Thread Ralf Baechle
On Mon, Feb 18, 2008 at 04:57:36AM -0800, Andrew Morton wrote:

> On Sun, 17 Feb 2008 18:46:03 +0200 Boaz Harrosh <[EMAIL PROTECTED]> wrote:
> 
> > ...
> >
> > All my testers have reported back that with these 5 patches applied they can
> > now run with a 2.6.24 kernel the same way they ran before. However there is
> > that reported issue, with the dma_free_coherent WARN_ON (above). The code 
> > was 
> > like that from day one and it is a very old issue, however it is a 
> > regression 
> > because 2.6.24 introduced that new WARN_ON.
> > (infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba)
> > >From posts on lkml and even recent one in linux-scsi about the arcmsr 
> > >driver
> > it looks that all a driver can do is work around it with different kernel 
> > mechanisms
> > and driver rewrites. I'm afraid I need your help here. I'm not sure I 
> > understand
> > why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and 
> > what
> > is needed to replace it. Could you please have a look in gdth_proc.c and 
> > also in
> > gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and 
> > advise
> > what can I do in it's place. Please bear in mind that we need it for 
> > 2.6.24, as
> > a bugfix.
> > 
> > Apart from the above issue, please accept patches 3,4,5 above they have now
> > been tested and are reported to bring broken system back to production.
> > (Given that you approve off course). And mark them for inclusion to the
> > 2.6.24 stable releases. (Or is there some thing that I should do)
> > 
> > ---
> > Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not
> > pose any harm. Some people have reported stability with temporarily 
> > disabling
> > it. For testers that want to try, here it is below. At your own risk.
> > 
> > ---
> > >From 50d3657bf6a138ee63ad1ce00052380edc75ace7 Mon Sep 17 00:00:00 2001
> > From: Boaz Harrosh <[EMAIL PROTECTED]>
> > Date: Sun, 17 Feb 2008 12:49:35 +0200
> > Subject: [PATCH] gdth: Hack to remove WARN_ON in 
> > arch/x86/kernel/pci-dma_32.c
> > 
> >   gdth uses dma_free_coherent() with interrupts disabled. Which
> >   is not portable, but is safe on the HW that supports gdth.
> > 
> > NOT Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
> > ---
> >  arch/x86/kernel/pci-dma_32.c |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/pci-dma_32.c b/arch/x86/kernel/pci-dma_32.c
> > index 5133032..350dcfd 100644
> > --- a/arch/x86/kernel/pci-dma_32.c
> > +++ b/arch/x86/kernel/pci-dma_32.c
> > @@ -63,7 +63,7 @@ void dma_free_coherent(struct device *dev, size_t size,
> > struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
> > int order = get_order(size);
> >  
> > -   WARN_ON(irqs_disabled());   /* for portability */
> > +/* WARN_ON(irqs_disabled());*/ /* for portability */
> > if (mem && vaddr >= mem->virt_base && vaddr < (mem->virt_base + 
> > (mem->size << PAGE_SHIFT))) {
> > int page = (vaddr - mem->virt_base) >> PAGE_SHIFT;
> >  
> 
> Yes.   Let's reprise aa24886e379d2b641c5117e178b15ce1d5d366ba:
> 
> : commit aa24886e379d2b641c5117e178b15ce1d5d366ba
> : Author: David Brownell <[EMAIL PROTECTED]>
> : Date:   Fri Aug 10 13:10:27 2007 -0700
> : 
> : dma_free_coherent() needs irqs enabled (sigh)
> : 
> : On at least ARM (and I'm told MIPS too) dma_free_coherent() has a newish
> : call context requirement: unlike its dma_alloc_coherent() sibling, it 
> may
> : not be called with IRQs disabled.  (This was new behavior on ARM as of 
> late
> : 2005, caused by ARM SMP updates.) This little surprise can be annoyingly
> : driver-visible.
> : 
> : Since it looks like that restriction won't be removed, this patch 
> changes
> : the definition of the API to include that requirement.  Also, to help 
> catch
> : nonportable drivers, it updates the x86 and swiotlb versions to include 
> the
> : relevant warnings.  (I already observed that it trips on the
> : bus_reset_tasklet of the new firewire_ohci driver.)
> : 
> 
> In general, all Linux memory-freeing functions can be called from all
> contexts.  (vfree is an irritating exception).  This is good, and provides
> maximum usefulness to callees, as all utility functions should seek to do. 
> It would be best to fix arm and mips.
> 
> But arm and mips require enabled local irqs because their
> dma_free_coherent() needs to do a cross-cpu IPI call.  Presumably because
> of certain unusual TLB protocols.
> 
> I'm not sure what we should do about this.  Presumably the gdth-on-arm
> usage base is, umm, zero, so we could lamely add
> CONFIG_DMA_FREE_COHERENT_WITH_LOCAL_IRQS_DISABLED_IS_OK and then use that
> to disable gdth (and similar) on arm amd mips.  But ugh.
> 
> Russell, Ralf: is there something we can do here to relax this requirement?

The current MIPS implementation of dma_alloc_coherent / dma_free_coherent
is a glorified wrapper around __get

Re: ips.c broken since 2.6.23 on x86_64?

2008-02-18 Thread Tim Pepper
On Mon 18 Feb at 22:32:46 +0900 [EMAIL PROTECTED] said:
> 
> diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
> index 05bb6ea..39cdd68 100644
> --- a/drivers/scsi/ips.c
> +++ b/drivers/scsi/ips.c
> @@ -6906,7 +6906,7 @@ ips_register_scsi(int index)
>   sh->max_channel = ha->nbus - 1;
>   sh->can_queue = ha->max_cmds - 1;
> 
> - scsi_add_host(sh, NULL);
> + scsi_add_host(sh, &ha->pcidev->dev);
>   scsi_scan_host(sh);
> 
>   return 0;

Fujita-san,

This applies and runs well on top of your 0005 patch!  The rest of the
patches also then apply in order and run successfully.

Just to confirm, I applied the above alone to a clean 2.6.24 and things
again build and run successfully.  For completeness I also reproduced
the problem against 2.6.23.16 and verified the above patch fixes on that
kernel version as well.

Assuming this patch is accepted for 2.6.25, please also queue it for
the 2.6.23/24 stable trees.

Thank you very much for your help in tracking this issue down!

-- 
Tim Pepper  <[EMAIL PROTECTED]>
IBM Linux Technology Center
-
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: ips.c broken since 2.6.23 on x86_64?

2008-02-18 Thread Tim Pepper
On Mon 18 Feb at 06:57:14 -0800 [EMAIL PROTECTED] said:
> The path needs to be triggered, it is the path to handle spoofing
> of the Adapter's inquiry.
> 
> You need more printk instrumentation to determine *why* it is not
> reaching that code path. What is the result of scb->scsi_cmd.
> scb->bus, ips_is_passthru(scb->scsi_cmd)?
> 
> The sg breakup issue may need to be looked at, but keep in mind
> the driver is going down a path that was not intended.

Mark,

Fujita Tomonori has noted the following:

-   scsi_add_host(sh, NULL);
+   scsi_add_host(sh, &ha->pcidev->dev);

which appears to fix things for me.

-- 
Tim Pepper  <[EMAIL PROTECTED]>
IBM Linux Technology Center
-
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: Device rescan does not find all new devices

2008-02-18 Thread Mike Anderson
Markus Naeher <[EMAIL PROTECTED]> wrote:
> The missing disk is always the first one (LUN 000). I have tested this by 
> changing
> the order of the disks on ESS 2. 
> I have also repeated the test scenario with only one path per disk. 
> In this testcase, I have divided the ESS's on the two Adapters (Adapter 1 
> accesses
> the disks on ESS 1, Adapter 2 those on ESS 2). 
> Sadly, with no change in the result. 
> 
> I think the first thing I would need help with is how to further analyze 
> the problem. 
> 

Output from dmesg with SCSI scan logging on would be helpful. You can use
./scsi_logging_level -s --scan 7 with the script (let me know if you
cannot locate the script) or you can adjust the logging by hand.

-andmike
--
Michael Anderson
[EMAIL PROTECTED]
-
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: ips.c broken since 2.6.23 on x86_64?

2008-02-18 Thread FUJITA Tomonori
On Mon, 18 Feb 2008 15:30:58 -0800
Tim Pepper <[EMAIL PROTECTED]> wrote:

> On Mon 18 Feb at 22:32:46 +0900 [EMAIL PROTECTED] said:
> > 
> > diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
> > index 05bb6ea..39cdd68 100644
> > --- a/drivers/scsi/ips.c
> > +++ b/drivers/scsi/ips.c
> > @@ -6906,7 +6906,7 @@ ips_register_scsi(int index)
> > sh->max_channel = ha->nbus - 1;
> > sh->can_queue = ha->max_cmds - 1;
> > 
> > -   scsi_add_host(sh, NULL);
> > +   scsi_add_host(sh, &ha->pcidev->dev);
> > scsi_scan_host(sh);
> > 
> > return 0;
> 
> Fujita-san,
> 
> This applies and runs well on top of your 0005 patch!  The rest of the
> patches also then apply in order and run successfully.

Great, thanks a lot!


> Just to confirm, I applied the above alone to a clean 2.6.24 and things
> again build and run successfully.  For completeness I also reproduced
> the problem against 2.6.23.16 and verified the above patch fixes on that
> kernel version as well.

Nice. There is another bug on 2.6.24 but we rarely hit this so 2.6.24
works most of the time:

http://marc.info/?l=linux-scsi&m=120303487528875&w=2


> Assuming this patch is accepted for 2.6.25, please also queue it for
> the 2.6.23/24 stable trees.

Yes, I will take care about it.

Can you please help me just once more? 2.6.25-rc2 fixed this bug in a
bit different way by chance. Please test 2.6.25-rc2 with the attached
patch to make sure that ips in 2.6.25 works well.


> Thank you very much for your help in tracking this issue down!

No problem. I should have fixed it long time ago. Really sorry about
it.


diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index bb152fb..429592a 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -1576,7 +1576,7 @@ ips_make_passthru(ips_ha_t *ha, struct scsi_cmnd *SC, 
ips_scb_t *scb, int intr)
METHOD_TRACE("ips_make_passthru", 1);
 
 scsi_for_each_sg(SC, sg, scsi_sg_count(SC), i)
-length += sg[i].length;
+length += sg->length;
 
if (length < sizeof (ips_passthru_t)) {
/* wrong size */
@@ -3510,15 +3510,16 @@ ips_scmd_buf_write(struct scsi_cmnd *scmd, void *data, 
unsigned int count)
 struct scatterlist *sg = scsi_sglist(scmd);
 
 for (i = 0, xfer_cnt = 0;
- (i < scsi_sg_count(scmd)) && (xfer_cnt < count); i++) {
-min_cnt = min(count - xfer_cnt, sg[i].length);
+ (i < scsi_sg_count(scmd)) && (xfer_cnt < count);
+ i++, sg = sg_next(sg)) {
+min_cnt = min(count - xfer_cnt, sg->length);
 
 /* kmap_atomic() ensures addressability of the data buffer.*/
 /* local_irq_save() protects the KM_IRQ0 address slot. */
 local_irq_save(flags);
-buffer = kmap_atomic(sg_page(&sg[i]), KM_IRQ0) + sg[i].offset;
+buffer = kmap_atomic(sg_page(sg), KM_IRQ0) + sg->offset;
 memcpy(buffer, &cdata[xfer_cnt], min_cnt);
-kunmap_atomic(buffer - sg[i].offset, KM_IRQ0);
+kunmap_atomic(buffer - sg->offset, KM_IRQ0);
 local_irq_restore(flags);
 
 xfer_cnt += min_cnt;
@@ -3543,15 +3544,16 @@ ips_scmd_buf_read(struct scsi_cmnd *scmd, void *data, 
unsigned int count)
 struct scatterlist *sg = scsi_sglist(scmd);
 
 for (i = 0, xfer_cnt = 0;
- (i < scsi_sg_count(scmd)) && (xfer_cnt < count); i++) {
-min_cnt = min(count - xfer_cnt, sg[i].length);
+ (i < scsi_sg_count(scmd)) && (xfer_cnt < count);
+ i++, sg = sg_next(sg)) {
+min_cnt = min(count - xfer_cnt, sg->length);
 
 /* kmap_atomic() ensures addressability of the data buffer.*/
 /* local_irq_save() protects the KM_IRQ0 address slot. */
 local_irq_save(flags);
-buffer = kmap_atomic(sg_page(&sg[i]), KM_IRQ0) + sg[i].offset;
+buffer = kmap_atomic(sg_page(sg), KM_IRQ0) + sg->offset;
 memcpy(&cdata[xfer_cnt], buffer, min_cnt);
-kunmap_atomic(buffer - sg[i].offset, KM_IRQ0);
+kunmap_atomic(buffer - sg->offset, KM_IRQ0);
 local_irq_restore(flags);
 
 xfer_cnt += min_cnt;
-
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: ips.c broken since 2.6.23 on x86_64?

2008-02-18 Thread Tim Pepper
On Feb 18, 2008 4:11 PM, FUJITA Tomonori <[EMAIL PROTECTED]> wrote:
> Can you please help me just once more? 2.6.25-rc2 fixed this bug in a
> bit different way by chance. Please test 2.6.25-rc2 with the attached
> patch to make sure that ips in 2.6.25 works well.

Confirmed...the patch below against 2.6.25-rc2 also works for me.

Thank you again Fujita-san!

>
> diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
> index bb152fb..429592a 100644
> --- a/drivers/scsi/ips.c
> +++ b/drivers/scsi/ips.c
> @@ -1576,7 +1576,7 @@ ips_make_passthru(ips_ha_t *ha, struct scsi_cmnd *SC, 
> ips_scb_t *scb, int intr)
> METHOD_TRACE("ips_make_passthru", 1);
>
>  scsi_for_each_sg(SC, sg, scsi_sg_count(SC), i)
> -length += sg[i].length;
> +length += sg->length;
>
> if (length < sizeof (ips_passthru_t)) {
> /* wrong size */
> @@ -3510,15 +3510,16 @@ ips_scmd_buf_write(struct scsi_cmnd *scmd, void 
> *data, unsigned int count)
>  struct scatterlist *sg = scsi_sglist(scmd);
>
>  for (i = 0, xfer_cnt = 0;
> - (i < scsi_sg_count(scmd)) && (xfer_cnt < count); i++) {
> -min_cnt = min(count - xfer_cnt, sg[i].length);
> + (i < scsi_sg_count(scmd)) && (xfer_cnt < count);
> + i++, sg = sg_next(sg)) {
> +min_cnt = min(count - xfer_cnt, sg->length);
>
>  /* kmap_atomic() ensures addressability of the data buffer.*/
>  /* local_irq_save() protects the KM_IRQ0 address slot. */
>  local_irq_save(flags);
> -buffer = kmap_atomic(sg_page(&sg[i]), KM_IRQ0) + 
> sg[i].offset;
> +buffer = kmap_atomic(sg_page(sg), KM_IRQ0) + sg->offset;
>  memcpy(buffer, &cdata[xfer_cnt], min_cnt);
> -kunmap_atomic(buffer - sg[i].offset, KM_IRQ0);
> +kunmap_atomic(buffer - sg->offset, KM_IRQ0);
>  local_irq_restore(flags);
>
>  xfer_cnt += min_cnt;
> @@ -3543,15 +3544,16 @@ ips_scmd_buf_read(struct scsi_cmnd *scmd, void *data, 
> unsigned int count)
>  struct scatterlist *sg = scsi_sglist(scmd);
>
>  for (i = 0, xfer_cnt = 0;
> - (i < scsi_sg_count(scmd)) && (xfer_cnt < count); i++) {
> -min_cnt = min(count - xfer_cnt, sg[i].length);
> + (i < scsi_sg_count(scmd)) && (xfer_cnt < count);
> + i++, sg = sg_next(sg)) {
> +min_cnt = min(count - xfer_cnt, sg->length);
>
>  /* kmap_atomic() ensures addressability of the data buffer.*/
>  /* local_irq_save() protects the KM_IRQ0 address slot. */
>  local_irq_save(flags);
> -buffer = kmap_atomic(sg_page(&sg[i]), KM_IRQ0) + 
> sg[i].offset;
> +buffer = kmap_atomic(sg_page(sg), KM_IRQ0) + sg->offset;
>  memcpy(&cdata[xfer_cnt], buffer, min_cnt);
> -kunmap_atomic(buffer - sg[i].offset, KM_IRQ0);
> +kunmap_atomic(buffer - sg->offset, KM_IRQ0);
>  local_irq_restore(flags);
>
>  xfer_cnt += min_cnt;
-
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