linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 00/11] libata cleanups and improvements
@ 2021-08-16  1:44 Damien Le Moal
  2021-08-16  1:44 ` [PATCH v7 01/11] libata: fix ata_host_alloc_pinfo() Damien Le Moal
                   ` (12 more replies)
  0 siblings, 13 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

The first three patches of this series fix sparse and kernel bot
warnings (potential NULL pointer dereference and locking imbalance).

The following three patches cleanup libata-core code in the area of
device configuration (ata_dev_configure() function).

Patch 7 improves ata_read_log_page() to avoid unnecessary warning
messages and patch 8 adds an informational message on device scan to
advertize the features supported by a device.

Path 9 adds the new sysfs ahci device attribute ncq_prio_supported to
indicate that a disk supports NCQ priority.

Patch 10 and 11 update the ABI user documentation files.

Changes from v6:
* Added patch 10 and 11

Changes from v5:
* Updated patch 9 to include adding the new ncq_prio_supported sysfs
  attribute for SATA adapters other than AHCI. Changed the patch title
  and commit message accordingly.

Changes from v4:
* Fixed patch 1 to avoid an out-of-bounds array access
* Changed title of patch 3 to describe the change (as opposed to only
  mentioning the tool that found the problem)
* Removed patch 10 from this series as Martin took it through the scsi
  tree
* Added reviewed-by tags

Changes from v3:
* Reworked patch 1
* Added patch 3 to fix a sparse warning discovered while checking
  patch 1 & 2
* Added reviewed-by tags

Changes from v2:
* Reworked patch 4 to avoid the need for an additional on-stack string
  for device information messages
* Added reviewed-by tags

Changes from v1:
* Added patch 1 and 2 to fix problems reported by the kernel test robot
* Use strscpy() instead of strcpy in patch 4
* Use sysfs_emit in patch 8 and 9 as suggested by Bart
* Fix typos in comments of the new sas_ncq_prio_supported attribute in
  patch 9

Damien Le Moal (10):
  libata: fix ata_host_alloc_pinfo()
  libata: fix ata_host_start()
  libata: simplify ata_scsi_rbuf_fill()
  libata: cleanup device sleep capability detection
  libata: cleanup ata_dev_configure()
  libata: cleanup NCQ priority handling
  libata: fix ata_read_log_page() warning
  libata: print feature list on device scan
  libata: Introduce ncq_prio_supported sysfs sttribute
  docs: sysfs-block-device: document ncq_prio_supported

Niklas Cassel (1):
  docs: sysfs-block-device: improve ncq_prio_enable documentation

 Documentation/ABI/testing/sysfs-block-device |  43 ++-
 drivers/ata/libahci.c                        |   1 +
 drivers/ata/libata-core.c                    | 290 ++++++++++---------
 drivers/ata/libata-sata.c                    |  62 ++--
 drivers/ata/libata-scsi.c                    |  60 +---
 include/linux/libata.h                       |   5 +
 6 files changed, 252 insertions(+), 209 deletions(-)

-- 
2.31.1


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

* [PATCH v7 01/11] libata: fix ata_host_alloc_pinfo()
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
@ 2021-08-16  1:44 ` Damien Le Moal
  2021-08-16  5:53   ` Hannes Reinecke
  2021-08-16 11:29   ` James Bottomley
  2021-08-16  1:44 ` [PATCH v7 02/11] libata: fix ata_host_start() Damien Le Moal
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

