Re: [PATCH 2.6.11.6] ide-scsi: kmap scatter/gather before doing PIO

[Jens Axboe <axboe@suse.de>]

On Fri, Apr 29 2005, Stuart_Hayes@Dell.com wrote:
> > With more testing, I've discovered a problem with thin patch--I used
> > the "KM_USER0" window for kmap_atomic(), but that's apparently not
> > safe to use in an interrupt handler, because there are places in the
> > kernel where KM_USER0 is used without masking interrupts (see
> > include/linux/highmem.h for several examples).  This patch is
> > identical to the previous one I sent in, except it uses the KM_IRQ0
> > window, which I verified is not used anywhere without IRQs masked.   
> > 
> > This patch is against 2.6.11.7... the latest kernel on kernel.org
> > didn't yet have the previous patch I submitted.  If it would be more
> > useful, I can make this patch against an ide-scsi.c that already has
> > my previous patch applied--I wasn't sure which would be better.   

(you should inline the patch so one can comment on it...)

The IO code typically uses the BIO defines for this.

diff -purN linux-2.6.11.7/drivers/scsi/ide-scsi.c linux-2.6.11.7mods/drivers/scsi/ide-scsi.c
--- linux-2.6.11.7/drivers/scsi/ide-scsi.c	2005-04-07 14:57:46.000000000 -0400
+++ linux-2.6.11.7mods/drivers/scsi/ide-scsi.c	2005-04-29 11:46:55.000000000 -0400
@@ -143,6 +143,9 @@ static void idescsi_input_buffers (ide_d
 {
 	int count;
 	char *buf;
+#ifdef CONFIG_HIGHMEM
+	unsigned long flags;
+#endif
 
 	while (bcount) {
 		if (pc->sg - (struct scatterlist *) pc->scsi_cmd->request_buffer > pc->scsi_cmd->use_sg) {
@@ -151,8 +154,15 @@ static void idescsi_input_buffers (ide_d
 			return;
 		}
 		count = min(pc->sg->length - pc->b_count, bcount);
-		buf = page_address(pc->sg->page) + pc->sg->offset;
+#ifdef CONFIG_HIGHMEM
+		local_irq_save(flags);
+#endif

This is just aweful, don't add tons of ifdefs. If you must
differentiate, just do something ala:

        if (PageHighMem(page)) {
                save interrupts
                buf = kmap
        } else
                buf = page_address(xxx).

and so on. But why are you still doing it this way? Did we not agree
that adding a host template no-high-mem defines was way better??

-- 
Jens Axboe