Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread David Miller
From: Benjamin Herrenschmidt [EMAIL PROTECTED]
Date: Mon, 19 Nov 2007 16:35:23 +1100

 I'm not sure what is the best way to fix that. Internally, I've done
 some test whacking some cacheline_aligned in the scsi_cmnd data
 structure to verify I no longer get random SLAB corruption when using my
 USB but that significantly bloats the size of the structure on archs
 such as ppc64 that don't need it and have a large cache line size.
 
 Unfortunately, I don't think there's any existing Kconfig symbol or arch
 provided #define to tell us that we are on a non-coherent arch afaik
 that could be used to make that conditional.
 
 Another option would be to kmalloc the buffer (wasn't it the case before
 btw ?) but I suppose some people will scream at the idea due to how the
 command pools are done...

You could make a dma_cacheline_aligned and use that.
It seems pretty reasonable.
-
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: [dm-devel] [PATCH 0/3] Workqueue framework and ALUA hardware handler

2007-11-19 Thread S. J. van Harmelen
Hi Hannes,

Can you tell me when these patches get in the kernel? Is that going to
be 2.6.24.x or can we expect them before that?

Sander



On Thu, 2007-11-15 at 10:13 +0100, Hannes Reinecke wrote:
 Hi all,
 
 this patchset adds some more abstraction to the multipath
 hardware handlers.
 - A generic workqueue framework is added, which allows the
   hardware handler to submit failover commands independent
   from multipath I/O.
 - The exsting RDAC hardware handler is converted to use that
   framework
 - A new SPC-3 ALUA hardware handler is added, using that
   framework
 - A controller abstraction is added to the framework, which
   allows for command throttling based on the used controller.
 - The RDAC hardware handler is modified to use the new
   controller framework.
 
 This patchset superseded my previous implementation of the
 SPC-3 ALUA handler.
 Next plans are to update the table definitions and multipath-tools
 to allow for a passing of the controller ID from userland; ALUA
 doesn't specify for any controller information, so we have to
 do this by hand based on prior knowledge of the storage array.
 
 As per normal, comments etc welcome.
 
 Cheers,
 
 Hannes

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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Matthew Wilcox
On Mon, Nov 19, 2007 at 04:35:23PM +1100, Benjamin Herrenschmidt wrote:
 The other one I'm hitting now is that the SCSI layer nowadays embeds the

'nowadays'?  It has always been so.

 sense_buffer inside the scsi_cmnd structure without any kind of
 alignment whatsoever. I've been hitting irregulary is a crash on SCSI command
 completion that seems to be related to corruption of the request
 pointer in struct scsi_cmnd and I think it might be the cause.
 I'm now trying to setup a proper repro-case.

What other drivers do is DMA to their own allocation and then memcpy to
the sense buffer.

There is a movement to allocate the sense data as its own sg list, but
I don't think that patch has even been posted yet.

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread James Bottomley

On Mon, 2007-11-19 at 05:32 -0700, Matthew Wilcox wrote:
 On Mon, Nov 19, 2007 at 04:35:23PM +1100, Benjamin Herrenschmidt wrote:
  The other one I'm hitting now is that the SCSI layer nowadays embeds the
 
 'nowadays'?  It has always been so.
 
  sense_buffer inside the scsi_cmnd structure without any kind of
  alignment whatsoever. I've been hitting irregulary is a crash on SCSI 
  command
  completion that seems to be related to corruption of the request
  pointer in struct scsi_cmnd and I think it might be the cause.
  I'm now trying to setup a proper repro-case.
 
 What other drivers do is DMA to their own allocation and then memcpy to
 the sense buffer.
 
 There is a movement to allocate the sense data as its own sg list, but
 I don't think that patch has even been posted yet.

I'd like to be rid of it inside the command for various reasons:  every
command has one of these, and they're expensive in the allocation (at 96
bytes).  There's no reason we have to allocate and free that amount of
space with every command.  In theory, the number of these is bounded at
the queue depth, in practice, there's usually only one, and this DMA
alignment issue does requires most drivers to double copy.

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


SCSI dynamic power management

2007-11-19 Thread Alan Stern
Oliver (or anybody else):

Adding dynamic (AKA runtime) power management to the SCSI core is
looking a little difficult.  (Actually, since as far as I know the SCSI
specification takes no heed of power management, perhaps this should be
called idle-device management.)

Imagine a SCSI disk has been idle long enough that we want to spin it
down.  When I/O requests do start to arrive later on, we want to delay
them in the request queue until the disk has a chance to spin back up.  
Or even to fail them, if the user has done a manual suspend.

These are conflicting requirements.  How can we send the START-STOP 
UNIT commands to spin the disk up/down through the request queue while 
delaying or failing all others?  What happens if one of the 
START-STOP UNIT commands encounters a problem and the error handler 
needs to send some commands of its own?

Any suggestions or ideas are welcome.

Alan Stern

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


Re: SCSI dynamic power management

2007-11-19 Thread Matthew Wilcox
On Mon, Nov 19, 2007 at 10:36:19AM -0500, Alan Stern wrote:
 These are conflicting requirements.  How can we send the START-STOP 
 UNIT commands to spin the disk up/down through the request queue while 
 delaying or failing all others?

You can insert commands at the head of a request queue.

 What happens if one of the 
 START-STOP UNIT commands encounters a problem and the error handler 
 needs to send some commands of its own?

It'll do a reset and recover through the normal paths?

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: SCSI dynamic power management

2007-11-19 Thread Alan Stern
On Mon, 19 Nov 2007, Matthew Wilcox wrote:

 On Mon, Nov 19, 2007 at 10:36:19AM -0500, Alan Stern wrote:
  These are conflicting requirements.  How can we send the START-STOP 
  UNIT commands to spin the disk up/down through the request queue while 
  delaying or failing all others?
 
 You can insert commands at the head of a request queue.

Sure.  But that won't do any good if the requests get held on the queue
(or failed immediately) because the disk is supposedly suspended.
Somehow those requests have to be allowed to proceed while all others 
are forced to wait (or to fail).

  What happens if one of the 
  START-STOP UNIT commands encounters a problem and the error handler 
  needs to send some commands of its own?
 
 It'll do a reset and recover through the normal paths?

Not if those normal paths can't send commands through to the device.

Alan Stern

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


Re: SCSI dynamic power management

2007-11-19 Thread Alan Stern
On Mon, 19 Nov 2007, James Smart wrote:

 Also, don't forget that you need to sequence spin-up. A jbod tower with
 multiple drives all spinning up concurrently can overload the power
 supply. Most towers avoid this at power-on by having drive jumpers
 sequence the drives. This is a hard nut to crack as it requires some
 notion of physical packaging. When I worked on this in the past, the
 only reasonable approach was to sequence based on target id.

What happens during an ordinary system resume?  Don't the drives all 
get spun-up concurrently then?  Or does the sd_start_stop_device() 
routine wait until the disk is fully spun-up before returning?

Alan Stern

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


RE: SCSI dynamic power management

2007-11-19 Thread Salyzyn, Mark
Alan Stern sez:
 Sure.  But that won't do any good if the requests get held on 
 the queue
 (or failed immediately) because the disk is supposedly suspended.
 Somehow those requests have to be allowed to proceed while all others 
 are forced to wait (or to fail).

Not a failure. Not ready is reported back in a check condition on a
media based request, a spin-up request is issued, then a subsequent loop
sits there probing every second with a Test Unit ready to wait for the
drive to spin back up. There is ample time provided in this path for the
drive to spin-up.

Sincerely -- Mark Salyzyn
-
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: Process Scheduling Issue using sg/libata

2007-11-19 Thread James Chapman
Mark Lord wrote:
 Fajun Chen wrote:

 As a matter of fact, I'm using /dev/sg*.  Due to the size of my test
 application, I have not be able to compress it into a small and
 publishable form. However, this issue can be easily reproduced on my
 ARM XScale target using sg3_util code as follows:
 1. Run printtime.c attached,  which prints message to console in a loop.
 2. Run sgm_dd (part of sg3_util package, source code attached) on the
 same system as follows:
 sgm_dd if=/dev/sg0 of=/dev/null count=10M bpt=1
 The print task can be delayed for as many as 25 seconds. Surprisingly,
 I can't reproduce the problem in an i386 test system with a more
 powerful processor.

 Some clarification to MAP_ANONYMOUS option in mmap(). After fixing a
 bug and more testing, this option seems to make no difference to cpu
 load. Sorry about previous report. Back to the drawing board now :-)
 ..
 
 Okay, I don't see anything unusual here.  The code is on a slow CPU,
 and is triggering 10MBytes of PIO over a (probably) slow bus to an ATA
 device.
 
 This *will* tie up the CPU at 100% for the duration of the I/O,
 because the I/O happens in interrupt handlers, which are outside
 of the realm of the CPU scheduler.
 
 This is a known shortcoming of Linux for real-time uses.
 
 When the I/O uses DMA transfers, it *may* still have a similar effect,
 depending upon the caching in the ATA device, and on how the DMA shares
 the memory bus with the CPU.
 
 Again, no surprise here.
 
 One way to deal with it in an embedded device, is to force the
 application that's generating the I/O to self-throttle.
 Or modify the device driver to self-throttle.

Does disk access have to be so interrupt driven? Could disk interrupt
handling be done in a softirq/kthread like the networking guys deal with
network device interrupts? This would prevent the system from
live-locking when it is being bombarded with disk IO events. It doesn't
seem right that the disk IO subsystem can cause interrupt live-lock on
relatively slow CPUs...

 You may want to find an embedded Linux consultant to help out
 with this situation if it's beyond your expertise.

Check out the rtlinux patch, which pushes all interrupt handling out to
per-cpu kernel threads (irqd). The kernel scheduler then regains control
of what runs when.

Another option is to change your ATA driver to do interrupt processing
at task level using a workqueue or similar.

-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

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


RE: SCSI dynamic power management

2007-11-19 Thread Alan Stern
On Mon, 19 Nov 2007, Salyzyn, Mark wrote:

 Alan Stern sez:
  Sure.  But that won't do any good if the requests get held on 
  the queue
  (or failed immediately) because the disk is supposedly suspended.
  Somehow those requests have to be allowed to proceed while all others 
  are forced to wait (or to fail).
 
 Not a failure. Not ready is reported back in a check condition on a
 media based request, a spin-up request is issued, then a subsequent loop
 sits there probing every second with a Test Unit ready to wait for the
 drive to spin back up. There is ample time provided in this path for the
 drive to spin-up.

