Page MenuHomeFreeBSD

Fix use of busdma(9) KPI in ahci(4).
ClosedPublic

Authored by kib on Jan 3 2019, 11:53 PM.

Details

Summary

Use BUS_DMA_NOWAIT for loads at initialization time.
Report actual numeric error code if any problem occurs at the initialization.

Reported by: pho

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kib created this revision.Jan 3 2019, 11:53 PM
mav requested changes to this revision.Jan 5 2019, 1:46 AM

I don't think this change fix anything. As I see, this code path is called without the lock held, so locking callback is pointless. What should be added instead is BUS_DMA_NOWAIT flag to the bus_dmamap_load() calls, since the code there really expects synchronous completion. Or it require some more complicated rework to make it asynchronous.

I am really surprised that sleep happened there. How it happened? Was ahci loaded as module long in run time on some really busy system, or how?

This revision now requires changes to proceed.Jan 5 2019, 1:46 AM
kib updated this revision to Diff 52596.Jan 5 2019, 9:26 PM

Use BUS_DMA_NOWAIT. Report actual errors occured.

kib retitled this revision from Add missed locking to the ahci busdma tags. to Fix use of busdma(9) KPI in ahci(4)..Jan 5 2019, 9:27 PM
kib edited the summary of this revision. (Show Details)
In D18741#400044, @mav wrote:

I don't think this change fix anything. As I see, this code path is called without the lock held, so locking callback is pointless. What should be added instead is BUS_DMA_NOWAIT flag to the bus_dmamap_load() calls, since the code there really expects synchronous completion. Or it require some more complicated rework to make it asynchronous.
I am really surprised that sleep happened there. How it happened? Was ahci loaded as module long in run time on some really busy system, or how?

Soon there will be much harder requirements for the i386 use of busdma to be correct. Basically, PAE is going to be turned on by default (while bus_addr_t still kept at 32bit for drivers compatibility). I found it much more painful to widen bus_addr_t.

mav accepted this revision.Jan 7 2019, 2:12 AM
This revision is now accepted and ready to land.Jan 7 2019, 2:12 AM
This revision was automatically updated to reflect the committed changes.