From owner-freebsd-arch Fri Feb 22 11:20:26 2002 Delivered-To: freebsd-arch@freebsd.org Received: from rwcrmhc52.attbi.com (rwcrmhc52.attbi.com [216.148.227.88]) by hub.freebsd.org (Postfix) with ESMTP id F3C1537B400; Fri, 22 Feb 2002 11:20:13 -0800 (PST) Received: from InterJet.elischer.org ([12.232.206.8]) by rwcrmhc52.attbi.com (InterMail vM.4.01.03.27 201-229-121-127-20010626) with ESMTP id <20020222192013.BWEU1147.rwcrmhc52.attbi.com@InterJet.elischer.org>; Fri, 22 Feb 2002 19:20:13 +0000 Received: from localhost (localhost.elischer.org [127.0.0.1]) by InterJet.elischer.org (8.9.1a/8.9.1) with ESMTP id LAA74339; Fri, 22 Feb 2002 11:19:05 -0800 (PST) Date: Fri, 22 Feb 2002 11:19:03 -0800 (PST) From: Julian Elischer To: John Baldwin Cc: Matthew Dillon , arch@FreeBSD.org Subject: Re: RE: that INVARIANT/ucred freeing stuff. In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG OK here is my suggestion: We add extra code under DIAGNOSTIC the code does: in proc.h add a field to thread of: #ifdef DIAGNOSTIC td_ucred_cache #endif /* DIAGNOSTIC */ on texiting the kernel: #ifdef DIAGNOSTIC if (td->td_ucred_cache) panic("thread already has cached ucred"); td->td_ucred_cache = td->td_ucred; td->td_ucred = NULL; #endif /* DIAGNOSTIC */ on entering the kernel we do: #ifdef DIAGNOSTIC if (td->td_ucred) panic("thread got a cred form somewhere in userspace"); td->td_cred = td->td_ucred_cache; td->td_ucred_cache = NULL; #endif /* DIAGNOSTIC */ if (td->ucred != p->p_ucred) cred_update_thread(td); we get good performance even when it it is optionned in and still have a NULL ucred pointer when in user space when DIAGNOSTIC is turned on. With no DIAGNOSTICS we get the best performance, and don't even bother to shift the reference. On Fri, 22 Feb 2002, John Baldwin wrote: > > On 22-Feb-02 Matthew Dillon wrote: > >:Fine, stick it under DIAGNOSTIC (which isn't dead.) The problem is that > >:there > >:aren't just 5 places in the kernel that you would need to stick this assert, > >:you would need it all over the place. But I guess no one else has looked at > >:all the places that p_ucred is used and thought about how to ensure we don't > >:use a bogus td_ucred. > >: > >: > >:John Baldwin <>< http://www.FreeBSD.org/~jhb/ > > > > Don't try to overengineer the problem. Unless you believe there is > > a serious problem, there is no need to put a check in every single > > conceivable place an error might occur. Just putting a few safety checks > > in a few critical places should be sufficient. > > I don't know where all the places we might look at a ucred wrongly are. That's > why I wanted the much simpler solution of just clearing td_ucred to NULL so we > had an implicit KASSERT for us in all those places. > > > -Matt > > Matthew Dillon > > > > -- > > John Baldwin <>< http://www.FreeBSD.org/~jhb/ > "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message