Date: Thu, 28 Mar 2002 10:07:39 -0500 (EST) From: John Baldwin <jhb@FreeBSD.org> To: Bruce Evans <bde@zeta.org.au> Cc: smp@FreeBSD.ORG, Robert Watson <rwatson@FreeBSD.ORG> Subject: Re: suser() API change patch Message-ID: <XFMail.20020328100739.jhb@FreeBSD.org> In-Reply-To: <20020328211326.P7433-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 28-Mar-2002 Bruce Evans wrote: > On Wed, 27 Mar 2002, Robert Watson wrote: > >> On Wed, 27 Mar 2002, Bruce Evans wrote: >> >> > On Tue, 26 Mar 2002, John Baldwin wrote: >> > >> > > The patch at the URL below changes the suser() API as follows. >> > > Currently we >> > > have four suser() functions that take the following args: >> > > >> > > suser(proc) >> > > suser_td(thread) >> > > suser_xxx(cred, proc, flag) >> > > suser_xxx_td(cred, thread, flag) >> > > >> > > In the new scheme (which has been approved by Robert Watson and is >> > > really >> > > his design) we go back to only two functions like so: >> > > >> > > suser(thread, flag) >> > > suser_cred(cred, flag) >> > >> > The former should be suser(thread). In the patch, the flag is nonzero >> > in less than 10% of the calls. >> >> This nice thing about the suser(thread) model, pointed out I think by >> Julian a few months ago, is that it hides the structure contents of thread >> and proc from the caller. suser_cred() requires that the caller not only >> include proc.h, but also that it dereference proc.h, meaning that if >> struct thread or struct proc changes, binary compatibility is broken for >> any precompiled code. Originally, my preference was to always expose the >> credential selection decision in the calling code, but in light of this >> argument, I fell back to using suser() in all places a specific credential >> decision wasn't made. This also had the advantage of hiding whether >> suser(thread) was extracting the credential from the thread or the process >> structure, during the migration. Other than the fact that the flag is >> frequently 0, is there another rationale you have in mind for this? The >> API is still changing (from proc to thread) regardless, so there's no >> compatibility throw-away. > > suser(thread, flag) could still exist (named somthing like suser_flag()) > if it is used enough to justify it. My main point is that the flag is > rarely used, so the interface shouldn't be bloated to pass it. I don't think it's very much bloat and it avoids having to complicate the API by adding a suser_flag() or some such. Originally Robert wanted to just have suser() as what is now suser_cred(). suser() exists for the reasons Robert highlighted above so that a thread can be passed in. > Another point: td->td_ucred can only be safely used without locking > if td is curthread. Our current code mostly assumes this. suser(td) > can easily check that td is curthread, but this is a silly reason to > use a bloated interface. It is just bug for bug compatible with passing > thread pointers around a lot. Actually, td_ucred can be safely used w/o locking for any thread which is guaranteed to not leave the kernel while it is being read. The easiest one to guarantee that for is curthread. Locks would never help since none protect writing to td_ucred. As far as the more general notion of passing thread pointers rather than using curthread, I'll just ignore that issue for now. It would need its own bikeshed. :) > Bruce -- 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-smp" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20020328100739.jhb>