diff mbox series

[04/13] ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels

Message ID 20201126014824.123831-5-tyreld@linux.ibm.com
State New
Headers show
Series ibmvfc: initial MQ development | expand

Commit Message

Tyrel Datwyler Nov. 26, 2020, 1:48 a.m. UTC
Allocate a set of Sub-CRQs in advance. During channel setup the client
and VIOS negotiate the number of queues the VIOS supports and the number
that the client desires to request. Its possible that the final channel
resources allocated is less than requested, but the client is still
responsible for sending handles for every queue it is hoping for.

Also, provide deallocation cleanup routines.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvfc.c | 115 +++++++++++++++++++++++++++++++++
 drivers/scsi/ibmvscsi/ibmvfc.h |   1 +
 2 files changed, 116 insertions(+)

Comments

Brian King Nov. 27, 2020, 5:46 p.m. UTC | #1
On 11/25/20 7:48 PM, Tyrel Datwyler wrote:
> Allocate a set of Sub-CRQs in advance. During channel setup the client

> and VIOS negotiate the number of queues the VIOS supports and the number

> that the client desires to request. Its possible that the final channel

> resources allocated is less than requested, but the client is still

> responsible for sending handles for every queue it is hoping for.

> 

> Also, provide deallocation cleanup routines.

> 

> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>

> ---

>  drivers/scsi/ibmvscsi/ibmvfc.c | 115 +++++++++++++++++++++++++++++++++

>  drivers/scsi/ibmvscsi/ibmvfc.h |   1 +

>  2 files changed, 116 insertions(+)

> 

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c

> index 260b82e3cc01..571abdb48384 100644

> --- a/drivers/scsi/ibmvscsi/ibmvfc.c

> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c

> @@ -4983,6 +4983,114 @@ static int ibmvfc_init_crq(struct ibmvfc_host *vhost)

>  	return retrc;

>  }

>  

> +static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,

> +				  int index)

> +{

> +	struct device *dev = vhost->dev;

> +	struct vio_dev *vdev = to_vio_dev(dev);

> +	struct ibmvfc_sub_queue *scrq = &vhost->scsi_scrqs.scrqs[index];

> +	int rc = -ENOMEM;

> +

> +	ENTER;

> +

> +	scrq->msgs = (struct ibmvfc_sub_crq *)get_zeroed_page(GFP_KERNEL);

> +	if (!scrq->msgs)

> +		return rc;

> +

> +	scrq->size = PAGE_SIZE / sizeof(*scrq->msgs);

> +	scrq->msg_token = dma_map_single(dev, scrq->msgs, PAGE_SIZE,

> +					 DMA_BIDIRECTIONAL);

> +

> +	if (dma_mapping_error(dev, scrq->msg_token))

> +		goto dma_map_failed;

> +

> +	rc = h_reg_sub_crq(vdev->unit_address, scrq->msg_token, PAGE_SIZE,

> +			   &scrq->cookie, &scrq->hw_irq);

> +

> +	if (rc) {

> +		dev_warn(dev, "Error registering sub-crq: %d\n", rc);

> +		dev_warn(dev, "Firmware may not support MQ\n");

> +		goto reg_failed;

> +	}

> +

> +	scrq->hwq_id = index;

> +	scrq->vhost = vhost;

> +

> +	LEAVE;

> +	return 0;

> +

> +reg_failed:

> +	dma_unmap_single(dev, scrq->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);

> +dma_map_failed:

> +	free_page((unsigned long)scrq->msgs);

> +	LEAVE;

> +	return rc;

> +}

> +

> +static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)

> +{

> +	struct device *dev = vhost->dev;

> +	struct vio_dev *vdev = to_vio_dev(dev);

> +	struct ibmvfc_sub_queue *scrq = &vhost->scsi_scrqs.scrqs[index];

> +	long rc;

> +

> +	ENTER;

> +

> +	do {

> +		rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,

> +					scrq->cookie);

> +	} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));

> +

> +	if (rc)

> +		dev_err(dev, "Failed to free sub-crq[%d]: rc=%ld\n", index, rc);

> +

> +	dma_unmap_single(dev, scrq->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);

> +	free_page((unsigned long)scrq->msgs);

