highmem pages

PATCH In review Jeff Garzik 2 participants 12 messages Oct 5, 2005 – Oct 9, 2005

Subsystems (2)

SCSI libata

lore

Share & tools

lore

Jeff Garzik

author

2005-10-05 07:35 UTC #0 lore raw

Here is an interesting code snippet from ide-scsi, that could be useful in libata:

Quoted 16 lines

                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->offse
t;
                        drive->hwif->atapi_input_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_input_bytes(drive, buf + pc->b_count,
 count);
               }

Albert Lee

2005-10-06 10:35 UTC #1 lore raw

Quoted 24 lines · Jeff Garzik

Here is an interesting code snippet from ide-scsi, that could be useful in libata:

Quoted 20 lines

                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->offse
t;
                        drive->hwif->atapi_input_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_input_bytes(drive, buf + 
pc->b_count,
 count);
               }

Thanks for the advice. The kmap_atomic() usage optimization patches are ready:

patch 1/3: reorganize ata_pio_sector() and __atapi_pio_bytes() patch 2/3: reorganize "buf + offset"
patch 3/3: use PageHighMem() to optimize the kmap_atomic() usage

Patch against libata-dev irq-pio branch (c71c18576d0d8aa4db876c737c3c597c724cf02f). For your review and advice, thanks.

Albert

Albert Lee

2005-10-06 10:40 UTC #2 lore raw

PATCH 1/3 irq-pio: reorganize ata_pio_sector() and __atapi_pio_bytes()

Patch 1/3:
reorganize ata_pio_sector() and __atapi_pio_bytes()

Changes:
- move some code out of the kmap_atomic() / kunmap_atomic() zone
- remove the redundant "do_write = (qc->tf.flags & ATA_TFLAG_WRITE);"

For your review, thanks.

Albert

Signed-off-by Albert Lee <[email protected]>

===========

drivers/scsi/libata-core.c +16 ~~-17~~

@@ -2763,22 +2763,21 @@ static void ata_pio_sector(struct ata_qu
 	local_irq_save(flags);
 	buf = kmap_atomic(page, KM_IRQ0) + offset;
 
-	qc->cursect++;
-	qc->cursg_ofs++;
-
-	if ((qc->cursg_ofs * ATA_SECT_SIZE) == (&sg[qc->cursg])->length) {
-		qc->cursg++;
-		qc->cursg_ofs = 0;
-	}
-
 	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
 
 	/* do the actual data transfer */
-	do_write = (qc->tf.flags & ATA_TFLAG_WRITE);
 	ata_data_xfer(ap, buf, ATA_SECT_SIZE, do_write);
 
 	kunmap_atomic(buf - offset, KM_IRQ0);
 	local_irq_restore(flags);
+
+	qc->cursect++;
+	qc->cursg_ofs++;
+
+	if ((qc->cursg_ofs * ATA_SECT_SIZE) == (&sg[qc->cursg])->length) {
+		qc->cursg++;
+		qc->cursg_ofs = 0;
+	}
 }
 
 /**

@@ -2956,6 +2955,14 @@ next_sg:
 	local_irq_save(flags);
 	buf = kmap_atomic(page, KM_IRQ0) + offset;
 
+	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
+
+	/* do the actual data transfer */
+	ata_data_xfer(ap, buf, count, do_write);
+
+	kunmap_atomic(buf - offset, KM_IRQ0);
+	local_irq_restore(flags);
+
 	bytes -= count;
 	qc->curbytes += count;
 	qc->cursg_ofs += count;

@@ -2965,14 +2972,6 @@ next_sg:
 		qc->cursg_ofs = 0;
 	}
 
-	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
-
-	/* do the actual data transfer */
-	ata_data_xfer(ap, buf, count, do_write);
-
-	kunmap_atomic(buf - offset, KM_IRQ0);
-	local_irq_restore(flags);
-
 	if (bytes)
 		goto next_sg;
 }

Albert Lee

2005-10-06 10:42 UTC #3 lore raw

