Re: [PATCH] Chaining sg lists for big IO commands v5

[Jens Axboe <jens.axboe@oracle.com>]

On Thu, May 24 2007, Jens Axboe wrote:
> > Oops: Kernel access of bad area, sig: 11 [#1]
> > SMP NR_CPUS=32 NUMA pSeries
> > Modules linked in: qla2xxx scsi_transport_fc
> > NIP: c0000000000414a0 LR: c00000000004162c CTR: 0000000000000001
> > REGS: c0000000047bb130 TRAP: 0300   Not tainted  (2.6.22-rc1)
> > MSR: 8000000000001032 <ME,IR,DR>  CR: 28000022  XER: 00000008
> > DAR: 0000000000000000, DSISR: 0000000040000000
> > TASK = c0000000047a6aa0[0] 'swapper' THREAD: c0000000047b8000 CPU: 7
> > GPR00: 0000000000000080 c0000000047bb3b0 c000000000692358 c0000000047a6aa0
> > GPR04: 0000000000000000 0000000000000070 0000000000000000 c0000000005ac0b8
> > GPR08: 000000000000e4b4 0000000000000001 0000000000000000 0000000000000280
> > GPR12: 0000000000000020 c0000000005a3e80 0000000000000000 0000000007a8dd70
> > GPR16: 0000000000000000 0000000000000000 0000000000000000 c0000000047b8000
> > GPR20: 000000003b9aca00 c0000000047a6c50 0000000000000001 0000000000000000
> > GPR24: d000000000909048 0001dee6d30c0d30 c000000003b7dd80 c0000000047a6aa0
> > GPR28: 00000001079027ca c0000000047a6aa0 c0000000005b7cb0 c000000000472c9c
> > NIP [c0000000000414a0] .dequeue_task+0x0/0x9c
> > LR [c00000000004162c] .deactivate_task+0x40/0x60
> > Call Trace:
> > [c0000000047bb3b0] [c00000000004bccc] .printk+0x38/0x48 (unreliable)
> > [c0000000047bb440] [c000000000471704] .schedule+0x1fc/0x8dc
> > [c0000000047bb540] [c000000000472c9c] .schedule_timeout+0xa8/0xe8
> > [c0000000047bb610] [c000000000057260] .msleep+0x20/0x38
> > [c0000000047bb690] [c00000000003f5ec] .eeh_dn_check_failure+0x114/0x268
> > [c0000000047bb740] [c00000000003fc64] .eeh_check_failure+0xec/0x114
> > [c0000000047bb7c0] [d00000000086190c] .qla2300_fw_dump+0x1130/0x1c00 [qla2xxx]
> > [c0000000047bb8a0] [d000000000858d50] .qla2300_intr_handler+0x1e8/0x60c [qla2xxx]
> > [c0000000047bb950] [c000000000078368] .handle_IRQ_event+0x70/0xe4
> > [c0000000047bb9f0] [c00000000007a7e0] .handle_fasteoi_irq+0x11c/0x1d0
> > [c0000000047bba90] [c00000000000c178] .do_IRQ+0x90/0xec
> > [c0000000047bbb10] [c000000000004790] hardware_interrupt_entry+0x18/0x1c
> 
> Not good. The qla changes are non-trivial (that hardware has a really
> funky sg setup), so I may have botched a part of it. I'll review the
> qla changes and get back to you.

Does this help?

diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 10251bf..5510b86 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -155,6 +155,8 @@ void qla2x00_build_scsi_iocbs_32(srb_t *sp, cmd_entry_t *cmd_pkt,
 	uint32_t	*cur_dsd;
 	scsi_qla_host_t	*ha;
 	struct scsi_cmnd *cmd;
+	struct scatterlist *sg;
+	int i;
 
 	cmd = sp->cmd;
 
@@ -178,13 +180,10 @@ void qla2x00_build_scsi_iocbs_32(srb_t *sp, cmd_entry_t *cmd_pkt,
 
 	/* Load data segments */
 	if (cmd->use_sg != 0) {
-		struct	scatterlist *cur_seg;
-		struct	scatterlist *end_seg;
-
-		cur_seg = (struct scatterlist *)cmd->request_buffer;
-		end_seg = cur_seg + tot_dsds;
-		while (cur_seg < end_seg) {
-			cont_entry_t	*cont_pkt;
+		struct scatterlist *sgl = cmd->request_buffer;
+	
+		for_each_sg(sgl, sg, tot_dsds, i) {
+			cont_entry_t *cont_pkt;
 
 			/* Allocate additional continuation packets? */
 			if (avail_dsds == 0) {
@@ -197,11 +196,9 @@ void qla2x00_build_scsi_iocbs_32(srb_t *sp, cmd_entry_t *cmd_pkt,
 				avail_dsds = 7;
 			}
 
-			*cur_dsd++ = cpu_to_le32(sg_dma_address(cur_seg));
-			*cur_dsd++ = cpu_to_le32(sg_dma_len(cur_seg));
+			*cur_dsd++ = cpu_to_le32(sg_dma_address(sg));
+			*cur_dsd++ = cpu_to_le32(sg_dma_len(sg));
 			avail_dsds--;
-
-			cur_seg++;
 		}
 	} else {
 		*cur_dsd++ = cpu_to_le32(sp->dma_handle);
@@ -224,6 +221,9 @@ void qla2x00_build_scsi_iocbs_64(srb_t *sp, cmd_entry_t *cmd_pkt,
 	uint32_t	*cur_dsd;
 	scsi_qla_host_t	*ha;
 	struct scsi_cmnd *cmd;
+	struct scatterlist *sgl;
+	struct scatterlist *sg;
+	int i;
 
 	cmd = sp->cmd;
 
@@ -247,12 +247,8 @@ void qla2x00_build_scsi_iocbs_64(srb_t *sp, cmd_entry_t *cmd_pkt,
 
 	/* Load data segments */
 	if (cmd->use_sg != 0) {
-		struct	scatterlist *cur_seg;
-		struct	scatterlist *end_seg;
-
-		cur_seg = (struct scatterlist *)cmd->request_buffer;
-		end_seg = cur_seg + tot_dsds;
-		while (cur_seg < end_seg) {
+		sgl = cmd->request_buffer;
+		for_each_sg(sgl, sg, tot_dsds, i) {
 			dma_addr_t	sle_dma;
 			cont_a64_entry_t *cont_pkt;
 
@@ -267,13 +263,11 @@ void qla2x00_build_scsi_iocbs_64(srb_t *sp, cmd_entry_t *cmd_pkt,
 				avail_dsds = 5;
 			}
 
-			sle_dma = sg_dma_address(cur_seg);
+			sle_dma = sg_dma_address(sg);
 			*cur_dsd++ = cpu_to_le32(LSD(sle_dma));
 			*cur_dsd++ = cpu_to_le32(MSD(sle_dma));
-			*cur_dsd++ = cpu_to_le32(sg_dma_len(cur_seg));
+			*cur_dsd++ = cpu_to_le32(sg_dma_len(sg));
 			avail_dsds--;
-
-			cur_seg++;
 		}
 	} else {
 		*cur_dsd++ = cpu_to_le32(LSD(sp->dma_handle));
@@ -642,6 +636,8 @@ qla24xx_build_scsi_iocbs(srb_t *sp, struct cmd_type_7 *cmd_pkt,
 	uint32_t	*cur_dsd;
 	scsi_qla_host_t	*ha;
 	struct scsi_cmnd *cmd;
+	struct scatterlist *sg, *sgl;
+	int i;
 
 	cmd = sp->cmd;
 
@@ -671,11 +667,8 @@ qla24xx_build_scsi_iocbs(srb_t *sp, struct cmd_type_7 *cmd_pkt,
 
 	/* Load data segments */
 	if (cmd->use_sg != 0) {
-		struct	scatterlist *sgl = cmd->request_buffer;
-		struct	scatterlist *cur_seg;
-		int i;
-
-		for_each_sg(sgl, cur_seg, tot_dsds, i) {
+		sgl = cmd->request_buffer;
+		for_each_sg(sgl, sg, tot_dsds, i) {
 			dma_addr_t	sle_dma;
 			cont_a64_entry_t *cont_pkt;
 
@@ -690,10 +683,10 @@ qla24xx_build_scsi_iocbs(srb_t *sp, struct cmd_type_7 *cmd_pkt,
 				avail_dsds = 5;
 			}
 
-			sle_dma = sg_dma_address(cur_seg);
+			sle_dma = sg_dma_address(sg);
 			*cur_dsd++ = cpu_to_le32(LSD(sle_dma));
 			*cur_dsd++ = cpu_to_le32(MSD(sle_dma));
-			*cur_dsd++ = cpu_to_le32(sg_dma_len(cur_seg));
+			*cur_dsd++ = cpu_to_le32(sg_dma_len(sg));
 			avail_dsds--;
 		}
 	} else {

-- 
Jens Axboe