Date: Mon, 11 Feb 2002 20:44:40 -0500 (EST) From: John Baldwin <jhb@FreeBSD.org> To: Terry Lambert <tlambert2@mindspring.com> Cc: current@freebsd.org, bde@freebsd.org, Subject: Re: ucred holding patch, BDE version Message-ID: <20020212021253.085A59F416@okeeffe.bestweb.net>
next in thread | raw e-mail | index | archive | help
On 12-Feb-02 Terry Lambert wrote: > Julian Elischer wrote: >> > This makes little sense to me. >> > >> > Maybe I'm missing something, but by virtue of ownership we don't >> > have to worry about the ucred's refcount on entry into the kernel >> > because it is the owner and no one else is allowed to change our >> > privledges besideds ourselves via set[ug]id(). >> >> multiple threads can do it.. >> >> The proclock is needed to get the reference, >> guarding against other threads, and giant is needed fo rnot to free it >> because if it reaches a refcount of 0 it needs to call free(). (which john >> assures me needs Giant at this time). >> We could avoid the proclock with judicious use of an atomic refcount >> incrementing method. >> >> When Giant goes away it won't be so bad but it will STILL be quicker to >> not drop it across userland. > > The "multiple threads" argument is bogus, since the calls > to [gs]et[ug]id() are on a per process, not a per thread > basis. Thread 1 makes a syscall that blocks. Say it blocks in one of 3 VOP's all of which need a credential. Thread 2 changes the credentials of the process. Thread 1 now resumes assuming that say a VADMIN or VACCESS suceeded with the old cred that may not be valid any longer and performs VOP's with the now newer credential (which it may even read a stale value of w/o a lock thus using some random memory for the creds) to do its other VOP's which may now fail weirdly. The idea of per-thread ucred references is so that a thread will have the same credentials for the lifetime of a syscall so that the entire syscall is self consistent. It also means that except for when we update the ucred reference, we don't need locks to access thread credentials since the thread references are to read-only credentials. We discussed this on -arch _months_ ago and everyone agreed with it then. > Perhaps this dicsussion is enough impetus to justify > revisiting the atomic_t type definitions, which would > be useful as reference counted hold/release mechanisms > that would obviate the need for locks here? This would > at least let you defer getting rid of the per thread > cred instances until later. All having the refcount_t and other refcount_* functions would do is let us get rid of the per-ucred mutex (actually a pool mutex, so the overhead per ucred is just a pointer right now). It wouln't change the fact that we need a lock to make sure p_ucred is up to date before we read a value we need to depend on or actually use. > -- Terry -- 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-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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020212021253.085A59F416>