mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ata: libata-sff: replace kmap_atomic() with kmap_local_page()
@ 2026-09-21 18:24 Danish Khateeb
  2026-09-22  3:15 ` Damien Le Moal
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Danish Khateeb @ 2026-09-21 18:24 UTC (permalink / raw)
  To: Damien Le Moal, Niklas Cassel; +Cc: linux-ide, linux-kernel, Danish Khateeb

kmap_atomic() is deprecated in favour of kmap_local_page(), as described
in Documentation/mm/highmem.rst.

ata_pio_xfer() and __atapi_pio_bytes() each map a single page, pass the
address to ->sff_data_xfer() for one transfer and unmap it again, so the
mapping never leaves the function that created it.

Neither needs the pagefault_disable() and preempt_disable() implied by
kmap_atomic(). ->sff_data_xfer() only moves data between the device and
the mapped buffer, and both functions are reached only through
ata_sff_hsm_move() with the host lock held, whether from the PIO task,
an interrupt handler or EH. kmap_local_page() is fine in all of these.

Tested on i386 with CONFIG_HIGHMEM4G and CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP
under QEMU, with the PIIX IDE controller limited to PIO by
libata.force=pio4: O_DIRECT and buffered reads from an ATA disk and an
ATAPI CD-ROM, and O_DIRECT writes to the disk, all checked against the
image files. The O_DIRECT buffers were in highmem. Both call sites ran
in hardirq context, and ata_pio_xfer() also from the PIO task. No
warnings with lockdep and CONFIG_DEBUG_ATOMIC_SLEEP enabled.

Assisted-by: LLM sparse
Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---
 drivers/ata/libata-sff.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index d5ef0338735c..a263f886abc7 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -580,9 +580,9 @@ static void ata_pio_xfer(struct ata_queued_cmd *qc, struct page *page,
 	bool do_write = (qc->tf.flags & ATA_TFLAG_WRITE);
 	unsigned char *buf;
 
-	buf = kmap_atomic(page);
+	buf = kmap_local_page(page);
 	qc->ap->ops->sff_data_xfer(qc, buf + offset, xfer_size, do_write);
-	kunmap_atomic(buf);
+	kunmap_local(buf);
 
 	if (!do_write && !PageSlab(page))
 		flush_dcache_page(page);
@@ -763,9 +763,9 @@ static int __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes)
 	trace_atapi_pio_transfer_data(qc, offset, count);
 
 	/* do the actual data transfer */
-	buf = kmap_atomic(page);
+	buf = kmap_local_page(page);
 	consumed = ap->ops->sff_data_xfer(qc, buf + offset, count, rw);
-	kunmap_atomic(buf);
+	kunmap_local(buf);
 
 	bytes -= min(bytes, consumed);
 	qc->curbytes += count;

base-commit: 3bab8c7e0c6ba187a47163a1f71477213a278c48
-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ata: libata-sff: replace kmap_atomic() with kmap_local_page()
  2026-09-21 18:24 [PATCH] ata: libata-sff: replace kmap_atomic() with kmap_local_page() Danish Khateeb
@ 2026-09-22  3:15 ` Damien Le Moal
  2026-09-22 13:02 ` Niklas Cassel
  2026-09-22 13:04 ` Niklas Cassel
  2 siblings, 0 replies; 4+ messages in thread
From: Damien Le Moal @ 2026-09-22  3:15 UTC (permalink / raw)
  To: Danish Khateeb, Niklas Cassel; +Cc: linux-ide, linux-kernel

On 9/22/26 03:24, Danish Khateeb wrote:
> kmap_atomic() is deprecated in favour of kmap_local_page(), as described
> in Documentation/mm/highmem.rst.
> 
> ata_pio_xfer() and __atapi_pio_bytes() each map a single page, pass the
> address to ->sff_data_xfer() for one transfer and unmap it again, so the
> mapping never leaves the function that created it.
> 
> Neither needs the pagefault_disable() and preempt_disable() implied by
> kmap_atomic(). ->sff_data_xfer() only moves data between the device and
> the mapped buffer, and both functions are reached only through
> ata_sff_hsm_move() with the host lock held, whether from the PIO task,
> an interrupt handler or EH. kmap_local_page() is fine in all of these.
> 
> Tested on i386 with CONFIG_HIGHMEM4G and CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP
> under QEMU, with the PIIX IDE controller limited to PIO by
> libata.force=pio4: O_DIRECT and buffered reads from an ATA disk and an
> ATAPI CD-ROM, and O_DIRECT writes to the disk, all checked against the
> image files. The O_DIRECT buffers were in highmem. Both call sites ran
> in hardirq context, and ata_pio_xfer() also from the PIO task. No
> warnings with lockdep and CONFIG_DEBUG_ATOMIC_SLEEP enabled.
> 
> Assisted-by: LLM sparse
> Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>

Looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ata: libata-sff: replace kmap_atomic() with kmap_local_page()
  2026-09-21 18:24 [PATCH] ata: libata-sff: replace kmap_atomic() with kmap_local_page() Danish Khateeb
  2026-09-22  3:15 ` Damien Le Moal
@ 2026-09-22 13:02 ` Niklas Cassel
  2026-09-22 13:04 ` Niklas Cassel
  2 siblings, 0 replies; 4+ messages in thread
From: Niklas Cassel @ 2026-09-22 13:02 UTC (permalink / raw)
  To: Danish Khateeb; +Cc: Damien Le Moal, linux-ide, linux-kernel

On Mon, Sep 21, 2026 at 01:24:48PM -0500, Danish Khateeb wrote:
> kmap_atomic() is deprecated in favour of kmap_local_page(), as described
> in Documentation/mm/highmem.rst.
> 
> ata_pio_xfer() and __atapi_pio_bytes() each map a single page, pass the
> address to ->sff_data_xfer() for one transfer and unmap it again, so the
> mapping never leaves the function that created it.
> 
> Neither needs the pagefault_disable() and preempt_disable() implied by
> kmap_atomic(). ->sff_data_xfer() only moves data between the device and
> the mapped buffer, and both functions are reached only through
> ata_sff_hsm_move() with the host lock held, whether from the PIO task,
> an interrupt handler or EH. kmap_local_page() is fine in all of these.

> 
> Tested on i386 with CONFIG_HIGHMEM4G and CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP
> under QEMU, with the PIIX IDE controller limited to PIO by
> libata.force=pio4: O_DIRECT and buffered reads from an ATA disk and an
> ATAPI CD-ROM, and O_DIRECT writes to the disk, all checked against the
> image files. The O_DIRECT buffers were in highmem. Both call sites ran
> in hardirq context, and ata_pio_xfer() also from the PIO task. No
> warnings with lockdep and CONFIG_DEBUG_ATOMIC_SLEEP enabled.

Testing instructions are usually written after "---", or in the cover-letter,
so that they are not picked up by git am.

No need to send a v2, I will fix up when applying.

When applying with b4 -sl, which adds a Link: tag, the patch, including
additional notes (and cover-letter etc) can still easily be found.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ata: libata-sff: replace kmap_atomic() with kmap_local_page()
  2026-09-21 18:24 [PATCH] ata: libata-sff: replace kmap_atomic() with kmap_local_page() Danish Khateeb
  2026-09-22  3:15 ` Damien Le Moal
  2026-09-22 13:02 ` Niklas Cassel
@ 2026-09-22 13:04 ` Niklas Cassel
  2 siblings, 0 replies; 4+ messages in thread
From: Niklas Cassel @ 2026-09-22 13:04 UTC (permalink / raw)
  To: Damien Le Moal, Danish Khateeb; +Cc: linux-ide, linux-kernel

On Mon, 21 Sep 2026 13:24:48 -0500, Danish Khateeb wrote:
> kmap_atomic() is deprecated in favour of kmap_local_page(), as described
> in Documentation/mm/highmem.rst.
> 
> ata_pio_xfer() and __atapi_pio_bytes() each map a single page, pass the
> address to ->sff_data_xfer() for one transfer and unmap it again, so the
> mapping never leaves the function that created it.
> 
> [...]

Applied to libata/linux.git (for-7.4), thanks!

[1/1] ata: libata-sff: replace kmap_atomic() with kmap_local_page()
      https://git.kernel.org/libata/linux/c/02178d4b

Kind regards,
Niklas


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-22 13:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-21 18:24 [PATCH] ata: libata-sff: replace kmap_atomic() with kmap_local_page() Danish Khateeb
2026-09-22  3:15 ` Damien Le Moal
2026-09-22 13:02 ` Niklas Cassel
2026-09-22 13:04 ` Niklas Cassel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®