Bruce,
Please find the attached patch for socreate() in uipc_socket.c.
I think the code was supposed to call soalloc(0) rather then
soalloc(M_NOWAIT). Note M_NOWAIT defined as 1.
Is that a real typo or i'm missing something here?
soalloc(1) was intended (and was obtained by misspelling 1 as M_NOWAIT),
but that was probably wrong. Things have regressed from RELENG_4 where
the code was:
so = soalloc(p != 0);
socreate()'s `p' arg was abused as a flag to say that the call is made
in process context. In -current, the corresponding `td' is never NULL
so it is harder to tell if we are in process context or if this
distinction actually matters. The code first rotted to:
so = soalloc(td != 0);
Then it rotted to:
so = soalloc(M_NOWAIT);
which is equivalent to the previously rotted state since td is never NULL
and M_NOWAIT happens to be 1 (boolean true).
I see, however I find it very confusing to use M_NOWAIT just because
it is defined as 1. I'm sure there are plenty other defines that can
be used :) Why M_NOWAIT? Why just not write soalloc(1)? When I was
looking at the code I got the impression that soalloc() should not
sleep.
Changing it to your version:
so = soalloc(M_NOWAIT);
^^^^^^^^ huh? I hope you meant 0 here :) That
is what I did in my patch. Otherwise I'm deeply confused :)
is safer since it assumes the worst case (that socreate() is called in
non-process context and/or with locks held), but it leaves soalloc()'s
interface bogus (it's waitfor arg is always 0).
Right, but isn't soalloc() interface bogus already? It's "waitok"
arguments is always set to 1. As far as I can tell there only two
functions that call soalloc(): socreate() and sonewconn(). BTW
sonewconn() calls soalloc(0).
In my code I open socket from inside kernel, i.e. I call socreate()
directly with mutex held. This gives me "could sleep with" WITNESS
warning. I could make the mutex sleepable, but, frankly, there is
no reason to do so. It is not the end of the world for my code if
I can't create a socket.
Perhaps socreate() could accept another flag that tell it where
it can sleep or not. Is there any other/better way?
I don't trust the locking for this even in RELENG_4. Networking code
liked to do things like:
s = splnet(); /* "Lock" something or other. */
lotsa_stuff();
splx(s);
where lotsa_stuff() calls malloc(..., M_WAITOK) from a deeply nested
routine. This may or may not be a race, depending on whether the code
depends on splnet() to lock _everything_ against softnet activity until
the splx(). I'm not sure if this happens for socreate(). Eventually,
locking bugs in this area will be caught by locking assertions. I
think they mostly aren't now, since the interrupt context check in
malloc() has rotted and other checks aren't reached unless malloc()
actually sleeps (which rarely happens).
thanks,
max
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
