Date: Mon, 11 Feb 2002 20:31:02 -0500 (EST) From: John Baldwin <jhb@FreeBSD.org> To: Alfred Perlstein <bright@mu.org> Cc: current@freebsd.org, bde@freebsd.org, Julian Elischer <julian@elischer.org> Subject: Re: ucred holding patch, BDE version Message-ID: <XFMail.020211203102.jhb@FreeBSD.org> In-Reply-To: <20020211153935.J63886@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 11-Feb-02 Alfred Perlstein wrote: > * Julian Elischer <julian@elischer.org> [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 <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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.020211203102.jhb>