linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Improve log directory handling and some cleanups
@ 2025-07-03 10:36 Damien Le Moal
  2025-07-03 10:36 ` [PATCH v2 1/4] ata: libata-core: Cache the general purpose log directory Damien Le Moal
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Damien Le Moal @ 2025-07-03 10:36 UTC (permalink / raw)
  To: linux-ide, Niklas Cassel

The first patch improves handling of a device general purpose log
directory log page by avoiding repeated accesses to it using a cache.

The following 3 patches are simple cleanups that do not introduce
functional changes.

Changes from v1:
 - Improved error handling in patch 1
 - Added review tag to patch 2
 - Split former patch 3 into current patch 3 and 4

Damien Le Moal (4):
  ata: libata-core: Cache the general purpose log directory
  ata: libata-core: Make ata_dev_cleanup_cdl_resources() static
  ata: libata-eh: Rename and make ata_set_mode() static
  ata: libata-core: Rename ata_do_set_mode()

 drivers/ata/libata-core.c   | 47 ++++++++++++++++++++++++++++++++-----
 drivers/ata/libata-eh.c     | 11 +++++----
 drivers/ata/libata.h        |  2 --
 drivers/ata/pata_optidma.c  |  4 +++-
 drivers/ata/pata_pcmcia.c   |  4 ++--
 drivers/ata/pata_pdc2027x.c |  2 +-
 drivers/ata/sata_sil.c      |  2 +-
 include/linux/libata.h      |  5 +++-
 8 files changed, 58 insertions(+), 19 deletions(-)

-- 
2.50.0


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

* [PATCH v2 1/4] ata: libata-core: Cache the general purpose log directory
  2025-07-03 10:36 [PATCH v2 0/4] Improve log directory handling and some cleanups Damien Le Moal
@ 2025-07-03 10:36 ` Damien Le Moal
  2025-07-03 10:45   ` Niklas Cassel
  2025-07-03 10:36 ` [PATCH v2 2/4] ata: libata-core: Make ata_dev_cleanup_cdl_resources() static Damien Le Moal
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Damien Le Moal @ 2025-07-03 10:36 UTC (permalink / raw)
  To: linux-ide, Niklas Cassel

The function ata_log_supported() tests if a log page is supported by a
device using the General Purpose Log Directory log page, which lists the
size of all surported log pages. However, this log page is read from the
device using ata_read_log_page() every time ata_log_supported() is
called. That is not necessary.

Avoid reading the General Purpose Log Directory log page by caching its
content in the gp_log_dir buffer defined as part of struct ata_device.
The functions ata_read_log_directory() and ata_clear_log_directory() are
introduced to manage this buffer. ata_clear_log_directory() zero-fill
the gp_log_dir buffer every time ata_dev_configure() is called, that is,
when the device is first scanned and when it is being revalidated.
The function ata_log_supported() is modified to call
ata_read_log_directory() instead of ata_read_log_page().

The function ata_read_log_directory() calls ata_read_log_page() to read
the General Purpose Log Directory log page from the device only if the
first 16-bits word of the log is not equal to 0x0001, that is, it is not
equal to the ACS mandated value for the log version.

With this, the log page is read from the device only once for every
ata_dev_configure() call. For instance, with pr_debug enabled, a call
to ata_dev_configure() before this patch generates the following log
page accesses:

ata3.00: read log page - log 0x0, page 0x0
ata3.00: read log page - log 0x13, page 0x0
ata3.00: read log page - log 0x0, page 0x0
ata3.00: read log page - log 0x12, page 0x0
ata3.00: read log page - log 0x0, page 0x0
ata3.00: read log page - log 0x30, page 0x0
ata3.00: read log page - log 0x30, page 0x8
ata3.00: read log page - log 0x0, page 0x0
ata3.00: read log page - log 0x0, page 0x0
ata3.00: read log page - log 0x0, page 0x0
ata3.00: read log page - log 0x30, page 0x0
ata3.00: read log page - log 0x0, page 0x0
ata3.00: read log page - log 0x30, page 0x0
ata3.00: read log page - log 0x30, page 0x3
ata3.00: read log page - log 0x30, page 0x4
ata3.00: read log page - log 0x18, page 0x0

