* [PATCH] libata: PIO bounce copies to avoid long IRQ off runs
@ 2008-04-29 13:47 Alan Cox
2008-04-29 15:51 ` Grant Grundler
2008-04-29 22:01 ` [PATCH] " Jeff Garzik
0 siblings, 2 replies; 5+ messages in thread
From: Alan Cox @ 2008-04-29 13:47 UTC (permalink / raw)
To: linux-ide, jeff
Right now with PIO transfers we can jam the box up horribly as lock IRQs
off when the transfer is from high memory. We still have other things we
need to fix in this area to really make a difference but this is the
buffering side of the fix (the state machine enable/disable irq stuff is
also needed)
Currently uses kmalloc - which seems to work best, but trivial to
allocate at init time if wanted
Signed-off-by: Alan Cox <alan@redhat.com>
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-mm1/drivers/ata/libata-sff.c linux-2.6.25-mm1/drivers/ata/libata-sff.c
--- linux.vanilla-2.6.25-mm1/drivers/ata/libata-sff.c 2008-04-28 11:36:48.000000000 +0100
+++ linux-2.6.25-mm1/drivers/ata/libata-sff.c 2008-04-28 11:42:05.000000000 +0100
@@ -665,6 +665,62 @@
}
/**
+ * ata_bounce_pio_xfer - Transfer a block by PIO from high
+ * @dev: target device
+ * @page: highmem page
+ * @offset: offset in page
+ * @count: bytes to transfer
+ * @do_write: writing if set, reading if not
+ *
+ * Transfer a page of high memory via PIO. Whenever possible use a bounce
+ * buffer to avoid transfers occuring with local IRQ disable
+ */
+
+static unsigned int ata_bounce_pio_xfer(struct ata_device *dev, struct page *page,
+ unsigned int offset, int count, int rw)
+{
+ struct ata_port *ap = dev->link->ap;
+ unsigned long flags;
+ unsigned char *zebedee;
+ unsigned char *buf;
+ unsigned int consumed;
+
+ BUG_ON(offset + count > PAGE_SIZE);
+
+ zebedee = kmalloc(count, GFP_ATOMIC);
+ if (likely(zebedee)) {
+ if (rw == WRITE) {
+ local_irq_save(flags);
+ buf = kmap_atomic(page, KM_IRQ0);
+ memcpy(zebedee, buf + offset, count);
+ kunmap_atomic(buf, KM_IRQ0);
+ local_irq_restore(flags);
+ }
+ /* do the actual data transfer */
+ consumed = ap->ops->sff_data_xfer(dev, zebedee, count, rw);
+ if (rw == READ) {
+ /* Read so bounce data upwards */
+ local_irq_save(flags);
+ buf = kmap_atomic(page, KM_IRQ0);
+ memcpy(buf + offset, zebedee, count);
+ kunmap_atomic(buf, KM_IRQ0);
+ local_irq_restore(flags);
+ }
+ kfree(zebedee);
+ } else {
+ /* Only used when we are out of buffer memory
+ as a last last resort */
+ local_irq_save(flags);
+ buf = kmap_atomic(page, KM_IRQ0);
+ /* do the actual data transfer */
+ consumed = ap->ops->sff_data_xfer(dev, buf + offset, count, rw);
+ kunmap_atomic(buf, KM_IRQ0);
+ local_irq_restore(flags);
+ }
+ return consumed;
+}
+
+/**
* ata_pio_sector - Transfer a sector of data.
* @qc: Command on going
*
@@ -675,7 +731,7 @@
*/
static void ata_pio_sector(struct ata_queued_cmd *qc)
{
- int do_write = (qc->tf.flags & ATA_TFLAG_WRITE);
+ int do_write = (qc->tf.flags & ATA_TFLAG_WRITE) ? WRITE : READ;
struct ata_port *ap = qc->ap;
struct page *page;
unsigned int offset;
@@ -693,19 +749,9 @@
DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
- if (PageHighMem(page)) {
- unsigned long flags;
-
- /* FIXME: use a bounce buffer */
- local_irq_save(flags);
- buf = kmap_atomic(page, KM_IRQ0);
-
- /* do the actual data transfer */
- ap->ops->sff_data_xfer(qc->dev, buf + offset, qc->sect_size,
- do_write);
-
- kunmap_atomic(buf, KM_IRQ0);
- local_irq_restore(flags);
+ if (PageHighMem(page) || 1 /* TEST FIXME */) {
+ ata_bounce_pio_xfer(qc->dev, page, offset, qc->sect_size,
+ do_write);
} else {
buf = page_address(page);
ap->ops->sff_data_xfer(qc->dev, buf + offset, qc->sect_size,
@@ -830,19 +876,9 @@
DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
- if (PageHighMem(page)) {
- unsigned long flags;
-
- /* FIXME: use bounce buffer */
- local_irq_save(flags);
- buf = kmap_atomic(page, KM_IRQ0);
-
- /* do the actual data transfer */
- consumed = ap->ops->sff_data_xfer(dev, buf + offset, count, rw);
-
- kunmap_atomic(buf, KM_IRQ0);
- local_irq_restore(flags);
- } else {
+ if (PageHighMem(page))
+ consumed = ata_bounce_pio_xfer(qc->dev, page, offset, count, rw);
+ else {
buf = page_address(page);
consumed = ap->ops->sff_data_xfer(dev, buf + offset, count, rw);
}
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] libata: PIO bounce copies to avoid long IRQ off runs
2008-04-29 13:47 [PATCH] libata: PIO bounce copies to avoid long IRQ off runs Alan Cox
@ 2008-04-29 15:51 ` Grant Grundler
2008-04-29 16:21 ` Alan Cox
2008-04-29 22:01 ` [PATCH] " Jeff Garzik
1 sibling, 1 reply; 5+ messages in thread
From: Grant Grundler @ 2008-04-29 15:51 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-ide, jeff
On Tue, Apr 29, 2008 at 6:47 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> Right now with PIO transfers we can jam the box up horribly as lock IRQs
> off when the transfer is from high memory. We still have other things we
> need to fix in this area to really make a difference but this is the
> buffering side of the fix (the state machine enable/disable irq stuff is
> also needed)
>
> Currently uses kmalloc - which seems to work best, but trivial to
> allocate at init time if wanted
>
> Signed-off-by: Alan Cox <alan@redhat.com>
>
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-mm1/drivers/ata/libata-sff.c linux-2.6.25-mm1/drivers/ata/libata-sff.c
> --- linux.vanilla-2.6.25-mm1/drivers/ata/libata-sff.c 2008-04-28 11:36:48.000000000 +0100
> +++ linux-2.6.25-mm1/drivers/ata/libata-sff.c 2008-04-28 11:42:05.000000000 +0100
> @@ -665,6 +665,62 @@
> }
>
> /**
> + * ata_bounce_pio_xfer - Transfer a block by PIO from high
> + * @dev: target device
> + * @page: highmem page
> + * @offset: offset in page
> + * @count: bytes to transfer
> + * @do_write: writing if set, reading if not
> + *
> + * Transfer a page of high memory via PIO. Whenever possible use a bounce
> + * buffer to avoid transfers occuring with local IRQ disable
> + */
> +
> +static unsigned int ata_bounce_pio_xfer(struct ata_device *dev, struct page *page,
> + unsigned int offset, int count, int rw)
> +{
> + struct ata_port *ap = dev->link->ap;
> + unsigned long flags;
> + unsigned char *zebedee;
> + unsigned char *buf;
> + unsigned int consumed;
> +
> + BUG_ON(offset + count > PAGE_SIZE);
> +
> + zebedee = kmalloc(count, GFP_ATOMIC);
> + if (likely(zebedee)) {
> + if (rw == WRITE) {
> + local_irq_save(flags);
> + buf = kmap_atomic(page, KM_IRQ0);
> + memcpy(zebedee, buf + offset, count);
> + kunmap_atomic(buf, KM_IRQ0);
> + local_irq_restore(flags);
> + }
> + /* do the actual data transfer */
> + consumed = ap->ops->sff_data_xfer(dev, zebedee, count, rw);
> + if (rw == READ) {
> + /* Read so bounce data upwards */
> + local_irq_save(flags);
> + buf = kmap_atomic(page, KM_IRQ0);
> + memcpy(buf + offset, zebedee, count);
> + kunmap_atomic(buf, KM_IRQ0);
> + local_irq_restore(flags);
> + }
> + kfree(zebedee);
> + } else {
> + /* Only used when we are out of buffer memory
> + as a last last resort */
> + local_irq_save(flags);
> + buf = kmap_atomic(page, KM_IRQ0);
> + /* do the actual data transfer */
> + consumed = ap->ops->sff_data_xfer(dev, buf + offset, count, rw);
> + kunmap_atomic(buf, KM_IRQ0);
> + local_irq_restore(flags);
> + }
> + return consumed;
> +}
> +
> +/**
> * ata_pio_sector - Transfer a sector of data.
> * @qc: Command on going
> *
> @@ -675,7 +731,7 @@
> */
> static void ata_pio_sector(struct ata_queued_cmd *qc)
> {
> - int do_write = (qc->tf.flags & ATA_TFLAG_WRITE);
> + int do_write = (qc->tf.flags & ATA_TFLAG_WRITE) ? WRITE : READ;
> struct ata_port *ap = qc->ap;
> struct page *page;
> unsigned int offset;
> @@ -693,19 +749,9 @@
>
> DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
>
> - if (PageHighMem(page)) {
> - unsigned long flags;
> -
> - /* FIXME: use a bounce buffer */
> - local_irq_save(flags);
> - buf = kmap_atomic(page, KM_IRQ0);
> -
> - /* do the actual data transfer */
> - ap->ops->sff_data_xfer(qc->dev, buf + offset, qc->sect_size,
> - do_write);
> -
> - kunmap_atomic(buf, KM_IRQ0);
> - local_irq_restore(flags);
> + if (PageHighMem(page) || 1 /* TEST FIXME */) {
Did you want the "|| 1" code committed as well?
thanks,
grant
> + ata_bounce_pio_xfer(qc->dev, page, offset, qc->sect_size,
> + do_write);
> } else {
> buf = page_address(page);
> ap->ops->sff_data_xfer(qc->dev, buf + offset, qc->sect_size,
> @@ -830,19 +876,9 @@
>
> DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
>
> - if (PageHighMem(page)) {
> - unsigned long flags;
> -
> - /* FIXME: use bounce buffer */
> - local_irq_save(flags);
> - buf = kmap_atomic(page, KM_IRQ0);
> -
> - /* do the actual data transfer */
> - consumed = ap->ops->sff_data_xfer(dev, buf + offset, count, rw);
> -
> - kunmap_atomic(buf, KM_IRQ0);
> - local_irq_restore(flags);
> - } else {
> + if (PageHighMem(page))
> + consumed = ata_bounce_pio_xfer(qc->dev, page, offset, count, rw);
> + else {
> buf = page_address(page);
> consumed = ap->ops->sff_data_xfer(dev, buf + offset, count, rw);
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] Re: libata: PIO bounce copies to avoid long IRQ off runs
2008-04-29 13:47 [PATCH] libata: PIO bounce copies to avoid long IRQ off runs Alan Cox
2008-04-29 15:51 ` Grant Grundler
@ 2008-04-29 22:01 ` Jeff Garzik
2008-04-29 22:01 ` Alan Cox
1 sibling, 1 reply; 5+ messages in thread
From: Jeff Garzik @ 2008-04-29 22:01 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-ide
I would tend to prefer this version...
- allocate buffer at init time
- use lesser-used KM_IRQ1
Original description:
Right now with PIO transfers we can jam the box up horribly as lock
IRQs off when the transfer is from high memory. We still have other
things we need to fix in this area to really make a difference but
this is the buffering side of the fix (the state machine enable/disable
irq stuff is also needed)
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
drivers/ata/libata-sff.c | 82 ++++++++++++++++++++++++++++++-----------------
include/linux/libata.h | 6 +++
2 files changed, 60 insertions(+), 28 deletions(-)
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 2ec65a8..7e0651c 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -665,6 +665,52 @@ unsigned int ata_sff_data_xfer_noirq(struct ata_device *dev, unsigned char *buf,
}
/**
+ * ata_bounce_pio_xfer - Transfer a block by PIO from high
+ * @dev: target device
+ * @page: highmem page
+ * @offset: offset in page
+ * @count: bytes to transfer
+ * @do_write: writing if set, reading if not
+ *
+ * Transfer a page of high memory via PIO. Whenever possible use a bounce
+ * buffer to avoid transfers occuring with local IRQ disable
+ */
+
+static unsigned int ata_bounce_pio_xfer(struct ata_device *dev,
+ struct page *page, unsigned int offset,
+ int count, int rw)
+{
+ struct ata_port *ap = dev->link->ap;
+ unsigned long flags;
+ unsigned char *buf;
+ unsigned int consumed;
+
+ BUG_ON(offset + count > PAGE_SIZE);
+
+ if (rw == WRITE) {
+ local_irq_save(flags);
+ buf = kmap_atomic(page, KM_IRQ1);
+ memcpy(dev->pio_bounce, buf + offset, count);
+ kunmap_atomic(buf, KM_IRQ1);
+ local_irq_restore(flags);
+ }
+
+ /* do the actual data transfer */
+ consumed = ap->ops->sff_data_xfer(dev, dev->pio_bounce, count, rw);
+
+ if (rw == READ) {
+ /* Read so bounce data upwards */
+ local_irq_save(flags);
+ buf = kmap_atomic(page, KM_IRQ1);
+ memcpy(buf + offset, dev->pio_bounce, count);
+ kunmap_atomic(buf, KM_IRQ1);
+ local_irq_restore(flags);
+ }
+
+ return consumed;
+}
+
+/**
* ata_pio_sector - Transfer a sector of data.
* @qc: Command on going
*
@@ -675,7 +721,7 @@ unsigned int ata_sff_data_xfer_noirq(struct ata_device *dev, unsigned char *buf,
*/
static void ata_pio_sector(struct ata_queued_cmd *qc)
{
- int do_write = (qc->tf.flags & ATA_TFLAG_WRITE);
+ int do_write = (qc->tf.flags & ATA_TFLAG_WRITE) ? WRITE : READ;
struct ata_port *ap = qc->ap;
struct page *page;
unsigned int offset;
@@ -693,20 +739,10 @@ static void ata_pio_sector(struct ata_queued_cmd *qc)
DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
- if (PageHighMem(page)) {
- unsigned long flags;
-
- /* FIXME: use a bounce buffer */
- local_irq_save(flags);
- buf = kmap_atomic(page, KM_IRQ0);
-
- /* do the actual data transfer */
- ap->ops->sff_data_xfer(qc->dev, buf + offset, qc->sect_size,
- do_write);
-
- kunmap_atomic(buf, KM_IRQ0);
- local_irq_restore(flags);
- } else {
+ if (PageHighMem(page))
+ ata_bounce_pio_xfer(qc->dev, page, offset, qc->sect_size,
+ do_write);
+ else {
buf = page_address(page);
ap->ops->sff_data_xfer(qc->dev, buf + offset, qc->sect_size,
do_write);
@@ -830,19 +866,9 @@ next_sg:
DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
- if (PageHighMem(page)) {
- unsigned long flags;
-
- /* FIXME: use bounce buffer */
- local_irq_save(flags);
- buf = kmap_atomic(page, KM_IRQ0);
-
- /* do the actual data transfer */
- consumed = ap->ops->sff_data_xfer(dev, buf + offset, count, rw);
-
- kunmap_atomic(buf, KM_IRQ0);
- local_irq_restore(flags);
- } else {
+ if (PageHighMem(page))
+ consumed = ata_bounce_pio_xfer(qc->dev, page, offset, count, rw);
+ else {
buf = page_address(page);
consumed = ap->ops->sff_data_xfer(dev, buf + offset, count, rw);
}
diff --git a/include/linux/libata.h b/include/linux/libata.h
index d1dfe87..ab43fad 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -576,6 +576,12 @@ struct ata_device {
u16 id[ATA_ID_WORDS]; /* IDENTIFY xxx DEVICE data */
u32 gscr[SATA_PMP_GSCR_DWORDS]; /* PMP GSCR block */
};
+
+ /* TODO: if bounce buffer is guaranteed never to be used on a
+ * system, elide it at compile time. It is only needed to
+ * bounce PageHighMem() pages.
+ */
+ u8 pio_bounce[PAGE_SIZE];
};
/* Offset into struct ata_device. Fields above it are maintained
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] Re: libata: PIO bounce copies to avoid long IRQ off runs
2008-04-29 22:01 ` [PATCH] " Jeff Garzik
@ 2008-04-29 22:01 ` Alan Cox
0 siblings, 0 replies; 5+ messages in thread
From: Alan Cox @ 2008-04-29 22:01 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-ide
On Tue, 29 Apr 2008 18:01:28 -0400
Jeff Garzik <jeff@garzik.org> wrote:
> I would tend to prefer this version...
>
> - allocate buffer at init time
> - use lesser-used KM_IRQ1
Seems reasonable enough. You actually want to deal with the buffer at run
time however because you care about two cases
1. Box with no highmem - easy to do compile time
2. Box with no high pages allocated - kind of hairy with gcc as
the crystal ball feature doesn't work.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-04-29 22:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-29 13:47 [PATCH] libata: PIO bounce copies to avoid long IRQ off runs Alan Cox
2008-04-29 15:51 ` Grant Grundler
2008-04-29 16:21 ` Alan Cox
2008-04-29 22:01 ` [PATCH] " Jeff Garzik
2008-04-29 22:01 ` Alan Cox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).