On Wed, Jun 27 2001, Jens Axboe wrote:
> > I can see one mm corruption race condition in the patch, you missed
> > nested irq in the for kmap_irq_bh (PIO). You must _always_
> > __cli/__save_flags before accessing the KMAP_IRQ_BH slot, in case the
> > remapping is required (so _only_ when the page is in the highmem zone).
> > Otherwise memory corruption will happen when the race triggers (for
> > example two ide disks in PIO mode doing I/O at the same time connected
> > to different irq sources).
>
> Ah yes, my bad. This requires some moving around, I'll post an updated
> patch later tonight. Thanks!
A prelim and untested fix just whipped up
--
Jens Axboe
--- include/linux/highmem.h~ Wed Jun 27 18:24:13 2001
+++ include/linux/highmem.h Wed Jun 27 18:41:55 2001
@@ -26,7 +26,9 @@
}
/*
- * remember to add offset!
+ * remember to add offset! caller must also rememer to have done __save_flags
+ * and __cli prior to calling bh_kmap_irq, and __restore_flags after calling
+ * bh_kunmap_irq
*/
static inline char *bh_kmap_irq(struct buffer_head *bh)
{
@@ -44,6 +46,37 @@
kunmap_atomic((void *) ptr, KM_BH_IRQ);
}
+
+static inline void bh_cpy_to_buf(char *buf, struct buffer_head *bh, int len)
+{
+ unsigned long flags;
+ char *bh_buf;
+
+ __save_flags(flags);
+ __cli();
+
+ bh_buf = bh_kmap_irq(bh);
+ memcpy(buf, bh_buf, len);
+ bh_kunmap_irq(bh_buf);
+
+ __restore_flags(flags);
+}
+
+static inline void bh_cpy_from_buf(struct buffer_head *bh, char *buf, int len)
+{
+ unsigned long flags;
+ char *bh_buf;
+
+ __save_flags(flags);
+ __cli();
+
+ bh_buf = bh_kmap_irq(bh);
+ memcpy(bh_buf, buf, len);
+ bh_kunmap_irq(bh_buf);
+
+ __restore_flags(flags);
+}
+
#else /* CONFIG_HIGHMEM */
--- include/linux/ide.h~ Wed Jun 27 18:43:46 2001
+++ include/linux/ide.h Wed Jun 27 18:44:18 2001
@@ -759,14 +759,17 @@
*/
#define ide_rq_offset(rq) (((rq)->hard_cur_sectors - (rq)->current_nr_sectors) << 9)
-extern inline void *ide_map_buffer(struct request *rq)
+extern inline void *ide_map_buffer(struct request *rq, unsigned long flags);
{
+ __save_flags(flags);
+ __cli();
return bh_kmap_irq(rq->bh) + ide_rq_offset(rq);
}
-extern inline void ide_unmap_buffer(char *buffer)
+extern inline void ide_unmap_buffer(char *buffer, unsigned long flags)
{
bh_kunmap_irq(buffer);
+ __restore_flags(flags);
}
/*
--- drivers/scsi/scsi_lib.c~ Wed Jun 27 18:32:58 2001
+++ drivers/scsi/scsi_lib.c Wed Jun 27 18:40:57 2001
@@ -566,11 +566,8 @@
scsi_free(SCpnt->buffer, SCpnt->sglist_len);
} else {
if (SCpnt->buffer != req->buffer) {
- if (req->cmd == READ) {
- char *to = bh_kmap_irq(req->bh);
- memcpy(to, SCpnt->buffer, SCpnt->bufflen);
- bh_kunmap_irq(to);
- }
+ if (req->cmd == READ)
+ bh_cpy_from_buf(req->bh, SCpnt->buffer,
+SCpnt->bufflen);
scsi_free(SCpnt->buffer, SCpnt->bufflen);
}
}
--- drivers/scsi/scsi_merge.c~ Wed Jun 27 18:34:15 2001
+++ drivers/scsi/scsi_merge.c Wed Jun 27 18:38:01 2001
@@ -1022,11 +1022,8 @@
dma_exhausted(SCpnt, 0);
}
}
- if (req->cmd == WRITE) {
- char *buf = bh_kmap_irq(bh);
- memcpy(buff, buf, this_count << 9);
- bh_kunmap_irq(buf);
- }
+ if (req->cmd == WRITE)
+ bh_cpy_buf_high(buff, bh, this_count << 9);
}
}
SCpnt->request_bufflen = this_count << 9;
--- drivers/ide/ide-disk.c~ Wed Jun 27 18:42:35 2001
+++ drivers/ide/ide-disk.c Wed Jun 27 18:43:40 2001
@@ -140,6 +140,7 @@
byte stat;
int i;
unsigned int msect, nsect;
+ unsigned long flags;
struct request *rq;
char *to;
@@ -162,14 +163,14 @@
msect -= nsect;
} else
nsect = 1;
- to = ide_map_buffer(rq);
+ to = ide_map_buffer(rq, flags);
idedisk_input_data(drive, to, nsect * SECTOR_WORDS);
#ifdef DEBUG
printk("%s: read: sectors(%ld-%ld), buffer=0x%08lx, remaining=%ld\n",
drive->name, rq->sector, rq->sector+nsect-1,
(unsigned long) rq->buffer+(nsect<<9), rq->nr_sectors-nsect);
#endif
- ide_unmap_buffer(to);
+ ide_unmap_buffer(to, flags);
rq->sector += nsect;
rq->errors = 0;
i = (rq->nr_sectors -= nsect);
@@ -193,6 +194,7 @@
int i;
ide_hwgroup_t *hwgroup = HWGROUP(drive);
struct request *rq = hwgroup->rq;
+ unsigned long flags;
if (!OK_STAT(stat=GET_STAT(),DRIVE_READY,drive->bad_wstat)) {
printk("%s: write_intr error1: nr_sectors=%ld, stat=0x%02x\n",
drive->name, rq->nr_sectors, stat);
@@ -210,9 +212,9 @@
if (((long)rq->current_nr_sectors) <= 0)
ide_end_request(1, hwgroup);
if (i > 0) {
- char *to = ide_map_buffer(rq);
+ char *to = ide_map_buffer(rq, flags);
idedisk_output_data (drive, to, SECTOR_WORDS);
- ide_unmap_buffer(to);
+ ide_unmap_buffer(to, flags);
ide_set_handler (drive, &write_intr, WAIT_CMD, NULL);
return ide_started;
}
@@ -242,12 +244,13 @@
do {
char *buffer;
int nsect = rq->current_nr_sectors;
+ unsigned long flags;
if (nsect > mcount)
nsect = mcount;
mcount -= nsect;
- buffer = ide_map_buffer(rq);
+ buffer = ide_map_buffer(rq, flags);
rq->sector += nsect;
rq->nr_sectors -= nsect;
rq->current_nr_sectors -= nsect;
@@ -271,7 +274,7 @@
* re-entering us on the last transfer.
*/
idedisk_output_data(drive, buffer, nsect<<7);
- ide_unmap_buffer(buffer);
+ ide_unmap_buffer(buffer, flags);
} while (mcount);
return 0;
@@ -455,10 +458,12 @@
return ide_stopped;
}
} else {
- char *buffer = ide_map_buffer(rq);
+ unsigned long flags;
+ char *buffer = ide_map_buffer(rq, flags);
+
ide_set_handler (drive, &write_intr, WAIT_CMD, NULL);
idedisk_output_data(drive, buffer, SECTOR_WORDS);
- ide_unmap_buffer(buffer);
+ ide_unmap_buffer(buffer, flags);
}
return ide_started;
}
