From: Brian King <brking@linux.vnet.ibm.com> To: Tyrel Datwyler <tyreld@linux.ibm.com>, james.bottomley@hansenpartnership.com Cc: martin.petersen@oracle.com, linux-scsi@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, brking@linux.ibm.com Subject: Re: [PATCH v3 06/18] ibmvfc: add handlers to drain and complete Sub-CRQ responses Date: Fri, 4 Dec 2020 08:51:38 -0600 [thread overview] Message-ID: <3fe8683a-47f6-8713-762a-02c57c2e4ec2@linux.vnet.ibm.com> (raw) In-Reply-To: <20201203020806.14747-7-tyreld@linux.ibm.com> On 12/2/20 8:07 PM, Tyrel Datwyler wrote: > The logic for iterating over the Sub-CRQ responses is similiar to that > of the primary CRQ. Add the necessary handlers for processing those > responses. > > Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com> > --- > drivers/scsi/ibmvscsi/ibmvfc.c | 80 ++++++++++++++++++++++++++++++++++ > 1 file changed, 80 insertions(+) > > diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c > index e082935f56cf..b61ae1df21e5 100644 > --- a/drivers/scsi/ibmvscsi/ibmvfc.c > +++ b/drivers/scsi/ibmvscsi/ibmvfc.c > @@ -3381,6 +3381,86 @@ static int ibmvfc_toggle_scrq_irq(struct ibmvfc_sub_queue *scrq, int enable) > return rc; > } > > +static void ibmvfc_handle_scrq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost) > +{ > + struct ibmvfc_event *evt = (struct ibmvfc_event *)be64_to_cpu(crq->ioba); > + unsigned long flags; > + > + switch (crq->valid) { > + case IBMVFC_CRQ_CMD_RSP: > + break; > + case IBMVFC_CRQ_XPORT_EVENT: > + return; > + default: > + dev_err(vhost->dev, "Got and invalid message type 0x%02x\n", crq->valid); > + return; > + } > + > + /* The only kind of payload CRQs we should get are responses to > + * things we send. Make sure this response is to something we > + * actually sent > + */ > + if (unlikely(!ibmvfc_valid_event(&vhost->pool, evt))) { > + dev_err(vhost->dev, "Returned correlation_token 0x%08llx is invalid!\n", > + crq->ioba); > + return; > + } > + > + if (unlikely(atomic_read(&evt->free))) { > + dev_err(vhost->dev, "Received duplicate correlation_token 0x%08llx!\n", > + crq->ioba); > + return; > + } > + > + del_timer(&evt->timer); > + list_del(&evt->queue); > + ibmvfc_trc_end(evt);> + spin_unlock_irqrestore(vhost->host->host_lock, flags); You can't do this here... You are grabbing the host lock in ibmvfc_drain_sub_crq and saving the irqflags to a local in that function, then doing a spin_unlock_irqrestore and restoring irqflags using an uninitialized local in this function... I'm assuming this will get sorted out with the locking changes we've been discussing off-list... > + evt->done(evt); > + spin_lock_irqsave(vhost->host->host_lock, flags); > +} > + > +static struct ibmvfc_crq *ibmvfc_next_scrq(struct ibmvfc_sub_queue *scrq) > +{ > + struct ibmvfc_crq *crq; > + > + crq = &scrq->msgs[scrq->cur].crq; > + if (crq->valid & 0x80) { > + if (++scrq->cur == scrq->size) > + scrq->cur = 0; > + rmb(); > + } else > + crq = NULL; > + > + return crq; > +} > + > +static void ibmvfc_drain_sub_crq(struct ibmvfc_sub_queue *scrq) > +{ > + struct ibmvfc_crq *crq; > + unsigned long flags; > + int done = 0; > + > + spin_lock_irqsave(scrq->vhost->host->host_lock, flags); > + while (!done) { > + while ((crq = ibmvfc_next_scrq(scrq)) != NULL) { > + ibmvfc_handle_scrq(crq, scrq->vhost); > + crq->valid = 0; > + wmb(); > + } > + > + ibmvfc_toggle_scrq_irq(scrq, 1); > + if ((crq = ibmvfc_next_scrq(scrq)) != NULL) { > + ibmvfc_toggle_scrq_irq(scrq, 0); > + ibmvfc_handle_scrq(crq, scrq->vhost); > + crq->valid = 0; > + wmb(); > + } else > + done = 1; > + } > + spin_unlock_irqrestore(scrq->vhost->host->host_lock, flags); > +} > + > /** > * ibmvfc_init_tgt - Set the next init job step for the target > * @tgt: ibmvfc target struct > -- Brian King Power Linux I/O IBM Linux Technology Center
WARNING: multiple messages have this Message-ID (diff)
From: Brian King <brking@linux.vnet.ibm.com> To: Tyrel Datwyler <tyreld@linux.ibm.com>, james.bottomley@hansenpartnership.com Cc: brking@linux.ibm.com, linuxppc-dev@lists.ozlabs.org, linux-scsi@vger.kernel.org, martin.petersen@oracle.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 06/18] ibmvfc: add handlers to drain and complete Sub-CRQ responses Date: Fri, 4 Dec 2020 08:51:38 -0600 [thread overview] Message-ID: <3fe8683a-47f6-8713-762a-02c57c2e4ec2@linux.vnet.ibm.com> (raw) In-Reply-To: <20201203020806.14747-7-tyreld@linux.ibm.com> On 12/2/20 8:07 PM, Tyrel Datwyler wrote: > The logic for iterating over the Sub-CRQ responses is similiar to that > of the primary CRQ. Add the necessary handlers for processing those > responses. > > Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com> > --- > drivers/scsi/ibmvscsi/ibmvfc.c | 80 ++++++++++++++++++++++++++++++++++ > 1 file changed, 80 insertions(+) > > diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c > index e082935f56cf..b61ae1df21e5 100644 > --- a/drivers/scsi/ibmvscsi/ibmvfc.c > +++ b/drivers/scsi/ibmvscsi/ibmvfc.c > @@ -3381,6 +3381,86 @@ static int ibmvfc_toggle_scrq_irq(struct ibmvfc_sub_queue *scrq, int enable) > return rc; > } > > +static void ibmvfc_handle_scrq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost) > +{ > + struct ibmvfc_event *evt = (struct ibmvfc_event *)be64_to_cpu(crq->ioba); > + unsigned long flags; > + > + switch (crq->valid) { > + case IBMVFC_CRQ_CMD_RSP: > + break; > + case IBMVFC_CRQ_XPORT_EVENT: > + return; > + default: > + dev_err(vhost->dev, "Got and invalid message type 0x%02x\n", crq->valid); > + return; > + } > + > + /* The only kind of payload CRQs we should get are responses to > + * things we send. Make sure this response is to something we > + * actually sent > + */ > + if (unlikely(!ibmvfc_valid_event(&vhost->pool, evt))) { > + dev_err(vhost->dev, "Returned correlation_token 0x%08llx is invalid!\n", > + crq->ioba); > + return; > + } > + > + if (unlikely(atomic_read(&evt->free))) { > + dev_err(vhost->dev, "Received duplicate correlation_token 0x%08llx!\n", > + crq->ioba); > + return; > + } > + > + del_timer(&evt->timer); > + list_del(&evt->queue); > + ibmvfc_trc_end(evt);> + spin_unlock_irqrestore(vhost->host->host_lock, flags); You can't do this here... You are grabbing the host lock in ibmvfc_drain_sub_crq and saving the irqflags to a local in that function, then doing a spin_unlock_irqrestore and restoring irqflags using an uninitialized local in this function... I'm assuming this will get sorted out with the locking changes we've been discussing off-list... > + evt->done(evt); > + spin_lock_irqsave(vhost->host->host_lock, flags); > +} > + > +static struct ibmvfc_crq *ibmvfc_next_scrq(struct ibmvfc_sub_queue *scrq) > +{ > + struct ibmvfc_crq *crq; > + > + crq = &scrq->msgs[scrq->cur].crq; > + if (crq->valid & 0x80) { > + if (++scrq->cur == scrq->size) > + scrq->cur = 0; > + rmb(); > + } else > + crq = NULL; > + > + return crq; > +} > + > +static void ibmvfc_drain_sub_crq(struct ibmvfc_sub_queue *scrq) > +{ > + struct ibmvfc_crq *crq; > + unsigned long flags; > + int done = 0; > + > + spin_lock_irqsave(scrq->vhost->host->host_lock, flags); > + while (!done) { > + while ((crq = ibmvfc_next_scrq(scrq)) != NULL) { > + ibmvfc_handle_scrq(crq, scrq->vhost); > + crq->valid = 0; > + wmb(); > + } > + > + ibmvfc_toggle_scrq_irq(scrq, 1); > + if ((crq = ibmvfc_next_scrq(scrq)) != NULL) { > + ibmvfc_toggle_scrq_irq(scrq, 0); > + ibmvfc_handle_scrq(crq, scrq->vhost); > + crq->valid = 0; > + wmb(); > + } else > + done = 1; > + } > + spin_unlock_irqrestore(scrq->vhost->host->host_lock, flags); > +} > + > /** > * ibmvfc_init_tgt - Set the next init job step for the target > * @tgt: ibmvfc target struct > -- Brian King Power Linux I/O IBM Linux Technology Center
next prev parent reply other threads:[~2020-12-04 14:52 UTC|newest] Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-03 2:07 [PATCH v3 00/18] ibmvfc: initial MQ development Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-03 2:07 ` [PATCH v3 01/18] ibmvfc: add vhost fields and defaults for MQ enablement Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-03 2:07 ` [PATCH v3 02/18] ibmvfc: define hcall wrapper for registering a Sub-CRQ Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-03 2:07 ` [PATCH v3 03/18] ibmvfc: add Subordinate CRQ definitions Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-03 2:07 ` [PATCH v3 04/18] ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-04 14:47 ` Brian King 2020-12-04 14:47 ` Brian King 2020-12-05 0:15 ` Tyrel Datwyler 2020-12-05 0:15 ` Tyrel Datwyler 2020-12-03 2:07 ` [PATCH v3 05/18] ibmvfc: add Sub-CRQ IRQ enable/disable routine Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-03 2:07 ` [PATCH v3 06/18] ibmvfc: add handlers to drain and complete Sub-CRQ responses Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-04 14:51 ` Brian King [this message] 2020-12-04 14:51 ` Brian King 2020-12-05 0:16 ` Tyrel Datwyler 2020-12-05 0:16 ` Tyrel Datwyler 2020-12-03 2:07 ` [PATCH v3 07/18] ibmvfc: define Sub-CRQ interrupt handler routine Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-03 2:07 ` [PATCH v3 08/18] ibmvfc: map/request irq and register Sub-CRQ interrupt handler Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-03 2:07 ` [PATCH v3 09/18] ibmvfc: implement channel enquiry and setup commands Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-03 2:07 ` [PATCH v3 10/18] ibmvfc: advertise client support for using hardware channels Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-03 2:07 ` [PATCH v3 11/18] ibmvfc: set and track hw queue in ibmvfc_event struct Tyrel Datwyler 2020-12-03 2:07 ` Tyrel Datwyler 2020-12-03 2:08 ` [PATCH v3 12/18] ibmvfc: send commands down HW Sub-CRQ when channelized Tyrel Datwyler 2020-12-03 2:08 ` Tyrel Datwyler 2020-12-03 2:08 ` [PATCH v3 13/18] ibmvfc: register Sub-CRQ handles with VIOS during channel setup Tyrel Datwyler 2020-12-03 2:08 ` Tyrel Datwyler 2020-12-03 2:08 ` [PATCH v3 14/18] ibmvfc: add cancel mad initialization helper Tyrel Datwyler 2020-12-03 2:08 ` Tyrel Datwyler 2020-12-03 2:08 ` [PATCH v3 15/18] ibmvfc: send Cancel MAD down each hw scsi channel Tyrel Datwyler 2020-12-03 2:08 ` Tyrel Datwyler 2020-12-04 21:26 ` Brian King 2020-12-04 21:26 ` Brian King 2020-12-03 2:08 ` [PATCH v3 16/18] ibmvfc: enable MQ and set reasonable defaults Tyrel Datwyler 2020-12-03 2:08 ` Tyrel Datwyler 2020-12-03 2:08 ` [PATCH v3 17/18] ibmvfc: provide modules parameters for MQ settings Tyrel Datwyler 2020-12-03 2:08 ` Tyrel Datwyler 2020-12-04 21:28 ` Brian King 2020-12-04 21:28 ` Brian King 2020-12-03 2:08 ` [PATCH v3 18/18] ibmvfc: drop host lock when completing commands in CRQ Tyrel Datwyler 2020-12-03 2:08 ` Tyrel Datwyler 2020-12-04 21:35 ` Brian King 2020-12-04 21:35 ` Brian King 2020-12-05 0:20 ` Tyrel Datwyler 2020-12-05 0:20 ` Tyrel Datwyler
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=3fe8683a-47f6-8713-762a-02c57c2e4ec2@linux.vnet.ibm.com \ --to=brking@linux.vnet.ibm.com \ --cc=brking@linux.ibm.com \ --cc=james.bottomley@hansenpartnership.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=martin.petersen@oracle.com \ --cc=tyreld@linux.ibm.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.