PATCH 2/3 irq-pio: reorganize "buf + offset"

In reply to Albert Lee

Patch 2/3:
reorganize "buf + offset" in ata_pio_sector() and __atapi_pio_bytes()

Changes:
- relocate DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
- buf + offset, buf - offset tidy up

For your review, thanks.

Albert

Signed-off-by Albert Lee <[email protected]>

===========

drivers/scsi/libata-core.c +10 ~~-10~~

@@ -2760,15 +2760,15 @@ static void ata_pio_sector(struct ata_qu
 	page = nth_page(page, (offset >> PAGE_SHIFT));
 	offset %= PAGE_SIZE;
 
-	local_irq_save(flags);
-	buf = kmap_atomic(page, KM_IRQ0) + offset;
-
 	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
 
+	local_irq_save(flags);
+	buf = kmap_atomic(page, KM_IRQ0);
+
 	/* do the actual data transfer */
-	ata_data_xfer(ap, buf, ATA_SECT_SIZE, do_write);
+	ata_data_xfer(ap, buf + offset, ATA_SECT_SIZE, do_write);
 
-	kunmap_atomic(buf - offset, KM_IRQ0);
+	kunmap_atomic(buf, KM_IRQ0);
 	local_irq_restore(flags);
 
 	qc->cursect++;

@@ -2952,15 +2952,15 @@ next_sg:
 	/* don't cross page boundaries */
 	count = min(count, (unsigned int)PAGE_SIZE - offset);
 
-	local_irq_save(flags);
-	buf = kmap_atomic(page, KM_IRQ0) + offset;
-
 	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
 
+	local_irq_save(flags);
+	buf = kmap_atomic(page, KM_IRQ0);
+
 	/* do the actual data transfer */
-	ata_data_xfer(ap, buf, count, do_write);
+	ata_data_xfer(ap, buf + offset, count, do_write);
 
-	kunmap_atomic(buf - offset, KM_IRQ0);
+	kunmap_atomic(buf, KM_IRQ0);
 	local_irq_restore(flags);
 
 	bytes -= count;

Albert Lee

2005-10-06 10:44 UTC #4 lore raw

PATCH 3/3 irq-pio: reorganize "buf + offset"

In reply to Albert Lee

Patch 3/3:
use PageHighMem() to optimize the kmap_atomic() usage, as done in ide-scsi.c

For your review, thanks.

Albert

Signed-off-by Albert Lee <[email protected]>

==========

drivers/scsi/libata-core.c +26 ~~-14~~

@@ -2748,7 +2748,6 @@ static void ata_pio_sector(struct ata_qu
 	struct page *page;
 	unsigned int offset;
 	unsigned char *buf;
-	unsigned long flags;
 
 	if (qc->cursect == (qc->nsect - 1))
 		ap->hsm_task_state = HSM_ST_LAST;

@@ -2762,14 +2761,21 @@ static void ata_pio_sector(struct ata_qu
 
 	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
 
-	local_irq_save(flags);
-	buf = kmap_atomic(page, KM_IRQ0);
+	if (PageHighMem(page)) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		buf = kmap_atomic(page, KM_IRQ0);
 
-	/* do the actual data transfer */
-	ata_data_xfer(ap, buf + offset, ATA_SECT_SIZE, do_write);
+		/* do the actual data transfer */
+		ata_data_xfer(ap, buf + offset, ATA_SECT_SIZE, do_write);
 
-	kunmap_atomic(buf, KM_IRQ0);
-	local_irq_restore(flags);
+		kunmap_atomic(buf, KM_IRQ0);
+		local_irq_restore(flags);
+	} else {
+		buf = page_address(page);
+		ata_data_xfer(ap, buf + offset, ATA_SECT_SIZE, do_write);
+	}
 
 	qc->cursect++;
 	qc->cursg_ofs++;

@@ -2908,7 +2914,6 @@ static void __atapi_pio_bytes(struct ata
 	struct page *page;
 	unsigned char *buf;
 	unsigned int offset, count;
-	unsigned long flags;
 
 	if (qc->curbytes + bytes >= qc->nbytes)
 		ap->hsm_task_state = HSM_ST_LAST;

@@ -2954,14 +2959,21 @@ next_sg:
 
 	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
 
-	local_irq_save(flags);
-	buf = kmap_atomic(page, KM_IRQ0);
+	if (PageHighMem(page)) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		buf = kmap_atomic(page, KM_IRQ0);
 
-	/* do the actual data transfer */
-	ata_data_xfer(ap, buf + offset, count, do_write);
+		/* do the actual data transfer */
+		ata_data_xfer(ap, buf + offset, count, do_write);
 
-	kunmap_atomic(buf, KM_IRQ0);
-	local_irq_restore(flags);
+		kunmap_atomic(buf, KM_IRQ0);
+		local_irq_restore(flags);
+	} else {
+		buf = page_address(page);
+		ata_data_xfer(ap, buf + offset, count, do_write);
+	}
 
 	bytes -= count;
 	qc->curbytes += count;

Jeff Garzik

author

2005-10-06 11:21 UTC #5 lore raw

In reply to Albert Lee

Quoted 5 lines · Albert Lee

Thanks for the advice. The kmap_atomic() usage optimization patches are ready:
patch 1/3: reorganize ata_pio_sector() and __atapi_pio_bytes() patch 2/3: reorganize "buf + offset"
patch 3/3: use PageHighMem() to optimize the kmap_atomic() usage

Thanks for the patches. All three patches look OK, but did not apply (probably due to mailer problems):

Quoted 1 line

[jgarzik@pretzel libata-dev]$ patch -sp1 < /g/tmp/mbox 3 out of 3 hunks FAILED -- saving rejects to file drivers/scsi/libata-core.c.rej

FWIW the most reliable email solution is to use a script to generate an RFC822 header and email message:

To: [email protected] (Jeff Garzik)
CC: [email protected] (Linux IDE mailing list)
Subject: [patch 1/3] irq-pio: ...
[blank line, as required by RFC822]
[patch description]
[patch]

and then send that to '/usr/bin/sendmail -t < msg.txt' for delivery.

Jeff

Albert Lee

2005-10-07 06:01 UTC #6 lore raw

Quoted 5 lines · Jeff Garzik

Quoted 3 lines · Albert Lee
Thanks for the advice. The kmap_atomic() usage optimization patches are ready:

Quoted 12 lines

Quoted 3 lines
patch 1/3: reorganize ata_pio_sector() and __atapi_pio_bytes() patch 2/3: reorganize "buf + offset"
patch 3/3: use PageHighMem() to optimize the kmap_atomic() usage
Thanks for the patches. All three patches look OK, but did not apply (probably due to mailer problems):
Quoted 2 lines
[jgarzik@pretzel libata-dev]$ patch -sp1 < /g/tmp/mbox 3 out of 3 hunks FAILED -- saving rejects to file drivers/scsi/libata-core.c.rej

Sorry for the mangled patch. :(

It seems Thunderbird add an extra space to the beginning of the line. e.g.
[space][tab]local_irq_save(flags);
was changed to
[space][space][tab]local_irq_save(flags);

I've reconfigured the Thunderbird according to

http://www.gelato.unsw.edu.au/archives/git/0508/7750.html

Will resubmit the patches later.

Albert

Albert Lee

2005-10-07 06:08 UTC #7 lore raw

cover libata-dev: use PageHighMem() to optimize kmap_atomic() usage

In reply to Jeff Garzik

(Resend due to white space mangled in the previous patches)

patch 1/3: reorganize ata_pio_sector() and __atapi_pio_bytes() patch 2/3: reorganize "buf + offset"
patch 3/3: use PageHighMem() to optimize the kmap_atomic() usage

Patch against libata-dev irq-pio branch (c71c18576d0d8aa4db876c737c3c597c724cf02f). Thanks,

Albert

Albert Lee

2005-10-07 06:12 UTC #8 lore raw

PATCH 1/3 irq-pio: reorganize ata_pio_sector() and __atapi_pio_bytes()

In reply to Jeff Garzik

Patch 1/3:
reorganize ata_pio_sector() and __atapi_pio_bytes()

Changes:
- move some code out of the kmap_atomic() / kunmap_atomic() zone
- remove the redundant "do_write = (qc->tf.flags & ATA_TFLAG_WRITE);"

Signed-off-by Albert Lee <[email protected]>

===========

drivers/scsi/libata-core.c +16 ~~-17~~

@@ -2763,22 +2763,21 @@ static void ata_pio_sector(struct ata_qu
 	local_irq_save(flags);
 	buf = kmap_atomic(page, KM_IRQ0) + offset;
 
-	qc->cursect++;
-	qc->cursg_ofs++;
-
-	if ((qc->cursg_ofs * ATA_SECT_SIZE) == (&sg[qc->cursg])->length) {
-		qc->cursg++;
-		qc->cursg_ofs = 0;
-	}
-
 	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
 
 	/* do the actual data transfer */
-	do_write = (qc->tf.flags & ATA_TFLAG_WRITE);
 	ata_data_xfer(ap, buf, ATA_SECT_SIZE, do_write);
 
 	kunmap_atomic(buf - offset, KM_IRQ0);
 	local_irq_restore(flags);
+
+	qc->cursect++;
+	qc->cursg_ofs++;
+
+	if ((qc->cursg_ofs * ATA_SECT_SIZE) == (&sg[qc->cursg])->length) {
+		qc->cursg++;
+		qc->cursg_ofs = 0;
+	}
 }
 
 /**

@@ -2956,6 +2955,14 @@ next_sg:
 	local_irq_save(flags);
 	buf = kmap_atomic(page, KM_IRQ0) + offset;
 
+	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
+
+	/* do the actual data transfer */
+	ata_data_xfer(ap, buf, count, do_write);
+
+	kunmap_atomic(buf - offset, KM_IRQ0);
+	local_irq_restore(flags);
+
 	bytes -= count;
 	qc->curbytes += count;
 	qc->cursg_ofs += count;

@@ -2965,14 +2972,6 @@ next_sg:
 		qc->cursg_ofs = 0;
 	}
 
-	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
-
-	/* do the actual data transfer */
-	ata_data_xfer(ap, buf, count, do_write);
-
-	kunmap_atomic(buf - offset, KM_IRQ0);
-	local_irq_restore(flags);
-
 	if (bytes)
 		goto next_sg;
 }

Albert Lee

2005-10-07 06:19 UTC #9 lore raw

PATCH 2/3 irq-pio: reorganize "buf + offset"

In reply to Jeff Garzik

(Resend)
Patch 2/3:
reorganize "buf + offset" in ata_pio_sector() and __atapi_pio_bytes()

Changes:
- relocate DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
- buf + offset, buf - offset tidy up

Signed-off-by Albert Lee <[email protected]>

===========

drivers/scsi/libata-core.c +10 ~~-10~~

@@ -2760,15 +2760,15 @@ static void ata_pio_sector(struct ata_qu
 	page = nth_page(page, (offset >> PAGE_SHIFT));
 	offset %= PAGE_SIZE;
 
-	local_irq_save(flags);
-	buf = kmap_atomic(page, KM_IRQ0) + offset;
-
 	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
 
+	local_irq_save(flags);
+	buf = kmap_atomic(page, KM_IRQ0);
+
 	/* do the actual data transfer */
-	ata_data_xfer(ap, buf, ATA_SECT_SIZE, do_write);
+	ata_data_xfer(ap, buf + offset, ATA_SECT_SIZE, do_write);
 
-	kunmap_atomic(buf - offset, KM_IRQ0);
+	kunmap_atomic(buf, KM_IRQ0);
 	local_irq_restore(flags);
 
 	qc->cursect++;

@@ -2952,15 +2952,15 @@ next_sg:
 	/* don't cross page boundaries */
 	count = min(count, (unsigned int)PAGE_SIZE - offset);
 
-	local_irq_save(flags);
-	buf = kmap_atomic(page, KM_IRQ0) + offset;
-
 	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
 
+	local_irq_save(flags);
+	buf = kmap_atomic(page, KM_IRQ0);
+
 	/* do the actual data transfer */
-	ata_data_xfer(ap, buf, count, do_write);
+	ata_data_xfer(ap, buf + offset, count, do_write);
 
-	kunmap_atomic(buf - offset, KM_IRQ0);
+	kunmap_atomic(buf, KM_IRQ0);
 	local_irq_restore(flags);
 
 	bytes -= count;

Albert Lee

2005-10-07 06:21 UTC #10 lore raw

PATCH 3/3 irq-pio: use PageHighMem() to optimize the kmap_atomic() usage

In reply to Jeff Garzik

(resend)
Patch 3/3:
use PageHighMem() to optimize the kmap_atomic() usage, as done in ide-scsi.c

Signed-off-by Albert Lee <[email protected]>

==========

drivers/scsi/libata-core.c +26 ~~-14~~

@@ -2748,7 +2748,6 @@ static void ata_pio_sector(struct ata_qu
 	struct page *page;
 	unsigned int offset;
 	unsigned char *buf;
-	unsigned long flags;
 
 	if (qc->cursect == (qc->nsect - 1))
 		ap->hsm_task_state = HSM_ST_LAST;

@@ -2762,14 +2761,21 @@ static void ata_pio_sector(struct ata_qu
 
 	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
 
-	local_irq_save(flags);
-	buf = kmap_atomic(page, KM_IRQ0);
+	if (PageHighMem(page)) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		buf = kmap_atomic(page, KM_IRQ0);
 
-	/* do the actual data transfer */
-	ata_data_xfer(ap, buf + offset, ATA_SECT_SIZE, do_write);
+		/* do the actual data transfer */
+		ata_data_xfer(ap, buf + offset, ATA_SECT_SIZE, do_write);
 
-	kunmap_atomic(buf, KM_IRQ0);
-	local_irq_restore(flags);
+		kunmap_atomic(buf, KM_IRQ0);
+		local_irq_restore(flags);
+	} else {
+		buf = page_address(page);
+		ata_data_xfer(ap, buf + offset, ATA_SECT_SIZE, do_write);
+	}
 
 	qc->cursect++;
 	qc->cursg_ofs++;

@@ -2908,7 +2914,6 @@ static void __atapi_pio_bytes(struct ata
 	struct page *page;
 	unsigned char *buf;
 	unsigned int offset, count;
-	unsigned long flags;
 
 	if (qc->curbytes + bytes >= qc->nbytes)
 		ap->hsm_task_state = HSM_ST_LAST;

@@ -2954,14 +2959,21 @@ next_sg:
 
 	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
 
-	local_irq_save(flags);
-	buf = kmap_atomic(page, KM_IRQ0);
+	if (PageHighMem(page)) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		buf = kmap_atomic(page, KM_IRQ0);
 
-	/* do the actual data transfer */
-	ata_data_xfer(ap, buf + offset, count, do_write);
+		/* do the actual data transfer */
+		ata_data_xfer(ap, buf + offset, count, do_write);
 
-	kunmap_atomic(buf, KM_IRQ0);
-	local_irq_restore(flags);
+		kunmap_atomic(buf, KM_IRQ0);
+		local_irq_restore(flags);
+	} else {
+		buf = page_address(page);
+		ata_data_xfer(ap, buf + offset, count, do_write);
+	}
 
 	bytes -= count;
 	qc->curbytes += count;

Jeff Garzik

author

2005-10-09 13:49 UTC #11 lore raw

Re: [PATCH 3/3] irq-pio: use PageHighMem() to optimize the kmap_atomic() usage

applied patches 1-3, after updating the irq-pio branch to the latest upstream branch.