| 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: <08d85db7-679b-b099-485b-744b8bb22839@suse.de>
Date: Tue, 4 Oct 2022 07:55:44 +0200
From: Hannes Reinecke <hare@...e.de>
To: John Garry <john.garry@...wei.com>, jejb@...ux.ibm.com,
martin.petersen@...cle.com, jinpu.wang@...ud.ionos.com,
damien.lemoal@....com
Cc: linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
linuxarm@...wei.com, ipylypiv@...gle.com, changyuanl@...gle.com,
hch@....de, yanaijie@...wei.com
Subject: Re: [PATCH v2 6/6] scsi: mvsas: Use sas_task_find_rq() for tagging
On 9/30/22 10:56, John Garry wrote:
> The request associated with a scsi command coming from the block layer
> has a unique tag, so use that when possible for getting a slot.
>
> Unfortunately we don't support reserved commands in the SCSI midlayer yet.
> As such, SMP tasks - as an example - will not have a request associated, so
> in the interim continue to manage those tags for that type of sas_task
> internally.
>
> We reserve an arbitrary 4 tags for these internal tags. Indeed, we already
> decrement MVS_RSVD_SLOTS by 2 for the shost can_queue when flag
> MVF_FLAG_SOC is set. This change was made in commit 20b09c2992fef
> ("[PATCH] [SCSI] mvsas: add support for 94xx; layout change; bug fixes"),
> but what those 2 slots are used for is not obvious.
>
> Also make the tag management functions static, where possible.
>
> Signed-off-by: John Garry <john.garry@...wei.com>
> ---
> drivers/scsi/mvsas/mv_defs.h | 1 +
> drivers/scsi/mvsas/mv_init.c | 9 +++++----
> drivers/scsi/mvsas/mv_sas.c | 38 ++++++++++++++++++++++++------------
> drivers/scsi/mvsas/mv_sas.h | 7 +------
> 4 files changed, 32 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/scsi/mvsas/mv_defs.h b/drivers/scsi/mvsas/mv_defs.h
> index 7123a2efbf58..8ef174cd4d37 100644
> --- a/drivers/scsi/mvsas/mv_defs.h
> +++ b/drivers/scsi/mvsas/mv_defs.h
> @@ -40,6 +40,7 @@ enum driver_configuration {
> MVS_ATA_CMD_SZ = 96, /* SATA command table buffer size */
> MVS_OAF_SZ = 64, /* Open address frame buffer size */
> MVS_QUEUE_SIZE = 64, /* Support Queue depth */
> + MVS_RSVD_SLOTS = 4,
> MVS_SOC_CAN_QUEUE = MVS_SOC_SLOTS - 2,
> };
>
> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> index c85fb812ad43..cfe84473a515 100644
> --- a/drivers/scsi/mvsas/mv_init.c
> +++ b/drivers/scsi/mvsas/mv_init.c
> @@ -142,7 +142,7 @@ static void mvs_free(struct mvs_info *mvi)
> scsi_host_put(mvi->shost);
> list_for_each_entry(mwq, &mvi->wq_list, entry)
> cancel_delayed_work(&mwq->work_q);
> - kfree(mvi->tags);
> + kfree(mvi->rsvd_tags);
> kfree(mvi);
> }
>
> @@ -284,7 +284,6 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
> printk(KERN_DEBUG "failed to create dma pool %s.\n", pool_name);
> goto err_out;
> }
> - mvi->tags_num = slot_nr;
>
> return 0;
> err_out:
> @@ -367,8 +366,8 @@ static struct mvs_info *mvs_pci_alloc(struct pci_dev *pdev,
> mvi->sas = sha;
> mvi->shost = shost;
>
> - mvi->tags = kzalloc(MVS_CHIP_SLOT_SZ>>3, GFP_KERNEL);
> - if (!mvi->tags)
> + mvi->rsvd_tags = bitmap_zalloc(MVS_RSVD_SLOTS, GFP_KERNEL);
> + if (!mvi->rsvd_tags)
> goto err_out;
>
> if (MVS_CHIP_DISP->chip_ioremap(mvi))
> @@ -469,6 +468,8 @@ static void mvs_post_sas_ha_init(struct Scsi_Host *shost,
> else
> can_queue = MVS_CHIP_SLOT_SZ;
>
> + can_queue -= MVS_RSVD_SLOTS;
> +
> shost->sg_tablesize = min_t(u16, SG_ALL, MVS_MAX_SG);
> shost->can_queue = can_queue;
> mvi->shost->cmd_per_lun = MVS_QUEUE_SIZE;
> diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
> index 0810e6c930e1..00b3a2781d21 100644
> --- a/drivers/scsi/mvsas/mv_sas.c
> +++ b/drivers/scsi/mvsas/mv_sas.c
> @@ -20,33 +20,39 @@ static int mvs_find_tag(struct mvs_info *mvi, struct sas_task *task, u32 *tag)
> return 0;
> }
>
> -void mvs_tag_clear(struct mvs_info *mvi, u32 tag)
> +static void mvs_tag_clear(struct mvs_info *mvi, u32 tag)
> {
> - void *bitmap = mvi->tags;
> + void *bitmap = mvi->rsvd_tags;
> clear_bit(tag, bitmap);
> }
>
> -void mvs_tag_free(struct mvs_info *mvi, u32 tag)
> +static void mvs_tag_free(struct mvs_info *mvi, u32 tag)
> {
> + if (tag < mvi->shost->can_queue)
> + return;
> +
> + tag -= mvi->shost->can_queue;
> +
> mvs_tag_clear(mvi, tag);
> }
>
> -void mvs_tag_set(struct mvs_info *mvi, unsigned int tag)
> +static void mvs_tag_set(struct mvs_info *mvi, unsigned int tag)
> {
> - void *bitmap = mvi->tags;
> + void *bitmap = mvi->rsvd_tags;
> set_bit(tag, bitmap);
> }
>
> -inline int mvs_tag_alloc(struct mvs_info *mvi, u32 *tag_out)
> +static int mvs_tag_alloc(struct mvs_info *mvi, u32 *tag_out)
> {
> unsigned int index, tag;
> - void *bitmap = mvi->tags;
> + void *bitmap = mvi->rsvd_tags;
>
> - index = find_first_zero_bit(bitmap, mvi->tags_num);
> + index = find_first_zero_bit(bitmap, MVS_RSVD_SLOTS);
> tag = index;
> - if (tag >= mvi->tags_num)
> + if (tag >= MVS_RSVD_SLOTS)
> return -SAS_QUEUE_FULL;
> mvs_tag_set(mvi, tag);
> + tag += mvi->shost->can_queue;
> *tag_out = tag;
> return 0;
> }
Also here: please move the reserved tags to the front, such that the tag
allocation scheme matches with the blk-mq tag allocation scheme.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@...e.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
Powered by blists - more mailing lists