Re: CVS commit: [jdolecek-ncq] src/sys/dev/ata

[Jaromír Doleček <jaromir.dolecek%gmail.com@localhost>]

2017-06-17 20:00 GMT+02:00 Jonathan A. Kollasch <jakllsch%kollasch.net@localhost>:
>
> I really think we need to make ata_xfer_init() private to
> ata_queue_alloc(), and use ata_get_xfer() everywhere.  That we can
> fabricate an xfer on the stack anywhere that will trample all over an
> active command slot is fundamentally wrong.

Any non-NCQ command which goes via atastart() is safe, since that routine

makes sure there is only one non-NCQ command being executed by HBA.

The on-stack xfer was carried over from previous design. I kept it mostly since I didn't want to change those codepaths to deal with xfer not being available.

As far as I see, ahcisata(4) reset should be actually fine, since it does the reset

without using the xfers/slots.

I had a look at siisata_reset_drive(). That thing indeed can't reliably work as-is.

Arguably it should be changed to use the on-stack xfer, currently it e.g. doesn't

do anything if all slots are taken, which is wrong. I'll have a look.

> ata_queue_downsize() currently appears to leak xfers.  Additionally once
> you put a QD1 drive on the channel you're stuck there forever, even if
> you replace the channel population entirely with QD32 drives.

Okay, I need to look at this - original assumption was that it's never called while there are other transfers. This is false with PMP.

Jaromir