That is, the general purpose log directory page is read 7 times.
With this patch applied, the number of accesses to this log page is
reduced to one:

ata3.00: read log page - log 0x0, page 0x0
ata3.00: read log page - log 0x13, page 0x0
ata3.00: read log page - log 0x12, page 0x0
ata3.00: read log page - log 0x30, page 0x0
ata3.00: read log page - log 0x30, page 0x8
ata3.00: read log page - log 0x30, page 0x0
ata3.00: read log page - log 0x30, page 0x0
ata3.00: read log page - log 0x30, page 0x3
ata3.00: read log page - log 0x30, page 0x4
ata3.00: read log page - log 0x18, page 0x0

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/ata/libata-core.c | 39 +++++++++++++++++++++++++++++++++++++--
 include/linux/libata.h    |  3 +++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 7f6cebe61b33..30913bc6fe21 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2154,14 +2154,46 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
 	return err_mask;
 }
 
+static inline void ata_clear_log_directory(struct ata_device *dev)
+{
+	memset(dev->gp_log_dir, 0, ATA_SECT_SIZE);
+}
+
+static int ata_read_log_directory(struct ata_device *dev)
+{
+	u16 version;
+
+	/* If the log page is already cached, do nothing. */
+	version = get_unaligned_le16(&dev->gp_log_dir[0]);
+	if (version == 0x0001)
+		return 0;
+
+	if (ata_read_log_page(dev, ATA_LOG_DIRECTORY, 0, dev->gp_log_dir, 1)) {
+		ata_clear_log_directory(dev);
+		return -EIO;
+	}
+
+	version = get_unaligned_le16(&dev->gp_log_dir[0]);
+	if (version != 0x0001) {
+		ata_dev_err(dev, "Invalid log directory version 0x%04x\n",
+			    version);
+		ata_clear_log_directory(dev);
+		dev->quirks |= ATA_QUIRK_NO_LOG_DIR;
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int ata_log_supported(struct ata_device *dev, u8 log)
 {
 	if (dev->quirks & ATA_QUIRK_NO_LOG_DIR)
 		return 0;
 
-	if (ata_read_log_page(dev, ATA_LOG_DIRECTORY, 0, dev->sector_buf, 1))
+	if (ata_read_log_directory(dev))
 		return 0;
-	return get_unaligned_le16(&dev->sector_buf[log * 2]);
+
+	return get_unaligned_le16(&dev->gp_log_dir[log * 2]);
 }
 
 static bool ata_identify_page_supported(struct ata_device *dev, u8 page)
@@ -2890,6 +2922,9 @@ int ata_dev_configure(struct ata_device *dev)
 		return 0;
 	}
 
+	/* Clear the general purpose log directory cache. */
+	ata_clear_log_directory(dev);
+
 	/* Set quirks */
 	dev->quirks |= ata_dev_quirks(dev);
 	ata_force_quirks(dev);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 7462218312ad..78a4addc6659 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -761,6 +761,9 @@ struct ata_device {
 		u32		gscr[SATA_PMP_GSCR_DWORDS]; /* PMP GSCR block */
 	} ____cacheline_aligned;
 
+	/* General Purpose Log Directory log page */
+	u8			gp_log_dir[ATA_SECT_SIZE] ____cacheline_aligned;
+
 	/* DEVSLP Timing Variables from Identify Device Data Log */
 	u8			devslp_timing[ATA_LOG_DEVSLP_SIZE];
 
-- 
2.50.0


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

* [PATCH v2 2/4] ata: libata-core: Make ata_dev_cleanup_cdl_resources() static
  2025-07-03 10:36 [PATCH v2 0/4] Improve log directory handling and some cleanups Damien Le Moal
  2025-07-03 10:36 ` [PATCH v2 1/4] ata: libata-core: Cache the general purpose log directory Damien Le Moal
@ 2025-07-03 10:36 ` Damien Le Moal
  2025-07-03 10:36 ` [PATCH v2 3/4] ata: libata-eh: Rename and make ata_set_mode() static Damien Le Moal
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Damien Le Moal @ 2025-07-03 10:36 UTC (permalink / raw)
  To: linux-ide, Niklas Cassel

The function ata_dev_cleanup_cdl_resources() is called only from
ata_dev_init_cdl_resources() and ata_dev_free_resources() which are
both defined in drivers/ata/libata-core.c. So there is no need for
ata_dev_cleanup_cdl_resources() to be visible from outside of this
file. Change this function definition to be a static function.

No functional changes.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libata-core.c | 2 +-
 drivers/ata/libata.h      | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 30913bc6fe21..72abd2996e9c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2516,7 +2516,7 @@ static void ata_dev_config_trusted(struct ata_device *dev)
 		dev->flags |= ATA_DFLAG_TRUSTED;
 }
 
-void ata_dev_cleanup_cdl_resources(struct ata_device *dev)
+static void ata_dev_cleanup_cdl_resources(struct ata_device *dev)
 {
 	kfree(dev->cdl);
 	dev->cdl = NULL;
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 8e68f4556962..767a61f8ff89 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -97,7 +97,6 @@ extern int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg);
 extern const char *sata_spd_string(unsigned int spd);
 extern unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
 				      u8 page, void *buf, unsigned int sectors);
-void ata_dev_cleanup_cdl_resources(struct ata_device *dev);
 
 #define to_ata_port(d) container_of(d, struct ata_port, tdev)
 
-- 
2.50.0


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

* [PATCH v2 3/4] ata: libata-eh: Rename and make ata_set_mode() static
  2025-07-03 10:36 [PATCH v2 0/4] Improve log directory handling and some cleanups Damien Le Moal
  2025-07-03 10:36 ` [PATCH v2 1/4] ata: libata-core: Cache the general purpose log directory Damien Le Moal
  2025-07-03 10:36 ` [PATCH v2 2/4] ata: libata-core: Make ata_dev_cleanup_cdl_resources() static Damien Le Moal
@ 2025-07-03 10:36 ` Damien Le Moal
  2025-07-03 10:48   ` Niklas Cassel
  2025-07-03 10:36 ` [PATCH v2 4/4] ata: libata-core: Rename ata_do_set_mode() Damien Le Moal
  2025-07-04  8:37 ` [PATCH v2 0/4] Improve log directory handling and some cleanups Niklas Cassel
  4 siblings, 1 reply; 9+ messages in thread
From: Damien Le Moal @ 2025-07-03 10:36 UTC (permalink / raw)
  To: linux-ide, Niklas Cassel

The function ata_set_mode() is defined and used only in libata-eh.c.
Make this function static and rename it ata_eh_set_mode() to make it
clear where its definition is.

No functional changes.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/ata/libata-eh.c | 9 +++++----
 drivers/ata/libata.h    | 1 -
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 0f9c30f9bc1e..8b2a0a56ffe1 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -3408,12 +3408,12 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
 }
 
 /**
- *	ata_set_mode - Program timings and issue SET FEATURES - XFER
+ *	ata_eh_set_mode - Program timings and issue SET FEATURES - XFER
  *	@link: link on which timings will be programmed
  *	@r_failed_dev: out parameter for failed device
  *
  *	Set ATA device disk transfer mode (PIO3, UDMA6, etc.).  If
- *	ata_set_mode() fails, pointer to the failing device is
+ *	ata_eh_set_mode() fails, pointer to the failing device is
  *	returned in @r_failed_dev.
  *
  *	LOCKING:
@@ -3422,7 +3422,8 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
  *	RETURNS:
  *	0 on success, negative errno otherwise
  */
-int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
+static int ata_eh_set_mode(struct ata_link *link,
+			   struct ata_device **r_failed_dev)
 {
 	struct ata_port *ap = link->ap;
 	struct ata_device *dev;
@@ -3926,7 +3927,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
 
 		/* configure transfer mode if necessary */
 		if (ehc->i.flags & ATA_EHI_SETMODE) {
-			rc = ata_set_mode(link, &dev);
+			rc = ata_eh_set_mode(link, &dev);
 			if (rc)
 				goto rest_fail;
 			ehc->i.flags &= ~ATA_EHI_SETMODE;
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 767a61f8ff89..135e1e5ee44e 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -177,7 +177,6 @@ extern void ata_eh_report(struct ata_port *ap);
 extern int ata_eh_reset(struct ata_link *link, int classify,
 			ata_prereset_fn_t prereset, ata_reset_fn_t softreset,
 			ata_reset_fn_t hardreset, ata_postreset_fn_t postreset);
-extern int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev);
 extern int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
 			  ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
 			  ata_postreset_fn_t postreset,
-- 
2.50.0


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

* [PATCH v2 4/4] ata: libata-core: Rename ata_do_set_mode()
  2025-07-03 10:36 [PATCH v2 0/4] Improve log directory handling and some cleanups Damien Le Moal
                   ` (2 preceding siblings ...)
  2025-07-03 10:36 ` [PATCH v2 3/4] ata: libata-eh: Rename and make ata_set_mode() static Damien Le Moal
@ 2025-07-03 10:36 ` Damien Le Moal
  2025-07-03 10:48   ` Niklas Cassel
  2025-07-04  8:37 ` [PATCH v2 0/4] Improve log directory handling and some cleanups Niklas Cassel
  4 siblings, 1 reply; 9+ messages in thread
From: Damien Le Moal @ 2025-07-03 10:36 UTC (permalink / raw)
  To: linux-ide, Niklas Cassel

With the renaming of libata-eh ata_set_mode() function to
ata_eh_set_mode(), libata-core function ata_do_set_mode() can now be
renamed to the simpler ata_set_mode().

All the call sites of the former ata_do_set_mode() are updated to use
the new function name.

No functional changes.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/ata/libata-core.c   | 6 +++---
 drivers/ata/libata-eh.c     | 2 +-
 drivers/ata/pata_optidma.c  | 4 +++-
 drivers/ata/pata_pcmcia.c   | 4 ++--
 drivers/ata/pata_pdc2027x.c | 2 +-
 drivers/ata/sata_sil.c      | 2 +-
 include/linux/libata.h      | 2 +-
 7 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 72abd2996e9c..bbf1318a2b9a 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3510,7 +3510,7 @@ static int ata_dev_set_mode(struct ata_device *dev)
 }
 
 /**
- *	ata_do_set_mode - Program timings and issue SET FEATURES - XFER
+ *	ata_set_mode - Program timings and issue SET FEATURES - XFER
  *	@link: link on which timings will be programmed
  *	@r_failed_dev: out parameter for failed device
  *
@@ -3526,7 +3526,7 @@ static int ata_dev_set_mode(struct ata_device *dev)
  *	0 on success, negative errno otherwise
  */
 
-int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
+int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
 {
 	struct ata_port *ap = link->ap;
 	struct ata_device *dev;
@@ -3607,7 +3607,7 @@ int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
 		*r_failed_dev = dev;
 	return rc;
 }
-EXPORT_SYMBOL_GPL(ata_do_set_mode);
+EXPORT_SYMBOL_GPL(ata_set_mode);
 
 /**
  *	ata_wait_ready - wait for link to become ready
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 8b2a0a56ffe1..e5fa61fb8a59 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -3444,7 +3444,7 @@ static int ata_eh_set_mode(struct ata_link *link,
 	if (ap->ops->set_mode)
 		rc = ap->ops->set_mode(link, r_failed_dev);
 	else
-		rc = ata_do_set_mode(link, r_failed_dev);
+		rc = ata_set_mode(link, r_failed_dev);
 
 	/* if transfer mode has changed, set DUBIOUS_XFER on device */
 	ata_for_each_dev(dev, link, ENABLED) {
diff --git a/drivers/ata/pata_optidma.c b/drivers/ata/pata_optidma.c
index dfc36b4ec9c6..cc876dc7a9d8 100644
--- a/drivers/ata/pata_optidma.c
+++ b/drivers/ata/pata_optidma.c
@@ -322,7 +322,9 @@ static int optidma_set_mode(struct ata_link *link, struct ata_device **r_failed)
 	u8 r;
 	int nybble = 4 * ap->port_no;
 	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
-	int rc  = ata_do_set_mode(link, r_failed);
+	int rc;
+
+	rc = ata_set_mode(link, r_failed);
 	if (rc == 0) {
 		pci_read_config_byte(pdev, 0x43, &r);
 
diff --git a/drivers/ata/pata_pcmcia.c b/drivers/ata/pata_pcmcia.c
index 5b602206c522..cf3810933a27 100644
--- a/drivers/ata/pata_pcmcia.c
+++ b/drivers/ata/pata_pcmcia.c
@@ -46,7 +46,7 @@ static int pcmcia_set_mode(struct ata_link *link, struct ata_device **r_failed_d
 	struct ata_device *slave = &link->device[1];
 
 	if (!ata_dev_enabled(master) || !ata_dev_enabled(slave))
-		return ata_do_set_mode(link, r_failed_dev);
+		return ata_set_mode(link, r_failed_dev);
 
 	if (memcmp(master->id + ATA_ID_FW_REV,  slave->id + ATA_ID_FW_REV,
 			   ATA_ID_FW_REV_LEN + ATA_ID_PROD_LEN) == 0) {
@@ -58,7 +58,7 @@ static int pcmcia_set_mode(struct ata_link *link, struct ata_device **r_failed_d
 			ata_dev_disable(slave);
 		}
 	}
-	return ata_do_set_mode(link, r_failed_dev);
+	return ata_set_mode(link, r_failed_dev);
 }
 
 /**
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
index 6820c5597b14..a4ee3b92c9aa 100644
--- a/drivers/ata/pata_pdc2027x.c
+++ b/drivers/ata/pata_pdc2027x.c
@@ -387,7 +387,7 @@ static int pdc2027x_set_mode(struct ata_link *link, struct ata_device **r_failed
 	struct ata_device *dev;
 	int rc;
 
-	rc = ata_do_set_mode(link, r_failed);
+	rc = ata_set_mode(link, r_failed);
 	if (rc < 0)
 		return rc;
 
diff --git a/drivers/ata/sata_sil.c b/drivers/ata/sata_sil.c
index 3a99f66198a9..1b6dc950a42a 100644
--- a/drivers/ata/sata_sil.c
+++ b/drivers/ata/sata_sil.c
@@ -351,7 +351,7 @@ static int sil_set_mode(struct ata_link *link, struct ata_device **r_failed)
 	u32 tmp, dev_mode[2] = { };
 	int rc;
 
-	rc = ata_do_set_mode(link, r_failed);
+	rc = ata_set_mode(link, r_failed);
 	if (rc)
 		return rc;
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 78a4addc6659..d092747be588 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1218,7 +1218,7 @@ extern int ata_ncq_prio_enabled(struct ata_port *ap, struct scsi_device *sdev,
 extern int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev,
 			       bool enable);
 extern struct ata_device *ata_dev_pair(struct ata_device *adev);
-extern int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev);
+int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev);
 extern void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap);
 extern void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap, struct list_head *eh_q);
 
-- 
2.50.0


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

* Re: [PATCH v2 1/4] ata: libata-core: Cache the general purpose log directory
  2025-07-03 10:36 ` [PATCH v2 1/4] ata: libata-core: Cache the general purpose log directory Damien Le Moal
@ 2025-07-03 10:45   ` Niklas Cassel
  0 siblings, 0 replies; 9+ messages in thread
From: Niklas Cassel @ 2025-07-03 10:45 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: linux-ide

On Thu, Jul 03, 2025 at 07:36:19PM +0900, Damien Le Moal wrote:
> The function ata_log_supported() tests if a log page is supported by a
> device using the General Purpose Log Directory log page, which lists the
> size of all surported log pages. However, this log page is read from the
> device using ata_read_log_page() every time ata_log_supported() is
> called. That is not necessary.
> 
> Avoid reading the General Purpose Log Directory log page by caching its
> content in the gp_log_dir buffer defined as part of struct ata_device.
> The functions ata_read_log_directory() and ata_clear_log_directory() are
> introduced to manage this buffer. ata_clear_log_directory() zero-fill
> the gp_log_dir buffer every time ata_dev_configure() is called, that is,
> when the device is first scanned and when it is being revalidated.
> The function ata_log_supported() is modified to call
> ata_read_log_directory() instead of ata_read_log_page().
> 
> The function ata_read_log_directory() calls ata_read_log_page() to read
> the General Purpose Log Directory log page from the device only if the
> first 16-bits word of the log is not equal to 0x0001, that is, it is not
> equal to the ACS mandated value for the log version.
> 
> With this, the log page is read from the device only once for every
> ata_dev_configure() call. For instance, with pr_debug enabled, a call
> to ata_dev_configure() before this patch generates the following log
> page accesses:
> 
> ata3.00: read log page - log 0x0, page 0x0
> ata3.00: read log page - log 0x13, page 0x0
> ata3.00: read log page - log 0x0, page 0x0
> ata3.00: read log page - log 0x12, page 0x0
> ata3.00: read log page - log 0x0, page 0x0
> ata3.00: read log page - log 0x30, page 0x0
> ata3.00: read log page - log 0x30, page 0x8
> ata3.00: read log page - log 0x0, page 0x0
> ata3.00: read log page - log 0x0, page 0x0
> ata3.00: read log page - log 0x0, page 0x0
> ata3.00: read log page - log 0x30, page 0x0
> ata3.00: read log page - log 0x0, page 0x0
> ata3.00: read log page - log 0x30, page 0x0
> ata3.00: read log page - log 0x30, page 0x3
> ata3.00: read log page - log 0x30, page 0x4
> ata3.00: read log page - log 0x18, page 0x0
> 
> That is, the general purpose log directory page is read 7 times.
> With this patch applied, the number of accesses to this log page is
> reduced to one:
> 
> ata3.00: read log page - log 0x0, page 0x0
> ata3.00: read log page - log 0x13, page 0x0
> ata3.00: read log page - log 0x12, page 0x0
> ata3.00: read log page - log 0x30, page 0x0
> ata3.00: read log page - log 0x30, page 0x8
> ata3.00: read log page - log 0x30, page 0x0
> ata3.00: read log page - log 0x30, page 0x0
> ata3.00: read log page - log 0x30, page 0x3
> ata3.00: read log page - log 0x30, page 0x4
> ata3.00: read log page - log 0x18, page 0x0
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>

Reviewed-by: Niklas Cassel <cassel@kernel.org>

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

* Re: [PATCH v2 4/4] ata: libata-core: Rename ata_do_set_mode()
  2025-07-03 10:36 ` [PATCH v2 4/4] ata: libata-core: Rename ata_do_set_mode() Damien Le Moal
@ 2025-07-03 10:48   ` Niklas Cassel
  0 siblings, 0 replies; 9+ messages in thread
From: Niklas Cassel @ 2025-07-03 10:48 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: linux-ide

On Thu, Jul 03, 2025 at 07:36:22PM +0900, Damien Le Moal wrote:
> With the renaming of libata-eh ata_set_mode() function to
> ata_eh_set_mode(), libata-core function ata_do_set_mode() can now be
> renamed to the simpler ata_set_mode().
> 
> All the call sites of the former ata_do_set_mode() are updated to use
> the new function name.
> 
> No functional changes.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>

Reviewed-by: Niklas Cassel <cassel@kernel.org>

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

* Re: [PATCH v2 3/4] ata: libata-eh: Rename and make ata_set_mode() static
  2025-07-03 10:36 ` [PATCH v2 3/4] ata: libata-eh: Rename and make ata_set_mode() static Damien Le Moal
@ 2025-07-03 10:48   ` Niklas Cassel
  0 siblings, 0 replies; 9+ messages in thread
From: Niklas Cassel @ 2025-07-03 10:48 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: linux-ide

On Thu, Jul 03, 2025 at 07:36:21PM +0900, Damien Le Moal wrote:
> The function ata_set_mode() is defined and used only in libata-eh.c.
> Make this function static and rename it ata_eh_set_mode() to make it
> clear where its definition is.
> 
> No functional changes.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>

Reviewed-by: Niklas Cassel <cassel@kernel.org>

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

* Re: [PATCH v2 0/4] Improve log directory handling and some cleanups
  2025-07-03 10:36 [PATCH v2 0/4] Improve log directory handling and some cleanups Damien Le Moal
                   ` (3 preceding siblings ...)
  2025-07-03 10:36 ` [PATCH v2 4/4] ata: libata-core: Rename ata_do_set_mode() Damien Le Moal
@ 2025-07-04  8:37 ` Niklas Cassel
  4 siblings, 0 replies; 9+ messages in thread
From: Niklas Cassel @ 2025-07-04  8:37 UTC (permalink / raw)
  To: linux-ide, Damien Le Moal

On Thu, 03 Jul 2025 19:36:18 +0900, Damien Le Moal wrote:
> The first patch improves handling of a device general purpose log
> directory log page by avoiding repeated accesses to it using a cache.
> 
> The following 3 patches are simple cleanups that do not introduce
> functional changes.
> 
> Changes from v1:
>  - Improved error handling in patch 1
>  - Added review tag to patch 2
>  - Split former patch 3 into current patch 3 and 4
> 
> [...]

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

[1/4] ata: libata-core: Cache the general purpose log directory
      https://git.kernel.org/libata/linux/c/6d4405b1
[2/4] ata: libata-core: Make ata_dev_cleanup_cdl_resources() static
      https://git.kernel.org/libata/linux/c/6cbd9896
[3/4] ata: libata-eh: Rename and make ata_set_mode() static
      https://git.kernel.org/libata/linux/c/6ba4d05c
[4/4] ata: libata-core: Rename ata_do_set_mode()
      https://git.kernel.org/libata/linux/c/31921e87

Kind regards,
Niklas


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

end of thread, other threads:[~2025-07-04  8:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-03 10:36 [PATCH v2 0/4] Improve log directory handling and some cleanups Damien Le Moal
2025-07-03 10:36 ` [PATCH v2 1/4] ata: libata-core: Cache the general purpose log directory Damien Le Moal
2025-07-03 10:45   ` Niklas Cassel
2025-07-03 10:36 ` [PATCH v2 2/4] ata: libata-core: Make ata_dev_cleanup_cdl_resources() static Damien Le Moal
2025-07-03 10:36 ` [PATCH v2 3/4] ata: libata-eh: Rename and make ata_set_mode() static Damien Le Moal
2025-07-03 10:48   ` Niklas Cassel
2025-07-03 10:36 ` [PATCH v2 4/4] ata: libata-core: Rename ata_do_set_mode() Damien Le Moal
2025-07-03 10:48   ` Niklas Cassel
2025-07-04  8:37 ` [PATCH v2 0/4] Improve log directory handling and some cleanups Niklas Cassel

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).