Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: Ming Lei <ming.lei@redhat.com>, Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	"Martin K . Petersen" <martin.petersen@oracle.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>,
	Bart Van Assche <bvanassche@acm.org>,
	Hannes Reinecke <hare@suse.com>,
	John Garry <john.garry@huawei.com>,
	Don Brace <don.brace@microsemi.com>,
	Kashyap Desai <kashyap.desai@broadcom.com>,
	Sathya Prakash <sathya.prakash@broadcom.com>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH 9/9] scsi: mp3sas: convert private reply queue to blk-mq hw queue
Date: Fri, 31 May 2019 08:23:41 +0200
Message-ID: <280cb566-a40c-9fb2-4aa6-0641442fdafd@suse.de> (raw)
In-Reply-To: <20190531022801.10003-10-ming.lei@redhat.com>

On 5/31/19 4:28 AM, Ming Lei wrote:
> SCSI's reply qeueue is very similar with blk-mq's hw queue, both
> assigned by IRQ vector, so map te private reply queue into blk-mq's hw
> queue via .host_tagset.
> 
> Then the private reply mapping can be removed.
> 
> Another benefit is that the request/irq lost issue may be solved in
> generic approach because managed IRQ may be shutdown during CPU
> hotplug.
> 
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c  | 74 +++++-----------------------
>  drivers/scsi/mpt3sas/mpt3sas_base.h  |  3 +-
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 17 +++++++
>  3 files changed, 31 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 8aacbd1e7db2..2b207d2925b4 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -2855,8 +2855,7 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index)
>  static void
>  _base_assign_reply_queues(struct MPT3SAS_ADAPTER *ioc)
>  {
> -	unsigned int cpu, nr_cpus, nr_msix, index = 0;
> -	struct adapter_reply_queue *reply_q;
> +	unsigned int nr_cpus, nr_msix;
>  
>  	if (!_base_is_controller_msix_enabled(ioc))
>  		return;
> @@ -2866,50 +2865,9 @@ _base_assign_reply_queues(struct MPT3SAS_ADAPTER *ioc)
>  		return;
>  	}
>  
> -	memset(ioc->cpu_msix_table, 0, ioc->cpu_msix_table_sz);
> -
>  	nr_cpus = num_online_cpus();
>  	nr_msix = ioc->reply_queue_count = min(ioc->reply_queue_count,
>  					       ioc->facts.MaxMSIxVectors);
> -	if (!nr_msix)
> -		return;
> -
> -	if (smp_affinity_enable) {
> -		list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
> -			const cpumask_t *mask = pci_irq_get_affinity(ioc->pdev,
> -							reply_q->msix_index);
> -			if (!mask) {
> -				ioc_warn(ioc, "no affinity for msi %x\n",
> -					 reply_q->msix_index);
> -				continue;
> -			}
> -
> -			for_each_cpu_and(cpu, mask, cpu_online_mask) {
> -				if (cpu >= ioc->cpu_msix_table_sz)
> -					break;
> -				ioc->cpu_msix_table[cpu] = reply_q->msix_index;
> -			}
> -		}
> -		return;
> -	}
> -	cpu = cpumask_first(cpu_online_mask);
> -
> -	list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
> -
> -		unsigned int i, group = nr_cpus / nr_msix;
> -
> -		if (cpu >= nr_cpus)
> -			break;
> -
> -		if (index < nr_cpus % nr_msix)
> -			group++;
> -
> -		for (i = 0 ; i < group ; i++) {
> -			ioc->cpu_msix_table[cpu] = reply_q->msix_index;
> -			cpu = cpumask_next(cpu, cpu_online_mask);
> -		}
> -		index++;
> -	}
>  }
>  
>  /**
> @@ -2924,6 +2882,7 @@ _base_disable_msix(struct MPT3SAS_ADAPTER *ioc)
>  		return;
>  	pci_disable_msix(ioc->pdev);
>  	ioc->msix_enable = 0;
> +	ioc->smp_affinity_enable = 0;
>  }
>  
>  /**
> @@ -2980,6 +2939,9 @@ _base_enable_msix(struct MPT3SAS_ADAPTER *ioc)
>  		goto try_ioapic;
>  	}
>  
> +	if (irq_flags & PCI_IRQ_AFFINITY)
> +		ioc->smp_affinity_enable = 1;
> +
>  	ioc->msix_enable = 1;
>  	ioc->reply_queue_count = r;
>  	for (i = 0; i < ioc->reply_queue_count; i++) {
> @@ -3266,7 +3228,7 @@ mpt3sas_base_get_reply_virt_addr(struct MPT3SAS_ADAPTER *ioc, u32 phys_addr)
>  }
>  
>  static inline u8
> -_base_get_msix_index(struct MPT3SAS_ADAPTER *ioc)
> +_base_get_msix_index(struct MPT3SAS_ADAPTER *ioc, struct scsi_cmnd *scmd)
>  {
>  	/* Enables reply_queue load balancing */
>  	if (ioc->msix_load_balance)
> @@ -3274,7 +3236,7 @@ _base_get_msix_index(struct MPT3SAS_ADAPTER *ioc)
>  		    base_mod64(atomic64_add_return(1,
>  		    &ioc->total_io_cnt), ioc->reply_queue_count) : 0;
>  
> -	return ioc->cpu_msix_table[raw_smp_processor_id()];
> +	return scsi_cmnd_hctx_index(ioc->shost, scmd);
>  }
>  
>  /**
> @@ -3325,7 +3287,7 @@ mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
>  
>  	smid = tag + 1;
>  	request->cb_idx = cb_idx;
> -	request->msix_io = _base_get_msix_index(ioc);
> +	request->msix_io = _base_get_msix_index(ioc, scmd);
>  	request->smid = smid;
>  	INIT_LIST_HEAD(&request->chain_list);
>  	return smid;
> @@ -3498,7 +3460,7 @@ _base_put_smid_mpi_ep_scsi_io(struct MPT3SAS_ADAPTER *ioc, u16 smid, u16 handle)
>  	_base_clone_mpi_to_sys_mem(mpi_req_iomem, (void *)mfp,
>  					ioc->request_sz);
>  	descriptor.SCSIIO.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_SCSI_IO;
> -	descriptor.SCSIIO.MSIxIndex =  _base_get_msix_index(ioc);
> +	descriptor.SCSIIO.MSIxIndex =  _base_get_msix_index(ioc, NULL);
>  	descriptor.SCSIIO.SMID = cpu_to_le16(smid);
>  	descriptor.SCSIIO.DevHandle = cpu_to_le16(handle);
>  	descriptor.SCSIIO.LMID = 0;
> @@ -3520,7 +3482,7 @@ _base_put_smid_scsi_io(struct MPT3SAS_ADAPTER *ioc, u16 smid, u16 handle)
>  
>  
>  	descriptor.SCSIIO.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_SCSI_IO;
> -	descriptor.SCSIIO.MSIxIndex =  _base_get_msix_index(ioc);
> +	descriptor.SCSIIO.MSIxIndex =  _base_get_msix_index(ioc, NULL);
>  	descriptor.SCSIIO.SMID = cpu_to_le16(smid);
>  	descriptor.SCSIIO.DevHandle = cpu_to_le16(handle);
>  	descriptor.SCSIIO.LMID = 0;
> @@ -3543,7 +3505,7 @@ mpt3sas_base_put_smid_fast_path(struct MPT3SAS_ADAPTER *ioc, u16 smid,
>  
>  	descriptor.SCSIIO.RequestFlags =
>  	    MPI25_REQ_DESCRIPT_FLAGS_FAST_PATH_SCSI_IO;
> -	descriptor.SCSIIO.MSIxIndex = _base_get_msix_index(ioc);
> +	descriptor.SCSIIO.MSIxIndex = _base_get_msix_index(ioc, NULL);
>  	descriptor.SCSIIO.SMID = cpu_to_le16(smid);
>  	descriptor.SCSIIO.DevHandle = cpu_to_le16(handle);
>  	descriptor.SCSIIO.LMID = 0;
> @@ -3607,7 +3569,7 @@ mpt3sas_base_put_smid_nvme_encap(struct MPT3SAS_ADAPTER *ioc, u16 smid)
>  
>  	descriptor.Default.RequestFlags =
>  		MPI26_REQ_DESCRIPT_FLAGS_PCIE_ENCAPSULATED;
> -	descriptor.Default.MSIxIndex =  _base_get_msix_index(ioc);
> +	descriptor.Default.MSIxIndex =  _base_get_msix_index(ioc, NULL);
>  	descriptor.Default.SMID = cpu_to_le16(smid);
>  	descriptor.Default.LMID = 0;
>  	descriptor.Default.DescriptorTypeDependent = 0;
> @@ -3639,7 +3601,7 @@ mpt3sas_base_put_smid_default(struct MPT3SAS_ADAPTER *ioc, u16 smid)
>  	}
>  	request = (u64 *)&descriptor;
>  	descriptor.Default.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_DEFAULT_TYPE;
> -	descriptor.Default.MSIxIndex =  _base_get_msix_index(ioc);
> +	descriptor.Default.MSIxIndex =  _base_get_msix_index(ioc, NULL);
>  	descriptor.Default.SMID = cpu_to_le16(smid);
>  	descriptor.Default.LMID = 0;
>  	descriptor.Default.DescriptorTypeDependent = 0;
> @@ -6524,19 +6486,11 @@ mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc)
>  
>  	dinitprintk(ioc, ioc_info(ioc, "%s\n", __func__));
>  
> -	/* setup cpu_msix_table */
>  	ioc->cpu_count = num_online_cpus();
>  	for_each_online_cpu(cpu_id)
>  		last_cpu_id = cpu_id;
>  	ioc->cpu_msix_table_sz = last_cpu_id + 1;
> -	ioc->cpu_msix_table = kzalloc(ioc->cpu_msix_table_sz, GFP_KERNEL);
>  	ioc->reply_queue_count = 1;
> -	if (!ioc->cpu_msix_table) {
> -		dfailprintk(ioc,
> -			    ioc_info(ioc, "allocation for cpu_msix_table failed!!!\n"));
> -		r = -ENOMEM;
> -		goto out_free_resources;
> -	}
>  
>  	if (ioc->is_warpdrive) {
>  		ioc->reply_post_host_index = kcalloc(ioc->cpu_msix_table_sz,
> @@ -6748,7 +6702,6 @@ mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc)
>  	mpt3sas_base_free_resources(ioc);
>  	_base_release_memory_pools(ioc);
>  	pci_set_drvdata(ioc->pdev, NULL);
> -	kfree(ioc->cpu_msix_table);
>  	if (ioc->is_warpdrive)
>  		kfree(ioc->reply_post_host_index);
>  	kfree(ioc->pd_handles);
> @@ -6789,7 +6742,6 @@ mpt3sas_base_detach(struct MPT3SAS_ADAPTER *ioc)
>  	_base_release_memory_pools(ioc);
>  	mpt3sas_free_enclosure_list(ioc);
>  	pci_set_drvdata(ioc->pdev, NULL);
> -	kfree(ioc->cpu_msix_table);
>  	if (ioc->is_warpdrive)
>  		kfree(ioc->reply_post_host_index);
>  	kfree(ioc->pd_handles);
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
> index 480219f0efc5..4d441e031025 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.h
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
> @@ -1022,7 +1022,6 @@ typedef void (*MPT3SAS_FLUSH_RUNNING_CMDS)(struct MPT3SAS_ADAPTER *ioc);
>   * @start_scan_failed: means port enable failed, return's the ioc_status
>   * @msix_enable: flag indicating msix is enabled
>   * @msix_vector_count: number msix vectors
> - * @cpu_msix_table: table for mapping cpus to msix index
>   * @cpu_msix_table_sz: table size
>   * @total_io_cnt: Gives total IO count, used to load balance the interrupts
>   * @msix_load_balance: Enables load balancing of interrupts across
> @@ -1183,6 +1182,7 @@ struct MPT3SAS_ADAPTER {
>  	u16		broadcast_aen_pending;
>  	u8		shost_recovery;
>  	u8		got_task_abort_from_ioctl;
> +	u8		smp_affinity_enable;
>  
>  	struct mutex	reset_in_progress_mutex;
>  	spinlock_t	ioc_reset_in_progress_lock;
> @@ -1199,7 +1199,6 @@ struct MPT3SAS_ADAPTER {
>  
>  	u8		msix_enable;
>  	u16		msix_vector_count;
> -	u8		*cpu_msix_table;
>  	u16		cpu_msix_table_sz;
>  	resource_size_t __iomem **reply_post_host_index;
>  	u32		ioc_reset_count;
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 1ccfbc7eebe0..59c1f9e694a0 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -55,6 +55,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/aer.h>
>  #include <linux/raid_class.h>
> +#include <linux/blk-mq-pci.h>
>  #include <asm/unaligned.h>
>  
>  #include "mpt3sas_base.h"
> @@ -10161,6 +10162,17 @@ scsih_scan_finished(struct Scsi_Host *shost, unsigned long time)
>  	return 1;
>  }
>  
> +static int mpt3sas_map_queues(struct Scsi_Host *shost)
> +{
> +	struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
> +	struct blk_mq_queue_map *qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT];
> +
> +	if (ioc->smp_affinity_enable)
> +		return blk_mq_pci_map_queues(qmap, ioc->pdev, 0);
> +	else
> +		return blk_mq_map_queues(qmap);
> +}
> +
>  /* shost template for SAS 2.0 HBA devices */
>  static struct scsi_host_template mpt2sas_driver_template = {
>  	.module				= THIS_MODULE,

As indicated, we should be using a common function here.

> @@ -10189,6 +10201,8 @@ static struct scsi_host_template mpt2sas_driver_template = {
>  	.sdev_attrs			= mpt3sas_dev_attrs,
>  	.track_queue_depth		= 1,
>  	.cmd_size			= sizeof(struct scsiio_tracker),
> +	.host_tagset			= 1,
> +	.map_queues			= mpt3sas_map_queues,
>  };
>  
>  /* raid transport support for SAS 2.0 HBA devices */
> @@ -10227,6 +10241,8 @@ static struct scsi_host_template mpt3sas_driver_template = {
>  	.sdev_attrs			= mpt3sas_dev_attrs,
>  	.track_queue_depth		= 1,
>  	.cmd_size			= sizeof(struct scsiio_tracker),
> +	.host_tagset			= 1,
> +	.map_queues			= mpt3sas_map_queues,
>  };
>  
>  /* raid transport support for SAS 3.0 HBA devices */
> @@ -10538,6 +10554,7 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	} else
>  		ioc->hide_drives = 0;
>  
> +	shost->nr_hw_queues = ioc->reply_queue_count;
>  	rv = scsi_add_host(shost, &pdev->dev);
>  	if (rv) {
>  		ioc_err(ioc, "failure at %s:%d/%s()!\n",
> 
Otherwise:

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)

  reply index

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-31  2:27 [PATCH 0/9] blk-mq/scsi: convert private reply queue into blk_mq " Ming Lei
2019-05-31  2:27 ` [PATCH 1/9] blk-mq: allow hw queues to share hostwide tags Ming Lei
2019-05-31  6:07   ` Hannes Reinecke
2019-05-31 15:37   ` Bart Van Assche
2019-06-24  8:44     ` Ming Lei
2019-06-05 14:10   ` John Garry
2019-06-24  8:46     ` Ming Lei
2019-06-24 13:14       ` John Garry
2019-05-31  2:27 ` [PATCH 2/9] block: null_blk: introduce module parameter of 'g_host_tags' Ming Lei
2019-05-31  6:08   ` Hannes Reinecke
2019-05-31 15:39   ` Bart Van Assche
2019-06-24  8:43     ` Ming Lei
2019-06-02  1:56   ` Minwoo Im
2019-05-31  2:27 ` [PATCH 3/9] scsi: Add template flag 'host_tagset' Ming Lei
2019-05-31  6:08   ` Hannes Reinecke
2019-05-31  2:27 ` [PATCH 4/9] scsi_debug: support host tagset Ming Lei
2019-05-31  6:09   ` Hannes Reinecke
2019-06-02  2:03   ` Minwoo Im
2019-06-02 17:01   ` Douglas Gilbert
2019-05-31  2:27 ` [PATCH 5/9] scsi: introduce scsi_cmnd_hctx_index() Ming Lei
2019-05-31  6:10   ` Hannes Reinecke
2019-05-31  2:27 ` [PATCH 6/9] scsi: hpsa: convert private reply queue to blk-mq hw queue Ming Lei
2019-05-31  6:15   ` Hannes Reinecke
2019-05-31  6:30     ` Ming Lei
2019-05-31  6:40       ` Hannes Reinecke
2019-05-31  2:27 ` [PATCH 7/9] scsi: hisi_sas_v3: " Ming Lei
2019-05-31  6:20   ` Hannes Reinecke
2019-05-31  6:34     ` Ming Lei
2019-05-31  6:42       ` Hannes Reinecke
2019-05-31  7:14         ` Ming Lei
2019-05-31 11:38       ` John Garry
2019-06-03 11:00         ` Ming Lei
2019-06-03 13:00           ` John Garry
2019-06-04 13:37             ` Ming Lei
2019-05-31  2:28 ` [PATCH 8/9] scsi: megaraid: " Ming Lei
2019-05-31  6:22   ` Hannes Reinecke
2019-06-01 21:41   ` Kashyap Desai
2019-06-02  6:42     ` Ming Lei
2019-06-02  7:48       ` Ming Lei
2019-06-02 16:34         ` Kashyap Desai
2019-06-03  3:56           ` Ming Lei
2019-06-03 10:00             ` Kashyap Desai
2019-06-07  9:45             ` Kashyap Desai
2019-05-31  2:28 ` [PATCH 9/9] scsi: mp3sas: " Ming Lei
2019-05-31  6:23   ` Hannes Reinecke [this message]
2019-06-06 11:58   ` Sreekanth Reddy
2019-06-04  8:49 ` [PATCH 0/9] blk-mq/scsi: convert private reply queue into blk_mq " John Garry
2019-08-13  8:30   ` John Garry

Reply instructions:

You may reply publically 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=280cb566-a40c-9fb2-4aa6-0641442fdafd@suse.de \
    --to=hare@suse.de \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=don.brace@microsemi.com \
    --cc=hare@suse.com \
    --cc=hch@lst.de \
    --cc=john.garry@huawei.com \
    --cc=kashyap.desai@broadcom.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=ming.lei@redhat.com \
    --cc=sathya.prakash@broadcom.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: link

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org
	public-inbox-index linux-block

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git