From owner-freebsd-current Thu Feb 7 18: 0:45 2002 Delivered-To: freebsd-current@freebsd.org Received: from rwcrmhc53.attbi.com (rwcrmhc53.attbi.com [204.127.198.39]) by hub.freebsd.org (Postfix) with ESMTP id B86D837B41E for ; Thu, 7 Feb 2002 18:00:24 -0800 (PST) Received: from InterJet.elischer.org ([12.232.206.8]) by rwcrmhc53.attbi.com (InterMail vM.4.01.03.27 201-229-121-127-20010626) with ESMTP id <20020208020020.RMEX2951.rwcrmhc53.attbi.com@InterJet.elischer.org>; Fri, 8 Feb 2002 02:00:20 +0000 Received: from localhost (localhost.elischer.org [127.0.0.1]) by InterJet.elischer.org (8.9.1a/8.9.1) with ESMTP id RAA98999; Thu, 7 Feb 2002 17:59:59 -0800 (PST) Date: Thu, 7 Feb 2002 17:59:58 -0800 (PST) From: Julian Elischer To: Terry Lambert Cc: current@freebsd.org Subject: Re: ucred for threads In-Reply-To: <3C632DEA.F522339D@mindspring.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Thu, 7 Feb 2002, Terry Lambert wrote: > Julian Elischer wrote: > > In the KSE code I have: > > in trap(), and syscall() > > > > if (td->td_ucred != p->p_ucred) { > > PROC_LOCK(p); > > if (td->td_ucred) { > > crfree(td->td_ucred); > > td->td_ucred = NULL; > > } > > if (p->p_ucred != NULL) { > > td->td_ucred = crhold(p->p_ucred); > > } > > PROC_UNLOCK(p); > > } > > > > THis is actually not dependent on KSE (though I originally needed it > > for KSE I have since decided that it would be a good idea anyhow). > > > > Do those who deal in ucreds (probably jhb and Robert W) > > agree or disagree.. (if so I'll happily commit it as it shrinks the KSE > > patches a bit more :-) > > You can't have a process with a thread without a ucred, right? > yes.. well in booting yuo can have a null ucred. ( I know,s I've hit it), but in general you are correct. > If so, then the idea is sound, but the code is broken. > > In the case that they are not equal, you free it, null > the pointer, and don't allocate a replacement. ummm yes I do, assuming that there is one to allocate... if (p->p_ucred != NULL) { td->td_ucred = crhold(p->p_ucred); } > > What is the default state of td->td_ucred? on creation, NULL followed very rapidly with being set to p->p_ucred. (via crhold) > > In theory, it should be initialized on creation, and then > left initialized (you are effectively lazy binding the cred > references). yes.. > > Since the reference count is positive in both cases, an > unlocked pointer comparison is fine. In the case where > there is a race on a credential change in the unlocked > value, that race exists in the calling code, anyway. The > failure case in a change for->to instead of to->from is > also safe, since the pointers will be inequal during the > update, the lock will be held during the update, so the > subsequent release and regrab with the increment never > dropping below 1. > > If that's the case, then the code should be: > > if (td->td_ucred != p->p_ucred) { > PROC_LOCK(p); > if (td->td_ucred) { > crfree(td->td_ucred); > } without the if it crashes in boot sometimes. (this may not be true right now but was during my testing of the KSE kernel) > td->td_ucred = crhold(p->p_ucred); > PROC_UNLOCK(p); > } > > Is a thread lock required? I think it's guaranteed to > be non-reentrant, so the answer should be no... > > -- Terry > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message