From owner-freebsd-hackers@FreeBSD.ORG Fri May 20 17:32:40 2005 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from green.homeunix.org (freefall.freebsd.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id 1D07C16A4CF; Fri, 20 May 2005 17:32:40 +0000 (GMT) Received: from green.homeunix.org (green@localhost [127.0.0.1]) by green.homeunix.org (8.13.3/8.13.1) with ESMTP id j4KHWdSA097541; Fri, 20 May 2005 13:32:39 -0400 (EDT) (envelope-from green@green.homeunix.org) Received: (from green@localhost) by green.homeunix.org (8.13.3/8.13.1/Submit) id j4KHWaGa097540; Fri, 20 May 2005 13:32:36 -0400 (EDT) (envelope-from green) Date: Fri, 20 May 2005 13:32:36 -0400 From: Brian Fundakowski Feldman To: Scott Long Message-ID: <20050520173236.GG1201@green.homeunix.org> References: <200505201340.36176.hselasky@c2i.net> <20050520151620.GF1201@green.homeunix.org> <428E12B0.9020208@samsco.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <428E12B0.9020208@samsco.org> User-Agent: Mutt/1.5.6i cc: hackers@freebsd.org cc: Hans Petter Selasky Subject: Re: problems with new the "contigmalloc" routine X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 May 2005 17:32:40 -0000 On Fri, May 20, 2005 at 10:39:12AM -0600, Scott Long wrote: > Brian Fundakowski Feldman wrote: > > >On Fri, May 20, 2005 at 01:40:35PM +0200, Hans Petter Selasky wrote: > > > >>Hi, > >> > >>I just hit some problems with the new "contigmalloc()" routine in > >>FreeBSD-6-current, which is used by "bus_dmamem_alloc()". > >> > >>Firstly it locks Giant, which cause locking order reversals when > >>allocating memory from certain contexts. Secondly it sleeps even if flag > >>M_NOWAIT is passed. Thirdly it does not support flag M_ZERO. > > > > > >Read the documentation. It supports M_ZERO just fine, and it does _not_ > >support M_NOWAIT. > > > > > >>Can someone explain why these changes have been made? > > > > > >Changes? > > > > > >>Why doesn't "contigmalloc()" give a warning when an invalid flag is > >>passed? > > > > > >The kernel would be significantly larger and almost certainly slower > >if it were to double-check that everywhere any bit fields are used > >that flags that are not defined to have any behavior are unset. > > > > > >>Are these bugs in "contigmalloc()"? > > > > > >No. > > > > > >>Or does the code using "contigmalloc()" have to be changed? > > > > > >Yes. The i386 bus_dmamem_alloc(), for example, calls it with M_NOWAIT > >which is not a valid flag. The contigmalloc(9) page is not entirely > >truthful about the fact that it doesn't sleep at all -- it calls those > >routines which can. They can both be documented to require no locks > >to be held when being called, except for M_NOWAIT specifically in the > >one-page-or-less allocation case. > > > > The amount of colateral damage that has been inflicted on > bus_dmamem_alloc by your changes to contigmalloc is truly impressive. > Since I've spent the better part of 8 months cleaning it up, please > let me know what else needs to be done so that there are no more > surprises. And no, this is not my happy face. 1. http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/vm/vm_contig.c.diff?r1=1.34&r2=1.41&f=h 2. Notice the lack of semantical changes. 3. Notice that M_ZERO does the same thing as ever. 4. Notice that the same goes for M_WAITOK/M_NOWAIT: ignored for anything that needs more than one page. 5. Negative points for Hans for posting corrupt backtraces and making me go through a lot more work than should be necessary. 6. Negative points to Hans for posting backtraces to an obviously- molested tree (src/sys/dev/usb2/_*?!?). 7. Negative points to Scott for attempting to imply that for some reason it's okay to call contigmalloc(9) in contexts that it has never been okay to call it in. 8. http://unix.derkeiler.com/Mailing-Lists/FreeBSD/current/2003-11/1488.html 9. Negative points to Scott for failure to spend a couple minutes of research and find out these are known bugs. 10. http://lists.freebsd.org/pipermail/freebsd-current/2005-January/045105.html 11. Negative points to Scott for failure to document system semantics when it is obvious they should have been clarified according to Scott's own notes on the matter. 12. Mega positive points to Hans for actually asking the right questions. 13. Go fix or document the brokenness in busdma, or implement the features in contigmalloc(9) that were imagined to exist. Out. -- Brian Fundakowski Feldman \'[ FreeBSD ]''''''''''\ <> green@FreeBSD.org \ The Power to Serve! \ Opinions expressed are my own. \,,,,,,,,,,,,,,,,,,,,,,\