Avoid static checkers warnings about a potential NULL pointer
dereference for the port info variable pi. To do so, test that at least
one port info is available on entry to ata_host_alloc_pinfo() and start
the ata port initialization for() loop with pi initialized to the first
port info passed as argument (which is already checked to be non NULL).
Within the for() loop, get the next port info, if it is not NULL,
after initializing the ata port using the previous port info.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/ata/libata-core.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 61c762961ca8..b237a718ea0f 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5441,16 +5441,17 @@ struct ata_host *ata_host_alloc_pinfo(struct device *dev,
 	struct ata_host *host;
 	int i, j;
 
+	/* We must have at least one port info */
+	if (!ppi[0])
+		return NULL;
+
 	host = ata_host_alloc(dev, n_ports);
 	if (!host)
 		return NULL;
 
-	for (i = 0, j = 0, pi = NULL; i < host->n_ports; i++) {
+	for (i = 0, j = 0, pi = ppi[0]; i < host->n_ports; i++) {
 		struct ata_port *ap = host->ports[i];
 
-		if (ppi[j])
-			pi = ppi[j++];
-
 		ap->pio_mask = pi->pio_mask;
 		ap->mwdma_mask = pi->mwdma_mask;
 		ap->udma_mask = pi->udma_mask;
@@ -5460,6 +5461,15 @@ struct ata_host *ata_host_alloc_pinfo(struct device *dev,
 
 		if (!host->ops && (pi->port_ops != &ata_dummy_port_ops))
 			host->ops = pi->port_ops;
+
+		/*
+		 * Check that the next port info is not NULL.
+		 * If it is, keep using the current one.
+		 */
+		if (j < n_ports - 1 && ppi[j + 1]) {
+			j++;
+			pi = ppi[j];
+		}
 	}
 
 	return host;
-- 
2.31.1


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

* [PATCH v7 02/11] libata: fix ata_host_start()
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
  2021-08-16  1:44 ` [PATCH v7 01/11] libata: fix ata_host_alloc_pinfo() Damien Le Moal
@ 2021-08-16  1:44 ` Damien Le Moal
  2021-08-16  1:44 ` [PATCH v7 03/11] libata: simplify ata_scsi_rbuf_fill() Damien Le Moal
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

The loop on entry of ata_host_start() may not initialize host->ops to a
non NULL value. The test on the host_stop field of host->ops must then
be preceded by a check that host->ops is not NULL.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index b237a718ea0f..d3f7830bda2e 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5583,7 +5583,7 @@ int ata_host_start(struct ata_host *host)
 			have_stop = 1;
 	}
 
-	if (host->ops->host_stop)
+	if (host->ops && host->ops->host_stop)
 		have_stop = 1;
 
 	if (have_stop) {
-- 
2.31.1


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

* [PATCH v7 03/11] libata: simplify ata_scsi_rbuf_fill()
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
  2021-08-16  1:44 ` [PATCH v7 01/11] libata: fix ata_host_alloc_pinfo() Damien Le Moal
  2021-08-16  1:44 ` [PATCH v7 02/11] libata: fix ata_host_start() Damien Le Moal
@ 2021-08-16  1:44 ` Damien Le Moal
  2021-08-16  1:44 ` [PATCH v7 04/11] libata: cleanup device sleep capability detection Damien Le Moal
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

Sparse complains about context imbalance in ata_scsi_rbuf_get() and
ata_scsi_rbuf_put() due to these functions respectively only taking
and releasing the ata_scsi_rbuf_lock spinlock. Since these functions are
only called from ata_scsi_rbuf_fill() with ata_scsi_rbuf_get() being
called with a copy_in argument always false, the code can be simplified
and ata_scsi_rbuf_{get|put} removed. This change both simplifies the
code and fixes the sparse warning.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-scsi.c | 60 ++++++---------------------------------
 1 file changed, 9 insertions(+), 51 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index b9588c52815d..0b7b4624e4df 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1765,53 +1765,6 @@ struct ata_scsi_args {
 	struct scsi_cmnd	*cmd;
 };
 
-/**
- *	ata_scsi_rbuf_get - Map response buffer.
- *	@cmd: SCSI command containing buffer to be mapped.
- *	@flags: unsigned long variable to store irq enable status
- *	@copy_in: copy in from user buffer
- *
- *	Prepare buffer for simulated SCSI commands.
- *
- *	LOCKING:
- *	spin_lock_irqsave(ata_scsi_rbuf_lock) on success
- *
- *	RETURNS:
- *	Pointer to response buffer.
- */
-static void *ata_scsi_rbuf_get(struct scsi_cmnd *cmd, bool copy_in,
-			       unsigned long *flags)
-{
-	spin_lock_irqsave(&ata_scsi_rbuf_lock, *flags);
-
-	memset(ata_scsi_rbuf, 0, ATA_SCSI_RBUF_SIZE);
-	if (copy_in)
-		sg_copy_to_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
-				  ata_scsi_rbuf, ATA_SCSI_RBUF_SIZE);
-	return ata_scsi_rbuf;
-}
-
-/**
- *	ata_scsi_rbuf_put - Unmap response buffer.
- *	@cmd: SCSI command containing buffer to be unmapped.
- *	@copy_out: copy out result
- *	@flags: @flags passed to ata_scsi_rbuf_get()
- *
- *	Returns rbuf buffer.  The result is copied to @cmd's buffer if
- *	@copy_back is true.
- *
- *	LOCKING:
- *	Unlocks ata_scsi_rbuf_lock.
- */
-static inline void ata_scsi_rbuf_put(struct scsi_cmnd *cmd, bool copy_out,
-				     unsigned long *flags)
-{
-	if (copy_out)
-		sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
-				    ata_scsi_rbuf, ATA_SCSI_RBUF_SIZE);
-	spin_unlock_irqrestore(&ata_scsi_rbuf_lock, *flags);
-}
-
 /**
  *	ata_scsi_rbuf_fill - wrapper for SCSI command simulators
  *	@args: device IDENTIFY data / SCSI command of interest.
@@ -1830,14 +1783,19 @@ static inline void ata_scsi_rbuf_put(struct scsi_cmnd *cmd, bool copy_out,
 static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
 		unsigned int (*actor)(struct ata_scsi_args *args, u8 *rbuf))
 {
-	u8 *rbuf;
 	unsigned int rc;
 	struct scsi_cmnd *cmd = args->cmd;
 	unsigned long flags;
 
-	rbuf = ata_scsi_rbuf_get(cmd, false, &flags);
-	rc = actor(args, rbuf);
-	ata_scsi_rbuf_put(cmd, rc == 0, &flags);
+	spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
+
+	memset(ata_scsi_rbuf, 0, ATA_SCSI_RBUF_SIZE);
+	rc = actor(args, ata_scsi_rbuf);
+	if (rc == 0)
+		sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
+				    ata_scsi_rbuf, ATA_SCSI_RBUF_SIZE);
+
+	spin_unlock_irqrestore(&ata_scsi_rbuf_lock, flags);
 
 	if (rc == 0)
 		cmd->result = SAM_STAT_GOOD;
-- 
2.31.1


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

* [PATCH v7 04/11] libata: cleanup device sleep capability detection
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
                   ` (2 preceding siblings ...)
  2021-08-16  1:44 ` [PATCH v7 03/11] libata: simplify ata_scsi_rbuf_fill() Damien Le Moal
@ 2021-08-16  1:44 ` Damien Le Moal
  2021-08-16  1:44 ` [PATCH v7 05/11] libata: cleanup ata_dev_configure() Damien Le Moal
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

Move the code to retrieve the device sleep capability and timings out of
ata_dev_configure() into the helper function ata_dev_config_devslp().
While at it, mark the device as supporting the device sleep capability
only if the sata settings page was retrieved successfully to ensure that
the timing information is correctly initialized.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-core.c | 55 +++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index d3f7830bda2e..499ec1380676 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2363,6 +2363,37 @@ static void ata_dev_config_trusted(struct ata_device *dev)
 		dev->flags |= ATA_DFLAG_TRUSTED;
 }
 
+static void ata_dev_config_devslp(struct ata_device *dev)
+{
+	u8 *sata_setting = dev->link->ap->sector_buf;
+	unsigned int err_mask;
+	int i, j;
+
+	/*
+	 * Check device sleep capability. Get DevSlp timing variables
+	 * from SATA Settings page of Identify Device Data Log.
+	 */
+	if (!ata_id_has_devslp(dev->id))
+		return;
+
+	err_mask = ata_read_log_page(dev,
+				     ATA_LOG_IDENTIFY_DEVICE,
+				     ATA_LOG_SATA_SETTINGS,
+				     sata_setting, 1);
+	if (err_mask) {
+		ata_dev_dbg(dev,
+			    "failed to get SATA Settings Log, Emask 0x%x\n",
+			    err_mask);
+		return;
+	}
+
+	dev->flags |= ATA_DFLAG_DEVSLP;
+	for (i = 0; i < ATA_LOG_DEVSLP_SIZE; i++) {
+		j = ATA_LOG_DEVSLP_OFFSET + i;
+		dev->devslp_timing[i] = sata_setting[j];
+	}
+}
+
 /**
  *	ata_dev_configure - Configure the specified ATA/ATAPI device
  *	@dev: Target device to configure
@@ -2565,29 +2596,7 @@ int ata_dev_configure(struct ata_device *dev)
 			}
 		}
 
-		/* Check and mark DevSlp capability. Get DevSlp timing variables
-		 * from SATA Settings page of Identify Device Data Log.
-		 */
-		if (ata_id_has_devslp(dev->id)) {
-			u8 *sata_setting = ap->sector_buf;
-			int i, j;
-
-			dev->flags |= ATA_DFLAG_DEVSLP;
-			err_mask = ata_read_log_page(dev,
-						     ATA_LOG_IDENTIFY_DEVICE,
-						     ATA_LOG_SATA_SETTINGS,
-						     sata_setting,
-						     1);
-			if (err_mask)
-				ata_dev_dbg(dev,
-					    "failed to get Identify Device Data, Emask 0x%x\n",
-					    err_mask);
-			else
-				for (i = 0; i < ATA_LOG_DEVSLP_SIZE; i++) {
-					j = ATA_LOG_DEVSLP_OFFSET + i;
-					dev->devslp_timing[i] = sata_setting[j];
-				}
-		}
+		ata_dev_config_devslp(dev);
 		ata_dev_config_sense_reporting(dev);
 		ata_dev_config_zac(dev);
 		ata_dev_config_trusted(dev);
-- 
2.31.1


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

* [PATCH v7 05/11] libata: cleanup ata_dev_configure()
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
                   ` (3 preceding siblings ...)
  2021-08-16  1:44 ` [PATCH v7 04/11] libata: cleanup device sleep capability detection Damien Le Moal
@ 2021-08-16  1:44 ` Damien Le Moal
  2021-08-16  1:44 ` [PATCH v7 06/11] libata: cleanup NCQ priority handling Damien Le Moal
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

Introduce the helper functions ata_dev_config_lba() and
ata_dev_config_chs() to configure the addressing capabilities of a
device. To control message printing in these new helpers, as well as
in ata_dev_configure() and in ata_hpa_resize(), add the helper function
ata_dev_print_info() to avoid open coding for the eh context
ATA_EHI_PRINTINFO flag in multiple functions.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-core.c | 131 ++++++++++++++++++++++----------------
 1 file changed, 75 insertions(+), 56 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 499ec1380676..660b450bc498 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -159,6 +159,12 @@ MODULE_DESCRIPTION("Library module for ATA devices");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
 
+static inline bool ata_dev_print_info(struct ata_device *dev)
+{
+	struct ata_eh_context *ehc = &dev->link->eh_context;
+
+	return ehc->i.flags & ATA_EHI_PRINTINFO;
+}
 
 static bool ata_sstatus_online(u32 sstatus)
 {
@@ -1266,8 +1272,7 @@ static int ata_set_max_sectors(struct ata_device *dev, u64 new_sectors)
  */
 static int ata_hpa_resize(struct ata_device *dev)
 {
-	struct ata_eh_context *ehc = &dev->link->eh_context;
-	int print_info = ehc->i.flags & ATA_EHI_PRINTINFO;
+	bool print_info = ata_dev_print_info(dev);
 	bool unlock_hpa = ata_ignore_hpa || dev->flags & ATA_DFLAG_UNLOCK_HPA;
 	u64 sectors = ata_id_n_sectors(dev->id);
 	u64 native_sectors;
@@ -2363,6 +2368,65 @@ static void ata_dev_config_trusted(struct ata_device *dev)
 		dev->flags |= ATA_DFLAG_TRUSTED;
 }
 
+static int ata_dev_config_lba(struct ata_device *dev)
+{
+	struct ata_port *ap = dev->link->ap;
+	const u16 *id = dev->id;
+	const char *lba_desc;
+	char ncq_desc[24];
+	int ret;
+
+	dev->flags |= ATA_DFLAG_LBA;
+
+	if (ata_id_has_lba48(id)) {
+		lba_desc = "LBA48";
+		dev->flags |= ATA_DFLAG_LBA48;
+		if (dev->n_sectors >= (1UL << 28) &&
+		    ata_id_has_flush_ext(id))
+			dev->flags |= ATA_DFLAG_FLUSH_EXT;
+	} else {
+		lba_desc = "LBA";
+	}
+
+	/* config NCQ */
+	ret = ata_dev_config_ncq(dev, ncq_desc, sizeof(ncq_desc));
+
+	/* print device info to dmesg */
+	if (ata_msg_drv(ap) && ata_dev_print_info(dev))
+		ata_dev_info(dev,
+			     "%llu sectors, multi %u: %s %s\n",
+			     (unsigned long long)dev->n_sectors,
+			     dev->multi_count, lba_desc, ncq_desc);
+
+	return ret;
+}
+
+static void ata_dev_config_chs(struct ata_device *dev)
+{
+	struct ata_port *ap = dev->link->ap;
+	const u16 *id = dev->id;
+
+	if (ata_id_current_chs_valid(id)) {
+		/* Current CHS translation is valid. */
+		dev->cylinders = id[54];
+		dev->heads     = id[55];
+		dev->sectors   = id[56];
+	} else {
+		/* Default translation */
+		dev->cylinders	= id[1];
+		dev->heads	= id[3];
+		dev->sectors	= id[6];
+	}
+
+	/* print device info to dmesg */
+	if (ata_msg_drv(ap) && ata_dev_print_info(dev))
+		ata_dev_info(dev,
+			     "%llu sectors, multi %u, CHS %u/%u/%u\n",
+			     (unsigned long long)dev->n_sectors,
+			     dev->multi_count, dev->cylinders,
+			     dev->heads, dev->sectors);
+}
+
 static void ata_dev_config_devslp(struct ata_device *dev)
 {
 	u8 *sata_setting = dev->link->ap->sector_buf;
@@ -2410,8 +2474,7 @@ static void ata_dev_config_devslp(struct ata_device *dev)
 int ata_dev_configure(struct ata_device *dev)
 {
 	struct ata_port *ap = dev->link->ap;
-	struct ata_eh_context *ehc = &dev->link->eh_context;
-	int print_info = ehc->i.flags & ATA_EHI_PRINTINFO;
+	bool print_info = ata_dev_print_info(dev);
 	const u16 *id = dev->id;
 	unsigned long xfer_mask;
 	unsigned int err_mask;
@@ -2538,62 +2601,18 @@ int ata_dev_configure(struct ata_device *dev)
 					dev->multi_count = cnt;
 		}
 
-		if (ata_id_has_lba(id)) {
-			const char *lba_desc;
-			char ncq_desc[24];
-
-			lba_desc = "LBA";
-			dev->flags |= ATA_DFLAG_LBA;
-			if (ata_id_has_lba48(id)) {
-				dev->flags |= ATA_DFLAG_LBA48;
-				lba_desc = "LBA48";
-
-				if (dev->n_sectors >= (1UL << 28) &&
-				    ata_id_has_flush_ext(id))
-					dev->flags |= ATA_DFLAG_FLUSH_EXT;
-			}
+		/* print device info to dmesg */
+		if (ata_msg_drv(ap) && print_info)
+			ata_dev_info(dev, "%s: %s, %s, max %s\n",
+				     revbuf, modelbuf, fwrevbuf,
+				     ata_mode_string(xfer_mask));
 
-			/* config NCQ */
-			rc = ata_dev_config_ncq(dev, ncq_desc, sizeof(ncq_desc));
+		if (ata_id_has_lba(id)) {
+			rc = ata_dev_config_lba(dev);
 			if (rc)
 				return rc;
-
-			/* print device info to dmesg */
-			if (ata_msg_drv(ap) && print_info) {
-				ata_dev_info(dev, "%s: %s, %s, max %s\n",
-					     revbuf, modelbuf, fwrevbuf,
-					     ata_mode_string(xfer_mask));
-				ata_dev_info(dev,
-					     "%llu sectors, multi %u: %s %s\n",
-					(unsigned long long)dev->n_sectors,
-					dev->multi_count, lba_desc, ncq_desc);
-			}
 		} else {
-			/* CHS */
-
-			/* Default translation */
-			dev->cylinders	= id[1];
-			dev->heads	= id[3];
-			dev->sectors	= id[6];
-
-			if (ata_id_current_chs_valid(id)) {
-				/* Current CHS translation is valid. */
-				dev->cylinders = id[54];
-				dev->heads     = id[55];
-				dev->sectors   = id[56];
-			}
-
-			/* print device info to dmesg */
-			if (ata_msg_drv(ap) && print_info) {
-				ata_dev_info(dev, "%s: %s, %s, max %s\n",
-					     revbuf,	modelbuf, fwrevbuf,
-					     ata_mode_string(xfer_mask));
-				ata_dev_info(dev,
-					     "%llu sectors, multi %u, CHS %u/%u/%u\n",
-					     (unsigned long long)dev->n_sectors,
-					     dev->multi_count, dev->cylinders,
-					     dev->heads, dev->sectors);
-			}
+			ata_dev_config_chs(dev);
 		}
 
 		ata_dev_config_devslp(dev);
-- 
2.31.1


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

* [PATCH v7 06/11] libata: cleanup NCQ priority handling
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
                   ` (4 preceding siblings ...)
  2021-08-16  1:44 ` [PATCH v7 05/11] libata: cleanup ata_dev_configure() Damien Le Moal
@ 2021-08-16  1:44 ` Damien Le Moal
  2021-08-16  1:44 ` [PATCH v7 07/11] libata: fix ata_read_log_page() warning Damien Le Moal
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

The ata device flag ATA_DFLAG_NCQ_PRIO indicates if a device supports
the NCQ Priority feature while the ATA_DFLAG_NCQ_PRIO_ENABLE device
flag indicates if the feature is enabled. Enabling NCQ priority use is
controlled by the user through the device sysfs attribute
ncq_prio_enable. As a result, the ATA_DFLAG_NCQ_PRIO flag should not be
cleared when ATA_DFLAG_NCQ_PRIO_ENABLE is not set as the device still
supports the feature even after the user disables it. This leads to the
following cleanups:
- In ata_build_rw_tf(), set a command high priority bit based on the
  ATA_DFLAG_NCQ_PRIO_ENABLE flag, not on the ATA_DFLAG_NCQ flag. That
  is, set a command high priority only if the user enabled NCQ priority
  use.
- In ata_dev_config_ncq_prio(), ATA_DFLAG_NCQ_PRIO should not be cleared
  if ATA_DFLAG_NCQ_PRIO_ENABLE is not set. If the device does not
  support NCQ priority, both ATA_DFLAG_NCQ_PRIO and
  ATA_DFLAG_NCQ_PRIO_ENABLE must be cleared.

With the above ata_dev_config_ncq_prio() change, ATA_DFLAG_NCQ_PRIO flag
is set on device scan and revalidation. There is no need to trigger a
device revalidation in ata_ncq_prio_enable_store() when the user enables
the use of NCQ priority. Remove the revalidation code from that funciton
to simplify it. Also change the return value from -EIO to -EINVAL when a
user tries to enable NCQ priority for a device that does not support
this feature.  While at it, also simplify ata_ncq_prio_enable_show().

Overall, there is no functional change introduced by this patch.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-core.c | 32 ++++++++++++++------------------
 drivers/ata/libata-sata.c | 37 ++++++++++++-------------------------
 2 files changed, 26 insertions(+), 43 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 660b450bc498..a845a2b8d899 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -712,11 +712,9 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
 		if (tf->flags & ATA_TFLAG_FUA)
 			tf->device |= 1 << 7;
 
-		if (dev->flags & ATA_DFLAG_NCQ_PRIO) {
-			if (class == IOPRIO_CLASS_RT)
-				tf->hob_nsect |= ATA_PRIO_HIGH <<
-						 ATA_SHIFT_PRIO;
-		}
+		if (dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE &&
+		    class == IOPRIO_CLASS_RT)
+			tf->hob_nsect |= ATA_PRIO_HIGH << ATA_SHIFT_PRIO;
 	} else if (dev->flags & ATA_DFLAG_LBA) {
 		tf->flags |= ATA_TFLAG_LBA;
 
@@ -2178,11 +2176,6 @@ static void ata_dev_config_ncq_prio(struct ata_device *dev)
 	struct ata_port *ap = dev->link->ap;
 	unsigned int err_mask;
 
-	if (!(dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE)) {
-		dev->flags &= ~ATA_DFLAG_NCQ_PRIO;
-		return;
-	}
-
 	err_mask = ata_read_log_page(dev,
 				     ATA_LOG_IDENTIFY_DEVICE,
 				     ATA_LOG_SATA_SETTINGS,
@@ -2190,18 +2183,21 @@ static void ata_dev_config_ncq_prio(struct ata_device *dev)
 				     1);
 	if (err_mask) {
 		ata_dev_dbg(dev,
-			    "failed to get Identify Device data, Emask 0x%x\n",
+			    "failed to get SATA settings log, Emask 0x%x\n",
 			    err_mask);
-		return;
+		goto not_supported;
 	}
 
-	if (ap->sector_buf[ATA_LOG_NCQ_PRIO_OFFSET] & BIT(3)) {
-		dev->flags |= ATA_DFLAG_NCQ_PRIO;
-	} else {
-		dev->flags &= ~ATA_DFLAG_NCQ_PRIO;
-		ata_dev_dbg(dev, "SATA page does not support priority\n");
-	}
+	if (!(ap->sector_buf[ATA_LOG_NCQ_PRIO_OFFSET] & BIT(3)))
+		goto not_supported;
+
+	dev->flags |= ATA_DFLAG_NCQ_PRIO;
+
+	return;
 
+not_supported:
+	dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLE;
+	dev->flags &= ~ATA_DFLAG_NCQ_PRIO;
 }
 
 static int ata_dev_config_ncq(struct ata_device *dev,
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index 8adeab76dd38..dc397ebda089 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -839,23 +839,17 @@ static ssize_t ata_ncq_prio_enable_show(struct device *device,
 					char *buf)
 {
 	struct scsi_device *sdev = to_scsi_device(device);
-	struct ata_port *ap;
+	struct ata_port *ap = ata_shost_to_port(sdev->host);
 	struct ata_device *dev;
 	bool ncq_prio_enable;
 	int rc = 0;
 
-	ap = ata_shost_to_port(sdev->host);
-
 	spin_lock_irq(ap->lock);
 	dev = ata_scsi_find_dev(ap, sdev);
-	if (!dev) {
+	if (!dev)
 		rc = -ENODEV;
-		goto unlock;
-	}
-
-	ncq_prio_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE;
-
-unlock:
+	else
+		ncq_prio_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE;
 	spin_unlock_irq(ap->lock);
 
 	return rc ? rc : snprintf(buf, 20, "%u\n", ncq_prio_enable);
@@ -869,7 +863,7 @@ static ssize_t ata_ncq_prio_enable_store(struct device *device,
 	struct ata_port *ap;
 	struct ata_device *dev;
 	long int input;
-	int rc;
+	int rc = 0;
 
 	rc = kstrtol(buf, 10, &input);
 	if (rc)
@@ -883,27 +877,20 @@ static ssize_t ata_ncq_prio_enable_store(struct device *device,
 		return  -ENODEV;
 
 	spin_lock_irq(ap->lock);
+
+	if (!(dev->flags & ATA_DFLAG_NCQ_PRIO)) {
+		rc = -EINVAL;
+		goto unlock;
+	}
+
 	if (input)
 		dev->flags |= ATA_DFLAG_NCQ_PRIO_ENABLE;
 	else
 		dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLE;
 
-	dev->link->eh_info.action |= ATA_EH_REVALIDATE;
-	dev->link->eh_info.flags |= ATA_EHI_QUIET;
-	ata_port_schedule_eh(ap);
+unlock:
 	spin_unlock_irq(ap->lock);
 
-	ata_port_wait_eh(ap);
-
-	if (input) {
-		spin_lock_irq(ap->lock);
-		if (!(dev->flags & ATA_DFLAG_NCQ_PRIO)) {
-			dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLE;
-			rc = -EIO;
-		}
-		spin_unlock_irq(ap->lock);
-	}
-
 	return rc ? rc : len;
 }
 
-- 
2.31.1


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

* [PATCH v7 07/11] libata: fix ata_read_log_page() warning
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
                   ` (5 preceding siblings ...)
  2021-08-16  1:44 ` [PATCH v7 06/11] libata: cleanup NCQ priority handling Damien Le Moal
@ 2021-08-16  1:44 ` Damien Le Moal
  2021-08-16  1:44 ` [PATCH v7 08/11] libata: print feature list on device scan Damien Le Moal
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

Support for the READ LOG PAGE DMA EXT command is indicated by words 119
and 120 of a device identify data. This is tested in
ata_read_log_page() with ata_id_has_read_log_dma_ext() and the
READ LOG PAGE DMA command used if the device reports supports for it.

However, some devices lie about this support and using the DMA version
of the command fails, generating the warning message "READ LOG DMA EXT
failed, trying PIO". Since READ LOG PAGE DMA EXT is an optional command,
this warning is not at all important but may be scary for the user.
Change ata_read_log_page() to suppres this warning and to print an
error message if both DMA and PIO attempts failed.

With this change, there is no need to print again an error message when
ata_read_log_page() returns an error. So simplify the users of this
function.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-core.c | 47 +++++++++++----------------------------
 1 file changed, 13 insertions(+), 34 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index a845a2b8d899..bad577dbbc0d 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2026,13 +2026,15 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
 	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE,
 				     buf, sectors * ATA_SECT_SIZE, 0);
 
-	if (err_mask && dma) {
-		dev->horkage |= ATA_HORKAGE_NO_DMA_LOG;
-		ata_dev_warn(dev, "READ LOG DMA EXT failed, trying PIO\n");
-		goto retry;
+	if (err_mask) {
+		if (dma) {
+			dev->horkage |= ATA_HORKAGE_NO_DMA_LOG;
+			goto retry;
+		}
+		ata_dev_err(dev, "Read log page 0x%02x failed, Emask 0x%x\n",
+			    (unsigned int)page, err_mask);
 	}
 
-	DPRINTK("EXIT, err_mask=%x\n", err_mask);
 	return err_mask;
 }
 
@@ -2061,12 +2063,8 @@ static bool ata_identify_page_supported(struct ata_device *dev, u8 page)
 	 */
 	err = ata_read_log_page(dev, ATA_LOG_IDENTIFY_DEVICE, 0, ap->sector_buf,
 				1);
-	if (err) {
-		ata_dev_info(dev,
-			     "failed to get Device Identify Log Emask 0x%x\n",
-			     err);
+	if (err)
 		return false;
-	}
 
 	for (i = 0; i < ap->sector_buf[8]; i++) {
 		if (ap->sector_buf[9 + i] == page)
@@ -2130,11 +2128,7 @@ static void ata_dev_config_ncq_send_recv(struct ata_device *dev)
 	}
 	err_mask = ata_read_log_page(dev, ATA_LOG_NCQ_SEND_RECV,
 				     0, ap->sector_buf, 1);
-	if (err_mask) {
-		ata_dev_dbg(dev,
-			    "failed to get NCQ Send/Recv Log Emask 0x%x\n",
-			    err_mask);
-	} else {
+	if (!err_mask) {
 		u8 *cmds = dev->ncq_send_recv_cmds;
 
 		dev->flags |= ATA_DFLAG_NCQ_SEND_RECV;
@@ -2160,11 +2154,7 @@ static void ata_dev_config_ncq_non_data(struct ata_device *dev)
 	}
 	err_mask = ata_read_log_page(dev, ATA_LOG_NCQ_NON_DATA,
 				     0, ap->sector_buf, 1);
-	if (err_mask) {
-		ata_dev_dbg(dev,
-			    "failed to get NCQ Non-Data Log Emask 0x%x\n",
-			    err_mask);
-	} else {
+	if (!err_mask) {
 		u8 *cmds = dev->ncq_non_data_cmds;
 
 		memcpy(cmds, ap->sector_buf, ATA_LOG_NCQ_NON_DATA_SIZE);
@@ -2181,12 +2171,8 @@ static void ata_dev_config_ncq_prio(struct ata_device *dev)
 				     ATA_LOG_SATA_SETTINGS,
 				     ap->sector_buf,
 				     1);
-	if (err_mask) {
-		ata_dev_dbg(dev,
-			    "failed to get SATA settings log, Emask 0x%x\n",
-			    err_mask);
+	if (err_mask)
 		goto not_supported;
-	}
 
 	if (!(ap->sector_buf[ATA_LOG_NCQ_PRIO_OFFSET] & BIT(3)))
 		goto not_supported;
@@ -2347,11 +2333,8 @@ static void ata_dev_config_trusted(struct ata_device *dev)
 
 	err = ata_read_log_page(dev, ATA_LOG_IDENTIFY_DEVICE, ATA_LOG_SECURITY,
 			ap->sector_buf, 1);
-	if (err) {
-		ata_dev_dbg(dev,
-			    "failed to read Security Log, Emask 0x%x\n", err);
+	if (err)
 		return;
-	}
 
 	trusted_cap = get_unaligned_le64(&ap->sector_buf[40]);
 	if (!(trusted_cap & (1ULL << 63))) {
@@ -2440,12 +2423,8 @@ static void ata_dev_config_devslp(struct ata_device *dev)
 				     ATA_LOG_IDENTIFY_DEVICE,
 				     ATA_LOG_SATA_SETTINGS,
 				     sata_setting, 1);
-	if (err_mask) {
-		ata_dev_dbg(dev,
-			    "failed to get SATA Settings Log, Emask 0x%x\n",
-			    err_mask);
+	if (err_mask)
 		return;
-	}
 
 	dev->flags |= ATA_DFLAG_DEVSLP;
 	for (i = 0; i < ATA_LOG_DEVSLP_SIZE; i++) {
-- 
2.31.1


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

* [PATCH v7 08/11] libata: print feature list on device scan
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
                   ` (6 preceding siblings ...)
  2021-08-16  1:44 ` [PATCH v7 07/11] libata: fix ata_read_log_page() warning Damien Le Moal
@ 2021-08-16  1:44 ` Damien Le Moal
  2021-08-16  1:44 ` [PATCH v7 09/11] libata: Introduce ncq_prio_supported sysfs sttribute Damien Le Moal
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

Print a list of features supported by a drive when it is configured in
ata_dev_configure() using the new function ata_dev_print_features().
The features printed are not already advertized and are: trusted
send-recev support, device attention support, device sleep support,
NCQ send-recv support and NCQ priority support.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-core.c | 17 +++++++++++++++++
 include/linux/libata.h    |  4 ++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index bad577dbbc0d..68a55d3e977a 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2433,6 +2433,20 @@ static void ata_dev_config_devslp(struct ata_device *dev)
 	}
 }
 
+static void ata_dev_print_features(struct ata_device *dev)
+{
+	if (!(dev->flags & ATA_DFLAG_FEATURES_MASK))
+		return;
+
+	ata_dev_info(dev,
+		     "Features:%s%s%s%s%s\n",
+		     dev->flags & ATA_DFLAG_TRUSTED ? " Trust" : "",
+		     dev->flags & ATA_DFLAG_DA ? " Dev-Attention" : "",
+		     dev->flags & ATA_DFLAG_DEVSLP ? " Dev-Sleep" : "",
+		     dev->flags & ATA_DFLAG_NCQ_SEND_RECV ? " NCQ-sndrcv" : "",
+		     dev->flags & ATA_DFLAG_NCQ_PRIO ? " NCQ-prio" : "");
+}
+
 /**
  *	ata_dev_configure - Configure the specified ATA/ATAPI device
  *	@dev: Target device to configure
@@ -2595,6 +2609,9 @@ int ata_dev_configure(struct ata_device *dev)
 		ata_dev_config_zac(dev);
 		ata_dev_config_trusted(dev);
 		dev->cdb_len = 32;
+
+		if (ata_msg_drv(ap) && print_info)
+			ata_dev_print_features(dev);
 	}
 
 	/* ATAPI-specific feature tests */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3fcd24236793..b23f28cfc8e0 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -161,6 +161,10 @@ enum {
 	ATA_DFLAG_D_SENSE	= (1 << 29), /* Descriptor sense requested */
 	ATA_DFLAG_ZAC		= (1 << 30), /* ZAC device */
 
+	ATA_DFLAG_FEATURES_MASK	= ATA_DFLAG_TRUSTED | ATA_DFLAG_DA | \
+				  ATA_DFLAG_DEVSLP | ATA_DFLAG_NCQ_SEND_RECV | \
+				  ATA_DFLAG_NCQ_PRIO,
+
 	ATA_DEV_UNKNOWN		= 0,	/* unknown device */
 	ATA_DEV_ATA		= 1,	/* ATA device */
 	ATA_DEV_ATA_UNSUP	= 2,	/* ATA device (unsupported) */
-- 
2.31.1


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

* [PATCH v7 09/11] libata: Introduce ncq_prio_supported sysfs sttribute
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
                   ` (7 preceding siblings ...)
  2021-08-16  1:44 ` [PATCH v7 08/11] libata: print feature list on device scan Damien Le Moal
@ 2021-08-16  1:44 ` Damien Le Moal
  2021-08-16  5:54   ` Hannes Reinecke
  2021-08-16  1:44 ` [PATCH v7 10/11] docs: sysfs-block-device: improve ncq_prio_enable documentation Damien Le Moal
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

Currently, the only way a user can determine if a SATA device supports
NCQ priority is to try to enable the use of this feature using the
ncq_prio_enable sysfs device attribute. If enabling the feature fails,
it is because the device does not support NCQ priority. Otherwise, the
feature is enabled and success indicates that the device supports NCQ
priority.

Improve this odd interface by introducing the read-only
ncq_prio_supported sysfs device attribute to indicate if a SATA device
supports NCQ priority. The value of this attribute reflects the status
of device flag ATA_DFLAG_NCQ_PRIO, which is set only for devices
supporting NCQ priority.

Add this new sysfs attribute to the device attributes group of libahci
and libata-sata.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/ata/libahci.c     |  1 +
 drivers/ata/libata-sata.c | 25 +++++++++++++++++++++++++
 include/linux/libata.h    |  1 +
 3 files changed, 27 insertions(+)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index fec2e9754aed..5b3fa2cbe722 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -125,6 +125,7 @@ EXPORT_SYMBOL_GPL(ahci_shost_attrs);
 struct device_attribute *ahci_sdev_attrs[] = {
 	&dev_attr_sw_activity,
 	&dev_attr_unload_heads,
+	&dev_attr_ncq_prio_supported,
 	&dev_attr_ncq_prio_enable,
 	NULL
 };
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index dc397ebda089..8f3ff830ab0c 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -834,6 +834,30 @@ DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
 	    ata_scsi_lpm_show, ata_scsi_lpm_store);
 EXPORT_SYMBOL_GPL(dev_attr_link_power_management_policy);
 
+static ssize_t ata_ncq_prio_supported_show(struct device *device,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(device);
+	struct ata_port *ap = ata_shost_to_port(sdev->host);
+	struct ata_device *dev;
+	bool ncq_prio_supported;
+	int rc = 0;
+
+	spin_lock_irq(ap->lock);
+	dev = ata_scsi_find_dev(ap, sdev);
+	if (!dev)
+		rc = -ENODEV;
+	else
+		ncq_prio_supported = dev->flags & ATA_DFLAG_NCQ_PRIO;
+	spin_unlock_irq(ap->lock);
+
+	return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_supported);
+}
+
+DEVICE_ATTR(ncq_prio_supported, S_IRUGO, ata_ncq_prio_supported_show, NULL);
+EXPORT_SYMBOL_GPL(dev_attr_ncq_prio_supported);
+
 static ssize_t ata_ncq_prio_enable_show(struct device *device,
 					struct device_attribute *attr,
 					char *buf)
@@ -901,6 +925,7 @@ EXPORT_SYMBOL_GPL(dev_attr_ncq_prio_enable);
 struct device_attribute *ata_ncq_sdev_attrs[] = {
 	&dev_attr_unload_heads,
 	&dev_attr_ncq_prio_enable,
+	&dev_attr_ncq_prio_supported,
 	NULL
 };
 EXPORT_SYMBOL_GPL(ata_ncq_sdev_attrs);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index b23f28cfc8e0..a2d1bae7900b 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -539,6 +539,7 @@ typedef void (*ata_postreset_fn_t)(struct ata_link *link, unsigned int *classes)
 extern struct device_attribute dev_attr_unload_heads;
 #ifdef CONFIG_SATA_HOST
 extern struct device_attribute dev_attr_link_power_management_policy;
+extern struct device_attribute dev_attr_ncq_prio_supported;
 extern struct device_attribute dev_attr_ncq_prio_enable;
 extern struct device_attribute dev_attr_em_message_type;
 extern struct device_attribute dev_attr_em_message;
-- 
2.31.1


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

* [PATCH v7 10/11] docs: sysfs-block-device: improve ncq_prio_enable documentation
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
                   ` (8 preceding siblings ...)
  2021-08-16  1:44 ` [PATCH v7 09/11] libata: Introduce ncq_prio_supported sysfs sttribute Damien Le Moal
@ 2021-08-16  1:44 ` Damien Le Moal
  2021-08-16  5:54   ` Hannes Reinecke
  2021-08-16  1:44 ` [PATCH v7 11/11] docs: sysfs-block-device: document ncq_prio_supported Damien Le Moal
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

From: Niklas Cassel <niklas.cassel@wdc.com>

NCQ priority is an optional feature of the NCQ feature set and should
not be confused with the NCQ feature set itself. Clarify the
description of the ncq_prio_enable attribute to avoid this confusion.

Also add the missing documentation for the equivalent
sas_ncq_prio_enable attribute.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 Documentation/ABI/testing/sysfs-block-device | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-block-device b/Documentation/ABI/testing/sysfs-block-device
index aa0fb500e3c9..cf1013df4b92 100644
--- a/Documentation/ABI/testing/sysfs-block-device
+++ b/Documentation/ABI/testing/sysfs-block-device
@@ -55,6 +55,20 @@ Date:		Oct, 2016
 KernelVersion:	v4.10
 Contact:	linux-ide@vger.kernel.org
 Description:
-		(RW) Write to the file to turn on or off the SATA ncq (native
-		command queueing) support. By default this feature is turned
-		off.
+		(RW) Write to the file to turn on or off the SATA NCQ (native
+		command queueing) priority support. By default this feature is
+		turned off. If the device does not support the SATA NCQ
+		priority feature, writing "1" to this file results in an error.
+
+
+What:		/sys/block/*/device/sas_ncq_prio_enable
+Date:		Oct, 2016
+KernelVersion:	v4.10
+Contact:	linux-ide@vger.kernel.org
+Description:
+		(RW) This is the equivalent of the ncq_prio_enable attribute
+		file for SATA devices connected to a SAS host-bus-adapter
+		(HBA) implementing support for the SATA NCQ priority feature.
+		This file does not exist if the HBA driver does not implement
+		support for the SATA NCQ priority feature, regardless of the
+		device support for this feature.
-- 
2.31.1


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

* [PATCH v7 11/11] docs: sysfs-block-device: document ncq_prio_supported
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
                   ` (9 preceding siblings ...)
  2021-08-16  1:44 ` [PATCH v7 10/11] docs: sysfs-block-device: improve ncq_prio_enable documentation Damien Le Moal
@ 2021-08-16  1:44 ` Damien Le Moal
  2021-08-16  5:55   ` Hannes Reinecke
  2021-08-18  9:54 ` [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
  2021-08-18 13:20 ` Jens Axboe
  12 siblings, 1 reply; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16  1:44 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

Add documentation for the new device attribute file ncq_prio_supported,
and its SAS HBA equivalent sas_ncq_prio_supported.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 Documentation/ABI/testing/sysfs-block-device | 25 +++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-block-device b/Documentation/ABI/testing/sysfs-block-device
index cf1013df4b92..7ac7b19b2f72 100644
--- a/Documentation/ABI/testing/sysfs-block-device
+++ b/Documentation/ABI/testing/sysfs-block-device
@@ -58,7 +58,8 @@ Description:
 		(RW) Write to the file to turn on or off the SATA NCQ (native
 		command queueing) priority support. By default this feature is
 		turned off. If the device does not support the SATA NCQ
-		priority feature, writing "1" to this file results in an error.
+		priority feature, writing "1" to this file results in an error
+		(see ncq_prio_supported).
 
 
 What:		/sys/block/*/device/sas_ncq_prio_enable
@@ -71,4 +72,26 @@ Description:
 		(HBA) implementing support for the SATA NCQ priority feature.
 		This file does not exist if the HBA driver does not implement
 		support for the SATA NCQ priority feature, regardless of the
+		device support for this feature (see sas_ncq_prio_supported).
+
+
+What:		/sys/block/*/device/ncq_prio_supported
+Date:		Aug, 2021
+KernelVersion:	v5.15
+Contact:	linux-ide@vger.kernel.org
+Description:
+		(RO) Indicates if the device supports the SATA NCQ (native
+		command queueing) priority feature.
+
+
+What:		/sys/block/*/device/sas_ncq_prio_supported
+Date:		Aug, 2021
+KernelVersion:	v5.15
+Contact:	linux-ide@vger.kernel.org
+Description:
+		(RO) This is the equivalent of the ncq_prio_supported attribute
+		file for SATA devices connected to a SAS host-bus-adapter
+		(HBA) implementing support for the SATA NCQ priority feature.
+		This file does not exist if the HBA driver does not implement
+		support for the SATA NCQ priority feature, regardless of the
 		device support for this feature.
-- 
2.31.1


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

* Re: [PATCH v7 01/11] libata: fix ata_host_alloc_pinfo()
  2021-08-16  1:44 ` [PATCH v7 01/11] libata: fix ata_host_alloc_pinfo() Damien Le Moal
@ 2021-08-16  5:53   ` Hannes Reinecke
  2021-08-16 11:29   ` James Bottomley
  1 sibling, 0 replies; 21+ messages in thread
From: Hannes Reinecke @ 2021-08-16  5:53 UTC (permalink / raw)
  To: Damien Le Moal, Jens Axboe, linux-ide; +Cc: linux-block

On 8/16/21 3:44 AM, Damien Le Moal wrote:
> Avoid static checkers warnings about a potential NULL pointer
> dereference for the port info variable pi. To do so, test that at least
> one port info is available on entry to ata_host_alloc_pinfo() and start
> the ata port initialization for() loop with pi initialized to the first
> port info passed as argument (which is already checked to be non NULL).
> Within the for() loop, get the next port info, if it is not NULL,
> after initializing the ata port using the previous port info.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   drivers/ata/libata-core.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH v7 09/11] libata: Introduce ncq_prio_supported sysfs sttribute
  2021-08-16  1:44 ` [PATCH v7 09/11] libata: Introduce ncq_prio_supported sysfs sttribute Damien Le Moal
@ 2021-08-16  5:54   ` Hannes Reinecke
  0 siblings, 0 replies; 21+ messages in thread
From: Hannes Reinecke @ 2021-08-16  5:54 UTC (permalink / raw)
  To: Damien Le Moal, Jens Axboe, linux-ide; +Cc: linux-block

On 8/16/21 3:44 AM, Damien Le Moal wrote:
> Currently, the only way a user can determine if a SATA device supports
> NCQ priority is to try to enable the use of this feature using the
> ncq_prio_enable sysfs device attribute. If enabling the feature fails,
> it is because the device does not support NCQ priority. Otherwise, the
> feature is enabled and success indicates that the device supports NCQ
> priority.
> 
> Improve this odd interface by introducing the read-only
> ncq_prio_supported sysfs device attribute to indicate if a SATA device
> supports NCQ priority. The value of this attribute reflects the status
> of device flag ATA_DFLAG_NCQ_PRIO, which is set only for devices
> supporting NCQ priority.
> 
> Add this new sysfs attribute to the device attributes group of libahci
> and libata-sata.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   drivers/ata/libahci.c     |  1 +
>   drivers/ata/libata-sata.c | 25 +++++++++++++++++++++++++
>   include/linux/libata.h    |  1 +
>   3 files changed, 27 insertions(+)
> Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH v7 10/11] docs: sysfs-block-device: improve ncq_prio_enable documentation
  2021-08-16  1:44 ` [PATCH v7 10/11] docs: sysfs-block-device: improve ncq_prio_enable documentation Damien Le Moal
@ 2021-08-16  5:54   ` Hannes Reinecke
  0 siblings, 0 replies; 21+ messages in thread
From: Hannes Reinecke @ 2021-08-16  5:54 UTC (permalink / raw)
  To: Damien Le Moal, Jens Axboe, linux-ide; +Cc: linux-block

On 8/16/21 3:44 AM, Damien Le Moal wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> NCQ priority is an optional feature of the NCQ feature set and should
> not be confused with the NCQ feature set itself. Clarify the
> description of the ncq_prio_enable attribute to avoid this confusion.
> 
> Also add the missing documentation for the equivalent
> sas_ncq_prio_enable attribute.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   Documentation/ABI/testing/sysfs-block-device | 20 +++++++++++++++++---
>   1 file changed, 17 insertions(+), 3 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH v7 11/11] docs: sysfs-block-device: document ncq_prio_supported
  2021-08-16  1:44 ` [PATCH v7 11/11] docs: sysfs-block-device: document ncq_prio_supported Damien Le Moal
@ 2021-08-16  5:55   ` Hannes Reinecke
  0 siblings, 0 replies; 21+ messages in thread
From: Hannes Reinecke @ 2021-08-16  5:55 UTC (permalink / raw)
  To: Damien Le Moal, Jens Axboe, linux-ide; +Cc: linux-block

On 8/16/21 3:44 AM, Damien Le Moal wrote:
> Add documentation for the new device attribute file ncq_prio_supported,
> and its SAS HBA equivalent sas_ncq_prio_supported.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   Documentation/ABI/testing/sysfs-block-device | 25 +++++++++++++++++++-
>   1 file changed, 24 insertions(+), 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH v7 01/11] libata: fix ata_host_alloc_pinfo()
  2021-08-16  1:44 ` [PATCH v7 01/11] libata: fix ata_host_alloc_pinfo() Damien Le Moal
  2021-08-16  5:53   ` Hannes Reinecke
@ 2021-08-16 11:29   ` James Bottomley
  2021-08-16 11:43     ` Damien Le Moal
  1 sibling, 1 reply; 21+ messages in thread
From: James Bottomley @ 2021-08-16 11:29 UTC (permalink / raw)
  To: Damien Le Moal, Jens Axboe, linux-ide; +Cc: linux-block

On Mon, 2021-08-16 at 10:44 +0900, Damien Le Moal wrote:
> Avoid static checkers warnings about a potential NULL pointer
> dereference for the port info variable pi. To do so, test that at
> least
> one port info is available on entry to ata_host_alloc_pinfo() and
> start
> the ata port initialization for() loop with pi initialized to the
> first
> port info passed as argument (which is already checked to be non
> NULL).
> Within the for() loop, get the next port info, if it is not NULL,
> after initializing the ata port using the previous port info.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  drivers/ata/libata-core.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 61c762961ca8..b237a718ea0f 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5441,16 +5441,17 @@ struct ata_host *ata_host_alloc_pinfo(struct
> device *dev,
>  	struct ata_host *host;
>  	int i, j;
>  
> +	/* We must have at least one port info */
> +	if (!ppi[0])
> +		return NULL;

I've got to ask why on this one: most libata drivers use a static array
for the port info.  If the first element is NULL that's a coding
failure inside the driver, so WARN_ON would probably be more helpful to
the driver writer.

What makes the static checker think ppi isn't NULL?

> +
>  	host = ata_host_alloc(dev, n_ports);
>  	if (!host)
>  		return NULL;
>  
> -	for (i = 0, j = 0, pi = NULL; i < host->n_ports; i++) {
> +	for (i = 0, j = 0, pi = ppi[0]; i < host->n_ports; i++) {
>  		struct ata_port *ap = host->ports[i];
>  
> -		if (ppi[j])
> -			pi = ppi[j++];
> -
>  		ap->pio_mask = pi->pio_mask;
>  		ap->mwdma_mask = pi->mwdma_mask;
>  		ap->udma_mask = pi->udma_mask;
> @@ -5460,6 +5461,15 @@ struct ata_host *ata_host_alloc_pinfo(struct
> device *dev,
>  
>  		if (!host->ops && (pi->port_ops !=
> &ata_dummy_port_ops))
>  			host->ops = pi->port_ops;
> +
> +		/*
> +		 * Check that the next port info is not NULL.
> +		 * If it is, keep using the current one.
> +		 */
> +		if (j < n_ports - 1 && ppi[j + 1]) {
> +			j++;
> +			pi = ppi[j];
> +		}

This looks completely pointless: once you've verified ppi[0] is not
NULL above, there's no possible NULL deref in that loop and the static
checker should see it.  If it doesn't we need a new static checker
because we shouldn't be perturbing code for broken tools.

James



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

* Re: [PATCH v7 01/11] libata: fix ata_host_alloc_pinfo()
  2021-08-16 11:29   ` James Bottomley
@ 2021-08-16 11:43     ` Damien Le Moal
  0 siblings, 0 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-16 11:43 UTC (permalink / raw)
  To: James Bottomley, Jens Axboe, linux-ide; +Cc: linux-block

On 2021/08/16 20:29, James Bottomley wrote:
> On Mon, 2021-08-16 at 10:44 +0900, Damien Le Moal wrote:
>> Avoid static checkers warnings about a potential NULL pointer
>> dereference for the port info variable pi. To do so, test that at
>> least
>> one port info is available on entry to ata_host_alloc_pinfo() and
>> start
>> the ata port initialization for() loop with pi initialized to the
>> first
>> port info passed as argument (which is already checked to be non
>> NULL).
>> Within the for() loop, get the next port info, if it is not NULL,
>> after initializing the ata port using the previous port info.
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
>> ---
>>  drivers/ata/libata-core.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index 61c762961ca8..b237a718ea0f 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -5441,16 +5441,17 @@ struct ata_host *ata_host_alloc_pinfo(struct
>> device *dev,
>>  	struct ata_host *host;
>>  	int i, j;
>>  
>> +	/* We must have at least one port info */
>> +	if (!ppi[0])
>> +		return NULL;
> 
> I've got to ask why on this one: most libata drivers use a static array
> for the port info.  If the first element is NULL that's a coding
> failure inside the driver, so WARN_ON would probably be more helpful to
> the driver writer.
> 
> What makes the static checker think ppi isn't NULL?
> 
>> +
>>  	host = ata_host_alloc(dev, n_ports);
>>  	if (!host)
>>  		return NULL;
>>  
>> -	for (i = 0, j = 0, pi = NULL; i < host->n_ports; i++) {
>> +	for (i = 0, j = 0, pi = ppi[0]; i < host->n_ports; i++) {
>>  		struct ata_port *ap = host->ports[i];
>>  
>> -		if (ppi[j])
>> -			pi = ppi[j++];
>> -
>>  		ap->pio_mask = pi->pio_mask;
>>  		ap->mwdma_mask = pi->mwdma_mask;
>>  		ap->udma_mask = pi->udma_mask;
>> @@ -5460,6 +5461,15 @@ struct ata_host *ata_host_alloc_pinfo(struct
>> device *dev,
>>  
>>  		if (!host->ops && (pi->port_ops !=
>> &ata_dummy_port_ops))
>>  			host->ops = pi->port_ops;
>> +
>> +		/*
>> +		 * Check that the next port info is not NULL.
>> +		 * If it is, keep using the current one.
>> +		 */
>> +		if (j < n_ports - 1 && ppi[j + 1]) {
>> +			j++;
>> +			pi = ppi[j];
>> +		}
> 
> This looks completely pointless: once you've verified ppi[0] is not
> NULL above, there's no possible NULL deref in that loop and the static
> checker should see it.  If it doesn't we need a new static checker
> because we shouldn't be perturbing code for broken tools.

I do not know how to run that static checker which sent the warnings initially.
I changed the code to avoid all the "dumb" cases it thinks are possible and
leading to the NULL deref signaled.

I think we should drop this patch. If the checker complains again, then I can
revisit in a different series.

Jens, can you drop this one when you apply the series (if you think it is good
to apply). Or should I resend a v8 without this patch ?

> 
> James
> 
> 
> 


-- 
Damien Le Moal
Western Digital Research

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

* Re: [PATCH v7 00/11] libata cleanups and improvements
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
                   ` (10 preceding siblings ...)
  2021-08-16  1:44 ` [PATCH v7 11/11] docs: sysfs-block-device: document ncq_prio_supported Damien Le Moal
@ 2021-08-18  9:54 ` Damien Le Moal
  2021-08-18 13:20 ` Jens Axboe
  12 siblings, 0 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-18  9:54 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

On 2021/08/16 10:45, Damien Le Moal wrote:
> The first three patches of this series fix sparse and kernel bot
> warnings (potential NULL pointer dereference and locking imbalance).
> 
> The following three patches cleanup libata-core code in the area of
> device configuration (ata_dev_configure() function).
> 
> Patch 7 improves ata_read_log_page() to avoid unnecessary warning
> messages and patch 8 adds an informational message on device scan to
> advertize the features supported by a device.
> 
> Path 9 adds the new sysfs ahci device attribute ncq_prio_supported to
> indicate that a disk supports NCQ priority.
> 
> Patch 10 and 11 update the ABI user documentation files.

Hi Jens,

Can you queue this minus patch 1 please ? I can resend v8 without patch 1 too if
needed. Thanks !

> 
> Changes from v6:
> * Added patch 10 and 11
> 
> Changes from v5:
> * Updated patch 9 to include adding the new ncq_prio_supported sysfs
>   attribute for SATA adapters other than AHCI. Changed the patch title
>   and commit message accordingly.
> 
> Changes from v4:
> * Fixed patch 1 to avoid an out-of-bounds array access
> * Changed title of patch 3 to describe the change (as opposed to only
>   mentioning the tool that found the problem)
> * Removed patch 10 from this series as Martin took it through the scsi
>   tree
> * Added reviewed-by tags
> 
> Changes from v3:
> * Reworked patch 1
> * Added patch 3 to fix a sparse warning discovered while checking
>   patch 1 & 2
> * Added reviewed-by tags
> 
> Changes from v2:
> * Reworked patch 4 to avoid the need for an additional on-stack string
>   for device information messages
> * Added reviewed-by tags
> 
> Changes from v1:
> * Added patch 1 and 2 to fix problems reported by the kernel test robot
> * Use strscpy() instead of strcpy in patch 4
> * Use sysfs_emit in patch 8 and 9 as suggested by Bart
> * Fix typos in comments of the new sas_ncq_prio_supported attribute in
>   patch 9
> 
> Damien Le Moal (10):
>   libata: fix ata_host_alloc_pinfo()
>   libata: fix ata_host_start()
>   libata: simplify ata_scsi_rbuf_fill()
>   libata: cleanup device sleep capability detection
>   libata: cleanup ata_dev_configure()
>   libata: cleanup NCQ priority handling
>   libata: fix ata_read_log_page() warning
>   libata: print feature list on device scan
>   libata: Introduce ncq_prio_supported sysfs sttribute
>   docs: sysfs-block-device: document ncq_prio_supported
> 
> Niklas Cassel (1):
>   docs: sysfs-block-device: improve ncq_prio_enable documentation
> 
>  Documentation/ABI/testing/sysfs-block-device |  43 ++-
>  drivers/ata/libahci.c                        |   1 +
>  drivers/ata/libata-core.c                    | 290 ++++++++++---------
>  drivers/ata/libata-sata.c                    |  62 ++--
>  drivers/ata/libata-scsi.c                    |  60 +---
>  include/linux/libata.h                       |   5 +
>  6 files changed, 252 insertions(+), 209 deletions(-)
> 


-- 
Damien Le Moal
Western Digital Research

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

* Re: [PATCH v7 00/11] libata cleanups and improvements
  2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
                   ` (11 preceding siblings ...)
  2021-08-18  9:54 ` [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
@ 2021-08-18 13:20 ` Jens Axboe
  2021-08-18 21:53   ` Damien Le Moal
  12 siblings, 1 reply; 21+ messages in thread
From: Jens Axboe @ 2021-08-18 13:20 UTC (permalink / raw)
  To: Damien Le Moal, linux-ide; +Cc: linux-block

On 8/15/21 7:44 PM, Damien Le Moal wrote:
> The first three patches of this series fix sparse and kernel bot
> warnings (potential NULL pointer dereference and locking imbalance).
> 
> The following three patches cleanup libata-core code in the area of
> device configuration (ata_dev_configure() function).
> 
> Patch 7 improves ata_read_log_page() to avoid unnecessary warning
> messages and patch 8 adds an informational message on device scan to
> advertize the features supported by a device.
> 
> Path 9 adds the new sysfs ahci device attribute ncq_prio_supported to
> indicate that a disk supports NCQ priority.
> 
> Patch 10 and 11 update the ABI user documentation files.

Applied 2-11, thanks.

-- 
Jens Axboe


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

* Re: [PATCH v7 00/11] libata cleanups and improvements
  2021-08-18 13:20 ` Jens Axboe
@ 2021-08-18 21:53   ` Damien Le Moal
  0 siblings, 0 replies; 21+ messages in thread
From: Damien Le Moal @ 2021-08-18 21:53 UTC (permalink / raw)
  To: Jens Axboe, linux-ide; +Cc: linux-block

On 2021/08/18 22:20, Jens Axboe wrote:
> On 8/15/21 7:44 PM, Damien Le Moal wrote:
>> The first three patches of this series fix sparse and kernel bot
>> warnings (potential NULL pointer dereference and locking imbalance).
>>
>> The following three patches cleanup libata-core code in the area of
>> device configuration (ata_dev_configure() function).
>>
>> Patch 7 improves ata_read_log_page() to avoid unnecessary warning
>> messages and patch 8 adds an informational message on device scan to
>> advertize the features supported by a device.
>>
>> Path 9 adds the new sysfs ahci device attribute ncq_prio_supported to
>> indicate that a disk supports NCQ priority.
>>
>> Patch 10 and 11 update the ABI user documentation files.
> 
> Applied 2-11, thanks.

Thank you.
Please note that this creates a small conflict with the libata patch from the
crange series. Do you want me to rebase that patch on block/libata ?


-- 
Damien Le Moal
Western Digital Research

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

end of thread, other threads:[~2021-08-18 21:54 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-16  1:44 [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
2021-08-16  1:44 ` [PATCH v7 01/11] libata: fix ata_host_alloc_pinfo() Damien Le Moal
2021-08-16  5:53   ` Hannes Reinecke
2021-08-16 11:29   ` James Bottomley
2021-08-16 11:43     ` Damien Le Moal
2021-08-16  1:44 ` [PATCH v7 02/11] libata: fix ata_host_start() Damien Le Moal
2021-08-16  1:44 ` [PATCH v7 03/11] libata: simplify ata_scsi_rbuf_fill() Damien Le Moal
2021-08-16  1:44 ` [PATCH v7 04/11] libata: cleanup device sleep capability detection Damien Le Moal
2021-08-16  1:44 ` [PATCH v7 05/11] libata: cleanup ata_dev_configure() Damien Le Moal
2021-08-16  1:44 ` [PATCH v7 06/11] libata: cleanup NCQ priority handling Damien Le Moal
2021-08-16  1:44 ` [PATCH v7 07/11] libata: fix ata_read_log_page() warning Damien Le Moal
2021-08-16  1:44 ` [PATCH v7 08/11] libata: print feature list on device scan Damien Le Moal
2021-08-16  1:44 ` [PATCH v7 09/11] libata: Introduce ncq_prio_supported sysfs sttribute Damien Le Moal
2021-08-16  5:54   ` Hannes Reinecke
2021-08-16  1:44 ` [PATCH v7 10/11] docs: sysfs-block-device: improve ncq_prio_enable documentation Damien Le Moal
2021-08-16  5:54   ` Hannes Reinecke
2021-08-16  1:44 ` [PATCH v7 11/11] docs: sysfs-block-device: document ncq_prio_supported Damien Le Moal
2021-08-16  5:55   ` Hannes Reinecke
2021-08-18  9:54 ` [PATCH v7 00/11] libata cleanups and improvements Damien Le Moal
2021-08-18 13:20 ` Jens Axboe
2021-08-18 21:53   ` Damien Le Moal

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