Re: [PATCH] ide-scsi highmem cleanup

[Andrew Morton <akpm@osdl.org>]

Jeff Garzik <jgarzik@pobox.com> wrote:
>
> > @@ -212,19 +205,12 @@ static void idescsi_output_buffers (ide_
>  >  			return;
>  >  		}
>  >  		count = min(pc->sg->length - pc->b_count, bcount);
>  > -		if (PageHighMem(pc->sg->page)) {
>  > -			unsigned long flags;
>  > -
>  > -			local_irq_save(flags);
>  > -			buf = kmap_atomic(pc->sg->page, KM_IRQ0) + pc->sg->offset;
>  > -			drive->hwif->atapi_output_bytes(drive, buf + pc->b_count, count);
>  > -			kunmap_atomic(buf - pc->sg->offset, KM_IRQ0);
>  > -			local_irq_restore(flags);
>  > -		} else {
>  > -			buf = page_address(pc->sg->page) + pc->sg->offset;
>  > -			drive->hwif->atapi_output_bytes(drive, buf + pc->b_count, count);
>  > -		}
>  > -		bcount -= count; pc->b_count += count;
>  > +		buf = kmap_atomic(pc->sg->page, KM_IRQ0);
>  > +		drive->hwif->atapi_output_bytes(drive,
>  > +				buf + pc->b_count + pc->sg->offset, count);
>  > +		kunmap_atomic(buf, KM_IRQ0);
>  > +		bcount -= count;
>  > +		pc->b_count += count;
> 
>  Unless I'm missing something, this patch looks very wrong.
> 
>  kmap_atomic(..., KM_IRQx) needs to be inside local_irq_save().

Yeah, shared interrupts.

>  As such, the PageHighMem() does have clear benefits.

Yep, thanks.  I'll fix that up.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/