From owner-freebsd-arch@freebsd.org Fri Nov 20 14:45:53 2015 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4DC9DA3440B for ; Fri, 20 Nov 2015 14:45:53 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id DD9221AD0; Fri, 20 Nov 2015 14:45:52 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id tAKEjju6085936 (version=TLSv1 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Fri, 20 Nov 2015 16:45:45 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua tAKEjju6085936 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id tAKEjidJ085934; Fri, 20 Nov 2015 16:45:44 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 20 Nov 2015 16:45:44 +0200 From: Konstantin Belousov To: Svatopluk Kraus Cc: FreeBSD Arch Subject: Re: a question about BUS_DMA_MIN_ALLOC_COMP flag meaning Message-ID: <20151120144544.GB58629@kib.kiev.ua> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Nov 2015 14:45:53 -0000 On Wed, Nov 18, 2015 at 05:00:49PM +0100, Svatopluk Kraus wrote: > 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. I think that this patch is correct. > > 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. Could we apply e.g. MAX_BPAGES limit to bounce zones, or should we allow the limit to change based on the tag ? But I am not sure if there is any reasonable way to formulate the limit. MAX_BPAGES looks like some arbitrary sanity limit, e.g. we could have unlimited maxsize, but also have an alignment constraints, and then tag requires bouncing. I am not sure that hard-coded values, esp. the amd64 32MB limit, makes much sense, or that basing the limit on the tag constraints makes more sense. Might be, we should allow some total percentage of the physical memory on machine to be consumed by all bounce zones altogether ?