Date: Wed, 18 Nov 2015 17:00:49 +0100 From: Svatopluk Kraus <skra@freebsd.org> To: FreeBSD Arch <freebsd-arch@freebsd.org>, Konstantin Belousov <kostikbel@gmail.com> Subject: a question about BUS_DMA_MIN_ALLOC_COMP flag meaning Message-ID: <CAFHCsPVNntJVs051PvRuaY2S17Jf6kqbPPWq4tZpA-=Y-S32YA@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
Hi,
I have fallen to some problem with inconsistent use of
BUS_DMA_MIN_ALLOC_COMP flag. This flag was introduced in x86 MD code
very very long ago and so, the problem covers all archs which came out
from it.
However, it's only about bus_dma_tag_t with BUS_DMA_COULD_BOUNCE flag set.
(1) When bus_dma_tag_t is being created with BUS_DMA_ALLOCNOW flag
specified, some bounce pages could be allocated in advance and
BUS_DMA_MIN_ALLOC_COMP flag is set to the tag. The bounce pages are
allocated only if the tag's maxsize property is higher than size of
all bounce pages already allocated in a bounce zone.
(2) When bus_dmamap_t is being created, then if BUS_DMA_MIN_ALLOC_COMP
is not set on associated tag, some bounce pages are ALWAYS allocated
and BUS_DMA_MIN_ALLOC_COMP is set afterwards,
(3) else some bounce pages could be allocated if there is not enough
pages in a bounce zone and BUS_DMA_MIN_ALLOC_COMP is set afterwards.
The problem is the following. Due to case (2), the number of pages in
bounce zone can grow infinitely, as bounce pages once allocated are
never freed. It can happen when a big number of bus_dma_tag_t together
with bus_dmamap_t are created, or they are created dynamically either
because of a loadable module or by design.
The inconsistency is that when bus_dma_tag_t is being created, there
is no limit for how much pages could be allocated. On the other hand,
when bus_dmamap_t is being created, there is MAX_BPAGES limitation.
I think that fix for case (2) presented as x86 fix is the following:
diff --git a/sys/x86/x86/busdma_bounce.c b/sys/x86/x86/busdma_bounce.c
index 4826a2b..a15139f 100644
--- a/sys/x86/x86/busdma_bounce.c
+++ b/sys/x86/x86/busdma_bounce.c
@@ -308,7 +308,7 @@ bounce_bus_dmamap_create(bus_dma_tag_t dmat, int
flags, bus_dmamap_t *mapp)
else
maxpages = MIN(MAX_BPAGES, Maxmem -
atop(dmat->common.lowaddr));
- if ((dmat->bounce_flags & BUS_DMA_MIN_ALLOC_COMP) == 0 ||
+ if ((dmat->bounce_flags & BUS_DMA_MIN_ALLOC_COMP) == 0 &&
(bz->map_count > 0 && bz->total_bpages < maxpages)) {
pages = MAX(atop(dmat->common.maxsize), 1);
pages = MIN(maxpages - bz->total_bpages, pages);
IMO, it also fixes logic by making it same as in bus_dma_tag_t case.
The next question is, if case (1) should be limited by MAX_BPAGES as
in case (3) or maybe better if there should be some internal
limitation for bounce zone itself.
As this is quite old code which covers many archs, I really would
appreciate your comments. Thanks,
Svatopluk Kraus
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFHCsPVNntJVs051PvRuaY2S17Jf6kqbPPWq4tZpA-=Y-S32YA>