On 11-Feb-02 Alfred Perlstein wrote:
> * Julian Elischer <[EMAIL PROTECTED]> [020211 15:00] wrote:
>> In the current world, when the thread enters userland, it does:
>> lock giant
>> crfree() (which includes mutexes)
>> unlock giant
> This isn't needed afaik.

Yes, calling free() without Giant is about as good as calling fdrop() without
Giant Alfred. :)

>> if there are ASTs it does this once again for each AST waiting as well.
>> And on the way into the system it does:
>> lock process
>> crhold() (which includes mutex ops)
>> unlock process
> This isn't needed, at least afaik.

Not strictly for the comparison as Julian and Terry pointed out since the race
can occur anyway (i.e., you don't need the lock to see if p_ucred changed),
however, if you are actually doing a crhold(), you want to make sure p_ucred
isn't stale, so you need the proc lock.

>> so if there is a single AST (not uncommon) it does on a system call, 4 to
>> 6 locks and 4 to 6 unlocks just to get a reference on the ucred it already
>> had a reference on. By not freeing it when going to userland, and checking
>> if it is the right one when returning to the kernel, we replace that with
>> a pointer comparison (well maybe 2) 99.999% of the time.
>> John still wants to free it if INVARIANTS is on so he canh trap on
>> inapropriate access. I'm not sure it's needed but am willing to do so..
> 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().

Umm, do you understand the entire thread ucred reference at all?  What if you
have two threads on two different CPU's from teh same process and one changes
your ucred out from under you while you are handlign a syscall?  The idea here
is to ensure that a thread has a consistent ucred for an entire user trap or
syscall to avoid races involving credentials.

> Therefore the additional hold on entry is completely useless no
> matter what and with that the release on exit is also useless.

No, you just haven't been keeping up.  I added td_ucred at least a month or so
ago and it was discussed on -arch before it went in.  I have a big honking
patch to test when I get back from BSDCon that changes the kernel to use
td_ucred almost everywhere instead of p_ucred so that we actualyl use the
per-thread ucred (which will be needed before we can stop being a 1:1:1:1
system) and also means credential ops such as suser(), and p_can* don't need
locks for the current thread.  (p_can* still need locks for the target process).

> -Alfred


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

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

Reply via email to