hyperv/stor: Use xpt_done_direct() whenever possible

[mav]

I see no problems from using direct completion here. Its efficiency may vary, depending on number of disks and interrupts per controller. On real hardware with many disks per controller and only one interrupt I sometimes could get higher parallel IOPS when used queued completion due to using multiple completion threads in CAM. Direct completion is the most efficient on HBAs with multiple interrupt vectors or with only one/few disks connected. Though VM may have very different performance characteristics to optimize for.

I am just not sure why is there XPT_FC_QUEUED check? Both xpt_done(ccb) and xpt_done_direct(ccb) actually just return immediately if they see this flag, so this condition is effectively a NOP. Also I doubt any non-queued requests are going that path any way, since I don't see cases when this code path would be called not form interrupt handler.

[sepherosa_gmail.com]

In D5955#126973, @mav wrote:

I see no problems from using direct completion here. Its efficiency may vary, depending on number of disks and interrupts per controller. On real hardware with many disks per controller and only one interrupt I sometimes could get higher parallel IOPS when used queued completion due to using multiple completion threads in CAM. Direct completion is the most efficient on HBAs with multiple interrupt vectors or with only one/few disks connected. Though VM may have very different performance characteristics to optimize for.

Mainly to avoid extra schedule, which could introduce potential latency on io done path. And this driver is using "multiple interrupts".

I am just not sure why is there XPT_FC_QUEUED check? Both xpt_done(ccb) and xpt_done_direct(ccb) actually just return immediately if they see this flag, so this condition is effectively a NOP. Also I doubt any non-queued requests are going that path any way, since I don't see cases when this code path would be called not form interrupt handler.

Hmm, I copied it from ahci bindly. I didn't see the reason why it should be checked in either ahci or here either. Maybe for some historical reasons?

[mav]

In D5955#126995, @sepherosa_gmail.com wrote:

Hmm, I copied it from ahci bindly. I didn't see the reason why it should be checked in either ahci or here either. Maybe for some historical reasons?

In AHCI situation is different. ahci_done() is called from both request and response contexts, and also indeed called for non-queued requests too, in which case doing extra work for direct completion (which is any way NOP) is pointless.

I think you should remove the check. After that it would be good for me.

[sepherosa_gmail.com]

In D5955#126996, @mav wrote:

In D5955#126995, @sepherosa_gmail.com wrote:

Hmm, I copied it from ahci bindly. I didn't see the reason why it should be checked in either ahci or here either. Maybe for some historical reasons?

In AHCI situation is different. ahci_done() is called from both request and response contexts, and also indeed called for non-queued requests too, in which case doing extra work for direct completion (which is any way NOP) is pointless.

Aha, I see. Thanks!

I think you should remove the check. After that it would be good for me.

[sepherosa_gmail.com]

No need to check FC_QUEUED. Suggested by mav

[sepherosa_gmail.com]

In D5955#126996, @mav wrote:

In D5955#126995, @sepherosa_gmail.com wrote:

Hmm, I copied it from ahci bindly. I didn't see the reason why it should be checked in either ahci or here either. Maybe for some historical reasons?

In AHCI situation is different. ahci_done() is called from both request and response contexts, and also indeed called for non-queued requests too, in which case doing extra work for direct completion (which is any way NOP) is pointless.

I think you should remove the check. After that it would be good for me.

Done. Thank you for the clarification and suggestion!