Re: [PATCH 7/7] sg_ring: convert core ATA code to sg_ring.
Hello, Rusty Russell. Rusty Russell wrote: ATA relies so heavily on scsi that it needs to be converted at the same time. ATA adds padding to scatterlists in scsi commands, but because there was no good way of appending to those scatterlists, it had to use boutique iterators to make sure the padding is included. With sg_ring, ATA can simply append an sg_ring entry with the padding, and normal iterators can be used. I renamed qc-cursg to qc-cur_sg to catch all the users: they should now be referring to 'qc-cur_sg[qc-cursg_i]' wherever they were using 'qc-cursg'. Signed-off-by: Rusty Russell [EMAIL PROTECTED] There's pending patchset to make libata use chained sg. Please search for [PATCHSET] libata: improve ATAPI data transfer handling, take #3 on your favorite mailing archive (I'm writing this message offline so can't give you the url at the moment). The 11th patch in the patchset converts libata to use chained sg and and the following ones extend it to also chain drain sg entries. I agree that the current chained sg isn't easy to use and it would be nice to have some abstraction on top of it but sg_ring seems to implement duplicate feature which is already available through sg chaining albeit cumbersome to use. It would be better to build upon sg chaining as we already have it. I think it can be made much easier with a bit more safe guards, generalization and some helpers. Thanks. (PS, I haven't followed the sg chaining discussion. Why is sg chaining an optional feature? Performance overhead on low end machines?) -- 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
[PATCH 3/4] drivers/scsi/lpfc: Use time_before, time_before_eq, etc.
From: Julia Lawall [EMAIL PROTECTED]
The functions time_before, time_before_eq, time_after, and time_after_eq
are more robust for comparing jiffies against other values.
A simplified version of the semantic patch making this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)
// smpl
@ change_compare_np @
expression E;
@@
(
- jiffies = E
+ time_before_eq(jiffies,E)
|
- jiffies = E
+ time_after_eq(jiffies,E)
|
- jiffies E
+ time_before(jiffies,E)
|
- jiffies E
+ time_after(jiffies,E)
)
@ include depends on change_compare_np @
@@
#include linux/jiffies.h
@ no_include depends on !include change_compare_np @
@@
#include linux/...
+ #include linux/jiffies.h
// /smpl
Signed-off-by: Julia Lawall [EMAIL PROTECTED]
---
diff -u -p linux-2.6/drivers/scsi/lpfc/lpfc_scsi.c
linuxcopy/drivers/scsi/lpfc/lpfc_scsi.c
--- linux-2.6/drivers/scsi/lpfc/lpfc_scsi.c 2007-11-08 08:00:52.0
+0100
+++ linuxcopy/drivers/scsi/lpfc/lpfc_scsi.c 2007-12-25 20:53:54.0
+0100
@@ -22,6 +22,7 @@
#include linux/pci.h
#include linux/interrupt.h
#include linux/delay.h
+#include linux/jiffies.h
#include scsi/scsi.h
#include scsi/scsi_device.h
@@ -55,7 +56,8 @@ lpfc_adjust_queue_depth(struct lpfc_hba
atomic_inc(phba-num_rsrc_err);
phba-last_rsrc_error_time = jiffies;
- if ((phba-last_ramp_down_time + QUEUE_RAMP_DOWN_INTERVAL) jiffies) {
+ if (time_before(jiffies,
+ phba-last_ramp_down_time+QUEUE_RAMP_DOWN_INTERVAL)) {
spin_unlock_irqrestore(phba-hbalock, flags);
return;
}
@@ -94,8 +96,10 @@ lpfc_rampup_queue_depth(struct lpfc_vpor
if (vport-cfg_lun_queue_depth = sdev-queue_depth)
return;
spin_lock_irqsave(phba-hbalock, flags);
- if (((phba-last_ramp_up_time + QUEUE_RAMP_UP_INTERVAL) jiffies) ||
-((phba-last_rsrc_error_time + QUEUE_RAMP_UP_INTERVAL ) jiffies)) {
+ if ((time_before(jiffies,
+phba-last_ramp_up_time + QUEUE_RAMP_UP_INTERVAL)) ||
+(time_before(jiffies,
+ phba-last_rsrc_error_time + QUEUE_RAMP_UP_INTERVAL))) {
spin_unlock_irqrestore(phba-hbalock, flags);
return;
}
@@ -617,10 +621,12 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba
lpfc_rampup_queue_depth(vport, sdev);
if (!result pnode != NULL
- ((jiffies - pnode-last_ramp_up_time)
- LPFC_Q_RAMP_UP_INTERVAL * HZ)
- ((jiffies - pnode-last_q_full_time)
- LPFC_Q_RAMP_UP_INTERVAL * HZ)
+ (time_after(jiffies,
+ pnode-last_ramp_up_time +
+ LPFC_Q_RAMP_UP_INTERVAL * HZ))
+ (time_after(jiffies,
+ pnode-last_q_full_time +
+ LPFC_Q_RAMP_UP_INTERVAL * HZ))
(vport-cfg_lun_queue_depth sdev-queue_depth)) {
shost_for_each_device(tmp_sdev, sdev-host) {
if (vport-cfg_lun_queue_depth tmp_sdev-queue_depth){
-
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 4/4] drivers/scsi/u14-34f.c: Use time_before, time_before_eq, etc.
From: Julia Lawall [EMAIL PROTECTED]
The functions time_before, time_before_eq, time_after, and time_after_eq
are more robust for comparing jiffies against other values.
A simplified version of the semantic patch making this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)
// smpl
@ change_compare_np @
expression E;
@@
(
- jiffies = E
+ time_before_eq(jiffies,E)
|
- jiffies = E
+ time_after_eq(jiffies,E)
|
- jiffies E
+ time_before(jiffies,E)
|
- jiffies E
+ time_after(jiffies,E)
)
@ include depends on change_compare_np @
@@
#include linux/jiffies.h
@ no_include depends on !include change_compare_np @
@@
#include linux/...
+ #include linux/jiffies.h
// /smpl
Signed-off-by: Julia Lawall [EMAIL PROTECTED]
---
diff -u -p linux-2.6/drivers/scsi/u14-34f.c linuxcopy/drivers/scsi/u14-34f.c
--- linux-2.6/drivers/scsi/u14-34f.c2007-10-22 11:25:24.0 +0200
+++ linuxcopy/drivers/scsi/u14-34f.c2007-12-26 16:09:12.0 +0100
@@ -420,6 +420,7 @@
#include linux/init.h
#include linux/ctype.h
#include linux/spinlock.h
+#include linux/jiffies.h
#include asm/dma.h
#include asm/irq.h
@@ -745,7 +746,8 @@ static int wait_on_busy(unsigned long io
static int board_inquiry(unsigned int j) {
struct mscp *cpp;
dma_addr_t id_dma_addr;
- unsigned int time, limit = 0;
+ unsigned long time;
+ unsigned int limit = 0;
id_dma_addr = pci_map_single(HD(j)-pdev, HD(j)-board_id,
sizeof(HD(j)-board_id), PCI_DMA_BIDIRECTIONAL);
@@ -778,7 +780,7 @@ static int board_inquiry(unsigned int j)
spin_unlock_irq(driver_lock);
time = jiffies;
- while ((jiffies - time) HZ limit++ 2) udelay(100L);
+ while (time_before(jiffies, time + HZ) limit++ 2) udelay(100L);
spin_lock_irq(driver_lock);
if (cpp-adapter_status || HD(j)-cp_stat[0] != FREE) {
@@ -1393,7 +1395,8 @@ static int u14_34f_eh_abort(struct scsi_
}
static int u14_34f_eh_host_reset(struct scsi_cmnd *SCarg) {
- unsigned int i, j, time, k, c, limit = 0;
+ unsigned long time;
+ unsigned int i, j, k, c, limit = 0;
int arg_done = FALSE;
struct scsi_cmnd *SCpnt;
@@ -1479,7 +1482,8 @@ static int u14_34f_eh_host_reset(struct
spin_unlock_irq(sh[j]-host_lock);
time = jiffies;
- while ((jiffies - time) (10 * HZ) limit++ 20) udelay(100L);
+ while (time_before(jiffies, time + 10 * HZ) limit++ 20)
+ udelay(100L);
spin_lock_irq(sh[j]-host_lock);
printk(%s: reset, interrupts disabled, loops %d.\n, BN(j), limit);
-
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] drivers/scsi: Use offsetof
From: Julia Lawall [EMAIL PROTECTED] In the patch cc154ac64aa8d3396b187f64cef01ce67f433324, Al Viro observed that the proper way to compute the distance between two structure fields is to use offsetof() or a cast to a pointer to character. The same change can be applied to a few more files. The change was made using the following semantic patch (http://www.emn.fr/x-info/coccinelle/) // smpl @r3 disable ptr_to_array@ type T; T *s; type T1, T2; identifier fld1; @@ ( (char *)s-fld1 - (char *)s | (uint8_t *)s-fld1 - (uint8_t *)s | (u8 *)s-fld1 - (u8 *)s | - (T1)s-fld1 - (T2)s + offsetof(T,fld1) ) // /smpl Signed-off-by: Julia Lawall [EMAIL PROTECTED] --- diff -u -p a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c --- a/drivers/scsi/BusLogic.c 2007-10-22 11:25:20.0 +0200 +++ b/drivers/scsi/BusLogic.c 2007-12-26 16:33:42.0 +0100 @@ -2856,7 +2856,10 @@ static int BusLogic_QueueCommand(struct CCB-Opcode = BusLogic_InitiatorCCB_ScatterGather; CCB-DataLength = Count * sizeof(struct BusLogic_ScatterGatherSegment); if (BusLogic_MultiMasterHostAdapterP(HostAdapter)) - CCB-DataPointer = (unsigned int) CCB-DMA_Handle + ((unsigned long) CCB-ScatterGatherList - (unsigned long) CCB); + CCB-DataPointer = + (unsigned int) CCB-DMA_Handle + + offsetof(struct BusLogic_CCB, +ScatterGatherList); else CCB-DataPointer = Virtual_to_32Bit_Virtual(CCB-ScatterGatherList); - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] sg_ring: convert core ATA code to sg_ring.
On Wed, 2007-12-26 at 17:36 +0900, Tejun Heo wrote: (PS, I haven't followed the sg chaining discussion. Why is sg chaining an optional feature? Performance overhead on low end machines?) The idea of SG chaining is to allow drivers that wish to take advantage of it to increase their transfer lengths beyond MAX_HW_SEGMENTS*PAGE_SIZE by using chaining. However, drivers that stay below MAX_HW_SEGMENTS for the scatterlist length don't need to be altered. The ultimate goal (well, perhaps more wish) is to have all drivers converted, so SCSI can use something small for the default scatterlist sizing and dump all the sglist mempool stuff (although this may never be reached). 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
Problems with LSI MegaRaid 1068e
To whom it may concern: I am working with a SuperMicro Super Server AS2020A-8RB with an integrated LSI MegaRaid 1068e (Firmware M1068e.01.08221427R). The configuation uses 3x 73 GB SAS drives in a RAID 5 configuration (using the RAID key). I am trying to get some combination of kernel and driver to detect these drives to know avail. I have tried a variety of boot CDs, pre-compiled modules, and even patching the kernel source myself and compiling the driver on the LSI website. Nothing detects the RAID-5 logical drive. Using vanilla kernel source version 2.6.23.12 and the LSISAS1068E Linux driver version 4.00.13.04-1, I get the following compilation error: # make modules CHK include/linux/version.h CHK include/linux/utsrelease.h CALLscripts/checksyscalls.sh CC [M] drivers/message/fusion/mptsas.o drivers/message/fusion/mptsas.c: In function `mptsas_hotplug_work': drivers/message/fusion/mptsas.c:3605: warning: passing arg 1 of `schedule_delayed_work' from incompatible pointer type drivers/message/fusion/mptsas.c:3651: warning: passing arg 1 of `schedule_delayed_work' from incompatible pointer type drivers/message/fusion/mptsas.c: In function `mptsas_broadcast_primative_work': drivers/message/fusion/mptsas.c:4141: error: structure has no member named `work' drivers/message/fusion/mptsas.c:4141: warning: type defaults to `int' in declaration of `__mptr' drivers/message/fusion/mptsas.c:4141: warning: initialization from incompatible pointer type drivers/message/fusion/mptsas.c:4141: error: structure has no member named `work' drivers/message/fusion/mptsas.c: In function `mptsas_reprobe_lun': drivers/message/fusion/mptsas.c:3289: warning: ignoring return value of `scsi_device_reprobe', declared with attribute warn_unused_result make[3]: *** [drivers/message/fusion/mptsas.o] Error 1 make[2]: *** [drivers/message/fusion] Error 2 make[1]: *** [drivers/message] Error 2 make: *** [drivers] Error 2 What is the easiest way to boot up on a LiveCD for Linux to conduct and install that will detect the RAID-5 logical drive on this controller? What is the best combination of kernel and driver source code that I can use on a production system? Please let me know if you have any suggestions or if there are links to a HOWTO or better drivers/patches to use with this hardware. Thank you, Cory Visi Managing Partner GC InfoTech, LLC E-mail: [EMAIL PROTECTED] Mobile: (203) 912-1173 Office: (203) 327-5700 x4103 Fax:(203) 353-1035 - 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: Problems with LSI MegaRaid 1068e
On Wednesday, December 26, 2007 12:39 PM, Cory Visi wrote: To whom it may concern: I am working with a SuperMicro Super Server AS2020A-8RB with an integrated LSI MegaRaid 1068e (Firmware M1068e.01.08221427R). The configuation uses 3x 73 GB SAS drives in a RAID 5 configuration (using the RAID key). MegaRAID != Fusion Please contact the MegaRAID team. I guess it will be either Sreenivas Bagalkote([EMAIL PROTECTED]) or Sumant Patro([EMAIL PROTECTED]). Their contact info should be available in the MAINTAINERS file in the top folder of the kernel source tree. Eric - 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
Fw: Problems with LSI MegaRaid 1068e
To whom it may concern: I am working with a SuperMicro Super Server AS2020A-8RB with an integrated LSI MegaRaid 1068e (Firmware M1068e.01.08221427R). The configuation uses 3x 73 GB SAS drives in a RAID 5 configuration (using the RAID key/iButton). I am trying to get some combination of kernel and driver to detect these drives to know avail. I have tried a variety of boot CDs, pre-compiled modules, and even patching the kernel source myself and compiling the driver on the LSI website. Nothing detects the RAID-5 logical drive. Using vanilla kernel source version 2.6.23.12 and the LSISAS1068E Linux driver version 4.00.13.04-1, I get the following compilation error: # make modules CHK include/linux/version.h CHK include/linux/utsrelease.h CALLscripts/checksyscalls.sh CC [M] drivers/message/fusion/mptsas.o drivers/message/fusion/mptsas.c: In function `mptsas_hotplug_work': drivers/message/fusion/mptsas.c:3605: warning: passing arg 1 of `schedule_delayed_work' from incompatible pointer type drivers/message/fusion/mptsas.c:3651: warning: passing arg 1 of `schedule_delayed_work' from incompatible pointer type drivers/message/fusion/mptsas.c: In function `mptsas_broadcast_primative_work': drivers/message/fusion/mptsas.c:4141: error: structure has no member named `work' drivers/message/fusion/mptsas.c:4141: warning: type defaults to `int' in declaration of `__mptr' drivers/message/fusion/mptsas.c:4141: warning: initialization from incompatible pointer type drivers/message/fusion/mptsas.c:4141: error: structure has no member named `work' drivers/message/fusion/mptsas.c: In function `mptsas_reprobe_lun': drivers/message/fusion/mptsas.c:3289: warning: ignoring return value of `scsi_device_reprobe', declared with attribute warn_unused_result make[3]: *** [drivers/message/fusion/mptsas.o] Error 1 make[2]: *** [drivers/message/fusion] Error 2 make[1]: *** [drivers/message] Error 2 make: *** [drivers] Error 2 What is the easiest way to boot up on a LiveCD for Linux to conduct and install that will detect the RAID-5 logical drive on this controller? What is the best combination of kernel and driver source code that I can use on a production system? Please let me know if you have any suggestions or if there are links to a HOWTO or better drivers/patches to use with this hardware. Thank you, Cory Visi Managing Partner GC InfoTech, LLC E-mail: [EMAIL PROTECTED] Mobile: (203) 912-1173 Office: (203) 327-5700 x4103 Fax:(203) 353-1035 - 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: Problems with LSI MegaRaid 1068e
Eric, Sorry about the confusion and thank you very much for the references to other support addresses. You may want to consider removing the [EMAIL PROTECTED] e-mail address from MAINTAINERS. They really have no idea how to respond to Linux kernel driver requests. Thank you again! Cory Visi Managing Partner GC InfoTech, LLC E-mail: [EMAIL PROTECTED] Mobile: (203) 912-1173 Office: (203) 327-5700 x4103 Fax:(203) 353-1035 Moore, Eric [EMAIL PROTECTED] 12/26/2007 04:35 PM To Cory Visi [EMAIL PROTECTED], DL-MPT Fusion Linux [EMAIL PROTECTED], Support, Software [EMAIL PROTECTED], linux-scsi@vger.kernel.org cc Subject RE: Problems with LSI MegaRaid 1068e On Wednesday, December 26, 2007 12:39 PM, Cory Visi wrote: To whom it may concern: I am working with a SuperMicro Super Server AS2020A-8RB with an integrated LSI MegaRaid 1068e (Firmware M1068e.01.08221427R). The configuation uses 3x 73 GB SAS drives in a RAID 5 configuration (using the RAID key). MegaRAID != Fusion Please contact the MegaRAID team. I guess it will be either Sreenivas Bagalkote([EMAIL PROTECTED]) or Sumant Patro([EMAIL PROTECTED]). Their contact info should be available in the MAINTAINERS file in the top folder of the kernel source tree. Eric - 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/5] sg_ring for scsi
On Wed, 26 Dec 2007 11:27:40 +1100
Rusty Russell [EMAIL PROTECTED] wrote:
On Friday 21 December 2007 15:37:46 FUJITA Tomonori wrote:
Some scsi drivers like ips access to sglist in a tricky way.
Indeed. I fail to see how this code works, in fact:
drivers/scsi/ips.c:ips_queue() line 1101
if (ips_is_passthru(SC)) {
ips_copp_wait_item_t *scratch;
/* A Reset IOCTL is only sent by the boot CD in extreme cases.
*/
/* There can never be any system activity ( network or disk ),
but check */
/* anyway just as a good practice.
*/
pt = (ips_passthru_t *) scsi_sglist(SC);
if ((pt-CoppCP.cmd.reset.op_code == IPS_CMD_RESET_CHANNEL)
(pt-CoppCP.cmd.reset.adapter_flag == 1)) {
Casting the scsi_sglist(SC) (which ips_is_passthrough treated as a
scatterlist) to an ips_passthrough_t seems completely bogus. It looks like
it wants to access the region mapped by the scatterlist.
Yeah, it seems to be broken.
There are many signs through the code that it needs a great deal of
work: what is the purpose of sg_break? Why does the code check if kmap_atomic
fails?
sg_break is a workaround for the hardware restrictions, I think.
===
Convert the ips SCSI driver to sg_ring.
Slightly non-trivial conversion, will need testing.
As I said, I don't think that converting SCSI drivers to sg_ring (with
lots of non-trivial work) provides any benefits. Surely, sg_ring
enables us to modify sg lists but SCSI drivers don't need the
feature. What SCSI drivers needs is just a efficient way to get the
next sg entry (they use 'sg++' in the past and sg_next now).
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] sg_ring: convert core ATA code to sg_ring.
Hello, Rusty. Rusty Russell wrote: On Wednesday 26 December 2007 19:36:36 Tejun Heo wrote: It would be better to build upon sg chaining as we already have it. I think it can be made much easier with a bit more safe guards, generalization and some helpers. I did this work to replace sg chaining. The current chaining code gives us longer sgs, and (if used carefully) the ability to append in some circumstances. But it's clumsy, and we can do better. This is my attempt. Ah.. okay. That makes sense then. I fully agree that sg chaining as it currently stands is pretty ugly to use. Appending extra entries requires reserving one extra space in the sg list to be appended to accommodate the last entry of the original list, which will be used for chaining now. Also, it's quite brittle in that information can get out of sync easily (there's no one API for sg list manipulation, things have to be done manually). I understand that people are experiencing whiplash from the idea of another change just after the sg chaining changes which have already gone in. At the very least, I think I'll have to come up with a better transition plan for SCSI drivers, rather than trying to convert them all at once... In theory, sg chaining can be done through the sg ring arrays, but that's pretty horrible. I don't necessarily think large one time conversion is bad. They're necessary at times and make sense when they can increase long term maintainability of the code. One thing I'm concerned about is the mixture of sg chaining and sg ring. It would be best if we can agree on conversion plan (or sg chaining extension plan). Thanks. -- 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 errors on powerpc with 2.6.24-rc6-mm1
FUJITA Tomonori wrote: On Mon, 24 Dec 2007 10:18:50 +0530 Balbir Singh [EMAIL PROTECTED] wrote: [snip] I might break the IOMMU code. Can you reproduce it easily? If so, reverting my IOMMU patches (I've attached a patch to revert them) fix the problem? [snip] Yes, this patch fixes the problem for me. Tested-by: Balbir Singh [EMAIL PROTECTED] -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL - 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 errors on powerpc with 2.6.24-rc6-mm1
On Thu, 27 Dec 2007 10:08:25 +0530 Balbir Singh [EMAIL PROTECTED] wrote: FUJITA Tomonori wrote: On Mon, 24 Dec 2007 10:18:50 +0530 Balbir Singh [EMAIL PROTECTED] wrote: [snip] I might break the IOMMU code. Can you reproduce it easily? If so, reverting my IOMMU patches (I've attached a patch to revert them) fix the problem? [snip] Yes, this patch fixes the problem for me. Thanks, so you can reproduce it easily, right? The problem is that I don't want to revert these changes. I'll see how these changes cause the problem shortly. - 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