> +	LEAVE;

> +}

> +

> +static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)

> +{

> +	int i, j;

> +

> +	ENTER;

> +

> +	vhost->scsi_scrqs.scrqs = kcalloc(vhost->client_scsi_channels,

> +					  sizeof(*vhost->scsi_scrqs.scrqs),

> +					  GFP_KERNEL);

> +	if (!vhost->scsi_scrqs.scrqs)

> +		return -1;

> +

> +	for (i = 0; i < vhost->client_scsi_channels; i++) {

> +		if (ibmvfc_register_scsi_channel(vhost, i)) {

> +			for (j = i; j > 0; j--)

> +				ibmvfc_deregister_scsi_channel(vhost, j - 1);

> +			kfree(vhost->scsi_scrqs.scrqs);

> +			LEAVE;

> +			return -1;

> +		}

> +	}

> +

> +	LEAVE;

> +	return 0;

> +}

> +

> +static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)

> +{

> +	int i;

> +

> +	ENTER;

> +	if (!vhost->scsi_scrqs.scrqs)

> +		return;

> +

> +	for (i = 0; i < vhost->client_scsi_channels; i++)

> +		ibmvfc_deregister_scsi_channel(vhost, i);

> +

> +	vhost->scsi_scrqs.active_queues = 0;

> +	kfree(vhost->scsi_scrqs.scrqs);


Do you want to NULL this out after you free it do you don't keep
a reference to a freed page around?

> +	LEAVE;

> +}

> +

