From owner-freebsd-current Mon Feb 11 18:59:18 2002 Delivered-To: freebsd-current@freebsd.org Received: from newman2.bestweb.net (newman2.bestweb.net [209.94.102.67]) by hub.freebsd.org (Postfix) with ESMTP id 68EFC37B48B for ; Mon, 11 Feb 2002 18:17:04 -0800 (PST) Received: from okeeffe.bestweb.net (okeefe.bestweb.net [209.94.100.110]) by newman2.bestweb.net (Postfix) with ESMTP id 641E22316A; Mon, 11 Feb 2002 21:16:47 -0500 (EST) Received: by okeeffe.bestweb.net (Postfix, from userid 0) id F08309EFC2; Mon, 11 Feb 2002 21:11:52 -0500 (EST) 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 Message-Id: <20020212021152.F08309EFC2@okeeffe.bestweb.net> 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 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message