Date: Mon, 11 Feb 2002 06:32:48 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Julian Elischer <julian@elischer.org> Cc: <current@FreeBSD.ORG> Subject: Re: final ucred patch Message-ID: <20020211060831.T9036-100000@gamplex.bde.org> In-Reply-To: <3C66133F.43D243E0@elischer.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> After comments by jhb and bde
>
> Index: i386/i386/trap.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/i386/i386/trap.c,v
> retrieving revision 1.211
> diff -u -r1.211 trap.c
> --- i386/i386/trap.c 10 Jan 2002 11:49:54 -0000 1.211
> +++ i386/i386/trap.c 10 Feb 2002 00:52:58 -0000
> @@ -256,9 +256,19 @@
> sticks = td->td_kse->ke_sticks;
> td->td_frame = &frame;
> KASSERT(td->td_ucred == NULL, ("already have a ucred"));
> - PROC_LOCK(p);
> - td->td_ucred = crhold(p->p_ucred);
> - PROC_UNLOCK(p);
> + if (td->td_ucred != p->p_ucred) {
> + if (td->td_ucred) {
> + mtx_lock(&Giant);
> + crfree(td->td_ucred);
> + td->td_ucred = NULL;
> + mtx_unlock(&Giant);
See below about placement of this unlock.
> + }
> + if (p->p_ucred) {
How can this be NULL? The old code didn't check.
> + PROC_LOCK(p);
> + td->td_ucred = crhold(p->p_ucred);
> + PROC_UNLOCK(p);
> + }
> + }
The inner block is large enough and repeated enough to turn into a function.
>
> switch (type) {
> case T_PRIVINFLT: /* privileged instruction fault */
> @@ -644,10 +654,12 @@
> userret(td, &frame, sticks);
> mtx_assert(&Giant, MA_NOTOWNED);
> userout:
> +#ifdef INVARIANTS
> mtx_lock(&Giant);
> crfree(td->td_ucred);
> - mtx_unlock(&Giant);
> td->td_ucred = NULL;
> + mtx_unlock(&Giant);
> +#endif
> out:
> return;
> }
I think moving the unlock is just an obfuscation. td_ucred isn't locked
by Giant.
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020211060831.T9036-100000>