You're talking about what the stack does now, but I'm talking about 
what it will do once the dynamic power management code is in place.  
Commands won't even get sent to the low-level driver, never mind coming 
back with a check condition status.

Here's how it will work: scsi_prep_state_check() will see that the
device is in a suspended state (probably a substate of SDEV_QUIESCE).  
The return value will depend on the type of suspend:

For manual suspend or system suspend, the routine simply 
returns BLKPREP_KILL.

For autosuspend, the routine will submit a workqueue request
to autoresume the device and will return BLKPREP_DEFER.

The trick is somehow to allow the START-STOP UNIT commands get past 
this filter.  Is this what the REQ_PREEMPT flag is intended for?

Alan Stern

-
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: Process Scheduling Issue using sg/libata

2007-11-19 Thread Tejun Heo
James Chapman wrote:
 Mark Lord wrote:
 One way to deal with it in an embedded device, is to force the
 application that's generating the I/O to self-throttle.
 Or modify the device driver to self-throttle.
 
 Does disk access have to be so interrupt driven? Could disk interrupt
 handling be done in a softirq/kthread like the networking guys deal with
 network device interrupts? This would prevent the system from
 live-locking when it is being bombarded with disk IO events. It doesn't
 seem right that the disk IO subsystem can cause interrupt live-lock on
 relatively slow CPUs...
 
 You may want to find an embedded Linux consultant to help out
 with this situation if it's beyond your expertise.
 
 Check out the rtlinux patch, which pushes all interrupt handling out to
 per-cpu kernel threads (irqd). The kernel scheduler then regains control
 of what runs when.
 
 Another option is to change your ATA driver to do interrupt processing
 at task level using a workqueue or similar.

SFF ATA controllers are peculiar in that...

1. it doesn't have reliable IRQ pending bit.

2. it doesn't have reliable IRQ mask bit.

3. some controllers tank the machine completely if status or data
register is accessed differently than the chip likes.

So, it's not like we're all dickheads.  We know it's good to take those
out of irq handler.  The hardware just isn't very forgiving and I bet
you'll get obscure machine lockups if the RT kernel arbitrarily pushes
ATA PIO data transfers into kernel threads.

I think doing what IDE has been doing (disabling IRQ from interrupt
controller) is the way to go.

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


RE: SCSI dynamic power management

2007-11-19 Thread James Bottomley

On Mon, 2007-11-19 at 11:46 -0500, Alan Stern wrote:
 On Mon, 19 Nov 2007, Salyzyn, Mark wrote:
 
  Alan Stern sez:
   Sure.  But that won't do any good if the requests get held on 
   the queue
   (or failed immediately) because the disk is supposedly suspended.
   Somehow those requests have to be allowed to proceed while all others 
   are forced to wait (or to fail).
  
  Not a failure. Not ready is reported back in a check condition on a
  media based request, a spin-up request is issued, then a subsequent loop
  sits there probing every second with a Test Unit ready to wait for the
  drive to spin back up. There is ample time provided in this path for the
  drive to spin-up.
 
 You're talking about what the stack does now, but I'm talking about 
 what it will do once the dynamic power management code is in place.  
 Commands won't even get sent to the low-level driver, never mind coming 
 back with a check condition status.
 
 Here's how it will work: scsi_prep_state_check() will see that the
 device is in a suspended state (probably a substate of SDEV_QUIESCE).  
 The return value will depend on the type of suspend:
 
   For manual suspend or system suspend, the routine simply 
   returns BLKPREP_KILL.
 
   For autosuspend, the routine will submit a workqueue request
   to autoresume the device and will return BLKPREP_DEFER.
 
 The trick is somehow to allow the START-STOP UNIT commands get past 
 this filter.  Is this what the REQ_PREEMPT flag is intended for?

Um, that model is pretty host centric ... we wouldn't be able to use
that for power management of shared devices like SAS/SATA devices in an
expander or FC devices.  The basic problem with power management of
external devices (whether shared or not) is that it's not just what the
host did to them, it's also what the environment or administrator did to
them.

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: Process Scheduling Issue using sg/libata

2007-11-19 Thread Alan Cox
 SFF ATA controllers are peculiar in that...
 
 1. it doesn't have reliable IRQ pending bit.
 
 2. it doesn't have reliable IRQ mask bit.
 
 3. some controllers tank the machine completely if status or data
 register is accessed differently than the chip likes.

And 4. which is a killer for a lot of RT users

An I/O cycle to a taskfile style controller generally goes at ISA type
speed down the wire to the drive and back again. The CPU is stalled for
this and there is nothing we can do about it.

 
 So, it's not like we're all dickheads.  We know it's good to take those
 out of irq handler.  The hardware just isn't very forgiving and I bet
 you'll get obscure machine lockups if the RT kernel arbitrarily pushes
 ATA PIO data transfers into kernel threads.
 
 I think doing what IDE has been doing (disabling IRQ from interrupt
 controller) is the way to go.

Agreed - at which point RT or otherwise you can push it out. If you need
to do serious (sub 1mS) ATA then also go get a non SFF controller.

Alan
-
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 16/26] hptiop: avoid buffer overflow when returning sense data

2007-11-19 Thread Greg Kroah-Hartman

2.6.22-stable review patch.  If anyone has any objections, please let us
know.

--
From: HighPoint Linux Team [EMAIL PROTECTED]

patch 0fec02c93f60fb44ba3a24a0d3e4a52521d34d3f in mainline.

avoid buffer overflow when returning sense data.

With current adapter firmware the driver is working but future firmware
updates may return sense data larger than 96 bytes, causing overflow on
scp-sense_buffer and a kernel crash.

This fix should be backported to earlier kernels.

Signed-off-by: HighPoint Linux Team [EMAIL PROTECTED]
Signed-off-by: James Bottomley [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
Signed-off-by: Greg Kroah-Hartman [EMAIL PROTECTED]

---
 drivers/scsi/hptiop.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--- a/drivers/scsi/hptiop.c
+++ b/drivers/scsi/hptiop.c
@@ -377,8 +377,9 @@ static void hptiop_host_request_callback
scp-result = SAM_STAT_CHECK_CONDITION;
memset(scp-sense_buffer,
0, sizeof(scp-sense_buffer));
-   memcpy(scp-sense_buffer,
-   req-sg_list, le32_to_cpu(req-dataxfer_length));
+   memcpy(scp-sense_buffer, req-sg_list,
+   min(sizeof(scp-sense_buffer),
+   le32_to_cpu(req-dataxfer_length)));
break;
 
default:

-- 
-
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 16/26] hptiop: avoid buffer overflow when returning sense data

2007-11-19 Thread Matthew Wilcox
On Mon, Nov 19, 2007 at 10:19:12AM -0800, Greg Kroah-Hartman wrote:
 
 2.6.22-stable review patch.  If anyone has any objections, please let us
 know.

Makes sense to backport this.

Acked-by: Matthew Wilcox [EMAIL PROTECTED]

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: SCSI dynamic power management

2007-11-19 Thread Alan Stern
On Mon, 19 Nov 2007, James Bottomley wrote:

  Here's how it will work: scsi_prep_state_check() will see that the
  device is in a suspended state (probably a substate of SDEV_QUIESCE).  
  The return value will depend on the type of suspend:
  
  For manual suspend or system suspend, the routine simply 
  returns BLKPREP_KILL.
  
  For autosuspend, the routine will submit a workqueue request
  to autoresume the device and will return BLKPREP_DEFER.
  
  The trick is somehow to allow the START-STOP UNIT commands get past 
  this filter.  Is this what the REQ_PREEMPT flag is intended for?
 
 Um, that model is pretty host centric ... we wouldn't be able to use
 that for power management of shared devices like SAS/SATA devices in an
 expander or FC devices.  The basic problem with power management of
 external devices (whether shared or not) is that it's not just what the
 host did to them, it's also what the environment or administrator did to
 them.

Well, think of this as idle-device management rather than power 
management.

It's no more host-centric than the existing code in sd_suspend().  
When I mentioned sending START-STOP UNIT commands above, that was a bit
sloppy.  What the code would actually do is call the high-level
driver's suspend method, which would then send the appropriate commands
to the device.  (Or maybe the code would simply call scsi_bus_suspend.)  
If sending START-STOP UNIT during a suspend is host-centric then
sd_suspend() is already guilty.


Regarding this thread's original question, the best idea I've come up
with so far is to store an extra flag in the scsi_device structure
indicating that a suspend or resume transition is underway.  When the
flag is set, commands with REQ_PREEMPT would be accepted.  If the
device is suspended and the flag is clear, then commands would be
delayed or killed in the prep function regardless of REQ_PREEMPT.  This 
scheme could potentially let unwanted commands go through, but I 
haven't been able to think of anything more suitable.

Alan Stern

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


Re: SCSI dynamic power management

2007-11-19 Thread Matthew Wilcox
On Mon, Nov 19, 2007 at 02:16:12PM -0500, Alan Stern wrote:
 Regarding this thread's original question, the best idea I've come up
 with so far is to store an extra flag in the scsi_device structure
 indicating that a suspend or resume transition is underway.  When the
 flag is set, commands with REQ_PREEMPT would be accepted.  If the
 device is suspended and the flag is clear, then commands would be
 delayed or killed in the prep function regardless of REQ_PREEMPT.  This 
 scheme could potentially let unwanted commands go through, but I 
 haven't been able to think of anything more suitable.

You could allow only START_STOP and TUR to go through?

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: SCSI dynamic power management

2007-11-19 Thread Alan Stern
On Mon, 19 Nov 2007, Matthew Wilcox wrote:

 On Mon, Nov 19, 2007 at 02:16:12PM -0500, Alan Stern wrote:
  Regarding this thread's original question, the best idea I've come up
  with so far is to store an extra flag in the scsi_device structure
  indicating that a suspend or resume transition is underway.  When the
  flag is set, commands with REQ_PREEMPT would be accepted.  If the
  device is suspended and the flag is clear, then commands would be
  delayed or killed in the prep function regardless of REQ_PREEMPT.  This 
  scheme could potentially let unwanted commands go through, but I 
  haven't been able to think of anything more suitable.
 
 You could allow only START_STOP and TUR to go through?

That's a pretty good idea...

There will be other problems to sort out.  For example, if someone is 
using a SCSI cdrom drive to play an audio CD, it will appear to the 
system that the drive is idle because no commands will flow.  But that 
sort of thing can be handled later.

Alan Stern

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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Benjamin Herrenschmidt

