On Fri, May 31, 2019 at 10:32:04AM +0200, Hannes Reinecke wrote:
> On 5/31/19 10:21 AM, Ming Lei wrote:
> > On Fri, May 31, 2019 at 09:41:58AM +0200, Hannes Reinecke wrote:
> >> (Resending due to missing mailing list submission)
> >>
> >> Update v3 to support SCSI multiqueue.
> >>
> >> Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
> >> ---
> >> drivers/scsi/hisi_sas/hisi_sas.h | 1 -
> >> drivers/scsi/hisi_sas/hisi_sas_main.c | 45 +++++++++++++++++-----------------
> >> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 44 +++++++++++----------------------
> >> 3 files changed, 36 insertions(+), 54 deletions(-)
> >>
> >> diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
> >> index fc87994b5d73..4b6f32f60689 100644
> >> --- a/drivers/scsi/hisi_sas/hisi_sas.h
> >> +++ b/drivers/scsi/hisi_sas/hisi_sas.h
> >> @@ -378,7 +378,6 @@ struct hisi_hba {
> >> u32 intr_coal_count; /* Interrupt count to coalesce */
> >>
> >> int cq_nvecs;
> >> - unsigned int *reply_map;
> >>
> >> /* debugfs memories */
> >> u32 *debugfs_global_reg;
> >> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
> >> index 8a7feb8ed8d6..f4237c4754a4 100644
> >> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> >> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> >> @@ -200,16 +200,12 @@ static void hisi_sas_slot_index_set(struct hisi_hba *hisi_hba, int slot_idx)
> >> set_bit(slot_idx, bitmap);
> >> }
> >>
> >> -static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba,
> >> - struct scsi_cmnd *scsi_cmnd)
> >> +static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba)
> >> {
> >> int index;
> >> void *bitmap = hisi_hba->slot_index_tags;
> >> unsigned long flags;
> >>
> >> - if (scsi_cmnd)
> >> - return scsi_cmnd->request->tag;
> >> -
> >> spin_lock_irqsave(&hisi_hba->lock, flags);
> >> index = find_next_zero_bit(bitmap, hisi_hba->slot_index_count,
> >> hisi_hba->last_slot_index + 1);
> >
> > Then you switch to hisi_sas_slot_index_alloc() for allocating the unique
> > tag via spin_lock & find_next_zero_bit(). Do you think this way is more
> > efficient than blk-mq's sbitmap?
> >
> slot_index_alloc() is only used for commands which do _not_ have a tag
> (eg internal commands), or for v2 hardware which has weird allocation rules.
But this patch has switched to this allocation unconditionally for all commands:
> - if (scsi_cmnd)
> - return scsi_cmnd->request->tag;
> -
Otherwise duplicated slot can be used from different blk-mq hw queue.
>
> > The worsen thing is that V3's actual max queue depth is (4096 - 96), but
> > this patch claims that the device can support (4096 - 96) * 32 command
> > slots, finally hisi_sas_slot_index_alloc() is used to respect the actual
> > max queue depth(4000).
> >
> Well, this patch is an RFC to demonstrate my idea. Of course the queue
> depth should be identical before and after the conversion.
That is why I call it is hard to partition the hostwide tags to MQ.
>
> > Big contention is caused on hisi_sas_slot_index_alloc(), meantime huge> memory is wasted for request pool.
> >
> See above. That allocation is only used if no blk tag is available.
This patch switches the allocation for all commands.
>
> Or, at least, that was the idea :-)
Agree, :-)
thanks,
Ming
