Commit 602bcf21 authored by Damien Le Moal's avatar Damien Le Moal
Browse files

ata: libata: Improve CDL resource management 



The ncq_sense_buf buffer field of struct ata_port is allocated and used
only for devices that support the Command Duration Limits (CDL) feature.
However, the cdl buffer of struct ata_device, which is used to cache the
command duration limits log page for devices supporting CDL is always
allocated as part of struct ata_device, which is wasteful of memory for
devices that do not support this feature.

Clean this up by defining both buffers as part of the new ata_cdl
structure and allocating this structure only for devices that support
the CDL feature. This new structure is attached to struct ata_device
using the cdl pointer.

The functions ata_dev_init_cdl_resources() and
ata_dev_cleanup_cdl_resources() are defined to manage this new structure
allocation, initialization and freeing when a port is removed or a
device disabled. ata_dev_init_cdl_resources() is called from
ata_dev_config_cdl() only for devices that support CDL.
ata_dev_cleanup_cdl_resources() is called from ata_dev_free_resources()
to free the ata_cdl structure when a device is being disabled by EH.

Note that the name of the former cdl log buffer of struct ata_device is
changed to desc_log_buf to make it clearer that it is a buffer for the
limit descriptors log page.

This change reduces the size of struct ata_device, thus reducing memory
usage for ATA devices that do not support the CDL feature.

Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
Reviewed-by: default avatarNiklas Cassel <cassel@kernel.org>
parent 5f8319c4

drivers/ata/libata-core.c

+37 −24
Original line number Diff line number Diff line
@@ -2464,12 +2464,41 @@ 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)

{

	kfree(dev->cdl);

	dev->cdl = NULL;

}



static int ata_dev_init_cdl_resources(struct ata_device *dev)

{

	struct ata_cdl *cdl = dev->cdl;

	unsigned int err_mask;



	if (!cdl) {

		cdl = kzalloc(sizeof(*cdl), GFP_KERNEL);

		if (!cdl)

			return -ENOMEM;

		dev->cdl = cdl;

	}



	err_mask = ata_read_log_page(dev, ATA_LOG_CDL, 0, cdl->desc_log_buf,

				     ATA_LOG_CDL_SIZE / ATA_SECT_SIZE);

	if (err_mask) {

		ata_dev_warn(dev, "Read Command Duration Limits log failed\n");

		ata_dev_cleanup_cdl_resources(dev);

		return -EIO;

	}



	return 0;

}



static void ata_dev_config_cdl(struct ata_device *dev)

{

	struct ata_port *ap = dev->link->ap;

	unsigned int err_mask;

	bool cdl_enabled;

	u64 val;

	int ret;



	if (ata_id_major_version(dev->id) < 11)

		goto not_supported;
@@ -2564,37 +2593,20 @@ static void ata_dev_config_cdl(struct ata_device *dev) 
		}

	}



	/*

	 * Allocate a buffer to handle reading the sense data for successful

	 * NCQ Commands log page for commands using a CDL with one of the limit

	 * policy set to 0xD (successful completion with sense data available

	 * bit set).

	 */

	if (!ap->ncq_sense_buf) {

		ap->ncq_sense_buf = kmalloc(ATA_LOG_SENSE_NCQ_SIZE, GFP_KERNEL);

		if (!ap->ncq_sense_buf)

			goto not_supported;

	}



	/*

	 * Command duration limits is supported: cache the CDL log page 18h

	 * (command duration descriptors).

	 */

	err_mask = ata_read_log_page(dev, ATA_LOG_CDL, 0, dev->sector_buf, 1);

	if (err_mask) {

		ata_dev_warn(dev, "Read Command Duration Limits log failed\n");

	/* CDL is supported: allocate and initialize needed resources. */

	ret = ata_dev_init_cdl_resources(dev);

	if (ret) {

		ata_dev_warn(dev, "Initialize CDL resources failed\n");

		goto not_supported;

	}



	memcpy(dev->cdl, dev->sector_buf, ATA_LOG_CDL_SIZE);

	dev->flags |= ATA_DFLAG_CDL;



	return;



not_supported:

	dev->flags &= ~(ATA_DFLAG_CDL | ATA_DFLAG_CDL_ENABLED);

	kfree(ap->ncq_sense_buf);

	ap->ncq_sense_buf = NULL;

	ata_dev_cleanup_cdl_resources(dev);

}



static int ata_dev_config_lba(struct ata_device *dev)
@@ -5451,7 +5463,6 @@ void ata_port_free(struct ata_port *ap) 


	kfree(ap->pmp_link);

	kfree(ap->slave_link);

	kfree(ap->ncq_sense_buf);

	ida_free(&ata_ida, ap->print_id);

	kfree(ap);

}
@@ -5989,6 +6000,8 @@ void ata_dev_free_resources(struct ata_device *dev) 
{

	if (zpodd_dev_enabled(dev))

		zpodd_exit(dev);



	ata_dev_cleanup_cdl_resources(dev);

}



/**

drivers/ata/libata-sata.c

+1 −1
Original line number Diff line number Diff line
@@ -1505,7 +1505,7 @@ int ata_eh_get_ncq_success_sense(struct ata_link *link) 
{

	struct ata_device *dev = link->device;

	struct ata_port *ap = dev->link->ap;

	u8 *buf = ap->ncq_sense_buf;

	u8 *buf = dev->cdl->ncq_sense_log_buf;

	struct ata_queued_cmd *qc;

	unsigned int err_mask, tag;

	u8 *sense, sk = 0, asc = 0, ascq = 0;

drivers/ata/libata-scsi.c

+1 −1
Original line number Diff line number Diff line
@@ -2248,7 +2248,7 @@ static inline u16 ata_xlat_cdl_limit(u8 *buf) 
static unsigned int ata_msense_control_spgt2(struct ata_device *dev, u8 *buf,

					     u8 spg)

{

	u8 *b, *cdl = dev->cdl, *desc;

	u8 *b, *cdl = dev->cdl->desc_log_buf, *desc;

	u32 policy;

	int i;

drivers/ata/libata.h

+1 −0
Original line number Diff line number Diff line
@@ -90,6 +90,7 @@ 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)

include/linux/libata.h

+17 −4
Original line number Diff line number Diff line
@@ -700,6 +700,21 @@ struct ata_cpr_log { 
	struct ata_cpr		cpr[] __counted_by(nr_cpr);

};



struct ata_cdl {

	/*

	 * Buffer to cache the CDL log page 18h (command duration descriptors)

	 * for SCSI-ATA translation.

	 */

	u8			desc_log_buf[ATA_LOG_CDL_SIZE];



	/*

	 * Buffer to handle reading the sense data for successful NCQ Commands

	 * log page for commands using a CDL with one of the limits policy set

	 * to 0xD (successful completion with sense data available bit set).

	 */

	u8			ncq_sense_log_buf[ATA_LOG_SENSE_NCQ_SIZE];

};



struct ata_device {

	struct ata_link		*link;

	unsigned int		devno;		/* 0 or 1 */
@@ -762,8 +777,8 @@ struct ata_device { 
	/* Concurrent positioning ranges */

	struct ata_cpr_log	*cpr_log;



	/* Command Duration Limits log support */

	u8			cdl[ATA_LOG_CDL_SIZE];

	/* Command Duration Limits support */

	struct ata_cdl		*cdl;



	/* error history */

	int			spdn_cnt;
@@ -917,8 +932,6 @@ struct ata_port { 
#ifdef CONFIG_ATA_ACPI

	struct ata_acpi_gtm	__acpi_init_gtm; /* use ata_acpi_init_gtm() */

#endif

	/* owned by EH */

	u8			*ncq_sense_buf;

};



/* The following initializer overrides a method to NULL whether one of