Page MenuHomeFreeBSD

Remove unnecessary WITNESS check in x86 bounce_bus_dmamem_alloc()
ClosedPublic

Authored by jah on Jun 2 2020, 8:18 PM.

Details

Summary

When I did some header cleanup in r320528, I copy-pasted some sketchy WITNESS
checks from the prior busdma wrappers. Two of these were removed in r346351
and r346851, but one remains in bounce_bus_dmamem_alloc(). This check could
be constrained to only apply if (flags & BUS_DMA_NOWAIT), but it's cleaner to
simply remove it and rely on the checks already present in the sleepable
allocation paths used by this function.

Noticed by: cem

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

jah created this revision.Jun 2 2020, 8:18 PM
jah requested review of this revision.Jun 2 2020, 8:18 PM
jah added reviewers: cem, kib.Jun 2 2020, 8:19 PM
jah updated this revision to Diff 72581.Jun 2 2020, 9:42 PM

Remove another unnecessary WITNESS check in the shared x86 bus_dma_tag_create()
implementation. Both DMAR and bounce implementations use
common_bus_dma_tag_create(), which allocates the tag with M_NOWAIT.

kib accepted this revision.Jun 2 2020, 9:45 PM
This revision is now accepted and ready to land.Jun 2 2020, 9:45 PM
cem accepted this revision.Jun 2 2020, 11:53 PM

Thanks!

sys/x86/x86/busdma_bounce.c
410–411 ↗(On Diff #72581)

This one might be reasonable without BUS_DMA_NOWAIT, since we will allocate sleepable in that context. But I suppose we'll just get the exact same warning from malloc(9), so maybe this was redundant?

493 ↗(On Diff #72581)

Might fix "allociated" if you're doing typos on this line

jah added inline comments.Jun 2 2020, 11:58 PM
sys/x86/x86/busdma_bounce.c
410–411 ↗(On Diff #72581)

Yeah, UMA directly issues the same WITNESS_WARN for sleepable allocations, and kmem_alloc_contig() will go through msleep() if it needs to wait for pages.

493 ↗(On Diff #72581)

Ha! My brain just substituted "associated" when I read that.

cem added inline comments.Jun 2 2020, 11:59 PM
sys/x86/x86/busdma_bounce.c
410–411 ↗(On Diff #72581)

Excellent! :-)

jah updated this revision to Diff 72585.Jun 3 2020, 12:05 AM

Fix another typo

This revision now requires review to proceed.Jun 3 2020, 12:05 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 3 2020, 12:16 AM
This revision was automatically updated to reflect the committed changes.
jah added inline comments.Jun 3 2020, 2:31 AM
sys/x86/x86/busdma_bounce.c
410–411 ↗(On Diff #72581)

Even better, the !M_NOWAIT case of kmem_alloc_* gets a WITNESS check through vmem_alloc(), which will catch abusers even if backing pages are readily available. Otherwise I was going to add one in kmem_alloc_contig_pages().