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
