From: Hannes Reinecke <hare@suse.de>
To: Muneendra <muneendra.kumar@broadcom.com>,
linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
tj@kernel.org, linux-nvme@lists.infradead.org
Cc: jsmart2021@gmail.com, emilne@redhat.com, mkumar@redhat.com,
Gaurav Srivastava <gaurav.srivastava@broadcom.com>
Subject: Re: [PATCH v9 13/13] lpfc: vmid: Introducing vmid in io path.
Date: Thu, 8 Apr 2021 10:46:11 +0200 [thread overview]
Message-ID: <91b0c309-6908-8fd9-ac60-a8572500c3ed@suse.de> (raw)
In-Reply-To: <1617750397-26466-14-git-send-email-muneendra.kumar@broadcom.com>
On 4/7/21 1:06 AM, Muneendra wrote:
> From: Gaurav Srivastava <gaurav.srivastava@broadcom.com>
>
> The patch introduces the vmid in the io path. It checks if the vmid is
> enabled and if io belongs to a vm or not and acts accordingly. Other
> supporing APIs are also included in the patch.
supporting (sp)
>
> Signed-off-by: Gaurav Srivastava <gaurav.srivastava@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
>
> ---
> v9:
> Added changes related to locking and new hashtable implementation
>
> v8:
> Added proper error codes
> updated logic while handling vmid
>
> v7:
> No change
>
> v6:
> No change
>
> v5:
> No change
>
> v4:
> No change
>
> v3:
> Replaced blkcg_get_app_identifier with blkcg_get_fc_appid
>
> v2:
> Ported the patch on top of 5.10/scsi-queue
> Added a fix for issuing QFPA command which was not included in the
> last submit
> ---
> drivers/scsi/lpfc/lpfc_scsi.c | 169 ++++++++++++++++++++++++++++++++++
> 1 file changed, 169 insertions(+)
>
> diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
> index f81178464436..3267c5858238 100644
> --- a/drivers/scsi/lpfc/lpfc_scsi.c
> +++ b/drivers/scsi/lpfc/lpfc_scsi.c
> @@ -5263,6 +5263,155 @@ static void lpfc_vmid_assign_cs_ctl(struct lpfc_vport *vport,
> }
> }
>
> +/*
> + * lpfc_vmid_get_appid- get the vmid associated with the uuid
> + * @vport: The virtual port for which this call is being executed.
> + * @uuid: uuid associated with the VE
> + * @cmd: address of scsi cmmd descriptor
> + * @tag: VMID tag
> + * Returns status of the function
> + */
> +static int lpfc_vmid_get_appid(struct lpfc_vport *vport, char *uuid, struct
> + scsi_cmnd * cmd, union lpfc_vmid_io_tag *tag)
> +{
> + struct lpfc_vmid *vmp = NULL;
> + int hash, len, rc, i;
> + u8 pending = 0;
> +
> + /* check if QFPA is complete */
> + if (lpfc_vmid_is_type_priority_tag(vport) && !(vport->vmid_flag &
> + LPFC_VMID_QFPA_CMPL)) {
> + vport->work_port_events |= WORKER_CHECK_VMID_ISSUE_QFPA;
> + return -EAGAIN;
> + }
> +
> + /* search if the uuid has already been mapped to the vmid */
> + len = strlen(uuid);
> + hash = lpfc_vmid_hash_fn(uuid, len);
> +
> + /* search for the VMID in the table */
> + read_lock(&vport->vmid_lock);
> + vmp = lpfc_get_vmid_from_hastable(vport, hash, uuid);
> +
> + /* if found, check if its already registered */
> + if (vmp && vmp->flag & LPFC_VMID_REGISTERED) {
> + read_unlock(&vport->vmid_lock);
> + lpfc_vmid_update_entry(vport, cmd, vmp, tag);
> + rc = 0;
> + } else if (vmp && (vmp->flag & LPFC_VMID_REQ_REGISTER ||
> + vmp->flag & LPFC_VMID_DE_REGISTER)) {
> + /* else if register or dereg request has already been sent */
> + /* Hence vmid tag will not be added for this IO */
> + read_unlock(&vport->vmid_lock);
> + rc = -EBUSY;
> + } else {
> + /* The vmid was not found in the hashtable. At this point, */
> + /* drop the read lock first before proceeding further */
> + read_unlock(&vport->vmid_lock);
> + /* start the process to obtain one as per the */
> + /* type of the vmid indicated */
> + write_lock(&vport->vmid_lock);
> + vmp = lpfc_get_vmid_from_hastable(vport, hash, uuid);
> +
> + /* while the read lock was released, in case the entry was */
> + /* added by other context or is in process of being added */
> + if (vmp && vmp->flag & LPFC_VMID_REGISTERED) {
> + lpfc_vmid_update_entry(vport, cmd, vmp, tag);
> + write_unlock(&vport->vmid_lock);
> + return 0;
> + } else if (vmp && vmp->flag & LPFC_VMID_REQ_REGISTER) {
> + write_unlock(&vport->vmid_lock);
> + return -EBUSY;
> + }
> +
> + /* else search and allocate a free slot in the hash table */
> + if (vport->cur_vmid_cnt < vport->max_vmid) {
> + for (i = 0; i < vport->max_vmid; ++i) {
> + vmp = vport->vmid + i;
> + if (vmp->flag == LPFC_VMID_SLOT_FREE) {
> + vmp = vport->vmid + i;
> + break;
> + }
> + }
> + } else {
> + write_unlock(&vport->vmid_lock);
> + return -ENOMEM;
> + }
> +
> + if (vmp && (vmp->flag == LPFC_VMID_SLOT_FREE)) {
> + /* Add the vmid and register */
> + lpfc_put_vmid_in_hashtable(vport, hash, vmp);
> + vmp->vmid_len = len;
> + memcpy(vmp->host_vmid, uuid, vmp->vmid_len);
> + vmp->io_rd_cnt = 0;
> + vmp->io_wr_cnt = 0;
> + vmp->flag = LPFC_VMID_SLOT_USED;
> +
> + vmp->delete_inactive =
> + vport->vmid_inactivity_timeout ? 1 : 0;
> +
> + /* if type priority tag, get next available vmid */
> + if (lpfc_vmid_is_type_priority_tag(vport))
> + lpfc_vmid_assign_cs_ctl(vport, vmp);
> +
> + /* allocate the per cpu variable for holding */
> + /* the last access time stamp only if vmid is enabled */
> + if (!vmp->last_io_time)
> + vmp->last_io_time =
> + __alloc_percpu(sizeof(u64),
> + __alignof__(struct
> + lpfc_vmid));
> +
> + /* registration pending */
> + pending = 1;
> + } else {
> + rc = -ENOMEM;
> + }
> + write_unlock(&vport->vmid_lock);
> +
> + /* complete transaction with switch */
> + if (pending) {
> + if (lpfc_vmid_is_type_priority_tag(vport))
> + rc = lpfc_vmid_uvem(vport, vmp, true);
> + else
> + rc = lpfc_vmid_cmd(vport,
> + SLI_CTAS_RAPP_IDENT,
> + vmp);
> + if (!rc) {
> + write_lock(&vport->vmid_lock);
> + vport->cur_vmid_cnt++;
> + vmp->flag |= LPFC_VMID_REQ_REGISTER;
> + write_unlock(&vport->vmid_lock);
> + }
> + }
> +
> + /* finally, enable the idle timer once */
> + if (!(vport->phba->pport->vmid_flag & LPFC_VMID_TIMER_ENBLD)) {
> + mod_timer(&vport->phba->inactive_vmid_poll,
> + jiffies +
> + msecs_to_jiffies(1000 * LPFC_VMID_TIMER));
> + vport->phba->pport->vmid_flag |= LPFC_VMID_TIMER_ENBLD;
> + }
> + }
> + return rc;
> +}
> +
> +/*
> + * lpfc_is_command_vm_io - get the uuid from blk cgroup
> + * @cmd:Pointer to scsi_cmnd data structure
> + * Returns uuid if present if not null
> + */
> +static char *lpfc_is_command_vm_io(struct scsi_cmnd *cmd)
> +{
> + char *uuid = NULL;
> +
> + if (cmd->request) {
> + if (cmd->request->bio)
> + uuid = blkcg_get_fc_appid(cmd->request->bio);
> + }
> + return uuid;
> +}
> +
> /**
> * lpfc_queuecommand - scsi_host_template queuecommand entry point
> * @shost: kernel scsi host pointer.
> @@ -5288,6 +5437,7 @@ lpfc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmnd)
> int err, idx;
> #ifdef CONFIG_SCSI_LPFC_DEBUG_FS
> uint64_t start = 0L;
> + u8 *uuid = NULL;
>
> if (phba->ktime_on)
> start = ktime_get_ns();
> @@ -5415,6 +5565,25 @@ lpfc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmnd)
> }
>
>
> + /* check the necessary and sufficient condition to support VMID */
> + if (lpfc_is_vmid_enabled(phba) &&
> + (ndlp->vmid_support ||
> + phba->pport->vmid_priority_tagging ==
> + LPFC_VMID_PRIO_TAG_ALL_TARGETS)) {
> + /* is the IO generated by a VM, get the associated virtual */
> + /* entity id */
> + uuid = lpfc_is_command_vm_io(cmnd);
> +
> + if (uuid) {
> + err = lpfc_vmid_get_appid(vport, uuid, cmnd,
> + (union lpfc_vmid_io_tag *)
> + &lpfc_cmd->cur_iocbq.vmid_tag);
> + if (!err)
> + lpfc_cmd->cur_iocbq.iocb_flag |= LPFC_IO_VMID;
> + }
> + }
> +
> + atomic_inc(&ndlp->cmd_pending);
> #ifdef CONFIG_SCSI_LPFC_DEBUG_FS
> if (unlikely(phba->hdwqstat_on & LPFC_CHECK_SCSI_IO))
> this_cpu_inc(phba->sli4_hba.c_stat->xmt_io);
>
And that's the bit which I don't particular like.
Essentially we'll have to inject additional ELS commands _on each I/O_
to get a valid VMID.
Where there are _so_ many things which might get wrong, causing an I/O
stall.
I would have vastly preferred if we could _avoid_ having to do
additional ELS commands for VMID registration in the I/O path
(ie only allow for I/O with a valid VMID), and reject the I/O otherwise
until VMID registration is complete.
IE return 'BUSY' (or even a command retry?) when no valid VMID for this
particular I/O is found, register the VMID (preferably in another
thread), and restart the queue once the VMID is registered.
That way we have a clear separation, and the I/O path will always work
with valid VMIDs.
Hmm?
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
next prev parent reply other threads:[~2021-04-08 8:46 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-06 23:06 [PATCH v9 00/13] blkcg:Support to track FC storage blk io traffic Muneendra
2021-04-06 23:06 ` [PATCH v9 01/13] cgroup: Added cgroup_get_from_id Muneendra
2021-04-08 8:26 ` Hannes Reinecke
2021-04-06 23:06 ` [PATCH v9 02/13] blkcg: Added a app identifier support for blkcg Muneendra
2021-04-08 8:26 ` Hannes Reinecke
2021-04-06 23:06 ` [PATCH v9 03/13] nvme: Added a newsysfs attribute appid_store Muneendra
2021-04-18 15:32 ` Benjamin Block
2021-04-20 6:54 ` Muneendra Kumar M
2021-04-20 11:09 ` Benjamin Block
2021-04-22 23:29 ` James Smart
2021-04-23 10:14 ` Benjamin Block
2021-04-06 23:06 ` [PATCH v9 04/13] lpfc: vmid: Add the datastructure for supporting VMID in lpfc Muneendra
2021-04-08 8:28 ` Hannes Reinecke
2021-04-06 23:06 ` [PATCH v9 05/13] lpfc: vmid: VMID params initialization Muneendra
2021-04-08 8:29 ` Hannes Reinecke
2021-04-06 23:06 ` [PATCH v9 06/13] lpfc: vmid: Add support for vmid in mailbox command, does vmid resource allocation and vmid cleanup Muneendra
2021-04-08 8:32 ` Hannes Reinecke
2021-04-06 23:06 ` [PATCH v9 07/13] lpfc: vmid: Implements ELS commands for appid patch Muneendra
2021-04-08 8:34 ` Hannes Reinecke
2021-04-20 12:38 ` Benjamin Block
2021-04-21 22:55 ` James Smart
2021-04-22 9:28 ` Benjamin Block
2021-04-06 23:06 ` [PATCH v9 08/13] lpfc: vmid: Functions to manage vmids Muneendra
2021-04-08 8:35 ` Hannes Reinecke
2021-04-06 23:06 ` [PATCH v9 09/13] lpfc: vmid: Implements CT commands for appid Muneendra
2021-04-08 8:37 ` Hannes Reinecke
2021-04-06 23:06 ` [PATCH v9 10/13] lpfc: vmid: Appends the vmid in the wqe before sending Muneendra
2021-04-06 23:06 ` [PATCH v9 11/13] lpfc: vmid: Timeout implementation for vmid Muneendra
2021-04-08 8:38 ` Hannes Reinecke
2021-04-06 23:06 ` [PATCH v9 12/13] lpfc: vmid: Adding qfpa and vmid timeout check in worker thread Muneendra
2021-04-06 23:06 ` [PATCH v9 13/13] lpfc: vmid: Introducing vmid in io path Muneendra
2021-04-08 8:46 ` Hannes Reinecke [this message]
2021-04-10 15:00 ` James Smart
2021-04-12 5:27 ` Hannes Reinecke
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=91b0c309-6908-8fd9-ac60-a8572500c3ed@suse.de \
--to=hare@suse.de \
--cc=emilne@redhat.com \
--cc=gaurav.srivastava@broadcom.com \
--cc=jsmart2021@gmail.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=linux-scsi@vger.kernel.org \
--cc=mkumar@redhat.com \
--cc=muneendra.kumar@broadcom.com \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).