From owner-freebsd-hackers@FreeBSD.ORG Sat May 21 18:06:17 2005 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id A162D16A4CE for ; Sat, 21 May 2005 18:06:17 +0000 (GMT) Received: from pooker.samsco.org (pooker.samsco.org [168.103.85.57]) by mx1.FreeBSD.org (Postfix) with ESMTP id B072443D64 for ; Sat, 21 May 2005 18:06:14 +0000 (GMT) (envelope-from scottl@samsco.org) Received: from [192.168.254.14] (imini.samsco.home [192.168.254.14]) (authenticated bits=0) by pooker.samsco.org (8.13.3/8.13.3) with ESMTP id j4LI91cl007758; Sat, 21 May 2005 12:09:01 -0600 (MDT) (envelope-from scottl@samsco.org) Message-ID: <428F784B.6010806@samsco.org> Date: Sat, 21 May 2005 12:04:59 -0600 From: Scott Long User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.7) Gecko/20050416 X-Accept-Language: en-us, en MIME-Version: 1.0 To: John-Mark Gurney References: <200505201340.36176.hselasky@c2i.net> <200505202151.35831.hselasky@c2i.net> <20050520224928.GI2129@cirb503493.alcatel.com.au> <200505211546.56111.hselasky@c2i.net> <20050521174408.GE959@funkthat.com> In-Reply-To: <20050521174408.GE959@funkthat.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-2.8 required=3.8 tests=ALL_TRUSTED autolearn=failed version=3.0.2 X-Spam-Checker-Version: SpamAssassin 3.0.2 (2004-11-16) on pooker.samsco.org cc: Peter Jeremy 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: Sat, 21 May 2005 18:06:17 -0000 John-Mark Gurney wrote: > Hans Petter Selasky wrote this message on Sat, May 21, 2005 at 15:46 +0200: > >>On Saturday 21 May 2005 00:49, Peter Jeremy wrote: >> >>>On Fri, 2005-May-20 21:51:34 +0200, Hans Petter Selasky wrote: >>> >>>>Non-blocking mode has got to be supported. Else you get a nightmare >>>>rewriting the code to support blocking mode. >>> >>>Your code either has to block somewhere or fail. contigmalloc() only >>>blocks if it couldn't satisfy the request immediately. If it returns >>>to your code, you still have the problem of needing the memory and >>>not being able to allocate it without blocking. >> >>That is not the problem. The problem is that it sleeps, which will exit the >>Giant lock, which means another thread can change the state of what I am >>about to setup meanwhile: >> >>one_thread: >> >> mtx_lock(&Giant); >> >> if(sc->gone == 0) >> { >> sc->data = contigmalloc(); >> } >> >> mtx_unlock(&Giant); >> >>another_thread: >> >> mtx_lock(&Giant); >> >> if(sc->data) >> { >> contigfree(); >> sc->data = NULL; >> } >> >> sc->gone = 1; >> >> mtx_unlock(&Giant); >> >> >>The problem is that the undefined state: "sc->data != NULL" and >>"sc->gone == 1" can be reached. > > > How about rewriting the code to be: > one_thread: > tmpdata = contigmalloc(); > mtx_lock(&Giant); > if(sc->gone == 0) { > sc->data = tmpdata; > } else { > contigfree(tmpdata); > } > mtx_unlock(&Giant); > > another_thread: > > mtx_lock(&Giant); > if(sc->data) { > tmpdata = sc->data; > sc->data = NULL; > } > > sc->gone = 1; > > mtx_unlock(&Giant); > contigfree(tmpdata); > > That should do it.. Though you do need to have your own ref count on sc > to prevent the entire sc from going away before the first thread has > locked... Anyways, you should be using your own lock that's in sc for > this instead of using Giant... > > Remeber that you can usually do some of the work before obtaining the > lock, and then swapping a few variables around... In fact, it's usually > better to do this since you don't have to do as much work with the lock > locked... > > I don't think -current should allow new code to use Giant... > I'd suggest just following a simplier and more deterministic path by either pre-allocating your contiguous buffers in a safe context, or allocating them on the fly before you depend on state being static. Our concept of 'sleep' and 'block' are a bit muddled now that we have liberal uses of sleep locks, and we as programmers need to cope and adjust. It's always good form in embedded and kernel programming to pre-allocate and closely manage resources when you can; this isn't userland where resources are cheap. Scott