On Sat, 10 Jun 2000, Alfred Perlstein wrote:
> * Brian Fundakowski Feldman <[EMAIL PROTECTED]> [000610 09:13] wrote:
> > On Fri, 9 Jun 2000, Alfred Perlstein wrote:
> >
> > > * Alfred Perlstein <[EMAIL PROTECTED]> [000609 16:45] wrote:
> > > > hi,
> > > >
> > > > Is it just me or does the fact that uidinfo structures (see
> > > > kern/kern_proc.c) are allocated with M_WAITOK after finding them
> > > > fails and then inserted into the uidhash structure a race condition?
> > > >
> > > > Index: kern_proc.c
> > > > ===================================================================
> > >
> > > Yes, I know i forgot to put the created ones back into the list, I was
> > > just a bit flusteres after reading over the code. I'll have some more
> > > later.
> >
> > With regard to sbsize itself, the test-and-branch conditions do not have
> > to be atomic. It really isn't that important. The incrementing does,
> > though, and to fix that a very lightweight mutex should be used. How
> > about a simplelock? That should work perfectly.
>
> Well if we get an atomic_t it could be done that way, even if we
> fail the race for updating and go beyond our rlimit slightly it's
> much cheaper than a spinlock from my PoV. The only problem is
> that afaik on some archs atomic_t can be quite small, we'd have
> to watch for overflow, perhaps a spinlock is a better idea however
> only if the next thing I mention here is realized:
An atomic_t can only be as large as the largest generally usable
integer register, unless I'm missing something important. That means
that an i386 would have a bigger rlim_t than atomic_t.
> I'm somewhat upset now that I understand sbsize; every operation
> on a socket involves a lookup of the uidinfo and manipulating it.
> You could optimize it by having a flag in the socket struct that
> says "created with a non-infinity RLIMIT", which means that chgsbsize
> needs to be called when the size changes, but for the common case
> it's not done.
Well, first of all, it's not every operation on a socket. It's specific
size changes, creation, and deletion. On ATM there's also a resize on
connect(), and PF_LOCAL has pretty stupid send/receive routines that
need it; however, performance is really not affected at all even there.
> Do you think you can do that?
I suppose so. I don't think that would change the behavior (exposed
to the users) at all; it's not solving the problem you brought up.
> > As for uidinfo itself, I feel it should be done with a mutex over
> > uihashtbl. It should be grabbed if the hash for the uid is not
> > found, tested again (to see if we lost the race), and allocated.
> > I can't see a way to poke holes in that, and it would be quite
> > efficient.
>
> A mutex on each of the hash entrie heads would work, I really don't
> like the manual inlining of the lookup and insert functions,
> seperating them would be a good idea, it really reduces the complexity
> of the code.
Yes, they should. The code should have the proper mutexes and be
inlineable. Might as well do this now; simplelocks are cheap.
> --
> -Alfred Perlstein - [[EMAIL PROTECTED]|[EMAIL PROTECTED]]
> "I have the heart of a child; I keep it in a jar on my desk."
--
Brian Fundakowski Feldman \ FreeBSD: The Power to Serve! /
[EMAIL PROTECTED] `------------------------------'
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
