Message-ID: <1537235d-ac22-6914-4b31-4482e9536098@suse.de>
Date:   Tue, 22 Mar 2022 12:30:22 +0100
From:   Hannes Reinecke <hare@...e.de>
To:     John Garry <john.garry@...wei.com>, axboe@...nel.dk,
        damien.lemoal@...nsource.wdc.com, bvanassche@....org,
        jejb@...ux.ibm.com, martin.petersen@...cle.com, hch@....de,
        ming.lei@...hat.com
Cc:     chenxiang66@...ilicon.com, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-ide@...r.kernel.org,
        linux-scsi@...r.kernel.org, dm-devel@...hat.com, beanhuo@...ron.com
Subject: Re: [PATCH RFC 00/11] blk-mq/libata/scsi: SCSI driver tagging
 improvements

On 3/22/22 11:39, John Garry wrote:
> Currently SCSI low-level drivers are required to manage tags for commands
> which do not come via the block layer - libata internal commands would be
> an example of one of these.
> 
> There was some work to provide "reserved commands" support in such series
> as https://lore.kernel.org/linux-scsi/20211125151048.103910-1-hare@suse.de/
> 
> This was based on allocating a request for the lifetime of the "internal"
> command.
> 
> This series tries to solve that problem by not just allocating the request
> but also sending it through the block layer, that being the normal flow
> for a request. We need to do this as we may only poll completion of
> requests through the block layer, so would need to do this for poll queue
> support.
> 
> There is still scope to allocate commands just to get a tag as token as
> that may suit some other scenarios, but it's not what we do here.
> 
> This series extends blk-mq to support a request queue having a custom set
> of ops. In addition SCSI core code adds support for these type of requests.
> 
> This series does not include SCSI core handling for enabling reserved
> tags per tagset, but that would be easy to add.
> 
> Based on mkp-scsi 5.18/scsi-staging @ 66daf3e6b993
> 
> Please consider as an RFC for now. I think that the libata change has the
> largest scope for improvement...
> 

Grand seeing that someone is taking it up. Thanks for doing this!

But:
Allocating a queue for every request (eg in patch 3) is overkill. If we 
want to go that route we should be allocating the queue upfront (eg when 
creating the device itself), and then just referencing it.

However, can't say I like this approach. I've been playing around with 
supporting internal commands, and really found two constraints really 
annoying:

- The tagset supports only _one_ set of payload via
   blk_mq_rq_(to,from)_pdu().
This requires each request to be of the same type, and with that making
it really hard for re-purposing the request for internal usage. In the
end I settled by just keeping it and skipping the SCSI command field.
If we could have a distinct PDU type for internal commands I guess 
things would be easier.

- The number of reserved commands is static.
With that it's getting really hard using reserved commands with 
low-queue depth devices like ATA; we only have 31 commands to work with, 
and setting one or two aside for TMF is really making a difference 
performance wise. It would be _awesome_ if we could allocate reserved 
commands dynamically (ie just marking a command as 'reserved' once 
allocated).
Sure, it won't have the same guarantees as 'real' reserved commands, but 
in most cases we don't actually need that.

Maybe these are some lines we could investigate?
Hmm?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@...e.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

Powered by blists - more mailing lists