Re: [PATCH RFC] hisi_sas_v3: multiqueue support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 31, 2019 at 10:42:12AM +0100, John Garry wrote:
> > > > > 
> > > > > -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?
> 
> These are not fast path, as used only for TMF, internal IO, etc.

OK, looks I miss that.

> 
> Having said that, hopefully we can move to scsi_{get,put}_reserved_cmd()
> when available, so that the LLDD has to stop managing them.

Agree.

> 
> > > > 
> > > 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:
> > 
> 
> As Hannes said, v2 had a few bugs which meant that we had to make a specific
> version of this function for that hw revision, cf.
> slot_index_alloc_quirk_v2_hw(), and it cannot use request queue tag.
> 
> But, indeed, we could consider sbitmap for v2 hw. I'm not sure if it would
> help, considering the weird rules.
> 
> > > -       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
> 
> To be clear about the hw, the hw supports max 4096 command tags and has 16

Is 4096 the max allowed host-wide command tags? Or per-queue's max commands
tags?

If it is per-queue's max command tags, V3 should be real MQ hardware,
otherwise it is still host wide. Otherwise, Hannes's patch can't work
because tag from different hw queue can be same.

> hw queues. The hw queue depth is configurable by software, and we configure
> it at 4096 per queue - same as max command tags - this is to support
> possibility of all commands using the same queue simultaneously.

Suppose 4096 is the host-wide command tags:

As Hannes's patch changed to allow each blk-mq hw queue to accept 4096 commands,
there will be big contention in driver, given now the actual .can_queue becomes
4096 * 32, and how to avoid the same tag from different hw queue? 

Thanks,
Ming



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux