Date: Fri, 22 Feb 2002 11:19:03 -0800 (PST) From: Julian Elischer <julian@elischer.org> To: John Baldwin <jhb@FreeBSD.org> Cc: Matthew Dillon <dillon@apollo.backplane.com>, arch@FreeBSD.org Subject: Re: RE: that INVARIANT/ucred freeing stuff. Message-ID: <Pine.BSF.4.21.0202221109330.74100-100000@InterJet.elischer.org> In-Reply-To: <XFMail.020222041535.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <jhb@FreeBSD.org> <>< 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
> > <dillon@backplane.com>
>
> --
>
> John Baldwin <jhb@FreeBSD.org> <>< 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0202221109330.74100-100000>