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 <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to