Page MenuHomeFreeBSD
Differential D18741

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

Authored by kib on Jan 3 2019, 11:53 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 28, 9:23 PM
Unknown Object (File)
Dec 3 2024, 3:39 PM
Unknown Object (File)
Nov 27 2024, 12:36 AM
Unknown Object (File)
Nov 26 2024, 10:39 AM
Unknown Object (File)
Nov 26 2024, 2:50 AM
Unknown Object (File)
Nov 25 2024, 9:37 AM
Unknown Object (File)
Nov 20 2024, 1:19 PM
Unknown Object (File)
Sep 30 2024, 9:42 AM
View All 43 Files
Subscribers
imp
pho

Details

Reviewers
mav
Commits
rS342826: Fix use of busdma(9) KPI in ahci(4).
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 - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib created this revision.Jan 3 2019, 11:53 PM
Herald added a subscriber: imp. · View Herald TranscriptJan 3 2019, 11:53 PM
Harbormaster completed remote builds in B21829: Diff 52536.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.

Harbormaster completed remote builds in B21846: Diff 52596.Jan 5 2019, 9:26 PM
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
Closed by commit rS342826: Fix use of busdma(9) KPI in ahci(4). (authored by kib). · Explain WhyJan 7 2019, 2:39 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
sys/
dev/
ahci/
40 lines

Diff 52616

View Options

head/sys/dev/ahci/ahci.c

Loading...