Re: [Real fix] Re: Kernel panic: can't push onto full stack

Andrea Arcangeli (andrea@e-mind.com)
Mon, 1 Mar 1999 01:22:18 +0100 (CET)


On Sun, 28 Feb 1999 kuznet@ms2.inr.ac.ru wrote:

>Another problem was that we may have up to 2*max_files unix sockets:
>not-yet-accepted ones are not associated with files/inodes.

Ah I think to understand. It seems that upon connect af_unix will generate
a new sock hashed in the global socket list in this line:

/* create new sock for complete connection */
newsk = unix_create1(NULL, 1);

If I understand well the connect of the client is allocating the sock of
the server side (the peer of the client). So for every file you can have
two sock allocated.

>Using list is always OK, however it also requires reviewing

And obviously using a list is also a nice thing from a MM point of view ;).

>af_unix.c: f.e. pending socket destruction must be blocked
>while gc is running.

Really it seems to me that while the sock is hashed in the unix-sock
hashtable, everything is done with the big kernel lock held and without
sleep in unix_gc. The only point that could race it seems to be the
delayed destroy of the sock, but at that time the sock is just been
removed from the hash table from ages. So I think the code is just safe.
Maybe I am missing something...

I did some cleanup thought. Here a patch against second Alexander's patch:

Index: net/unix//garbage.c
===================================================================
RCS file: /var/cvs/linux/net/unix/garbage.c,v
retrieving revision 1.1.2.2
diff -u -r1.1.2.2 garbage.c
--- garbage.c 1999/02/28 20:05:45 1.1.2.2
+++ linux/net/unix/garbage.c 1999/03/01 00:15:03
@@ -17,7 +17,7 @@
*
* - explicit stack instead of recursion
* - tail recurse on first born instead of immediate push/pop
- * - we gather the stuff that should not be killed into tree
+ * - we gather the stuff that should not be killed into list
* and stack is just a path from root to the current pointer.
*
* Future optimizations:
@@ -53,12 +53,12 @@
* unrelated processes.
*
* AV 28 Feb 1999
- * Kill the explicit allocation of stack. Now we keep the tree
- * with root in dummy + pointer (gc_current) to one of the nodes.
- * Stack is represented as path from gc_current to dummy. Unmark
- * now means "add to tree". Push == "make it a son of gc_current".
- * Pop == "move gc_current to parent". We keep only pointers to
- * parents (->gc_tree).
+ * Kill the explicit allocation of stack. Now we keep the marked
+ * socks in the list with root in gc_current to one of the nodes.
+ * Stack is represented as path from gc_current to the last elem.
+ * Unmark now means "add to list". Push == "make it a son of
+ * gc_current". Pop == "move gc_current to parent". We keep only
+ * pointers to parents (->gc_next).
*
*/

@@ -82,8 +82,8 @@

/* Internal data structures and random procedures: */

-static unix_socket dummy;
-static unix_socket *gc_current=&dummy; /* stack of objects to mark */
+#define GC_HEAD ((unix_socket *) -1)
+static unix_socket *gc_current=GC_HEAD; /* stack of objects to mark */

extern inline unix_socket *unix_get_socket(struct file *filp)
{
@@ -133,20 +133,20 @@
extern inline unix_socket *pop_stack(void)
{
unix_socket *p=gc_current;
- gc_current = p->protinfo.af_unix.gc_tree;
+ gc_current = p->protinfo.af_unix.gc_next;
return p;
}

extern inline int empty_stack(void)
{
- return gc_current == &dummy;
+ return gc_current == GC_HEAD;
}

extern inline void maybe_unmark_and_push(unix_socket *x)
{
- if (x->protinfo.af_unix.gc_tree)
+ if (x->protinfo.af_unix.gc_next)
return;
- x->protinfo.af_unix.gc_tree = gc_current;
+ x->protinfo.af_unix.gc_next = gc_current;
gc_current = x;
}

@@ -171,7 +171,7 @@

forall_unix_sockets(i, s)
{
- s->protinfo.af_unix.gc_tree=NULL;
+ s->protinfo.af_unix.gc_next=NULL;
}
/*
* Everything is now marked
@@ -262,9 +262,9 @@

if (f)
{
- if (!f->protinfo.af_unix.gc_tree)
+ if (!f->protinfo.af_unix.gc_next)
{
- f->protinfo.af_unix.gc_tree=&dummy;
+ f->protinfo.af_unix.gc_next=GC_HEAD;
x=f;
f=NULL;
goto tail;
@@ -276,7 +276,7 @@

forall_unix_sockets(i, s)
{
- if (!s->protinfo.af_unix.gc_tree)
+ if (!s->protinfo.af_unix.gc_next)
{
struct sk_buff *nextsk;
skb=skb_peek(&s->receive_queue);
Index: include/net//sock.h
===================================================================
RCS file: /var/cvs/linux/include/net/sock.h,v
retrieving revision 1.1.2.5
diff -u -r1.1.2.5 sock.h
--- sock.h 1999/02/28 20:05:54 1.1.2.5
+++ linux/include/net/sock.h 1999/02/28 22:41:16
@@ -99,7 +99,7 @@
struct semaphore readsem;
struct sock * other;
struct sock ** list;
- struct sock * gc_tree;
+ struct sock * gc_next;
int inflight;
};

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/