Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
IronPort-SDR: 7t7nCOeTPhnjQtCUYCTBUj7a5aB0Wlq/+3cHYjyJUmnasf4aa1PNn9xv58ZmqNFie7YSuD/otU
 LBCckygnFovDHPcDc7qlpHwTrWhjEAlodj2vnXnEQLmtZR5sVqi0Mry9403wNynG+BistNm3Wh
 lE7vSLMr69PGzNVqTwQfMLWw/aFtXRUtuUabqAZ0GJVtCElL4kJxWiktGLt6WU06X7NoP5JZH8
 AWNtkqh0R2tW7XRp/1QYgjKD5k6psC/dU6UXRq6umKrJRXs7XSYfFAqQqmF9Wk6z60iZJtlHLs
 xYkZ8CSthj9dZ80uc5iLytRd
IronPort-SDR: t6C81hq3BKeE3Dtw7va0CaNIDbYX07tQoLvbDY8e0i8XgbPdMqAVOi+dUT1atf8ty1c1IvxKsN
 pFwPzTgxlFzIPw9wI6krkBlT/ZZIfWOUIbSJvdaH5tLMso8ts2+zGxQF6vrDpzXMI+tYnjmEew
 wPETpPwM9aXNEi0Nne36uZaPkckpqg8/0yPtQIXY/ATwYppbEsUsbaoiGTGpjeipYW8b/z4XVy
 iPr7duBrStI12ILWoMyCixpvyOw7fDbe5kVyy2NJWAdhg807LItJ88oEoeJbCShUs/xgAvZczp
 cdI=
WDCIronportException: Internal
Message-ID: <71b56949-e4d7-fd94-c44a-867080b7a4fa@opensource.wdc.com>
Date:   Fri, 28 Oct 2022 17:07:26 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH RFC v3 2/7] ata: libata-scsi: Add
 ata_internal_queuecommand()
Content-Language: en-US
To:     John Garry <john.garry@huawei.com>, jejb@linux.ibm.com,
        martin.petersen@oracle.com, hare@suse.de, bvanassche@acm.org,
        hch@lst.de, ming.lei@redhat.com, niklas.cassel@wdc.com
Cc:     axboe@kernel.dk, jinpu.wang@cloud.ionos.com,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,
        linuxarm@huawei.com, john.garry2@mail.dcu.ie
References: <1666693976-181094-1-git-send-email-john.garry@huawei.com>
 <1666693976-181094-3-git-send-email-john.garry@huawei.com>
 <08fdb698-0df3-7bc8-e6af-7d13cc96acfa@opensource.wdc.com>
 <83d9dc82-ea37-4a3c-7e67-1c097f777767@huawei.com>
 <9a2f30cc-d0e9-b454-d7cd-1b0bd3cf0bb9@opensource.wdc.com>
 <0e60fab5-8a76-9b7e-08cf-fb791e01ae08@huawei.com>
From:   Damien Le Moal <damien.lemoal@opensource.wdc.com>
Organization: Western Digital Research
In-Reply-To: <0e60fab5-8a76-9b7e-08cf-fb791e01ae08@huawei.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/28/22 17:01, John Garry wrote:
> On 27/10/2022 23:25, Damien Le Moal wrote:
>>> So we have this overall flow:
>>>
>>> ata_exec_internal_sg():
>>>    -> alloc request
>>>    -> blk_execute_rq_nowait()
>>>        ... -> scsi_queue_rq()
>>> 		-> sht->reserved_queuecommd()
>>> 			-> ata_internal_queuecommand()
>>>
>>> And then we have ata_internal_queuecommand() -> ata_sas_queuecmd() ->
>>> ata_scsi_queue_internal() -> ata_qc_issue().
>>>
>>> Hope it makes sense.
>> OK. Got it.
>> However, ata_exec_internal_sg() being used only from EH context with the
>> queue quiesced, will blk_execute_rq_nowait() work ? Is there an exception
>> for internal reserved tags ?
>>
> 
> Well, yeah. So if some error happens and EH kicks in, then full queue 
> depth of requests may be allocated. I have seen this for NCQ error. So 
> this is why I make in very first patch change allow us to allocate 
> reserved request from sdev request queue even when budget is fully 
> allocated.
> 
> Please also note that for AHCI, I make reserved depth =1, while for SAS 
> controllers it is greater. This means that in theory we could alloc > 1x 
> reserved command for SATA disk, but I don't think it matters.

Yes, 1 is enough. However, is 1 reserved out of 32 total, meaning that the
user can only use 31 tags ? or is it 32+1 reserved ? which we can do since
when using the reserved request, we will not use a hw tag (all reserved
requests will be non-ncq).

The 32 + 1 scheme will work. But for CDL command completion handling, we
will need a NCQ command to do a read log, to avoid forcing a queue drain.
For that to reliably work, we'll need a 31+1+1 setup...

> 
> Thanks,
> John

-- 
Damien Le Moal
Western Digital Research