On Mon, 2007-11-19 at 00:38 -0800, David Miller wrote:
 From: Benjamin Herrenschmidt [EMAIL PROTECTED]
 Date: Mon, 19 Nov 2007 16:35:23 +1100
 
  I'm not sure what is the best way to fix that. Internally, I've done
  some test whacking some cacheline_aligned in the scsi_cmnd data
  structure to verify I no longer get random SLAB corruption when using my
  USB but that significantly bloats the size of the structure on archs
  such as ppc64 that don't need it and have a large cache line size.
  
  Unfortunately, I don't think there's any existing Kconfig symbol or arch
  provided #define to tell us that we are on a non-coherent arch afaik
  that could be used to make that conditional.
  
  Another option would be to kmalloc the buffer (wasn't it the case before
  btw ?) but I suppose some people will scream at the idea due to how the
  command pools are done...
 
 You could make a dma_cacheline_aligned and use that.
 It seems pretty reasonable.

I was thinking about that. What archs would need it ? arm, mips, what
else ?

Cheers,
Ben.

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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Benjamin Herrenschmidt

On Mon, 2007-11-19 at 05:32 -0700, Matthew Wilcox wrote:
 On Mon, Nov 19, 2007 at 04:35:23PM +1100, Benjamin Herrenschmidt wrote:
  The other one I'm hitting now is that the SCSI layer nowadays embeds the
 
 'nowadays'?  It has always been so.

Wasn't it kmalloc'ed at one point ?

  sense_buffer inside the scsi_cmnd structure without any kind of
  alignment whatsoever. I've been hitting irregulary is a crash on SCSI 
  command
  completion that seems to be related to corruption of the request
  pointer in struct scsi_cmnd and I think it might be the cause.
  I'm now trying to setup a proper repro-case.
 
 What other drivers do is DMA to their own allocation and then memcpy to
 the sense buffer.

What other drivers ? Those architectures use the same drivers as
everything else.

 There is a movement to allocate the sense data as its own sg list, but
 I don't think that patch has even been posted yet.

I've seen code creating an sglist from the scsi_cmnd-sense_buffer and
passing that to drivers. That breaks.

Ben.

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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Benjamin Herrenschmidt

On Mon, 2007-11-19 at 09:09 -0600, James Bottomley wrote:
  What other drivers do is DMA to their own allocation and then memcpy to
  the sense buffer.
  
  There is a movement to allocate the sense data as its own sg list, but
  I don't think that patch has even been posted yet.
 
 I'd like to be rid of it inside the command for various reasons:  every
 command has one of these, and they're expensive in the allocation (at 96
 bytes).  There's no reason we have to allocate and free that amount of
 space with every command.  In theory, the number of these is bounded at
 the queue depth, in practice, there's usually only one, and this DMA
 alignment issue does requires most drivers to double copy.

And most drivers don't and break. Take USB storage, I -think- (code path
aren't trivial to follow) it just gets the sglist cooked up by the code
in scsi_error.c no ? That just points to the buffer in scsi_cmnd. It
then pass that for DMA to the USB stack.

Ben.


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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Benjamin Herrenschmidt

 I'd like to be rid of it inside the command for various reasons:  every
 command has one of these, and they're expensive in the allocation (at 96
 bytes).  There's no reason we have to allocate and free that amount of
 space with every command.  In theory, the number of these is bounded at
 the queue depth, in practice, there's usually only one, and this DMA
 alignment issue does requires most drivers to double copy.

Do you have a plan for short term here ? I'd like something fixed
for .25, so I may have to introduce a __dma_aligned macro of some sort
to deal with that in the meantime...

Ben.


-
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: [Bugme-new] [Bug 9405] New: iSCSI does not implement ordering guarantees required by e.g. journaling filesystems

2007-11-19 Thread Andrew Morton
On Mon, 19 Nov 2007 05:44:01 -0800 (PST)
[EMAIL PROTECTED] wrote:

 http://bugzilla.kernel.org/show_bug.cgi?id=9405
 
Summary: iSCSI does not implement ordering guarantees required by
 e.g. journaling filesystems
Product: IO/Storage
Version: 2.5
  KernelVersion: 2.6.23.1
   Platform: All
 OS/Version: Linux
   Tree: Mainline
 Status: NEW
   Severity: high
   Priority: P1
  Component: SCSI
 AssignedTo: [EMAIL PROTECTED]
 ReportedBy: [EMAIL PROTECTED]
 
 
 Most recent kernel where this bug did not occur: (new issue)
 Distribution: any
 Hardware Environment: (does not apply)
 Software Environment: (does not apply) 
 Problem Description: The sd (SCSI disk) driver ignores block device barriers
 (REQ_HARDBARRIER). The iSCSI code in the kernel sends all iSCSI commands with
 flag ISCSI_ATTR_SIMPLE to the iSCSI target. This means that the target may
 reorder these commands. Since a.o. correct operation of journaling filesystems
 depends on being able to enforce the order of certain block write operations,
 not enforcing write ordering is a bug. This can be solved by either adding
 support for REQ_HARDBARRIER in the sd device or by replacing ISCSI_ATTR_SIMPLE
 by ISCSI_ATTR_ORDERED.
 
 Steps to reproduce: Source reading of drivers/scsi/sd.c and
 drivers/scsi/libiscsi.c.
 
 References: SCSI Architecture Model - 3, paragraph 8.6
 (http://www.t10.org/ftp/t10/drafts/sam3/sam3r14.pdf).
 

(does iscsi have a maintainer?)
-
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: [Bugme-new] [Bug 9405] New: iSCSI does not implement ordering guarantees required by e.g. journaling filesystems

2007-11-19 Thread James Bottomley

On Mon, 2007-11-19 at 12:50 -0800, Andrew Morton wrote:
 On Mon, 19 Nov 2007 05:44:01 -0800 (PST)
 [EMAIL PROTECTED] wrote:
 
  http://bugzilla.kernel.org/show_bug.cgi?id=9405
  
 Summary: iSCSI does not implement ordering guarantees required by
  e.g. journaling filesystems
 Product: IO/Storage
 Version: 2.5
   KernelVersion: 2.6.23.1
Platform: All
  OS/Version: Linux
Tree: Mainline
  Status: NEW
Severity: high
Priority: P1
   Component: SCSI
  AssignedTo: [EMAIL PROTECTED]
  ReportedBy: [EMAIL PROTECTED]
  
  
  Most recent kernel where this bug did not occur: (new issue)
  Distribution: any
  Hardware Environment: (does not apply)
  Software Environment: (does not apply) 
  Problem Description: The sd (SCSI disk) driver ignores block device barriers
  (REQ_HARDBARRIER). The iSCSI code in the kernel sends all iSCSI commands 
  with
  flag ISCSI_ATTR_SIMPLE to the iSCSI target. This means that the target may
  reorder these commands. Since a.o. correct operation of journaling 
  filesystems
  depends on being able to enforce the order of certain block write 
  operations,
  not enforcing write ordering is a bug. This can be solved by either adding
  support for REQ_HARDBARRIER in the sd device or by replacing 
  ISCSI_ATTR_SIMPLE
  by ISCSI_ATTR_ORDERED.
  
  Steps to reproduce: Source reading of drivers/scsi/sd.c and
  drivers/scsi/libiscsi.c.
  
  References: SCSI Architecture Model - 3, paragraph 8.6
  (http://www.t10.org/ftp/t10/drafts/sam3/sam3r14.pdf).
  
 
 (does iscsi have a maintainer?)

Yes, mike christie

And please close this as invalid.  FS ordering guarantees in linux
aren't done via ordered tags.

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: [Bugme-new] [Bug 9405] New: iSCSI does not implement ordering guarantees required by e.g. journaling filesystems

2007-11-19 Thread Mike Christie

Andrew Morton wrote:

On Mon, 19 Nov 2007 05:44:01 -0800 (PST)
[EMAIL PROTECTED] wrote:


http://bugzilla.kernel.org/show_bug.cgi?id=9405

   Summary: iSCSI does not implement ordering guarantees required by
e.g. journaling filesystems
   Product: IO/Storage
   Version: 2.5
 KernelVersion: 2.6.23.1
  Platform: All
OS/Version: Linux
  Tree: Mainline
Status: NEW
  Severity: high
  Priority: P1
 Component: SCSI
AssignedTo: [EMAIL PROTECTED]
ReportedBy: [EMAIL PROTECTED]


Most recent kernel where this bug did not occur: (new issue)
Distribution: any
Hardware Environment: (does not apply)
Software Environment: (does not apply) 
Problem Description: The sd (SCSI disk) driver ignores block device barriers

(REQ_HARDBARRIER). The iSCSI code in the kernel sends all iSCSI commands with
flag ISCSI_ATTR_SIMPLE to the iSCSI target. This means that the target may
reorder these commands. Since a.o. correct operation of journaling filesystems
depends on being able to enforce the order of certain block write operations,
not enforcing write ordering is a bug. This can be solved by either adding
support for REQ_HARDBARRIER in the sd device or by replacing ISCSI_ATTR_SIMPLE
by ISCSI_ATTR_ORDERED.

Steps to reproduce: Source reading of drivers/scsi/sd.c and
drivers/scsi/libiscsi.c.

References: SCSI Architecture Model - 3, paragraph 8.6
(http://www.t10.org/ftp/t10/drafts/sam3/sam3r14.pdf).



(does iscsi have a maintainer?)


Attached is a patch to add me to the maintainers file so it will be 
easier to hunt me down in the future. It was made over 2.6.24-rc2.
Add Mike Christie to MAINTAINERS file.

Signed-off-by: Mike Christie [EMAIL PROTECTED]

diff --git a/MAINTAINERS b/MAINTAINERS
index 1c7c229..82b5751 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2111,6 +2111,14 @@ L:	[EMAIL PROTECTED] (subscribers-only)
 W:	http://irda.sourceforge.net/
 S:	Maintained
 
+iSCSI
+P:	Mike Christie
+M:	[EMAIL PROTECTED]
+L:	[EMAIL PROTECTED]
+W:	www.open-iscsi.org
+T:	git kernel.org:/pub/scm/linux/kernel/mnc/linux-2.6-iscsi.git
+S:	Maintained
+
 ISAPNP
 P:	Jaroslav Kysela
 M:	[EMAIL PROTECTED]


Re: [Bugme-new] [Bug 9405] New: iSCSI does not implement ordering guarantees required by e.g. journaling filesystems

2007-11-19 Thread Matthew Wilcox
On Mon, Nov 19, 2007 at 03:15:22PM -0600, Mike Christie wrote:
 +iSCSI

It's traditional to use all-caps here, even when the normal
capitalisation is different.  so I think this should be ISCSI.
Damned if I know why we have this convention, though.

 +P:   Mike Christie
 +M:   [EMAIL PROTECTED]
 +L:   [EMAIL PROTECTED]
 +W:   www.open-iscsi.org
 +T:   git kernel.org:/pub/scm/linux/kernel/mnc/linux-2.6-iscsi.git
 +S:   Maintained


-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Bugme-new] [Bug 9405] New: iSCSI does not implement ordering guarantees required by e.g. journaling filesystems

2007-11-19 Thread Mike Christie

James Bottomley wrote:

On Mon, 2007-11-19 at 12:50 -0800, Andrew Morton wrote:

On Mon, 19 Nov 2007 05:44:01 -0800 (PST)
[EMAIL PROTECTED] wrote:


http://bugzilla.kernel.org/show_bug.cgi?id=9405

   Summary: iSCSI does not implement ordering guarantees required by
e.g. journaling filesystems
   Product: IO/Storage
   Version: 2.5
 KernelVersion: 2.6.23.1
  Platform: All
OS/Version: Linux
  Tree: Mainline
Status: NEW
  Severity: high
  Priority: P1
 Component: SCSI
AssignedTo: [EMAIL PROTECTED]
ReportedBy: [EMAIL PROTECTED]


Most recent kernel where this bug did not occur: (new issue)
Distribution: any
Hardware Environment: (does not apply)
Software Environment: (does not apply) 
Problem Description: The sd (SCSI disk) driver ignores block device barriers

(REQ_HARDBARRIER). The iSCSI code in the kernel sends all iSCSI commands with
flag ISCSI_ATTR_SIMPLE to the iSCSI target. This means that the target may
reorder these commands. Since a.o. correct operation of journaling filesystems
depends on being able to enforce the order of certain block write operations,
not enforcing write ordering is a bug. This can be solved by either adding
support for REQ_HARDBARRIER in the sd device or by replacing ISCSI_ATTR_SIMPLE
by ISCSI_ATTR_ORDERED.

Steps to reproduce: Source reading of drivers/scsi/sd.c and
drivers/scsi/libiscsi.c.

References: SCSI Architecture Model - 3, paragraph 8.6
(http://www.t10.org/ftp/t10/drafts/sam3/sam3r14.pdf).


(does iscsi have a maintainer?)


Yes, mike christie

And please close this as invalid.  FS ordering guarantees in linux
aren't done via ordered tags.



I had a related question. I was working on the attached patch for soe 
other testing (patch made against scsi-rc-fixes, but is not stable so do 
not apply), which does the scsi_populate_tag_msg conversion from MSG_* 
to ISCSI_ATTR and sets the proper iscsi bits.


If I do this patch where I call scsi_activate_tcq on a device and that 
concertsion, does this require that my driver not reorder commands? I 
was just a little worried on some of the error handling paths where we 
requeue commands to the mid layer.
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 57ce225..d256cf3 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -37,6 +37,7 @@
 #include linux/scatterlist.h
 #include net/tcp.h
 #include scsi/scsi_cmnd.h
+#include scsi/scsi_tcq.h
 #include scsi/scsi_device.h
 #include scsi/scsi_host.h
 #include scsi/scsi.h
@@ -2211,8 +2212,17 @@ static void iscsi_tcp_session_destroy(struct iscsi_cls_session *cls_session)
 
 static int iscsi_tcp_slave_configure(struct scsi_device *sdev)
 {
+	int depth = 1, tag = 0;
+
 	blk_queue_bounce_limit(sdev-request_queue, BLK_BOUNCE_ANY);
 	blk_queue_dma_alignment(sdev-request_queue, 0);
+
+	if (sdev-tagged_supported) {
+		scsi_activate_tcq(sdev, ISCSI_DEF_CMD_PER_LUN);
+		depth = ISCSI_DEF_CMD_PER_LUN;
+		tag = MSG_ORDERED_TAG;
+	}
+	scsi_adjust_queue_depth(sdev, tag, depth);
 	return 0;
 }
 
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 8b57af5..7e13a03 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -122,6 +122,27 @@ void iscsi_prep_unsolicit_data_pdu(struct iscsi_cmd_task *ctask,
 }
 EXPORT_SYMBOL_GPL(iscsi_prep_unsolicit_data_pdu);
 
+static uint32_t iscsi_command_attr(struct scsi_cmnd *cmd)
+{
+	unsigned int attr = ISCSI_ATTR_UNTAGGED;
+	char msg[2];
+
+	if (scsi_populate_tag_msg(cmd, msg) == 2) {
+		switch (msg[0]) {
+		case MSG_SIMPLE_TAG:
+			attr = ISCSI_ATTR_SIMPLE;
+			break;
+		case MSG_HEAD_TAG:
+			attr = ISCSI_ATTR_HEAD_OF_QUEUE;
+			break;
+		case MSG_ORDERED_TAG:
+			attr = ISCSI_ATTR_ORDERED;
+			break;
+		}
+	}
+	return attr;
+}
+
 /**
  * iscsi_prep_scsi_cmd_pdu - prep iscsi scsi cmd pdu
  * @ctask: iscsi cmd task
@@ -137,7 +158,8 @@ static void iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task *ctask)
 	struct scsi_cmnd *sc = ctask-sc;
 
 hdr-opcode = ISCSI_OP_SCSI_CMD;
-hdr-flags = ISCSI_ATTR_SIMPLE;
+hdr-flags = hdr-flags = (iscsi_command_attr(sc) 
+		ISCSI_FLAG_CMD_ATTR_MASK);
 int_to_scsilun(sc-device-lun, (struct scsi_lun *)hdr-lun);
 hdr-itt = build_itt(ctask-itt, conn-id, session-age);
 hdr-data_length = cpu_to_be32(scsi_bufflen(sc));


Re: [Bugme-new] [Bug 9405] New: iSCSI does not implement ordering guarantees required by e.g. journaling filesystems

2007-11-19 Thread Mike Christie

Matthew Wilcox wrote:

On Mon, Nov 19, 2007 at 03:15:22PM -0600, Mike Christie wrote:

+iSCSI


It's traditional to use all-caps here, even when the normal
capitalisation is different.  so I think this should be ISCSI.
Damned if I know why we have this convention, though.



Thanks. Here is a updated patch.
Add Mike Christie to MAINTAINERS file.

v2 - used all caps for system

Signed-off-by: Mike Christie [EMAIL PROTECTED]

diff --git a/MAINTAINERS b/MAINTAINERS
index 1c7c229..1fdbb72 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2111,6 +2111,14 @@ L:	[EMAIL PROTECTED] (subscribers-only)
 W:	http://irda.sourceforge.net/
 S:	Maintained
 
+ISCSI
+P:	Mike Christie
+M:	[EMAIL PROTECTED]
+L:	[EMAIL PROTECTED]
+W:	www.open-iscsi.org
+T:	git kernel.org:/pub/scm/linux/kernel/mnc/linux-2.6-iscsi.git
+S:	Maintained
+
 ISAPNP
 P:	Jaroslav Kysela
 M:	[EMAIL PROTECTED]


Re: [Bugme-new] [Bug 9405] New: iSCSI does not implement ordering guarantees required by e.g. journaling filesystems

2007-11-19 Thread James Bottomley

On Mon, 2007-11-19 at 15:22 -0600, Mike Christie wrote:
 James Bottomley wrote:
  On Mon, 2007-11-19 at 12:50 -0800, Andrew Morton wrote:
  On Mon, 19 Nov 2007 05:44:01 -0800 (PST)
  [EMAIL PROTECTED] wrote:
 
  http://bugzilla.kernel.org/show_bug.cgi?id=9405
 
 Summary: iSCSI does not implement ordering guarantees required 
  by
  e.g. journaling filesystems
 Product: IO/Storage
 Version: 2.5
   KernelVersion: 2.6.23.1
Platform: All
  OS/Version: Linux
Tree: Mainline
  Status: NEW
Severity: high
Priority: P1
   Component: SCSI
  AssignedTo: [EMAIL PROTECTED]
  ReportedBy: [EMAIL PROTECTED]
 
 
  Most recent kernel where this bug did not occur: (new issue)
  Distribution: any
  Hardware Environment: (does not apply)
  Software Environment: (does not apply) 
  Problem Description: The sd (SCSI disk) driver ignores block device 
  barriers
  (REQ_HARDBARRIER). The iSCSI code in the kernel sends all iSCSI commands 
  with
  flag ISCSI_ATTR_SIMPLE to the iSCSI target. This means that the target may
  reorder these commands. Since a.o. correct operation of journaling 
  filesystems
  depends on being able to enforce the order of certain block write 
  operations,
  not enforcing write ordering is a bug. This can be solved by either adding
  support for REQ_HARDBARRIER in the sd device or by replacing 
  ISCSI_ATTR_SIMPLE
  by ISCSI_ATTR_ORDERED.
 
  Steps to reproduce: Source reading of drivers/scsi/sd.c and
  drivers/scsi/libiscsi.c.
 
  References: SCSI Architecture Model - 3, paragraph 8.6
  (http://www.t10.org/ftp/t10/drafts/sam3/sam3r14.pdf).
 
  (does iscsi have a maintainer?)
  
  Yes, mike christie
  
  And please close this as invalid.  FS ordering guarantees in linux
  aren't done via ordered tags.
  
 
 I had a related question. I was working on the attached patch for soe 
 other testing (patch made against scsi-rc-fixes, but is not stable so do 
 not apply), which does the scsi_populate_tag_msg conversion from MSG_* 
 to ISCSI_ATTR and sets the proper iscsi bits.
 
 If I do this patch where I call scsi_activate_tcq on a device and that 
 concertsion, does this require that my driver not reorder commands? I 
 was just a little worried on some of the error handling paths where we 
 requeue commands to the mid layer.

Right, there's no way of guaranteeing that commands aren't reordered in
the error path (or even the queue full submission path) which is why we
don't use ordered tags to enforce barriers.

James


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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Roland Dreier
  I've been debugging various issues on the PowerPC 44x embedded
  architecture which happens to have non-coherent PCI DMA.
  
  One of the problem I'm hitting is that one really need to enforce
  kmalloc alignement to cache lines or bad things will happen (among
  others with USB), for some reasons, powerpc failed to do so, I fixed it.

Heh... I hit the same problem literally 5 years ago:
http://lwn.net/Articles/1783/

I implemented the __dma_buffer annotation:
http://lwn.net/Articles/2269/

But DaveM said we should just use the PCI pool code instead:
http://lwn.net/Articles/2270/

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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Benjamin Herrenschmidt

On Mon, 2007-11-19 at 13:43 -0800, Roland Dreier wrote:
  I've been debugging various issues on the PowerPC 44x embedded
   architecture which happens to have non-coherent PCI DMA.
   
   One of the problem I'm hitting is that one really need to enforce
   kmalloc alignement to cache lines or bad things will happen (among
   others with USB), for some reasons, powerpc failed to do so, I fixed it.
 
 Heh... I hit the same problem literally 5 years ago:
 http://lwn.net/Articles/1783/
 
 I implemented the __dma_buffer annotation:
 http://lwn.net/Articles/2269/
 
 But DaveM said we should just use the PCI pool code instead:
 http://lwn.net/Articles/2270/

Heh, well... 

In this case, DaveM just proposed something akin to your
__dma_buffer :-)

On the other hand, after discussing with James, it looks like we'll just
be reverting the patch that removed the kmalloc of the sense buffer
since non cache-coherent archs are supposed to enforce kmalloc alignment
to cache lines.

__dma_buffer still seems like a good thing to have if too many other
drivers are hitting that but for this specific problem, it's not the
approach that we'll be taking.

Cheers,
Ben.


-
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] cciss: export more sysfs attributes

2007-11-19 Thread Mike Miller
Patch 1 of 3
This patch creates more sysfs attributes to be exported by cciss. Hopefully
we can work better with udev. Please consider this patch for inclusion.

Signed-off-by: Mike Miller [EMAIL PROTECTED]

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 7d70496..2ba5a89 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -229,20 +229,483 @@ static inline CommandList_struct 
*removeQ(CommandList_struct **Qptr,
return c;
 }
 
+static inline int find_drv_index(int ctlr, drive_info_struct *drv){
+int i;
+for (i=0; i  CISS_MAX_LUN; i++) {
+if (hba[ctlr]-drv[i].LunID == drv-LunID)
+return i;
+}
+return i;
+}
+
 #include cciss_scsi.c/* For SCSI tape support */
 
+#define ENG_GIG 10
+#define ENG_GIG_FACTOR (ENG_GIG/512)
 #define RAID_UNKNOWN 6
+static const char *raid_label[] = { 0, 4, 1(1+0), 5, 5+1, ADG,
+   UNKNOWN};
+
+
+static spinlock_t sysfs_lock = SPIN_LOCK_UNLOCKED;
+
+static void cciss_sysfs_stat_inquiry(int ctlr, int logvol,
+   int withirq, drive_info_struct *drv)
+{
+   int return_code;
+   InquiryData_struct *inq_buff;
+
+   /* If there are no heads then this is the controller disk and
+* not a valid logical drive so don't query it.
+*/
+   if (!drv-heads)
+   return;
+
+   inq_buff = kzalloc(sizeof(InquiryData_struct), GFP_KERNEL);
+   if (!inq_buff) {
+   printk(KERN_ERR cciss: out of memory\n);
+   goto err;
+   }
+
+   if (withirq)
+   return_code = sendcmd_withirq(CISS_INQUIRY, ctlr,
+   inq_buff, sizeof(*inq_buff), 1, logvol ,0, TYPE_CMD);
+   else
+   return_code = sendcmd(CISS_INQUIRY, ctlr, inq_buff,
+   sizeof(*inq_buff), 1, logvol , 0, NULL, TYPE_CMD);
+   if (return_code == IO_OK) {
+   memcpy(drv-vendor, inq_buff-data_byte[8], 8);
+   drv-vendor[8]='\0';
+   memcpy(drv-model, inq_buff-data_byte[16], 16);
+   drv-model[16] = '\0';
+   memcpy(drv-rev, inq_buff-data_byte[32], 4);
+   drv-rev[4] = '\0';
+   } else { /* Get geometry failed */
+   printk(KERN_WARNING cciss: inquiry for VPD page 0 failed\n);
+   }
+
+   if (withirq)
+   return_code = sendcmd_withirq(CISS_INQUIRY, ctlr,
+   inq_buff, sizeof(*inq_buff), 1, logvol ,0x83, TYPE_CMD);
+   else
+   return_code = sendcmd(CISS_INQUIRY, ctlr, inq_buff,
+   sizeof(*inq_buff), 1, logvol , 0x83, NULL, TYPE_CMD);
+
+   if (return_code == IO_OK) {
+   memcpy(drv-uid, inq_buff-data_byte[8], 16);
+   } else { /* Get geometry failed */
+   printk(KERN_WARNING cciss: id logical drive failed\n);
+   }
+
+   kfree(inq_buff);
+err:
+   drv-vendor[8] = '\0';
+   drv-model[16] = '\0';
+   drv-rev[4] = '\0';
+
+}
+
+static ssize_t cciss_show_raid_level(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+   struct drv_dynamic *d;
+   drive_info_struct *drv;
+   ctlr_info_t *h;
+   unsigned long flags;
+   int raid;
+
+   d = container_of(dev, struct drv_dynamic, dev);
+   spin_lock(sysfs_lock);
+   if (!d-disk) {
+   spin_unlock(sysfs_lock);
+   return -ENOENT;
+   }
+
+   h = get_host(d-disk);
+
+   spin_lock_irqsave(CCISS_LOCK(h-ctlr), flags);
+   if (h-busy_configuring) {
+   spin_unlock_irqrestore(CCISS_LOCK(h-ctlr), flags);
+   spin_unlock(sysfs_lock);
+   return snprintf(buf, 30, Device busy configuring\n);
+   }
+
+   drv = d-disk-private_data;
+   if ((drv-raid_level  0) || (drv-raid_level)  5)
+   raid = RAID_UNKNOWN;
+   else
+   raid = drv-raid_level;
+
+   spin_unlock_irqrestore(CCISS_LOCK(h-ctlr), flags);
+   spin_unlock(sysfs_lock);
+   return snprintf(buf, 20, RAID %s\n, raid_label[raid]);
+}
+
+static ssize_t cciss_show_disk_size(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct drv_dynamic *d;
+   drive_info_struct *drv;
+   ctlr_info_t *h;
+   unsigned long flags;
+   sector_t vol_sz, vol_sz_frac;
+
+   d = container_of(dev, struct drv_dynamic, dev);
+   spin_lock(sysfs_lock);
+   if (!d-disk) {
+   spin_unlock(sysfs_lock);
+   return -ENOENT;
+   }
+   h = get_host(d-disk);
+
+   spin_lock_irqsave(CCISS_LOCK(h-ctlr), flags);
+   if (h-busy_configuring) {
+   spin_unlock_irqrestore(CCISS_LOCK(h-ctlr), flags);
+   spin_unlock(sysfs_lock);
+   return snprintf(buf, 30, Device busy configuring\n);
+   }
+
+   drv = 

[PATCH 2/3] cciss: add support for blktrace

2007-11-19 Thread Mike Miller
Patch 2 of 3
This patch adds support for the blktrace utility. Please consider this for
inclusion. Seems there was already a call to blk_add_trace. This patch adds
ifdef's and includes the header file.

Signed-off-by: Mike Miller [EMAIL PROTECTED]


diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 2ba5a89..61bc0f3 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -41,6 +41,10 @@
 #include asm/uaccess.h
 #include asm/io.h
 
+#ifdef CONFIG_BLK_DEV_IO_TRACE
+#include linux/blktrace_api.h
+#endif /* CONFIG_BLK_DEV_IO_TRACE */
+
 #include linux/dma-mapping.h
 #include linux/blkdev.h
 #include linux/genhd.h
@@ -3013,7 +3017,9 @@ after_error_processing:
}
cmd-rq-data_len = 0;
cmd-rq-completion_data = cmd;
+#ifdef CONFIG_BLK_DEV_IO_TRACE
blk_add_trace_rq(cmd-rq-q, cmd-rq, BLK_TA_COMPLETE);
+#endif /* CONFIG_BLK_DEV_IO_TRACE */
blk_complete_request(cmd-rq);
 }
 
-
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] cciss: change version to 3.6.18

2007-11-19 Thread Mike Miller
Patch 3 of 3
This patch bumps the version of the driver from .14 to .18 to more closely
match the driver that HP ships. This driver already supports all the
hardware that the HP 3.6.18 version supports. So the versions should match,
also. Please consider this for inclusion.

Signed-off-by: Mike Miller [EMAIL PROTECTED]


diff --git a/Documentation/cciss.txt b/Documentation/cciss.txt
index e65736c..0467639 100644
--- a/Documentation/cciss.txt
+++ b/Documentation/cciss.txt
@@ -21,6 +21,7 @@ This driver is known to work with the following cards:
* SA E200
* SA E200i
* SA E500
+   * SA P700m
 
 Detecting drive failures:
 -
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 61bc0f3..6ddf543 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -55,15 +55,15 @@
 #include linux/cdrom.h
 
 #define CCISS_DRIVER_VERSION(maj,min,submin) ((maj16)|(min8)|(submin))
-#define DRIVER_NAME HP CISS Driver (v 3.6.14)
-#define DRIVER_VERSION CCISS_DRIVER_VERSION(3,6,14)
+#define DRIVER_NAME HP CISS Driver (v 3.6.18)
+#define DRIVER_VERSION CCISS_DRIVER_VERSION(3,6,18)
 
 /* Embedded module documentation macros - see modules.h */
 MODULE_AUTHOR(Hewlett-Packard Company);
-MODULE_DESCRIPTION(Driver for HP Controller SA5xxx SA6xxx version 3.6.14);
+MODULE_DESCRIPTION(Driver for HP Controller SA5xxx SA6xxx version 3.6.18);
 MODULE_SUPPORTED_DEVICE(HP SA5i SA5i+ SA532 SA5300 SA5312 SA641 SA642 SA6400
-SA6i P600 P800 P400 P400i E200 E200i E500);
-MODULE_VERSION(3.6.14);
+SA6i P600 P800 P400 P400i E200 E200i E500 P700m);
+MODULE_VERSION(3.6.18);
 MODULE_LICENSE(GPL);
 
 #include cciss_cmd.h
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread David Miller
From: Benjamin Herrenschmidt [EMAIL PROTECTED]
Date: Tue, 20 Nov 2007 06:51:14 +1100

 On Mon, 2007-11-19 at 00:38 -0800, David Miller wrote:
  From: Benjamin Herrenschmidt [EMAIL PROTECTED]
  Date: Mon, 19 Nov 2007 16:35:23 +1100
  
  You could make a dma_cacheline_aligned and use that.
  It seems pretty reasonable.
 
 I was thinking about that. What archs would need it ? arm, mips, what
 else ?

