Re: [PATCH RFC v3 2/7] ata: libata-scsi: Add ata_internal_queuecommand()

[Hannes Reinecke <hare@suse.de>]

On 11/2/22 11:07, Damien Le Moal wrote:
> On 11/2/22 18:52, John Garry wrote:
>> Hi Damien,
>>
[ .. ]
>>>> Or re-use 1 from 32 (and still also have 1 separate internal command)?
>>>
>>> I am not yet 100% sure if we can treat that internal NCQ read log like
>>> any other read/write request... If we can, then the 1-out-of-32
>>> reservation would not be needed. Need to revisit all the cases we need
>>> to take care of (because in the middle of this CDL completion handling,
>>> regular NCQ errors can happen, resulting in a drive reset that could
>>> wreck everything as we lose the sense data for the completed requests).
>>>
>>> In any case, I think that we can deal with that extra reserved command
>>> on top of you current series. No need to worry about it for now I think.
>>>
>>
>> So are you saying that you are basing current CDL support on libata
>> internally managing this extra reserved tag (and so do not need this
>> SCSI midlayer reserved tag support yet)?
> 
> Not really. For now, it is using libata EH, that is, when we need the
> internal command for the read log, we know the device is idle and no
> command is on-going. So we send a non-NCQ command which does not have a tag.
> 
> Ideally, all of this should use a real reserved tag to allow for an NCQ
> read log outside of EH, avoiding the drive queue drain.
> 
But with the current design you'll only get that if you reserve one 
precious tag.

OTOH, we might not need that tag at all, as _if_ we get an error for a 
specific command the tag associated with it is necessarily free after 
completion, right?

So we only need to find a way of 're-using' that tag, then we won't have 
to set aside a reserved tag and everything would be dandy...

Maybe we can stop processing when we receive an error (should be doing 
that anyway as otherwise the log might be overwritten), then we should 
be having a pretty good chance of getting that tag.
Or, precisely, getting _any_ tag as at least one tag is free at that point.
Hmm?

Cheers,

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