Hi
On Fri, 5 Aug 2005, Pierre Ossman wrote:
> Ok, I've now figured out where the bug appears. 2.6.12-rc4 runs fine
> while -rc5 does not.
Ohhhh... Ok, before you discover it yourself - the fault is mine, to be
more precise, my patch, that somehow got applied... The long interesting
discussion that followed the review by Jens Axboe, who revealed its bogus
nature, didn't produce a replacement patch, mainly because there wasn't
anybody with dc395x and highmem, until you came...:-) I just forgot /
didn't realise it actually made its way into the mainline...
So, now that we have somebody to test alternative patches, let's try it...
Please, first revert the first patch (apply with -R), and then apply the
second one. There, probably, will be offsets, don't worry about them.
Warning: untested. Please, try some read-only test first, if you get so
far at all... I prepared this patch a long time ago, and didn't revisit it
since then, hope, it's still fresh enough:-)
Including as attachments for easier separation.
Good luck
Guennadi
---
Guennadi Liakhovetski
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
diff -u a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
--- a/drivers/scsi/dc395x.c 17 Nov 2004 21:04:51
+++ b/drivers/scsi/dc395x.c 22 Jan 2005 22:55:45
@@ -182,7 +182,7 @@
* cross a page boundy.
*/
#define SEGMENTX_LEN (sizeof(struct SGentry)*DC395x_MAX_SG_LISTENTRY)
-
+#define VIRTX_LEN (sizeof(void *) * DC395x_MAX_SG_LISTENTRY)
struct SGentry {
u32 address; /* bus! address */
@@ -234,6 +234,7 @@
u8 sg_count; /* No of HW sg entries for this request */
u8 sg_index; /* Index of HW sg entry for this request */
u32 total_xfer_length; /* Total number of bytes remaining to be transfered */
+ void **virt_map;
unsigned char *virt_addr; /* Virtual address of current transfer position */
/*
@@ -1020,14 +1021,14 @@
reqlen, cmd->request_buffer, cmd->use_sg,
srb->sg_count);
- srb->virt_addr = page_address(sl->page);
for (i = 0; i < srb->sg_count; i++) {
- u32 busaddr = (u32)sg_dma_address(&sl[i]);
- u32 seglen = (u32)sl[i].length;
- sgp[i].address = busaddr;
+ u32 seglen = (u32)sg_dma_len(sl + i);
+ sgp[i].address = (u32)sg_dma_address(sl + i);
sgp[i].length = seglen;
srb->total_xfer_length += seglen;
+ srb->virt_map[i] = kmap(sl[i].page);
}
+ srb->virt_addr = srb->virt_map[0];
sgp += srb->sg_count - 1;
/*
@@ -1964,6 +1965,7 @@
int segment = cmd->use_sg;
u32 xferred = srb->total_xfer_length - left; /* bytes transfered */
struct SGentry *psge = srb->segment_x + srb->sg_index;
+ void **virt = srb->virt_map;
dprintkdbg(DBG_0,
"sg_update_list: Transfered %i of %i bytes, %i remain\n",
@@ -2003,16 +2005,16 @@
/* We have to walk the scatterlist to find it */
sg = (struct scatterlist *)cmd->request_buffer;
+ idx = 0;
while (segment--) {
unsigned long mask =
~((unsigned long)sg->length - 1) & PAGE_MASK;
if ((sg_dma_address(sg) & mask) == (psge->address & mask)) {
- srb->virt_addr = (page_address(sg->page)
- + psge->address -
- (psge->address & PAGE_MASK));
+ srb->virt_addr = virt[idx] + (psge->address & ~PAGE_MASK);
return;
}
++sg;
+ ++idx;
}
dprintkl(KERN_ERR, "sg_update_list: sg_to_virt failed\n");
@@ -2138,7 +2140,7 @@
DC395x_read32(acb, TRM_S1040_DMA_CXCNT));
}
/*
- * calculate all the residue data that not yet tranfered
+ * calculate all the residue data that not yet transfered
* SCSI transfer counter + left in SCSI FIFO data
*
* .....TRM_S1040_SCSI_COUNTER (24bits)
@@ -3256,6 +3258,7 @@
struct scsi_cmnd *cmd = srb->cmd;
enum dma_data_direction dir = cmd->sc_data_direction;
if (cmd->use_sg && dir != PCI_DMA_NONE) {
+ int i;
/* unmap DC395x SG list */
dprintkdbg(DBG_SG, "pci_unmap_srb: list=%08x(%05x)\n",
srb->sg_bus_addr, SEGMENTX_LEN);
@@ -3265,6 +3268,8 @@
dprintkdbg(DBG_SG, "pci_unmap_srb: segs=%i buffer=%p\n",
cmd->use_sg, cmd->request_buffer);
/* unmap the sg segments */
+ for (i = 0; i < srb->sg_count; i++)
+ kunmap(virt_to_page(srb->virt_map[i]));
pci_unmap_sg(acb->dev,
(struct scatterlist *)cmd->request_buffer,
cmd->use_sg, dir);
@@ -3311,7 +3316,7 @@
if (cmd->use_sg) {
struct scatterlist* sg = (struct scatterlist *)cmd->request_buffer;
- ptr = (struct ScsiInqData *)(page_address(sg->page) + sg->offset);
+ ptr = (struct ScsiInqData *)(srb->virt_map[0] + sg->offset);
} else {
ptr = (struct ScsiInqData *)(cmd->request_buffer);
}
@@ -4246,8 +4251,9 @@
const unsigned srbs_per_page = PAGE_SIZE/SEGMENTX_LEN;
for (i = 0; i < DC395x_MAX_SRB_CNT; i += srbs_per_page)
- if (acb->srb_array[i].segment_x)
- kfree(acb->srb_array[i].segment_x);
+ kfree(acb->srb_array[i].segment_x);
+
+ vfree(acb->srb_array[0].virt_map);
}
@@ -4263,9 +4269,12 @@
int srb_idx = 0;
unsigned i = 0;
struct SGentry *ptr;
+ void **virt_array;
- for (i = 0; i < DC395x_MAX_SRB_CNT; i++)
+ for (i = 0; i < DC395x_MAX_SRB_CNT; i++) {
acb->srb_array[i].segment_x = NULL;
+ acb->srb_array[i].virt_map = NULL;
+ }
dprintkdbg(DBG_1, "Allocate %i pages for SG tables\n", pages);
while (pages--) {
@@ -4286,6 +4295,19 @@
ptr + (i * DC395x_MAX_SG_LISTENTRY);
else
dprintkl(KERN_DEBUG, "No space for tmsrb SG table reserved?!\n");
+
+ virt_array = vmalloc((DC395x_MAX_SRB_CNT + 1) * DC395x_MAX_SG_LISTENTRY * sizeof(void*));
+
+ if (!virt_array) {
+ adapter_sg_tables_free(acb);
+ return 1;
+ }
+
+ for (i = 0; i < DC395x_MAX_SRB_CNT + 1; i++) {
+ acb->srb_array[i].virt_map = virt_array;
+ virt_array += DC395x_MAX_SG_LISTENTRY;
+ }
+
return 0;
}
diff -u a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
--- a/drivers/scsi/dc395x.c 4 Mar 2005 22:42:35
+++ b/drivers/scsi/dc395x.c 29 Apr 2005 20:36:14
@@ -229,12 +230,8 @@
struct scsi_cmnd *cmd;
struct SGentry *segment_x; /* Linear array of hw sg entries (up to 64 entries) */
- u32 sg_bus_addr; /* Bus address of sg list (ie, of segment_x) */
-
- u8 sg_count; /* No of HW sg entries for this request */
- u8 sg_index; /* Index of HW sg entry for this request */
- u32 total_xfer_length; /* Total number of bytes remaining to be transfered */
- unsigned char *virt_addr; /* Virtual address of current transfer position */
+ size_t total_xfer_length; /* Total number of bytes remaining to be transfered */
+ size_t request_length; /* Total number of bytes in this request */
/*
* The sense buffer handling function, request_sense, uses
@@ -245,7 +242,12 @@
* total_xfer_length in xferred. These values are restored in
* pci_unmap_srb_sense. This is the only place xferred is used.
*/
- u32 xferred; /* Saved copy of total_xfer_length */
+ size_t xferred; /* Saved copy of total_xfer_length */
+
+ u32 sg_bus_addr; /* Bus address of sg list (ie, of segment_x) */
+
+ u8 sg_count; /* No of HW sg entries for this request */
+ u8 sg_index; /* Index of HW sg entry for this request */
u16 state;
@@ -989,7 +991,6 @@
srb->sg_count = 0;
srb->total_xfer_length = 0;
srb->sg_bus_addr = 0;
- srb->virt_addr = NULL;
srb->sg_index = 0;
srb->adapter_status = 0;
srb->target_status = 0;
@@ -1020,7 +1021,6 @@
reqlen, cmd->request_buffer, cmd->use_sg,
srb->sg_count);
- srb->virt_addr = page_address(sl->page);
for (i = 0; i < srb->sg_count; i++) {
u32 busaddr = (u32)sg_dma_address(&sl[i]);
u32 seglen = (u32)sl[i].length;
@@ -1065,12 +1065,14 @@
srb->total_xfer_length++;
srb->segment_x[0].length = srb->total_xfer_length;
- srb->virt_addr = cmd->request_buffer;
+
dprintkdbg(DBG_0,
"build_srb: [1] len=%d buf=%p use_sg=%d map=%08x\n",
srb->total_xfer_length, cmd->request_buffer,
cmd->use_sg, srb->segment_x[0].address);
}
+
+ srb->request_length = srb->total_xfer_length;
}
@@ -1954,14 +1956,12 @@
/*
* Compute the next Scatter Gather list index and adjust its length
- * and address if necessary; also compute virt_addr
+ * and address if necessary
*/
static void sg_update_list(struct ScsiReqBlk *srb, u32 left)
{
u8 idx;
- struct scatterlist *sg;
struct scsi_cmnd *cmd = srb->cmd;
- int segment = cmd->use_sg;
u32 xferred = srb->total_xfer_length - left; /* bytes transfered */
struct SGentry *psge = srb->segment_x + srb->sg_index;
@@ -1994,29 +1994,6 @@
psge++;
}
sg_verify_length(srb);
-
- /* we need the corresponding virtual address */
- if (!segment) {
- srb->virt_addr += xferred;
- return;
- }
-
- /* We have to walk the scatterlist to find it */
- sg = (struct scatterlist *)cmd->request_buffer;
- while (segment--) {
- unsigned long mask =
- ~((unsigned long)sg->length - 1) & PAGE_MASK;
- if ((sg_dma_address(sg) & mask) == (psge->address & mask)) {
- srb->virt_addr = (page_address(sg->page)
- + psge->address -
- (psge->address & PAGE_MASK));
- return;
- }
- ++sg;
- }
-
- dprintkl(KERN_ERR, "sg_update_list: sg_to_virt failed\n");
- srb->virt_addr = NULL;
}
@@ -2035,7 +2012,6 @@
if (debug_enabled(DBG_PIO))
printk(" (next segment)");
srb->sg_index++;
- sg_update_list(srb, srb->total_xfer_length);
}
}
@@ -2195,7 +2171,6 @@
srb->total_xfer_length - diff;
sg_update_list(srb, d_left_counter);
/*srb->total_xfer_length -= diff; */
- /*srb->virt_addr += diff; */
/*if (srb->cmd->use_sg) */
/* srb->sg_index++; */
}
@@ -2217,12 +2192,47 @@
data_io_transfer(acb, srb, XFERDATAOUT);
}
+/**
+ * dc390_kmap_atomic_sg - find and atomically map an sg-elemnt
+ * @sg: scatter-gather list
+ * @sg_count: number of segments in sg
+ * @offset: offset in bytes into sg, on return offset into the mapped area
+ * @len: on return number of bytes mapped
+ *
+ * Returns virtual address of the start of the mapped page
+ */
+static void *dc395x_kmap_atomic_sg(struct scatterlist *sg, int sg_count, size_t *offset, size_t *len)
+{
+ int i;
+ size_t sg_len = 0;
+ struct page *page;
+
+ for (i = 0; i < sg_count; i++) {
+ sg_len += sg[i].length;
+ if (sg_len > *offset)
+ break;
+ }
+
+ BUG_ON(i == sg_count);
+
+ *len = sg_len - *offset;
+ *offset = sg[i].offset + sg[i].length - *len;
+
+ page = sg[i].page + (*offset >> PAGE_SHIFT);
+ *offset &= ~PAGE_MASK;
+
+ return kmap_atomic(page, KM_BIO_SRC_IRQ);
+}
+
+static void dc395x_kunmap_atomic_sg(void *virt)
+{
+ kunmap_atomic(virt, KM_BIO_SRC_IRQ);
+}
static void data_in_phase0(struct AdapterCtlBlk *acb, struct ScsiReqBlk *srb,
u16 *pscsi_status)
{
u16 scsi_status = *pscsi_status;
- u32 d_left_counter = 0;
dprintkdbg(DBG_0, "data_in_phase0: (pid#%li) <%02i-%i>\n",
srb->cmd->pid, srb->cmd->device->id, srb->cmd->device->lun);
@@ -2240,6 +2250,7 @@
* seem to be a bad idea, actually.
*/
if (!(srb->state & SRB_XFERPAD)) {
+ unsigned int d_left_counter, max_left;
if (scsi_status & PARITYERROR) {
dprintkl(KERN_INFO, "data_in_phase0: (pid#%li) "
"Parity Error\n", srb->cmd->pid);
@@ -2292,42 +2303,83 @@
DC395x_read32(acb, TRM_S1040_DMA_CXCNT),
srb->total_xfer_length, d_left_counter);
#if DC395x_LASTPIO
+ max_left = max(d_left_counter, srb->total_xfer_length);
/* KG: Less than or equal to 4 bytes can not be transfered via DMA, it seems. */
if (d_left_counter
&& srb->total_xfer_length <= DC395x_LASTPIO) {
+ size_t left_io = srb->total_xfer_length;
+
/*u32 addr = (srb->segment_x[srb->sg_index].address); */
/*sg_update_list (srb, d_left_counter); */
- dprintkdbg(DBG_PIO, "data_in_phase0: PIO (%i %s) to "
- "%p for remaining %i bytes:",
+ dprintkdbg(DBG_PIO, "data_in_phase0: PIO (%i %s) "
+ "for remaining %i bytes:",
DC395x_read8(acb, TRM_S1040_SCSI_FIFOCNT) & 0x1f,
(srb->dcb->sync_period & WIDE_SYNC) ?
"words" : "bytes",
- srb->virt_addr,
srb->total_xfer_length);
if (srb->dcb->sync_period & WIDE_SYNC)
DC395x_write8(acb, TRM_S1040_SCSI_CONFIG2,
CFG2_WIDEFIFO);
- while (DC395x_read8(acb, TRM_S1040_SCSI_FIFOCNT) != 0x40) {
- u8 byte = DC395x_read8(acb, TRM_S1040_SCSI_FIFO);
- *(srb->virt_addr)++ = byte;
- if (debug_enabled(DBG_PIO))
- printk(" %02x", byte);
- d_left_counter--;
- sg_subtract_one(srb);
- }
- if (srb->dcb->sync_period & WIDE_SYNC) {
-#if 1
- /* Read the last byte ... */
- if (srb->total_xfer_length > 0) {
- u8 byte = DC395x_read8(acb, TRM_S1040_SCSI_FIFO);
- *(srb->virt_addr)++ = byte;
- srb->total_xfer_length--;
+
+ while (left_io) {
+ unsigned char *virt, *base = NULL;
+ unsigned long flags = 0;
+ size_t len;
+ u8 fifocnt = 0;
+
+ if (srb->cmd->use_sg) {
+ size_t offset = srb->request_length - left_io;
+ local_irq_save(flags);
+ base = dc395x_kmap_atomic_sg((struct scatterlist *)srb->cmd->request_buffer,
+ srb->sg_count, &offset, &len);
+ virt = base + offset;
+ } else {
+ virt = srb->cmd->request_buffer + srb->cmd->request_bufflen - left_io;
+ len = left_io;
+ }
+ left_io -= len;
+
+ while (len) {
+ u8 byte;
+ fifocnt = DC395x_read8(acb, TRM_S1040_SCSI_FIFOCNT);
+
+ if (fifocnt == 0x40) {
+ left_io = 0;
+ break;
+ }
+
+ byte = DC395x_read8(acb, TRM_S1040_SCSI_FIFO);
+ *virt++ = byte;
+
if (debug_enabled(DBG_PIO))
printk(" %02x", byte);
+
+ d_left_counter--;
+ sg_subtract_one(srb);
+
+ len--;
}
+
+ if (fifocnt == 0x40 && (srb->dcb->sync_period & WIDE_SYNC)) {
+#if 1
+ /* Read the last byte ... */
+ if (srb->total_xfer_length > 0) {
+ u8 byte = DC395x_read8(acb, TRM_S1040_SCSI_FIFO);
+ *virt++ = byte;
+ srb->total_xfer_length--;
+ if (debug_enabled(DBG_PIO))
+ printk(" %02x", byte);
+ }
#endif
- DC395x_write8(acb, TRM_S1040_SCSI_CONFIG2, 0);
+ DC395x_write8(acb, TRM_S1040_SCSI_CONFIG2, 0);
+ }
+
+ if (srb->cmd->use_sg) {
+ dc395x_kunmap_atomic_sg(base);
+ local_irq_restore(flags);
+ }
}
+
/*printk(" %08x", *(u32*)(bus_to_virt (addr))); */
/*srb->total_xfer_length = 0; */
if (debug_enabled(DBG_PIO))
@@ -2485,22 +2537,44 @@
SCMD_FIFO_IN);
} else { /* write */
int ln = srb->total_xfer_length;
+ size_t left_io = srb->total_xfer_length;
+
if (srb->dcb->sync_period & WIDE_SYNC)
DC395x_write8(acb, TRM_S1040_SCSI_CONFIG2,
CFG2_WIDEFIFO);
- dprintkdbg(DBG_PIO,
- "data_io_transfer: PIO %i bytes from %p:",
- srb->total_xfer_length, srb->virt_addr);
-
- while (srb->total_xfer_length) {
- if (debug_enabled(DBG_PIO))
- printk(" %02x", (unsigned char) *(srb->virt_addr));
- DC395x_write8(acb, TRM_S1040_SCSI_FIFO,
- *(srb->virt_addr)++);
+ while (left_io) {
+ unsigned char *virt, *base = NULL;
+ unsigned long flags = 0;
+ size_t len;
+
+ if (srb->cmd->use_sg) {
+ size_t offset = srb->request_length - left_io;
+ local_irq_save(flags);
+ base = dc395x_kmap_atomic_sg((struct scatterlist *)srb->cmd->request_buffer,
+ srb->sg_count, &offset, &len);
+ virt = base + offset;
+ } else {
+ virt = srb->cmd->request_buffer + srb->cmd->request_bufflen - left_io;
+ len = left_io;
+ }
+ left_io -= len;
- sg_subtract_one(srb);
+ while (len--) {
+ if (debug_enabled(DBG_PIO))
+ printk(" %02x", *virt);
+
+ DC395x_write8(acb, TRM_S1040_SCSI_FIFO, *virt++);
+
+ sg_subtract_one(srb);
+ }
+
+ if (srb->cmd->use_sg) {
+ dc395x_kunmap_atomic_sg(base);
+ local_irq_restore(flags);
+ }
}
+
if (srb->dcb->sync_period & WIDE_SYNC) {
if (ln % 2) {
DC395x_write8(acb, TRM_S1040_SCSI_FIFO, 0);