The sparc32 port would need it too.

-
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: [stable] [patch 16/26] hptiop: avoid buffer overflow when returning sense data

2007-11-19 Thread Greg KH
On Mon, Nov 19, 2007 at 11:38:35AM -0700, Matthew Wilcox wrote:
 On Mon, Nov 19, 2007 at 10:19:12AM -0800, Greg Kroah-Hartman wrote:
  
  2.6.22-stable review patch.  If anyone has any objections, please let us
  know.
 
 Makes sense to backport this.
 
 Acked-by: Matthew Wilcox [EMAIL PROTECTED]

Thanks for the review.

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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Benjamin Herrenschmidt

On Mon, 2007-11-19 at 14:31 -0800, David Miller wrote:
 From: Benjamin Herrenschmidt [EMAIL PROTECTED]
 Date: Tue, 20 Nov 2007 06:51:14 +1100
 
  On Mon, 2007-11-19 at 00:38 -0800, David Miller wrote:
   From: Benjamin Herrenschmidt [EMAIL PROTECTED]
   Date: Mon, 19 Nov 2007 16:35:23 +1100
   
   You could make a dma_cacheline_aligned and use that.
   It seems pretty reasonable.
  
  I was thinking about that. What archs would need it ? arm, mips, what
  else ?
 
 The sparc32 port would need it too.

