Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
From: Ming Lei <ming.lei@redhat.com>
To: John Garry <john.garry@huawei.com>
Cc: Ming Lei <tom.leiming@gmail.com>, Hannes Reinecke <hare@suse.de>,
	Jens Axboe <axboe@kernel.dk>,
	linux-block <linux-block@vger.kernel.org>,
	Linux SCSI List <linux-scsi@vger.kernel.org>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	James Bottomley <James.Bottomley@hansenpartnership.com>,
	Bart Van Assche <bvanassche@acm.org>,
	Hannes Reinecke <hare@suse.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 7/9] scsi: hisi_sas_v3: convert private reply queue to blk-mq hw queue
Date: Mon, 3 Jun 2019 19:00:56 +0800
Message-ID: <20190603110054.GG11812@ming.t460p> (raw)
In-Reply-To: <c11faee4-fc38-9636-59b4-bc5c0d94ffbf@huawei.com>

On Fri, May 31, 2019 at 12:38:10PM +0100, John Garry wrote:
> 
> > > > -fallback:
> > > > -     for_each_possible_cpu(cpu)
> > > > -             hisi_hba->reply_map[cpu] = cpu % hisi_hba->queue_count;
> > > > -     /* Don't clean all CQ masks */
> > > > -}
> > > > -
> > > >  static int interrupt_init_v3_hw(struct hisi_hba *hisi_hba)
> > > >  {
> > > >       struct device *dev = hisi_hba->dev;
> > > > @@ -2383,11 +2359,6 @@ static int interrupt_init_v3_hw(struct hisi_hba *hisi_hba)
> > > > 
> > > >               min_msi = MIN_AFFINE_VECTORS_V3_HW;
> > > > 
> > > > -             hisi_hba->reply_map = devm_kcalloc(dev, nr_cpu_ids,
> > > > -                                                sizeof(unsigned int),
> > > > -                                                GFP_KERNEL);
> > > > -             if (!hisi_hba->reply_map)
> > > > -                     return -ENOMEM;
> > > >               vectors = pci_alloc_irq_vectors_affinity(hisi_hba->pci_dev,
> > > >                                                        min_msi, max_msi,
> > > >                                                        PCI_IRQ_MSI |
> > > > @@ -2395,7 +2366,6 @@ static int interrupt_init_v3_hw(struct hisi_hba *hisi_hba)
> > > >                                                        &desc);
> > > >               if (vectors < 0)
> > > >                       return -ENOENT;
> > > > -             setup_reply_map_v3_hw(hisi_hba, vectors - BASE_VECTORS_V3_HW);
> > > >       } else {
> > > >               min_msi = max_msi;
> > > >               vectors = pci_alloc_irq_vectors(hisi_hba->pci_dev, min_msi,
> > > > @@ -2896,6 +2866,18 @@ static void debugfs_snapshot_restore_v3_hw(struct hisi_hba *hisi_hba)
> > > >       clear_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags);
> > > >  }
> > > > 
> > > > +static int hisi_sas_map_queues(struct Scsi_Host *shost)
> > > > +{
> > > > +     struct hisi_hba *hisi_hba = shost_priv(shost);
> > > > +     struct blk_mq_queue_map *qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT];
> > > > +
> > > > +     if (auto_affine_msi_experimental)
> > > > +             return blk_mq_pci_map_queues(qmap, hisi_hba->pci_dev,
> > > > +                             BASE_VECTORS_V3_HW);
> > > > +     else
> > > > +             return blk_mq_map_queues(qmap);
> 
> I don't think that the mapping which blk_mq_map_queues() creates are not
> want we want. I'm guessing that we still would like a mapping similar to
> what blk_mq_pci_map_queues() produces, which is an even spread, putting
> adjacent CPUs on the same queue.
> 
> For my system with 96 cpus and 16 queues, blk_mq_map_queues() would map
> queue 0 to cpu 0, 16, 32, 48 ..., queue 1 to cpu 1, 17, 33 and so on.

blk_mq_map_queues() is the default or fallback mapping in case that managed
irq isn't used. If the mapping isn't good enough, we still can improve it
in future, then any driver applying it can got improved.

> 
> > > > +}
> > > > +
> > > >  static struct scsi_host_template sht_v3_hw = {
> > > >       .name                   = DRV_NAME,
> > > >       .module                 = THIS_MODULE,
> > > 
> > > As mentioned, we should be using a common function here.
> > > 
> > > > @@ -2906,6 +2888,8 @@ static struct scsi_host_template sht_v3_hw = {
> > > >       .scan_start             = hisi_sas_scan_start,
> > > >       .change_queue_depth     = sas_change_queue_depth,
> > > >       .bios_param             = sas_bios_param,
> > > > +     .map_queues             = hisi_sas_map_queues,
> > > > +     .host_tagset            = 1,
> > > >       .this_id                = -1,
> > > >       .sg_tablesize           = HISI_SAS_SGE_PAGE_CNT,
> > > >       .sg_prot_tablesize      = HISI_SAS_SGE_PAGE_CNT,
> > > > @@ -3092,6 +3076,8 @@ hisi_sas_v3_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > > >       if (hisi_sas_debugfs_enable)
> > > >               hisi_sas_debugfs_init(hisi_hba);
> > > > 
> > > > +     shost->nr_hw_queues = hisi_hba->cq_nvecs;
> 
> There's an ordering issue here, which can be fixed without too much trouble.
> 
> Value hisi_hba->cq_nvecs is not set until after this point, in
> hisi_sas_v3_probe()->hw->hw_init->hisi_sas_v3_init()->interrupt_init_v3_hw()
> 
> 
> Please see revised patch, below.

Good catch, will integrate it in V2.

Thanks,
Ming

  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 [this message]
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
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=20190603110054.GG11812@ming.t460p \
    --to=ming.lei@redhat.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=don.brace@microsemi.com \
    --cc=hare@suse.com \
    --cc=hare@suse.de \
    --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=sathya.prakash@broadcom.com \
    --cc=tom.leiming@gmail.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 linux-block@archiver.kernel.org
	public-inbox-index linux-block


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