>  /**

>   * ibmvfc_free_mem - Free memory for vhost

>   * @vhost:	ibmvfc host struct




-- 
Brian King
Power Linux I/O
IBM Linux Technology Center
Tyrel Datwyler Nov. 30, 2020, 5:26 p.m. UTC | #2
On 11/27/20 9:46 AM, Brian King wrote:
> On 11/25/20 7:48 PM, Tyrel Datwyler wrote:

>> Allocate a set of Sub-CRQs in advance. During channel setup the client

>> and VIOS negotiate the number of queues the VIOS supports and the number

>> that the client desires to request. Its possible that the final channel

>> resources allocated is less than requested, but the client is still

>> responsible for sending handles for every queue it is hoping for.

>>

>> Also, provide deallocation cleanup routines.

>>

>> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>

>> ---

>>  drivers/scsi/ibmvscsi/ibmvfc.c | 115 +++++++++++++++++++++++++++++++++

>>  drivers/scsi/ibmvscsi/ibmvfc.h |   1 +

>>  2 files changed, 116 insertions(+)

>>

>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c

>> index 260b82e3cc01..571abdb48384 100644

>> --- a/drivers/scsi/ibmvscsi/ibmvfc.c

>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c

>> @@ -4983,6 +4983,114 @@ static int ibmvfc_init_crq(struct ibmvfc_host *vhost)

>>  	return retrc;

>>  }

>>  

>> +static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,

>> +				  int index)

>> +{

>> +	struct device *dev = vhost->dev;

>> +	struct vio_dev *vdev = to_vio_dev(dev);

>> +	struct ibmvfc_sub_queue *scrq = &vhost->scsi_scrqs.scrqs[index];

>> +	int rc = -ENOMEM;

>> +

>> +	ENTER;

>> +

>> +	scrq->msgs = (struct ibmvfc_sub_crq *)get_zeroed_page(GFP_KERNEL);

>> +	if (!scrq->msgs)

>> +		return rc;

>> +

>> +	scrq->size = PAGE_SIZE / sizeof(*scrq->msgs);

>> +	scrq->msg_token = dma_map_single(dev, scrq->msgs, PAGE_SIZE,

>> +					 DMA_BIDIRECTIONAL);

>> +

>> +	if (dma_mapping_error(dev, scrq->msg_token))

>> +		goto dma_map_failed;

>> +

>> +	rc = h_reg_sub_crq(vdev->unit_address, scrq->msg_token, PAGE_SIZE,

>> +			   &scrq->cookie, &scrq->hw_irq);

>> +

>> +	if (rc) {

>> +		dev_warn(dev, "Error registering sub-crq: %d\n", rc);

>> +		dev_warn(dev, "Firmware may not support MQ\n");

>> +		goto reg_failed;

>> +	}

>> +

>> +	scrq->hwq_id = index;

>> +	scrq->vhost = vhost;

>> +

>> +	LEAVE;

>> +	return 0;

>> +

>> +reg_failed:

>> +	dma_unmap_single(dev, scrq->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);

>> +dma_map_failed:

>> +	free_page((unsigned long)scrq->msgs);

>> +	LEAVE;

>> +	return rc;

>> +}

>> +

>> +static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)

>> +{

>> +	struct device *dev = vhost->dev;

>> +	struct vio_dev *vdev = to_vio_dev(dev);

>> +	struct ibmvfc_sub_queue *scrq = &vhost->scsi_scrqs.scrqs[index];

>> +	long rc;

>> +

>> +	ENTER;

>> +

>> +	do {

>> +		rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,

>> +					scrq->cookie);

>> +	} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));

>> +

>> +	if (rc)

>> +		dev_err(dev, "Failed to free sub-crq[%d]: rc=%ld\n", index, rc);

>> +

>> +	dma_unmap_single(dev, scrq->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);

>> +	free_page((unsigned long)scrq->msgs);

>> +	LEAVE;

>> +}

>> +

>> +static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)

>> +{

>> +	int i, j;

>> +

>> +	ENTER;

>> +

>> +	vhost->scsi_scrqs.scrqs = kcalloc(vhost->client_scsi_channels,

>> +					  sizeof(*vhost->scsi_scrqs.scrqs),

>> +					  GFP_KERNEL);

>> +	if (!vhost->scsi_scrqs.scrqs)

>> +		return -1;

>> +

>> +	for (i = 0; i < vhost->client_scsi_channels; i++) {

>> +		if (ibmvfc_register_scsi_channel(vhost, i)) {

>> +			for (j = i; j > 0; j--)

>> +				ibmvfc_deregister_scsi_channel(vhost, j - 1);

>> +			kfree(vhost->scsi_scrqs.scrqs);

>> +			LEAVE;

>> +			return -1;

>> +		}

>> +	}

>> +

>> +	LEAVE;

>> +	return 0;

>> +}

>> +

>> +static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)

>> +{

>> +	int i;

>> +

>> +	ENTER;

>> +	if (!vhost->scsi_scrqs.scrqs)

>> +		return;

>> +

>> +	for (i = 0; i < vhost->client_scsi_channels; i++)

>> +		ibmvfc_deregister_scsi_channel(vhost, i);

>> +

>> +	vhost->scsi_scrqs.active_queues = 0;

>> +	kfree(vhost->scsi_scrqs.scrqs);

> 

> Do you want to NULL this out after you free it do you don't keep

> a reference to a freed page around?


This isn't actually a page, but a dynamically allocated array of
ibmvfc_sub_queues, but it should be NULL'ed regardless.

-Tyrel

> 

>> +	LEAVE;

>> +}

>> +

>>  /**

>>   * ibmvfc_free_mem - Free memory for vhost

>>   * @vhost:	ibmvfc host struct

> 

> 

>
diff mbox series

Patch

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 260b82e3cc01..571abdb48384 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -4983,6 +4983,114 @@  static int ibmvfc_init_crq(struct ibmvfc_host *vhost)
 	return retrc;
 }
 
+static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
+				  int index)
+{
+	struct device *dev = vhost->dev;
+	struct vio_dev *vdev = to_vio_dev(dev);
+	struct ibmvfc_sub_queue *scrq = &vhost->scsi_scrqs.scrqs[index];
+	int rc = -ENOMEM;
+
+	ENTER;
+
+	scrq->msgs = (struct ibmvfc_sub_crq *)get_zeroed_page(GFP_KERNEL);
+	if (!scrq->msgs)
+		return rc;
+
+	scrq->size = PAGE_SIZE / sizeof(*scrq->msgs);
+	scrq->msg_token = dma_map_single(dev, scrq->msgs, PAGE_SIZE,
+					 DMA_BIDIRECTIONAL);
+
+	if (dma_mapping_error(dev, scrq->msg_token))
+		goto dma_map_failed;
+
+	rc = h_reg_sub_crq(vdev->unit_address, scrq->msg_token, PAGE_SIZE,
+			   &scrq->cookie, &scrq->hw_irq);
+
+	if (rc) {
+		dev_warn(dev, "Error registering sub-crq: %d\n", rc);
+		dev_warn(dev, "Firmware may not support MQ\n");
+		goto reg_failed;
+	}
+
+	scrq->hwq_id = index;
+	scrq->vhost = vhost;
+
+	LEAVE;
+	return 0;
+
+reg_failed:
+	dma_unmap_single(dev, scrq->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);
+dma_map_failed:
+	free_page((unsigned long)scrq->msgs);
+	LEAVE;
+	return rc;
+}
+
+static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
+{
+	struct device *dev = vhost->dev;
+	struct vio_dev *vdev = to_vio_dev(dev);
+	struct ibmvfc_sub_queue *scrq = &vhost->scsi_scrqs.scrqs[index];
+	long rc;
+
+	ENTER;
+
+	do {
+		rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
+					scrq->cookie);
+	} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
+
+	if (rc)
+		dev_err(dev, "Failed to free sub-crq[%d]: rc=%ld\n", index, rc);
+
+	dma_unmap_single(dev, scrq->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);
+	free_page((unsigned long)scrq->msgs);
+	LEAVE;
+}
+
+static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
+{
+	int i, j;
+
+	ENTER;
+
+	vhost->scsi_scrqs.scrqs = kcalloc(vhost->client_scsi_channels,
+					  sizeof(*vhost->scsi_scrqs.scrqs),
+					  GFP_KERNEL);
+	if (!vhost->scsi_scrqs.scrqs)
+		return -1;
+
+	for (i = 0; i < vhost->client_scsi_channels; i++) {
+		if (ibmvfc_register_scsi_channel(vhost, i)) {
+			for (j = i; j > 0; j--)
+				ibmvfc_deregister_scsi_channel(vhost, j - 1);
+			kfree(vhost->scsi_scrqs.scrqs);
+			LEAVE;
+			return -1;
+		}
+	}
+
+	LEAVE;
+	return 0;
+}
+
+static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)
+{
+	int i;
+
+	ENTER;
+	if (!vhost->scsi_scrqs.scrqs)
+		return;
+
+	for (i = 0; i < vhost->client_scsi_channels; i++)
+		ibmvfc_deregister_scsi_channel(vhost, i);
+
+	vhost->scsi_scrqs.active_queues = 0;
+	kfree(vhost->scsi_scrqs.scrqs);
+	LEAVE;
+}
+
 /**
  * ibmvfc_free_mem - Free memory for vhost
  * @vhost:	ibmvfc host struct
@@ -5239,6 +5347,12 @@  static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 		goto remove_shost;
 	}
 
+	if (vhost->mq_enabled) {
+		rc = ibmvfc_init_sub_crqs(vhost);
+		if (rc)
+			dev_warn(dev, "Failed to allocate Sub-CRQs. rc=%d\n", rc);
+	}
+
 	if (shost_to_fc_host(shost)->rqst_q)
 		blk_queue_max_segments(shost_to_fc_host(shost)->rqst_q, 1);
 	dev_set_drvdata(dev, vhost);
@@ -5296,6 +5410,7 @@  static int ibmvfc_remove(struct vio_dev *vdev)
 	ibmvfc_purge_requests(vhost, DID_ERROR);
 	spin_unlock_irqrestore(vhost->host->host_lock, flags);
 	ibmvfc_free_event_pool(vhost);
+	ibmvfc_release_sub_crqs(vhost);
 
 	ibmvfc_free_mem(vhost);
 	spin_lock(&ibmvfc_driver_lock);
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 084ecdfe51ea..d80c7eeef409 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -838,6 +838,7 @@  struct ibmvfc_host {
 	mempool_t *tgt_pool;
 	struct ibmvfc_crq_queue crq;
 	struct ibmvfc_async_crq_queue async_crq;
+	struct ibmvfc_scsi_channels scsi_scrqs;
 	struct ibmvfc_npiv_login login_info;
 	union ibmvfc_npiv_login_data *login_buf;
 	dma_addr_t login_buf_dma;