James preference seem to go for a revert of the patch that removed the
kmalloc of the buffer instead. Sounds definitely like an easier plan
for .24 (and maybe even backport to stable).

I'll produce a patch for that later today or tomorrow.

Do you still think we should introduce this __dma_cacheline_aligned ? Do
you see other cases of drivers where it would be useful ? It tend to
agree with your earlier statement that drivers doing that are broken and
should be using a separate allocator for DMA'ble objects (in fact, on
non-cache coherent archs, kmalloc is just fine).

Cheers,
Ben.


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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Benjamin Herrenschmidt

On Mon, 2007-11-19 at 16:46 -0800, David Miller wrote:
 
 1) Require that entire buffers are commited by call sites,
and thus embedding DMA'd within non-DMA stuff isn't allowed
 
 2) Add the __dma_cacheline_aligned tag.
 
 But note that with #2 it could get quite ugly because the
 alignment and size both have a minimum that needs to be
 enforced, not just the alignment alone.  So either:

Yup.

 struct foo {
 unsigned int other_unrelated_stuff;
 
 struct object dma_thing __dma_cacheline_aligned;
 
 unsigned int more_nondma_stuff __dma_cacheline_aligned;
 };

In my tests, I had used a fuckton_t object defined to be an empty
thing with alignment constraint, seemed to work :-) But I'd rather
require #1.

BTW. What is the status nowadays with skb's ?

Cheers,
Ben.

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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread David Miller
From: Benjamin Herrenschmidt [EMAIL PROTECTED]
Date: Tue, 20 Nov 2007 11:55:01 +1100

 BTW. What is the status nowadays with skb's ?

Good question.

Some drivers are problematic (or were) because they put
DMA descriptor chaining information at the head of the
buffer, but those have been fixed either to use alternate
descriptor ring facilities or make the proper DMA sync
calls.
-
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 42/59] drivers/scsi/lpfc: Add missing space

2007-11-19 Thread Joe Perches

Signed-off-by: Joe Perches [EMAIL PROTECTED]
---
 drivers/scsi/lpfc/lpfc_attr.c |2 +-
 drivers/scsi/lpfc/lpfc_ct.c   |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index 80a1121..d9d6791 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -1182,7 +1182,7 @@ lpfc_nodev_tmo_set(struct lpfc_vport *vport, int val)
}
lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT,
 0403 lpfc_nodev_tmo attribute cannot be set to
-%d, allowed range is [%d, %d]\n,
+ %d, allowed range is [%d, %d]\n,
 val, LPFC_MIN_DEVLOSS_TMO, LPFC_MAX_DEVLOSS_TMO);
return -EINVAL;
 }
diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
index c701e4d..88b39d5 100644
--- a/drivers/scsi/lpfc/lpfc_ct.c
+++ b/drivers/scsi/lpfc/lpfc_ct.c
@@ -473,7 +473,7 @@ lpfc_ns_rsp(struct lpfc_vport *vport, struct lpfc_dmabuf 
*mp, uint32_t Size)
KERN_INFO,
LOG_DISCOVERY,
0238 Process 
-   x%x NameServer Rsp
+   x%x NameServer Rsp 
Data: x%x x%x x%x\n,
Did, ndlp-nlp_flag,
vport-fc_flag,
-- 
1.5.3.5.652.gf192c

-
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 41/59] drivers/scsi/aic94xx: Add missing space

2007-11-19 Thread Joe Perches

Signed-off-by: Joe Perches [EMAIL PROTECTED]
---
 drivers/scsi/aic94xx/aic94xx_sds.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c 
b/drivers/scsi/aic94xx/aic94xx_sds.c
index 06509bf..f45f0b9 100644
--- a/drivers/scsi/aic94xx/aic94xx_sds.c
+++ b/drivers/scsi/aic94xx/aic94xx_sds.c
@@ -325,8 +325,8 @@ asd_hwi_check_ocm_access (struct asd_ha_struct *asd_ha)
goto out;
}
 
-   printk(KERN_INFO OCM is not initialized by BIOS,
-  reinitialize it and ignore it, current IntrptStatus
+   printk(KERN_INFO OCM is not initialized by BIOS, 
+  reinitialize it and ignore it, current IntrptStatus 
   is 0x%x\n, v);
 
if (v)
-- 
1.5.3.5.652.gf192c

-
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 40/59] drivers/scsi/aic7xxx: Add missing space

2007-11-19 Thread Joe Perches

Signed-off-by: Joe Perches [EMAIL PROTECTED]
---
 drivers/scsi/aic7xxx/aic79xx_osm.c |2 +-
 drivers/scsi/aic7xxx/aic79xx_osm_pci.c |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c 
