Re: [PATCH RFC v2 03/18] scsi: core: Implement reserved command handling

From: Hannes Reinecke <hare@suse.de>
To: Damien Le Moal <damien.lemoal@opensource.wdc.com>,
	John Garry <john.garry@huawei.com>,
	axboe@kernel.dk, jejb@linux.ibm.com, martin.petersen@oracle.com,
	brking@us.ibm.com, hch@lst.de
Cc: linux-block@vger.kernel.org, linux-ide@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
	chenxiang66@hisilicon.com
Subject: Re: [PATCH RFC v2 03/18] scsi: core: Implement reserved command handling
Date: Mon, 20 Jun 2022 08:45:00 +0200	[thread overview]
Message-ID: <543fcfb2-6baf-1b19-d0f5-83979090db31@suse.de> (raw)
In-Reply-To: <e4b108ba-cbc9-5237-f873-2fcea94f2b85@opensource.wdc.com>

On 6/13/22 11:06, Damien Le Moal wrote:
> On 6/13/22 17:25, John Garry wrote:
[ .. ]
>>
>> We may have 32 regular tags and 1 reserved tag for SATA.
> 
> Right. But that is the messy part though. That extra 1 tag is actually not
> a tag since all internal commands are non-NCQ commands that do not need a
> tag...
> 
> I am working on command duration limits support currently. This feature
> set has a new horrendous "improvement": a command can be aborted by the
> device if it fails its duration limit, but the abort is done with a good
> status + sense data available bit set so that the device queue is not
> aborted entirely like with a regular NCQ command error.
> 
> For such aborted commands, the command sense data is set to
> "COMPLETED/DATA UNAVAILABLE". In this case, the host needs to go read the
> new "successful NCQ sense data log" to check that the command sense is
> indeed "COMPLETED/DATA UNAVAILABLE". And to go read that log page without
> stalling the device queue, we would need an internal NCQ (queuable) command.
> 
> Currently, that is not possible to do cleanly as there are no guarantees
> we can get a free tag (there is a race between block layer tag allocation
> and libata internal tag counting). So a reserved tag for that would be
> nice. We would end up with 31 IO tags at most + 1 reserved tag for NCQ
> commands + ATA_TAG_INTERNAL for non-NCQ. That last one would be rendered
> rather useless. But that also means that we kind-of go back to the days
> when Linux showed ATA drives max QD of 31...
> 
> I am still struggling with this particular use case and trying to make it
> fit with your series. Trying out different things right now.
> 
Hmm. Struggling on how that is supposed to work in general.
As we're reading from a log to get the sense information I guess that 
log is organized by tag index. Meaning we have to keep hold of the tag 
which generated that error.
Q1: Can we (re-) use that tag to read the log information?
Q2: What do you do if all 32 command generate such an error?

But really, this sounds no different from the 'classical' request sense 
handling in SCSI ML. Have you considered just run with that an map 
'REQUEST SENSE' on your new NCQ Get Log page command?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman