| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <99541f2d-2aea-6bd7-e3b6-21dbc355875d@opensource.wdc.com>
Date: Wed, 16 Mar 2022 12:21:35 +0900
From: Damien Le Moal <damien.lemoal@...nsource.wdc.com>
To: John Garry <john.garry@...wei.com>, jejb@...ux.ibm.com,
martin.petersen@...cle.com, bvanassche@....org,
ming.lei@...hat.com, hch@....de, hare@...e.de
Cc: linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-scsi@...r.kernel.org, martin.wilck@...e.com
Subject: Re: [PATCH RFC 2/2] libata: Use scsi cmnd budget token for qc tag for
SAS host
On 3/15/22 19:39, John Garry wrote:
> For attaining a qc tag for a SAS host we need to allocate a bit in
> ata_port.sas_tag_allocated bitmap.
>
> However we already have a unique tag per device in range
> [0, ATA_MAX_QUEUE) in the scsi cmnd budget token, so just use that
> instead.
>
> Signed-off-by: John Garry <john.garry@...wei.com>
> ---
> drivers/ata/libata-core.c | 5 +++--
> drivers/ata/libata-sata.c | 21 ++++-----------------
> drivers/ata/libata-scsi.c | 2 +-
> drivers/ata/libata.h | 4 ++--
> include/linux/libata.h | 1 -
> 5 files changed, 10 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 0c854aebfe0b..2c0a550d3ecd 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -4572,8 +4572,9 @@ void swap_buf_le16(u16 *buf, unsigned int buf_words)
> * None.
> */
>
> -struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, int tag)
> +struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, struct scsi_cmnd *scmd)
> {
> + int tag = scsi_cmd_to_rq(scmd)->tag;
> struct ata_port *ap = dev->link->ap;
> struct ata_queued_cmd *qc;
>
> @@ -4583,7 +4584,7 @@ struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, int tag)
>
> /* libsas case */
> if (ap->flags & ATA_FLAG_SAS_HOST) {
> - tag = ata_sas_allocate_tag(ap);
> + tag = ata_sas_allocate_tag(ap, scmd);
> if (tag < 0)
> return NULL;
> }
> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
> index 071158c0c44c..a4374fdffc43 100644
> --- a/drivers/ata/libata-sata.c
> +++ b/drivers/ata/libata-sata.c
> @@ -1268,29 +1268,16 @@ int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap)
> }
> EXPORT_SYMBOL_GPL(ata_sas_queuecmd);
>
> -int ata_sas_allocate_tag(struct ata_port *ap)
> +int ata_sas_allocate_tag(struct ata_port *ap, struct scsi_cmnd *scmd)
> {
> - unsigned int max_queue = ap->host->n_tags;
> - unsigned int i, tag;
> + if (scmd->budget_token >= ATA_MAX_QUEUE)
> + return -1;
>
> - for (i = 0, tag = ap->sas_last_tag + 1; i < max_queue; i++, tag++) {
> - tag = tag < max_queue ? tag : 0;
> -
> - /* the last tag is reserved for internal command. */
> - if (ata_tag_internal(tag))
> - continue;
> -
> - if (!test_and_set_bit(tag, &ap->sas_tag_allocated)) {
> - ap->sas_last_tag = tag;
> - return tag;
> - }
> - }
> - return -1;
> + return scmd->budget_token;
> }
Since this is now not actually allocating a tag, I would rename this
something like ata_sas_get_tag(). Or even better, simply open code this
in ata_qc_new_init() since that is the only caller.
>
> void ata_sas_free_tag(unsigned int tag, struct ata_port *ap)
> {
> - clear_bit(tag, &ap->sas_tag_allocated);
> }
This is called only in ata_qc_free(). With this change, the function is
empty, so let's completely remove it.
>
> /**
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index ed8be585a98f..45d63a2ba3ee 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -640,7 +640,7 @@ static struct ata_queued_cmd *ata_scsi_qc_new(struct ata_device *dev,
> {
> struct ata_queued_cmd *qc;
>
> - qc = ata_qc_new_init(dev, scsi_cmd_to_rq(cmd)->tag);
> + qc = ata_qc_new_init(dev, cmd);
> if (qc) {
> qc->scsicmd = cmd;
> qc->scsidone = scsi_done;
> diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
> index 51e01acdd241..65302d7829fe 100644
> --- a/drivers/ata/libata.h
> +++ b/drivers/ata/libata.h
> @@ -44,7 +44,7 @@ static inline void ata_force_cbl(struct ata_port *ap) { }
> #endif
> extern u64 ata_tf_to_lba(const struct ata_taskfile *tf);
> extern u64 ata_tf_to_lba48(const struct ata_taskfile *tf);
> -extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, int tag);
> +extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, struct scsi_cmnd *scmd);
> extern int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
> u64 block, u32 n_block, unsigned int tf_flags,
> unsigned int tag, int class);
> @@ -93,7 +93,7 @@ extern unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
>
> /* libata-sata.c */
> #ifdef CONFIG_SATA_HOST
> -int ata_sas_allocate_tag(struct ata_port *ap);
> +int ata_sas_allocate_tag(struct ata_port *ap, struct scsi_cmnd *scmd);
> void ata_sas_free_tag(unsigned int tag, struct ata_port *ap);
> #else
> static inline int ata_sas_allocate_tag(struct ata_port *ap)
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 7f99b4d78822..3b9399f67b39 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -814,7 +814,6 @@ struct ata_port {
> unsigned int cbl; /* cable type; ATA_CBL_xxx */
>
> struct ata_queued_cmd qcmd[ATA_MAX_QUEUE + 1];
> - unsigned long sas_tag_allocated; /* for sas tag allocation only */
> u64 qc_active;
> int nr_active_links; /* #links with active qcs */
> unsigned int sas_last_tag; /* track next tag hw expects */
--
Damien Le Moal
Western Digital Research
Powered by blists - more mailing lists