From owner-freebsd-current Mon Feb 11 17:45:10 2002 Delivered-To: freebsd-current@freebsd.org Received: from mail12.speakeasy.net (mail12.speakeasy.net [216.254.0.212]) by hub.freebsd.org (Postfix) with ESMTP id C2EFF37B417 for ; Mon, 11 Feb 2002 17:44:51 -0800 (PST) Received: (qmail 5654 invoked from network); 12 Feb 2002 01:44:50 -0000 Received: from unknown (HELO laptop.baldwin.cx) ([65.91.174.125]) (envelope-sender ) by mail12.speakeasy.net (qmail-ldap-1.03) with SMTP for ; 12 Feb 2002 01:44:50 -0000 Message-ID: X-Mailer: XFMail 1.4.0 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 In-Reply-To: <3C686E1E.84A42678@mindspring.com> Date: Mon, 11 Feb 2002 20:44:40 -0500 (EST) From: John Baldwin To: Terry Lambert Subject: Re: ucred holding patch, BDE version Cc: current@freebsd.org, bde@freebsd.org, Alfred Perlstein , Julian Elischer Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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 <>< 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