b/drivers/scsi/aic7xxx/aic79xx_osm.c
index 2d02040..581ac18 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
@@ -338,7 +338,7 @@ MODULE_PARM_DESC(aic79xx,
   amplitude:int Set the signal amplitude (0-7).\n
   seltime:int   Selection Timeout:\n
   (0/256ms,1/128ms,2/64ms,3/32ms)\n
-  slowcrc Turn on the SLOWCRC bit (Rev B only)\n 
 
+  slowcrc Turn on the SLOWCRC bit (Rev B only)\n
 \n
   Sample /etc/modprobe.conf line:\n
   Enable verbose logging\n
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm_pci.c 
b/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
index 66f0259..8ec99e2 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
@@ -374,7 +374,7 @@ ahd_pci_map_registers(struct ahd_softc *ahd)
ahd-bshs[1].ioport = base2;
command |= PCIM_CMD_PORTEN;
} else {
-   printf(aic79xx: PCI%d:%d:%d IO regions 0x%lx and 0x%lx
+   printf(aic79xx: PCI%d:%d:%d IO regions 0x%lx and 0x%lx 

   unavailable. Cannot map device.\n,
   ahd_get_pci_bus(ahd-dev_softc),
   ahd_get_pci_slot(ahd-dev_softc),
-- 
1.5.3.5.652.gf192c

-
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 43/59] drivers/scsi/qla4xxx: Add missing space

2007-11-19 Thread Joe Perches

Signed-off-by: Joe Perches [EMAIL PROTECTED]
---
 drivers/scsi/qla4xxx/ql4_init.c |2 +-
 drivers/scsi/qla4xxx/ql4_os.c   |4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_init.c b/drivers/scsi/qla4xxx/ql4_init.c
index d692c71..ab619a8 100644
--- a/drivers/scsi/qla4xxx/ql4_init.c
+++ b/drivers/scsi/qla4xxx/ql4_init.c
@@ -1184,7 +1184,7 @@ int qla4xxx_initialize_adapter(struct scsi_qla_host *ha,
 */
status = qla4xxx_initialize_ddb_list(ha);
if (status == QLA_ERROR) {
-   DEBUG2(printk(%s(%ld) Error occurred during build
+   DEBUG2(printk(%s(%ld) Error occurred during build 
  ddb list\n, __func__, ha-host_no));
goto exit_init_hba;
}
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 89460d2..7aa64ca 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -1016,7 +1016,7 @@ static void qla4xxx_do_dpc(struct work_struct *work)
struct ddb_entry *ddb_entry, *dtemp;
int status = QLA_ERROR;
 
-   DEBUG2(printk(scsi%ld: %s: DPC handler waking up.
+   DEBUG2(printk(scsi%ld: %s: DPC handler waking up. 
flags = 0x%08lx, dpc_flags = 0x%08lx ctrl_stat = 0x%08x\n,
ha-host_no, __func__, ha-flags, ha-dpc_flags,
readw(ha-reg-ctrl_status)));
@@ -1563,7 +1563,7 @@ static int qla4xxx_eh_device_reset(struct scsi_cmnd *cmd)
   cmd-device-channel, cmd-device-id, cmd-device-lun);
 
DEBUG2(printk(KERN_INFO
- scsi%ld: DEVICE_RESET cmd=%p jiffies = 0x%lx, to=%x,
+ scsi%ld: DEVICE_RESET cmd=%p jiffies = 0x%lx, to=%x, 
  dpc_flags=%lx, status=%x allowed=%d\n, ha-host_no,
  cmd, jiffies, cmd-timeout_per_command / HZ,
  ha-dpc_flags, cmd-result, cmd-allowed));
-- 
1.5.3.5.652.gf192c

-
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 43/59] drivers/scsi/qla4xxx: Add missing space

2007-11-19 Thread Joe Perches

Signed-off-by: Joe Perches [EMAIL PROTECTED]
---
 drivers/scsi/qla4xxx/ql4_init.c |2 +-
 drivers/scsi/qla4xxx/ql4_os.c   |4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_init.c b/drivers/scsi/qla4xxx/ql4_init.c
index d692c71..ab619a8 100644
--- a/drivers/scsi/qla4xxx/ql4_init.c
+++ b/drivers/scsi/qla4xxx/ql4_init.c
@@ -1184,7 +1184,7 @@ int qla4xxx_initialize_adapter(struct scsi_qla_host *ha,
 */
status = qla4xxx_initialize_ddb_list(ha);
if (status == QLA_ERROR) {
-   DEBUG2(printk(%s(%ld) Error occurred during build
+   DEBUG2(printk(%s(%ld) Error occurred during build 
  ddb list\n, __func__, ha-host_no));
goto exit_init_hba;
}
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 89460d2..7aa64ca 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -1016,7 +1016,7 @@ static void qla4xxx_do_dpc(struct work_struct *work)
struct ddb_entry *ddb_entry, *dtemp;
int status = QLA_ERROR;
 
-   DEBUG2(printk(scsi%ld: %s: DPC handler waking up.
+   DEBUG2(printk(scsi%ld: %s: DPC handler waking up. 
flags = 0x%08lx, dpc_flags = 0x%08lx ctrl_stat = 0x%08x\n,
ha-host_no, __func__, ha-flags, ha-dpc_flags,
readw(ha-reg-ctrl_status)));
@@ -1563,7 +1563,7 @@ static int qla4xxx_eh_device_reset(struct scsi_cmnd *cmd)
   cmd-device-channel, cmd-device-id, cmd-device-lun);
 
DEBUG2(printk(KERN_INFO
- scsi%ld: DEVICE_RESET cmd=%p jiffies = 0x%lx, to=%x,
+ scsi%ld: DEVICE_RESET cmd=%p jiffies = 0x%lx, to=%x, 
  dpc_flags=%lx, status=%x allowed=%d\n, ha-host_no,
  cmd, jiffies, cmd-timeout_per_command / HZ,
  ha-dpc_flags, cmd-result, cmd-allowed));
-- 
1.5.3.5.652.gf192c

-
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 44/59] drivers/scsi: Add missing space

2007-11-19 Thread Joe Perches

Signed-off-by: Joe Perches [EMAIL PROTECTED]
---
 drivers/scsi/NCR_D700.c|2 +-
 drivers/scsi/aic7xxx_old.c |2 +-
 drivers/scsi/dc395x.c  |2 +-
 drivers/scsi/hosts.c   |2 +-
 drivers/scsi/iscsi_tcp.c   |6 +++---
 drivers/scsi/scsi_proc.c   |2 +-
 drivers/scsi/scsi_scan.c   |2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/NCR_D700.c b/drivers/scsi/NCR_D700.c
index 9e64b21..99403a6 100644
--- a/drivers/scsi/NCR_D700.c
+++ b/drivers/scsi/NCR_D700.c
@@ -182,7 +182,7 @@ NCR_D700_probe_one(struct NCR_D700_private *p, int siop, 
int irq,
 
hostdata = kzalloc(sizeof(*hostdata), GFP_KERNEL);
if (!hostdata) {
-   printk(KERN_ERR NCR D700: SIOP%d: Failed to allocate host
+   printk(KERN_ERR NCR D700: SIOP%d: Failed to allocate host 
   data, detatching\n, siop);
return -ENOMEM;
}
diff --git a/drivers/scsi/aic7xxx_old.c b/drivers/scsi/aic7xxx_old.c
index 8f8db5f..c79a452 100644
--- a/drivers/scsi/aic7xxx_old.c
+++ b/drivers/scsi/aic7xxx_old.c
@@ -3716,7 +3716,7 @@ aic7xxx_pci_intr(struct aic7xxx_host *p)
   pci_read_config_byte(p-pdev, PCI_STATUS + 1, status1);
 
   if ( (status1  DPE)  (aic7xxx_verbose  VERBOSE_MINOR_ERROR) )
-printk(WARN_LEAD Data Parity Error during PCI address or PCI write
+printk(WARN_LEAD Data Parity Error during PCI address or PCI write 
   phase.\n, p-host_no, -1, -1, -1);
   if ( (status1  SSE)  (aic7xxx_verbose  VERBOSE_MINOR_ERROR) )
 printk(WARN_LEAD Signal System Error Detected\n, p-host_no,
diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index a9def6e..f98747c 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -1520,7 +1520,7 @@ static u8 start_scsi(struct AdapterCtlBlk* acb, struct 
DeviceCtlBlk* dcb,
}
 #endif
if (acb-active_dcb) {
-   dprintkl(KERN_DEBUG, start_scsi: (pid#%li) Attempt to start a
+   dprintkl(KERN_DEBUG, start_scsi: (pid#%li) Attempt to start a 
command while another command (pid#%li) is active.,
srb-cmd-serial_number,
acb-active_dcb-active_srb ?
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 24271a8..b53f681 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -141,7 +141,7 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum 
scsi_host_state state)
  illegal:
SCSI_LOG_ERROR_RECOVERY(1,
shost_printk(KERN_ERR, shost,
-Illegal host state transition
+Invalid host state transition 
 %s-%s\n,
 scsi_host_state_name(oldstate),
 scsi_host_state_name(state)));
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 4bcf916..2e0d9a1 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -389,7 +389,7 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, struct 
iscsi_cmd_task *ctask)
}
 
if (r2t-data_length  session-max_burst)
-   debug_scsi(invalid R2T with data len %u and max burst %u.
+   debug_scsi(invalid R2T with data len %u and max burst %u. 
   Attempting to execute request.\n,
r2t-data_length, session-max_burst);
 
@@ -900,13 +900,13 @@ more:
 
memcpy(recv_digest, conn-data, sizeof(uint32_t));
if (recv_digest != tcp_conn-in.datadgst) {
-   debug_tcp(iscsi_tcp: data digest error!
+   debug_tcp(iscsi_tcp: data digest error! 
  0x%x != 0x%x\n, recv_digest,
  tcp_conn-in.datadgst);
iscsi_conn_failure(conn, ISCSI_ERR_DATA_DGST);
return 0;
} else {
-   debug_tcp(iscsi_tcp: data digest match!
+   debug_tcp(iscsi_tcp: data digest match! 
  0x%x == 0x%x\n, recv_digest,
  tcp_conn-in.datadgst);
tcp_conn-in_progress = IN_PROGRESS_WAIT_HEADER;
diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c
index bb6f051..781dc85 100644
--- a/drivers/scsi/scsi_proc.c
+++ b/drivers/scsi/scsi_proc.c
@@ -123,7 +123,7 @@ void scsi_proc_host_add(struct Scsi_Host *shost)
p = create_proc_read_entry(name, S_IFREG | S_IRUGO | S_IWUSR,
sht-proc_dir, proc_scsi_read, shost);
if (!p) {
-   printk(KERN_ERR %s: Failed to register host %d in
+   printk(KERN_ERR %s: Failed to register host %d in 
   %s\n, __FUNCTION__, shost-host_no,
   sht-proc_name);

Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Roland Dreier
  2) Add the __dma_cacheline_aligned tag.
  
  But note that with #2 it could get quite ugly because the
  alignment and size both have a minimum that needs to be
  enforced, not just the alignment alone.  So either:
  
  struct foo {
   unsigned int other_unrelated_stuff;
  
   struct object dma_thing __dma_cacheline_aligned;
  
   unsigned int more_nondma_stuff __dma_cacheline_aligned;
  };
  
  or:
  
  struct foo {
   unsigned int other_unrelated_stuff;
  
   union {
   struct object dma_thing __dma_cacheline_aligned;
   char __pad[(sizeof(object) + DMA_CACHELINE_SIZE 
  ~DMA_CACHELINE_SIZE)];
   } u;
  
   unsigned int more_nondma_stuff __dma_cacheline_aligned;
  };

I wrapped this ugliness up inside the macro back in what I posted in
2002 (http://lkml.org/lkml/2002/6/12/234):

#define __dma_buffer __dma_buffer_line(__LINE__)
#define __dma_buffer_line(line) __dma_buffer_expand_line(line)
#define __dma_buffer_expand_line(line) \
__attribute__ ((aligned(L1_CACHE_BYTES))); \
char __dma_pad_ ## line [0] __attribute__ ((aligned(L1_CACHE_BYTES)))

then you just need to tag the actual member like:

char foo[3] __dma_buffer;

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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Benjamin Herrenschmidt

On Mon, 2007-11-19 at 18:10 -0800, Roland Dreier wrote:
 
 I wrapped this ugliness up inside the macro back in what I posted in
 2002 (http://lkml.org/lkml/2002/6/12/234):
 
 #define __dma_buffer __dma_buffer_line(__LINE__)
 #define __dma_buffer_line(line) __dma_buffer_expand_line(line)
 #define __dma_buffer_expand_line(line) \
 __attribute__ ((aligned(L1_CACHE_BYTES))); \
 char __dma_pad_ ## line [0] __attribute__
 ((aligned(L1_CACHE_BYTES)))
 
 then you just need to tag the actual member like:
 
 char foo[3] __dma_buffer;

That's actually not too bad ... 

I'm having a problem with reverting SCSI to use an allocation for the
sense buffer, because it can fail and the new scso_eh_prep_cmnd() isn't
supposed to return a failure. I've changed that but now I get into
trying to fix the various drivers that use it without checking the
result code and it's becoming much more complicated than initially
thought.

So I may do the above instead and revive your patch.

Any objection ? James ? David ?

Cheers,
Ben.


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


Re: SCSI breakage on non-cache coherent architectures

2007-11-19 Thread Benjamin Herrenschmidt
FYI, Here's what I have for the SCSI change. I haven't updated drivers
to care for the new return code though, help appreciated with that as I
don't know much about these drivers.

Index: linux-work/drivers/scsi/scsi_error.c
===
--- linux-work.orig/drivers/scsi/scsi_error.c   2007-11-20 13:26:18.0 
+1100
+++ linux-work/drivers/scsi/scsi_error.c2007-11-20 13:43:05.0 
+1100
@@ -602,8 +602,9 @@ static void scsi_abort_eh_cmnd(struct sc
  * @cmnd is ignored and this functions sets up a REQUEST_SENSE command
  * and cmnd buffers to read @sense_bytes into @scmd-sense_buffer.
  **/
-void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
-   unsigned char *cmnd, int cmnd_size, unsigned 
sense_bytes)
+int scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
+ unsigned char *cmnd, int cmnd_size, unsigned sense_bytes,
+ gfp_t gfp_mask)
 {
struct scsi_device *sdev = scmd-device;
 
@@ -622,12 +623,20 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd 
ses-use_sg = scmd-use_sg;
ses-resid = scmd-resid;
ses-result = scmd-result;
+   sg_init_table(ses-sense_sgl, 1);
 
if (sense_bytes) {
+   struct page *pg;
+
+   if (sdev-host-hostt-unchecked_isa_dma)
+   gfp_mask |= __GFP_DMA;
scmd-request_bufflen = min_t(unsigned,
   sizeof(scmd-sense_buffer), sense_bytes);
-   sg_init_one(ses-sense_sgl, scmd-sense_buffer,
-  scmd-request_bufflen);
+   pg = alloc_page(gfp_mask);
+   if (!pg)
+   return FAILED;
+   memset(page_address(pg), 0, scmd-request_bufflen);
+   sg_set_page(ses-sense_sgl, pg, scmd-request_bufflen, 0);
scmd-request_buffer = ses-sense_sgl;
scmd-sc_data_direction = DMA_FROM_DEVICE;
scmd-use_sg = 1;
@@ -658,6 +667,8 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd 
 * untransferred sense data should be interpreted as being zero.
 */
memset(scmd-sense_buffer, 0, sizeof(scmd-sense_buffer));
+
+   return SUCCESS;
 }
 EXPORT_SYMBOL(scsi_eh_prep_cmnd);
 
@@ -670,9 +681,17 @@ EXPORT_SYMBOL(scsi_eh_prep_cmnd);
  **/
 void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
 {
+   struct page *pg;
+
/*
 * Restore original data
 */
+   pg = sg_page(ses-sense_sgl);
+   if (pg) {
+   memcpy(scmd-sense_buffer, page_address(pg),
+  scmd-request_bufflen);
+   __free_page(pg);
+   }
scmd-cmd_len = ses-cmd_len;
memcpy(scmd-cmnd, ses-cmnd, sizeof(scmd-cmnd));
scmd-sc_data_direction = ses-data_direction;
@@ -709,7 +728,10 @@ static int scsi_send_eh_cmnd(struct scsi
struct scsi_eh_save ses;
int rtn;
 
-   scsi_eh_prep_cmnd(scmd, ses, cmnd, cmnd_size, sense_bytes);
+   if (scsi_eh_prep_cmnd(scmd, ses, cmnd, cmnd_size, sense_bytes,
+ GFP_KERNEL) != SUCCESS)
+   return FAILED;
+
shost-eh_action = done;
 
spin_lock_irqsave(shost-host_lock, flags);
Index: linux-work/include/scsi/scsi_eh.h
===
--- linux-work.orig/include/scsi/scsi_eh.h  2007-11-20 13:36:44.0 
+1100
+++ linux-work/include/scsi/scsi_eh.h   2007-11-20 13:42:49.0 +1100
@@ -81,9 +81,10 @@ struct scsi_eh_save {
struct scatterlist sense_sgl;
 };
 
-extern void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
+extern int scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
struct scsi_eh_save *ses, unsigned char *cmnd,
-   int cmnd_size, unsigned sense_bytes);
+   int cmnd_size, unsigned sense_bytes,
+   gfp_t gfp_mask);
 
 extern void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd,
struct scsi_eh_save *ses);
Index: linux-work/drivers/scsi/NCR5380.c
===
--- linux-work.orig/drivers/scsi/NCR5380.c  2007-11-20 13:46:41.0 
+1100
+++ linux-work/drivers/scsi/NCR5380.c   2007-11-20 13:46:47.0 +1100
@@ -2283,7 +2283,7 @@ static void NCR5380_information_transfer
}
 
if ((cmd-cmnd[0] != REQUEST_SENSE)  
(status_byte(cmd-SCp.Status) == CHECK_CONDITION)) {
-   scsi_eh_prep_cmnd(cmd, 
hostdata-ses, NULL, 0, ~0);
+   scsi_eh_prep_cmnd(cmd, 
hostdata-ses, NULL, 0, ~0, GFP_ATOMIC);
 
dprintk(NDEBUG_AUTOSENSE, 
(scsi%d : performing request sense\n, 

Re: [PATCH 2/3] cciss: add support for blktrace

2007-11-19 Thread Andrew Morton
On Mon, 19 Nov 2007 16:07:17 -0600 Mike Miller [EMAIL PROTECTED] wrote:

 Patch 2 of 3
 This patch adds support for the blktrace utility. Please consider this for
 inclusion. Seems there was already a call to blk_add_trace. This patch adds
 ifdef's and includes the header file.
 
 Signed-off-by: Mike Miller [EMAIL PROTECTED]
 
 
 diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
 index 2ba5a89..61bc0f3 100644
 --- a/drivers/block/cciss.c
 +++ b/drivers/block/cciss.c
 @@ -41,6 +41,10 @@
  #include asm/uaccess.h
  #include asm/io.h
  
 +#ifdef CONFIG_BLK_DEV_IO_TRACE
 +#include linux/blktrace_api.h
 +#endif /* CONFIG_BLK_DEV_IO_TRACE */

The ifdefs shouldn't be needed here.  If they are needed, blktrace_api.h needs
fixing.

  #include linux/dma-mapping.h
  #include linux/blkdev.h
  #include linux/genhd.h
 @@ -3013,7 +3017,9 @@ after_error_processing:
   }
   cmd-rq-data_len = 0;
   cmd-rq-completion_data = cmd;
 +#ifdef CONFIG_BLK_DEV_IO_TRACE
   blk_add_trace_rq(cmd-rq-q, cmd-rq, BLK_TA_COMPLETE);
 +#endif /* CONFIG_BLK_DEV_IO_TRACE */
   blk_complete_request(cmd-rq);
  }

Add if you remove the first set of ifdefs, these ifdefs can also be
removed.

-
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] cciss: export more sysfs attributes

2007-11-19 Thread Andrew Morton
On Mon, 19 Nov 2007 16:03:07 -0600 Mike Miller [EMAIL PROTECTED] wrote:

 Patch 1 of 3
 This patch creates more sysfs attributes to be exported by cciss. Hopefully
 we can work better with udev. Please consider this patch for inclusion.
 

It would be appropriate if the changelog were to describe what the problem
is with udev, and how this patch attemtps to address it.

 
 diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
 index 7d70496..2ba5a89 100644
 --- a/drivers/block/cciss.c
 +++ b/drivers/block/cciss.c
 @@ -229,20 +229,483 @@ static inline CommandList_struct 
 *removeQ(CommandList_struct **Qptr,
   return c;
  }
  
 +static inline int find_drv_index(int ctlr, drive_info_struct *drv){
 +int i;
 +for (i=0; i  CISS_MAX_LUN; i++) {
 +if (hba[ctlr]-drv[i].LunID == drv-LunID)
 +return i;
 +}
 +return i;
 +}

Please pass all patches though scripts/checkpatch.pl before sending.  It
will detect things like the codingstyle errors in the above code.

Also, that function seems to be too large to be inlined.

  #include cciss_scsi.c  /* For SCSI tape support */
  
 +#define ENG_GIG 10
 +#define ENG_GIG_FACTOR (ENG_GIG/512)
  #define RAID_UNKNOWN 6
 +static const char *raid_label[] = { 0, 4, 1(1+0), 5, 5+1, ADG,
 + UNKNOWN};
 +
 +
 +static spinlock_t sysfs_lock = SPIN_LOCK_UNLOCKED;

And that's a bug which checkpatch would have detected.  Please use
DEFINE_SPINLOCK() to avoid confusing lockdep.

 +static void cciss_sysfs_stat_inquiry(int ctlr, int logvol,
 + int withirq, drive_info_struct *drv)
 +{
 + int return_code;
 + InquiryData_struct *inq_buff;
 +
 + /* If there are no heads then this is the controller disk and
 +  * not a valid logical drive so don't query it.
 +  */
 + if (!drv-heads)
 + return;
 +
 + inq_buff = kzalloc(sizeof(InquiryData_struct), GFP_KERNEL);
 + if (!inq_buff) {
 + printk(KERN_ERR cciss: out of memory\n);
 + goto err;
 + }
 +
 + if (withirq)
 + return_code = sendcmd_withirq(CISS_INQUIRY, ctlr,
 + inq_buff, sizeof(*inq_buff), 1, logvol ,0, TYPE_CMD);
 + else
 + return_code = sendcmd(CISS_INQUIRY, ctlr, inq_buff,
 + sizeof(*inq_buff), 1, logvol , 0, NULL, TYPE_CMD);
 + if (return_code == IO_OK) {
 + memcpy(drv-vendor, inq_buff-data_byte[8], 8);
 + drv-vendor[8]='\0';
 + memcpy(drv-model, inq_buff-data_byte[16], 16);
 + drv-model[16] = '\0';
 + memcpy(drv-rev, inq_buff-data_byte[32], 4);
 + drv-rev[4] = '\0';
 + } else { /* Get geometry failed */
 + printk(KERN_WARNING cciss: inquiry for VPD page 0 failed\n);
 + }
 +
 + if (withirq)
 + return_code = sendcmd_withirq(CISS_INQUIRY, ctlr,
 + inq_buff, sizeof(*inq_buff), 1, logvol ,0x83, TYPE_CMD);
 + else
 + return_code = sendcmd(CISS_INQUIRY, ctlr, inq_buff,
 + sizeof(*inq_buff), 1, logvol , 0x83, NULL, TYPE_CMD);
 +
 + if (return_code == IO_OK) {
 + memcpy(drv-uid, inq_buff-data_byte[8], 16);
 + } else { /* Get geometry failed */
 + printk(KERN_WARNING cciss: id logical drive failed\n);
 + }
 +
 + kfree(inq_buff);
 +err:
 + drv-vendor[8] = '\0';
 + drv-model[16] = '\0';
 + drv-rev[4] = '\0';
 +
 +}
 +
 +static ssize_t cciss_show_raid_level(struct device *dev,
 +  struct device_attribute *attr, char *buf)
 +{
 + struct drv_dynamic *d;
 + drive_info_struct *drv;
 + ctlr_info_t *h;
 + unsigned long flags;
 + int raid;
 +
 + d = container_of(dev, struct drv_dynamic, dev);
 + spin_lock(sysfs_lock);
 + if (!d-disk) {
 + spin_unlock(sysfs_lock);
 + return -ENOENT;
 + }
 +
 + h = get_host(d-disk);
 +
 + spin_lock_irqsave(CCISS_LOCK(h-ctlr), flags);
 + if (h-busy_configuring) {
 + spin_unlock_irqrestore(CCISS_LOCK(h-ctlr), flags);
 + spin_unlock(sysfs_lock);
 + return snprintf(buf, 30, Device busy configuring\n);
 + }
 +
 + drv = d-disk-private_data;
 + if ((drv-raid_level  0) || (drv-raid_level)  5)
 + raid = RAID_UNKNOWN;
 + else
 + raid = drv-raid_level;
 +
 + spin_unlock_irqrestore(CCISS_LOCK(h-ctlr), flags);
 + spin_unlock(sysfs_lock);
 + return snprintf(buf, 20, RAID %s\n, raid_label[raid]);
 +}
 +
 +static ssize_t cciss_show_disk_size(struct device *dev,
 + struct device_attribute *attr, char *buf)
 +{
 + struct drv_dynamic *d;
 + drive_info_struct *drv;
 + ctlr_info_t *h;
 + unsigned long flags;
 + sector_t vol_sz, vol_sz_frac;
 +
 + d = container_of(dev, struct drv_dynamic, dev);
 + spin_lock(sysfs_lock);
 +