Linux HAM/Amateur Radio development
 help / color / mirror / Atom feed
* [PATCH] ax25_cb refcounting & waitqueue usage
@ 2003-07-06 17:27 Jeroen Vreeken
  2003-07-07 13:37 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 6+ messages in thread
From: Jeroen Vreeken @ 2003-07-06 17:27 UTC (permalink / raw)
  To: linux-hams; +Cc: ralf

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

Hi,

Second patch for this weekend...
This patch does two things, it adds reference counting to the ax25_cb
structure (until now only the ax25_cb list was protected, not the parts
using it) and it fixes an oops on interrupted socket syscalls. (e.g.
pressing Ctrl-C while in the connect function) The waitqueue was not
removed from the list and caused an oops in the socket release.

I have had only a quick look at Steven's patch but I don't think the two
will clash...

Jeroen

[-- Attachment #2: ax25-2.5.74.rxq.diff --]
[-- Type: application/octet-stream, Size: 6320 bytes --]

diff -ru linux-2.5.74/net/ax25/af_ax25.c linux-2.5.74.rxq/net/ax25/af_ax25.c
--- linux-2.5.74/net/ax25/af_ax25.c	2003-07-06 01:05:13.000000000 +0200
+++ linux-2.5.74.rxq/net/ax25/af_ax25.c	2003-07-06 18:43:45.000000000 +0200
@@ -54,13 +54,20 @@
 ax25_cb *ax25_list;
 spinlock_t ax25_list_lock = SPIN_LOCK_UNLOCKED;
 
+spinlock_t ax25_cb_lock = SPIN_LOCK_UNLOCKED;
+
 static struct proto_ops ax25_proto_ops;
 
 /*
  *	Free an allocated ax25 control block. This is done to centralise
  *	the MOD count code.
  */
-void ax25_free_cb(ax25_cb *ax25)
+/*
+ *	Changed name to include undescores. Nobody should call this without
+ *	going through the ref counter anymore 
+ *	- PE1RXQ
+ */
+void __ax25_free_cb(ax25_cb *ax25)
 {
 	if (ax25->digipeat != NULL) {
 		kfree(ax25->digipeat);
@@ -70,9 +77,25 @@
 	kfree(ax25);
 }
 
+void ax25_up_cb(ax25_cb *ax25)
+{
+	spin_lock_bh(&ax25_cb_lock);
+	ax25->refcount++;
+	spin_unlock_bh(&ax25_cb_lock);
+}
+
+void ax25_down_cb(ax25_cb *ax25)
+{
+	spin_lock_bh(&ax25_cb_lock);
+	ax25->refcount--;
+	if (!ax25->refcount)
+		__ax25_free_cb(ax25);
+	spin_unlock_bh(&ax25_cb_lock);
+}
+
 static void ax25_free_sock(struct sock *sk)
 {
-	ax25_free_cb(ax25_sk(sk));
+	ax25_down_cb(ax25_sk(sk));
 }
 
 /*
@@ -85,6 +108,7 @@
 	spin_lock_bh(&ax25_list_lock);
 	if ((s = ax25_list) == ax25) {
 		ax25_list = s->next;
+		ax25_down_cb(ax25);
 		spin_unlock_bh(&ax25_list_lock);
 		return;
 	}
@@ -92,6 +116,7 @@
 	while (s != NULL && s->next != NULL) {
 		if (s->next == ax25) {
 			s->next = ax25->next;
+			ax25_down_cb(ax25);
 			spin_unlock_bh(&ax25_list_lock);
 			return;
 		}
@@ -156,6 +181,7 @@
 void ax25_insert_socket(ax25_cb *ax25)
 {
 	spin_lock_bh(&ax25_list_lock);
+	ax25_up_cb(ax25);
 	ax25->next = ax25_list;
 	ax25_list  = ax25;
 	spin_unlock_bh(&ax25_list_lock);
@@ -240,6 +266,7 @@
 				if (s->digipeat != NULL && s->digipeat->ndigi != 0)
 					continue;
 			}
+			ax25_up_cb(s);
 			spin_unlock_bh(&ax25_list_lock);
 
 			return s;
@@ -352,7 +379,7 @@
 			sk_free(ax25->sk);
 		}
 	} else {
-		ax25_free_cb(ax25);
+		ax25_down_cb(ax25);
 	}
 }
 
@@ -507,6 +534,7 @@
 		return NULL;
 
 	memset(ax25, 0x00, sizeof(*ax25));
+	ax25_up_cb(ax25);
 
 	skb_queue_head_init(&ax25->write_queue);
 	skb_queue_head_init(&ax25->frag_queue);
@@ -877,7 +905,7 @@
 		break;
 	default:
 		sk_free(sk);
-		ax25_free_cb(ax25);
+		ax25_down_cb(ax25);
 		return NULL;
 	}
 
@@ -1092,7 +1120,7 @@
 	int addr_len, int flags)
 {
 	struct sock *sk = sock->sk;
-	ax25_cb *ax25 = ax25_sk(sk);
+	ax25_cb *ax25 = ax25_sk(sk), *ax25t;
 	struct full_sockaddr_ax25 *fsa = (struct full_sockaddr_ax25 *)uaddr;
 	ax25_digi *digi = NULL;
 	int ct = 0, err = 0;
@@ -1207,11 +1235,12 @@
 	}
 
 	if (sk->sk_type == SOCK_SEQPACKET &&
-	    ax25_find_cb(&ax25->source_addr, &fsa->fsa_ax25.sax25_call, digi,
-		    	 ax25->ax25_dev->dev)) {
+	    (ax25t=ax25_find_cb(&ax25->source_addr, &fsa->fsa_ax25.sax25_call, digi,
+		    	 ax25->ax25_dev->dev))) {
 		if (digi != NULL)
 			kfree(digi);
 		err = -EADDRINUSE;		/* Already such a connection */
+		ax25_down_cb(ax25t);
 		goto out;
 	}
 
@@ -1272,6 +1301,8 @@
 				lock_sock(sk);
 				continue;
 			}
+			current->state = TASK_RUNNING;
+			remove_wait_queue(sk->sk_sleep, &wait);
 			return -ERESTARTSYS;
 		}
 		current->state = TASK_RUNNING;
@@ -1339,6 +1370,8 @@
 			lock_sock(sk);
 			continue;
 		}
+		current->state = TASK_RUNNING;
+		remove_wait_queue(sk->sk_sleep, &wait);
 		return -ERESTARTSYS;
 	}
 	current->state = TASK_RUNNING;
@@ -1955,6 +1988,8 @@
 EXPORT_SYMBOL(ax25_rebuild_header);
 EXPORT_SYMBOL(ax25_findbyuid);
 EXPORT_SYMBOL(ax25_find_cb);
+EXPORT_SYMBOL(ax25_up_cb);
+EXPORT_SYMBOL(ax25_down_cb);
 EXPORT_SYMBOL(ax25_linkfail_register);
 EXPORT_SYMBOL(ax25_linkfail_release);
 EXPORT_SYMBOL(ax25_listen_register);
diff -ru linux-2.5.74/net/ax25/ax25_in.c linux-2.5.74.rxq/net/ax25/ax25_in.c
--- linux-2.5.74/net/ax25/ax25_in.c	2003-07-06 01:05:12.000000000 +0200
+++ linux-2.5.74.rxq/net/ax25/ax25_in.c	2003-07-06 18:44:42.000000000 +0200
@@ -329,6 +329,7 @@
 		if (ax25_process_rx_frame(ax25, skb, type, dama) == 0)
 			kfree_skb(skb);
 
+		ax25_down_cb(ax25);
 		return 0;
 	}
 
diff -ru linux-2.5.74/net/ax25/ax25_ip.c linux-2.5.74.rxq/net/ax25/ax25_ip.c
--- linux-2.5.74/net/ax25/ax25_ip.c	2003-07-06 01:05:13.000000000 +0200
+++ linux-2.5.74.rxq/net/ax25/ax25_ip.c	2003-07-06 18:46:14.000000000 +0200
@@ -107,6 +107,7 @@
 	ax25_address *src, *dst;
 	ax25_dev *ax25_dev;
 	ax25_route _route, *route = &_route;
+	ax25_cb *ax25;
 
 	dst = (ax25_address *)(bp + 1);
 	src = (ax25_address *)(bp + 8);
@@ -167,9 +168,14 @@
 			skb_pull(ourskb, AX25_HEADER_LEN - 1);	/* Keep PID */
 			ourskb->nh.raw = ourskb->data;
 
-			ax25_send_frame(ourskb, ax25_dev->values[AX25_VALUES_PACLEN], &src_c,
-&dst_c, route->digipeat, dev);
-
+			ax25=ax25_send_frame(
+			    ourskb, 
+			    ax25_dev->values[AX25_VALUES_PACLEN], 
+			    &src_c,
+			    &dst_c, route->digipeat, dev);
+			if (ax25) {
+				ax25_down_cb(ax25);
+			}
 			goto put;
 		}
 	}
diff -ru linux-2.5.74/net/ax25/ax25_out.c linux-2.5.74.rxq/net/ax25/ax25_out.c
--- linux-2.5.74/net/ax25/ax25_out.c	2003-07-06 01:05:13.000000000 +0200
+++ linux-2.5.74.rxq/net/ax25/ax25_out.c	2003-07-06 18:46:53.000000000 +0200
@@ -71,7 +71,7 @@
 
 	if (digi != NULL) {
 		if ((ax25->digipeat = kmalloc(sizeof(ax25_digi), GFP_ATOMIC)) == NULL) {
-			ax25_free_cb(ax25);
+			ax25_down_cb(ax25);
 			return NULL;
 		}
 		memcpy(ax25->digipeat, digi, sizeof(ax25_digi));
diff -ru linux-2.5.74/include/net/ax25.h linux-2.5.74.rxq/include/net/ax25.h
--- linux-2.5.74/include/net/ax25.h	2003-07-06 01:12:50.000000000 +0200
+++ linux-2.5.74.rxq/include/net/ax25.h	2003-07-05 23:19:25.000000000 +0200
@@ -199,6 +199,7 @@
 	unsigned char		window;
 	struct timer_list	timer;
 	struct sock		*sk;		/* Backlink to socket */
+	int			refcount;
 } ax25_cb;
 
 #define ax25_sk(__sk) ((ax25_cb *)(__sk)->sk_protinfo)
@@ -206,7 +207,8 @@
 /* af_ax25.c */
 extern ax25_cb *ax25_list;
 extern spinlock_t ax25_list_lock;
-extern void ax25_free_cb(ax25_cb *);
+extern void ax25_up_cb(ax25_cb *);
+extern void ax25_down_cb(ax25_cb *);
 extern void ax25_insert_socket(ax25_cb *);
 struct sock *ax25_find_listener(ax25_address *, int, struct net_device *, int);
 struct sock *ax25_get_socket(ax25_address *, ax25_address *, int);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ax25_cb refcounting & waitqueue usage
  2003-07-06 17:27 [PATCH] ax25_cb refcounting & waitqueue usage Jeroen Vreeken
@ 2003-07-07 13:37 ` Arnaldo Carvalho de Melo
  2003-07-07 17:23   ` Jeroen Vreeken
  0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2003-07-07 13:37 UTC (permalink / raw)
  To: Jeroen Vreeken; +Cc: linux-hams, ralf

Em Sun, Jul 06, 2003 at 07:27:34PM +0200, Jeroen Vreeken escreveu:
> Hi,
> 
> Second patch for this weekend...
> This patch does two things, it adds reference counting to the ax25_cb
> structure (until now only the ax25_cb list was protected, not the parts
> using it) and it fixes an oops on interrupted socket syscalls. (e.g.
> pressing Ctrl-C while in the connect function) The waitqueue was not
> removed from the list and caused an oops in the socket release.
> 
> I have had only a quick look at Steven's patch but I don't think the two
> will clash...

Hey, why not use sk->refcnt by means of sock_hold/sock_put? Current sources
even do that implicitely when you do a sk_add_node (sock_hold) and a
sk_del_node{_init} (sock_put)... Now it is just a matter of doing a sock_hold
when searching in some list and then dropping the refcnt when done using the
sock returned by the search function.

- Arnaldo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ax25_cb refcounting & waitqueue usage
  2003-07-07 13:37 ` Arnaldo Carvalho de Melo
@ 2003-07-07 17:23   ` Jeroen Vreeken
  2003-07-07 17:25     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 6+ messages in thread
From: Jeroen Vreeken @ 2003-07-07 17:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-hams, ralf

On 2003.07.07 15:37:10 +0200 Arnaldo Carvalho de Melo wrote:
> Em Sun, Jul 06, 2003 at 07:27:34PM +0200, Jeroen Vreeken escreveu:
> > Hi,
> > 
> > Second patch for this weekend...
> > This patch does two things, it adds reference counting to the ax25_cb
> > structure (until now only the ax25_cb list was protected, not the parts
> > using it) and it fixes an oops on interrupted socket syscalls. (e.g.
> > pressing Ctrl-C while in the connect function) The waitqueue was not
> > removed from the list and caused an oops in the socket release.
> > 
> > I have had only a quick look at Steven's patch but I don't think the
> two
> > will clash...
> 
> Hey, why not use sk->refcnt by means of sock_hold/sock_put? Current
> sources
> even do that implicitely when you do a sk_add_node (sock_hold) and a
> sk_del_node{_init} (sock_put)... Now it is just a matter of doing a
> sock_hold
> when searching in some list and then dropping the refcnt when done using
> the
> sock returned by the search function.

Unfortunatly the ax25_cbs are not directly related to a sock structure...
Sometimes they don't even have them, e.g. when you do ip in connected mode
or netrom and rose interlinks. In those cases you can even share an ax25_cb
between a socket and those protocols in which case you absolutly need the
refcounting....
Consider the following:

- ip datagram is send to ax25 layer.
- ax25 layer uses a lookup in the ax25_cb list to find an existing
connection
- list code locks, searches, unlocks, returns
***** preempt/interrupt/other processor
- application closes socket
- ax25_cb is removed from the list (again with proper list locking)
- ax25_cb is freed
***** preempt/interrupt/other processor back to ip handling routine
- ip handling routine uses ax25_cb
- BOOM!


Jeroen


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ax25_cb refcounting & waitqueue usage
  2003-07-07 17:23   ` Jeroen Vreeken
@ 2003-07-07 17:25     ` Arnaldo Carvalho de Melo
  2003-07-08 22:52       ` Jeroen Vreeken
  0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2003-07-07 17:25 UTC (permalink / raw)
  To: Jeroen Vreeken; +Cc: linux-hams, ralf

Em Mon, Jul 07, 2003 at 07:23:40PM +0200, Jeroen Vreeken escreveu:
> On 2003.07.07 15:37:10 +0200 Arnaldo Carvalho de Melo wrote:
> > Em Sun, Jul 06, 2003 at 07:27:34PM +0200, Jeroen Vreeken escreveu:
> > > Hi,
> > > 
> > > Second patch for this weekend...
> > > This patch does two things, it adds reference counting to the ax25_cb
> > > structure (until now only the ax25_cb list was protected, not the parts
> > > using it) and it fixes an oops on interrupted socket syscalls. (e.g.
> > > pressing Ctrl-C while in the connect function) The waitqueue was not
> > > removed from the list and caused an oops in the socket release.
> > > 
> > > I have had only a quick look at Steven's patch but I don't think the
> > two
> > > will clash...
> > 
> > Hey, why not use sk->refcnt by means of sock_hold/sock_put? Current
> > sources
> > even do that implicitely when you do a sk_add_node (sock_hold) and a
> > sk_del_node{_init} (sock_put)... Now it is just a matter of doing a
> > sock_hold
> > when searching in some list and then dropping the refcnt when done using
> > the
> > sock returned by the search function.
> 
> Unfortunatly the ax25_cbs are not directly related to a sock structure...
> Sometimes they don't even have them, e.g. when you do ip in connected mode
> or netrom and rose interlinks. In those cases you can even share an ax25_cb
> between a socket and those protocols in which case you absolutly need the
> refcounting....
> Consider the following:
> 
> - ip datagram is send to ax25 layer.
> - ax25 layer uses a lookup in the ax25_cb list to find an existing
> connection
> - list code locks, searches, unlocks, returns
> ***** preempt/interrupt/other processor
> - application closes socket
> - ax25_cb is removed from the list (again with proper list locking)
> - ax25_cb is freed
> ***** preempt/interrupt/other processor back to ip handling routine
> - ip handling routine uses ax25_cb
> - BOOM!

Got it, i.e. the fact that ax25_cb structs are not directly always associated
with a struct sock, the scenario above is the same for struct sock refcounting,
so may I suggest that you use the same arquitecture of list handling in struct
sock? i.e. use hlist and have a ax25_cb_add, ax25_cb_del{_init} that does the
refcounting, etc, so that it "looks like" what we have for struct sock and that
way a casual ax25 hacker can take advantage of what he learnt from studying
struct sock handling? I plan to attack the list searching problem in a general
way, i.e. abstracting more the search in a bsearch(3) way, using inlines and
macros so that all the protocols have a more consistent implementation.

- Arnaldo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ax25_cb refcounting & waitqueue usage
  2003-07-07 17:25     ` Arnaldo Carvalho de Melo
@ 2003-07-08 22:52       ` Jeroen Vreeken
  2003-07-09 17:06         ` Jeroen Vreeken
  0 siblings, 1 reply; 6+ messages in thread
From: Jeroen Vreeken @ 2003-07-08 22:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-hams, ralf

On 2003.07.07 19:25:04 +0200 Arnaldo Carvalho de Melo wrote:

> so may I suggest that you use the same arquitecture of list handling in
> struct
> sock? i.e. use hlist and have a ax25_cb_add, ax25_cb_del{_init} that does
> the
> refcounting, etc, so that it "looks like" what we have for struct sock
> and that
> way a casual ax25 hacker can take advantage of what he learnt from
> studying
> struct sock handling? I plan to attack the list searching problem in a
> general
> way, i.e. abstracting more the search in a bsearch(3) way, using inlines
> and
> macros so that all the protocols have a more consistent implementation.

Good idea...

I now have the first ax25 stack with hlists running on my test system...
It needs some cleaning up (some old junk commented out at the moment and I
need to rename some functions to be more descriptive) which will probably
happen in the next few days.
I also removed the bpq header length stuff in the skb allocation, it now
uses the hard_header_length of the network device... The stack should not
have to now which devices there might possibly be.

Jeroen


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ax25_cb refcounting & waitqueue usage
  2003-07-08 22:52       ` Jeroen Vreeken
@ 2003-07-09 17:06         ` Jeroen Vreeken
  0 siblings, 0 replies; 6+ messages in thread
From: Jeroen Vreeken @ 2003-07-09 17:06 UTC (permalink / raw)
  To: ralf; +Cc: Arnaldo Carvalho de Melo, linux-hams

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

Hi,

This is a new patch to replace my first one...
(Anybody on this list should know by now that I always send at least two
versions of each patch ;)

It does the following things:
-fixes waitqueue oops on interrupted socket call
-Adds refcounting to the ax25_cb struct with two new functions: 
 ax25_cb_hold() and ax25_cb_put()
-skb allocation uses dev->hard_header_len instead of some predicted worst
case.
-The ax25_cb list uses hlists

The biggest change is the hlist one combined with the refcounting.
This makes ax25 look somewhat like the other network stack as Arnaldo
sugested.

Jeroen

[-- Attachment #2: ax25-2.5.74.rxq.diff --]
[-- Type: application/octet-stream, Size: 14158 bytes --]

diff -ru linux-2.5.74/net/ax25/af_ax25.c linux-2.5.74.rxq/net/ax25/af_ax25.c
--- linux-2.5.74/net/ax25/af_ax25.c	2003-07-06 01:05:13.000000000 +0200
+++ linux-2.5.74.rxq/net/ax25/af_ax25.c	2003-07-09 18:22:17.000000000 +0200
@@ -51,16 +51,23 @@
 
 
 
-ax25_cb *ax25_list;
+HLIST_HEAD(ax25_list);
 spinlock_t ax25_list_lock = SPIN_LOCK_UNLOCKED;
 
+spinlock_t ax25_cb_lock = SPIN_LOCK_UNLOCKED;
+
 static struct proto_ops ax25_proto_ops;
 
 /*
  *	Free an allocated ax25 control block. This is done to centralise
  *	the MOD count code.
  */
-void ax25_free_cb(ax25_cb *ax25)
+/*
+ *	Changed name to include undescores. Nobody should call this without
+ *	going through the ref counter anymore 
+ *	- PE1RXQ
+ */
+void __ax25_free_cb(ax25_cb *ax25)
 {
 	if (ax25->digipeat != NULL) {
 		kfree(ax25->digipeat);
@@ -70,35 +77,36 @@
 	kfree(ax25);
 }
 
+void ax25_cb_hold(ax25_cb *ax25)
+{
+	spin_lock_bh(&ax25_cb_lock);
+	ax25->refcount++;
+	spin_unlock_bh(&ax25_cb_lock);
+}
+
+void ax25_cb_put(ax25_cb *ax25)
+{
+	spin_lock_bh(&ax25_cb_lock);
+	ax25->refcount--;
+	if (!ax25->refcount)
+		__ax25_free_cb(ax25);
+	spin_unlock_bh(&ax25_cb_lock);
+}
+
 static void ax25_free_sock(struct sock *sk)
 {
-	ax25_free_cb(ax25_sk(sk));
+	ax25_cb_put(ax25_sk(sk));
 }
 
 /*
  *	Socket removal during an interrupt is now safe.
  */
-static void ax25_remove_socket(ax25_cb *ax25)
+static void ax25_cb_del(ax25_cb *ax25)
 {
-	ax25_cb *s;
-
 	spin_lock_bh(&ax25_list_lock);
-	if ((s = ax25_list) == ax25) {
-		ax25_list = s->next;
-		spin_unlock_bh(&ax25_list_lock);
-		return;
-	}
-
-	while (s != NULL && s->next != NULL) {
-		if (s->next == ax25) {
-			s->next = ax25->next;
-			spin_unlock_bh(&ax25_list_lock);
-			return;
-		}
-
-		s = s->next;
-	}
+	hlist_del_init(&ax25->ax25_node);
 	spin_unlock_bh(&ax25_list_lock);
+	ax25_cb_put(ax25);
 }
 
 /*
@@ -108,17 +116,17 @@
 {
 	ax25_dev *ax25_dev;
 	ax25_cb *s;
+	struct hlist_node *node;
 
 	if ((ax25_dev = ax25_dev_ax25dev(dev)) == NULL)
 		return;
 
 	spin_lock_bh(&ax25_list_lock);
-	for (s = ax25_list; s != NULL; s = s->next) {
+	ax25_for_each(s, node, &ax25_list)
 		if (s->ax25_dev == ax25_dev) {
 			s->ax25_dev = NULL;
 			ax25_disconnect(s, ENETUNREACH);
 		}
-	}
 	spin_unlock_bh(&ax25_list_lock);
 }
 
@@ -153,11 +161,11 @@
 /*
  *	Add a socket to the bound sockets list.
  */
-void ax25_insert_socket(ax25_cb *ax25)
+void ax25_cb_add(ax25_cb *ax25)
 {
 	spin_lock_bh(&ax25_list_lock);
-	ax25->next = ax25_list;
-	ax25_list  = ax25;
+	ax25_cb_hold(ax25);
+	hlist_add_head(&ax25->ax25_node, &ax25_list);
 	spin_unlock_bh(&ax25_list_lock);
 }
 
@@ -169,9 +177,10 @@
 	struct net_device *dev, int type)
 {
 	ax25_cb *s;
+	struct hlist_node *node;
 
 	spin_lock_bh(&ax25_list_lock);
-	for (s = ax25_list; s != NULL; s = s->next) {
+	ax25_for_each(s, node, &ax25_list) {
 		if ((s->iamdigi && !digi) || (!s->iamdigi && digi))
 			continue;
 		if (s->sk && !ax25cmp(&s->source_addr, addr) &&
@@ -197,9 +206,10 @@
 {
 	struct sock *sk = NULL;
 	ax25_cb *s;
+	struct hlist_node *node;
 
 	spin_lock_bh(&ax25_list_lock);
-	for (s = ax25_list; s != NULL; s = s->next) {
+	ax25_for_each(s, node, &ax25_list) {
 		if (s->sk && !ax25cmp(&s->source_addr, my_addr) &&
 		    !ax25cmp(&s->dest_addr, dest_addr) &&
 		    s->sk->sk_type == type) {
@@ -223,9 +233,10 @@
 	ax25_digi *digi, struct net_device *dev)
 {
 	ax25_cb *s;
+	struct hlist_node *node;
 
 	spin_lock_bh(&ax25_list_lock);
-	for (s = ax25_list; s != NULL; s = s->next) {
+	ax25_for_each(s, node, &ax25_list) {
 		if (s->sk && s->sk->sk_type != SOCK_SEQPACKET)
 			continue;
 		if (s->ax25_dev == NULL)
@@ -240,6 +251,7 @@
 				if (s->digipeat != NULL && s->digipeat->ndigi != 0)
 					continue;
 			}
+			ax25_cb_hold(s);
 			spin_unlock_bh(&ax25_list_lock);
 
 			return s;
@@ -257,16 +269,17 @@
 {
 	struct sock *sk = NULL;
 	ax25_cb *s;
+	struct hlist_node *node;
 
 	spin_lock_bh(&ax25_list_lock);
-	for (s = ax25_list; s != NULL; s = s->next) {
+	ax25_for_each(s, node, &ax25_list)
 		if (s->sk != NULL && ax25cmp(&s->source_addr, addr) == 0 &&
 		    s->sk->sk_type == SOCK_RAW) {
 			sk = s->sk;
 			lock_sock(sk);
 			break;
 		}
-	}
+
 	spin_unlock_bh(&ax25_list_lock);
 
 	return sk;
@@ -312,7 +325,7 @@
 {
 	struct sk_buff *skb;
 
-	ax25_remove_socket(ax25);
+	ax25_cb_del(ax25);
 
 	ax25_stop_heartbeat(ax25);
 	ax25_stop_t1timer(ax25);
@@ -352,7 +365,7 @@
 			sk_free(ax25->sk);
 		}
 	} else {
-		ax25_free_cb(ax25);
+		ax25_cb_put(ax25);
 	}
 }
 
@@ -507,6 +520,7 @@
 		return NULL;
 
 	memset(ax25, 0x00, sizeof(*ax25));
+	ax25_cb_hold(ax25);
 
 	skb_queue_head_init(&ax25->write_queue);
 	skb_queue_head_init(&ax25->frag_queue);
@@ -877,7 +891,7 @@
 		break;
 	default:
 		sk_free(sk);
-		ax25_free_cb(ax25);
+		ax25_cb_put(ax25);
 		return NULL;
 	}
 
@@ -1076,7 +1090,7 @@
 		ax25_fillin_cb(ax25, ax25_dev);
 
 done:
-	ax25_insert_socket(ax25);
+	ax25_cb_add(ax25);
 	sk->sk_zapped = 0;
 
 out:
@@ -1092,7 +1106,7 @@
 	int addr_len, int flags)
 {
 	struct sock *sk = sock->sk;
-	ax25_cb *ax25 = ax25_sk(sk);
+	ax25_cb *ax25 = ax25_sk(sk), *ax25t;
 	struct full_sockaddr_ax25 *fsa = (struct full_sockaddr_ax25 *)uaddr;
 	ax25_digi *digi = NULL;
 	int ct = 0, err = 0;
@@ -1198,7 +1212,7 @@
 			goto out;
 
 		ax25_fillin_cb(ax25, ax25->ax25_dev);
-		ax25_insert_socket(ax25);
+		ax25_cb_add(ax25);
 	} else {
 		if (ax25->ax25_dev == NULL) {
 			err = -EHOSTUNREACH;
@@ -1207,11 +1221,12 @@
 	}
 
 	if (sk->sk_type == SOCK_SEQPACKET &&
-	    ax25_find_cb(&ax25->source_addr, &fsa->fsa_ax25.sax25_call, digi,
-		    	 ax25->ax25_dev->dev)) {
+	    (ax25t=ax25_find_cb(&ax25->source_addr, &fsa->fsa_ax25.sax25_call, digi,
+		    	 ax25->ax25_dev->dev))) {
 		if (digi != NULL)
 			kfree(digi);
 		err = -EADDRINUSE;		/* Already such a connection */
+		ax25_cb_put(ax25t);
 		goto out;
 	}
 
@@ -1272,6 +1287,8 @@
 				lock_sock(sk);
 				continue;
 			}
+			current->state = TASK_RUNNING;
+			remove_wait_queue(sk->sk_sleep, &wait);
 			return -ERESTARTSYS;
 		}
 		current->state = TASK_RUNNING;
@@ -1339,6 +1356,8 @@
 			lock_sock(sk);
 			continue;
 		}
+		current->state = TASK_RUNNING;
+		remove_wait_queue(sk->sk_sleep, &wait);
 		return -ERESTARTSYS;
 	}
 	current->state = TASK_RUNNING;
@@ -1518,7 +1537,7 @@
 	SOCK_DEBUG(sk, "AX.25: sendto: building packet.\n");
 
 	/* Assume the worst case */
-	size = len + 3 + ax25_addr_size(dp) + AX25_BPQ_HEADER_LEN;
+	size = len + ax25->ax25_dev->dev->hard_header_len;
 
 	skb = sock_alloc_send_skb(sk, size, msg->msg_flags&MSG_DONTWAIT, &err);
 	if (skb == NULL)
@@ -1844,6 +1863,7 @@
 	int len = 0;
 	off_t pos = 0;
 	off_t begin = 0;
+	struct hlist_node *node;
 
 	spin_lock_bh(&ax25_list_lock);
 
@@ -1852,7 +1872,7 @@
 	 * magic dev src_addr dest_addr,digi1,digi2,.. st vs vr va t1 t1 t2 t2 t3 t3 idle idle n2 n2 rtt window paclen Snd-Q Rcv-Q inode
 	 */
 
-	for (ax25 = ax25_list; ax25 != NULL; ax25 = ax25->next) {
+	ax25_for_each(ax25, node, &ax25_list) {
 		len += sprintf(buffer+len, "%8.8lx %s %s%s ",
 				(long) ax25,
 				ax25->ax25_dev == NULL? "???" : ax25->ax25_dev->dev->name,
@@ -1955,6 +1975,8 @@
 EXPORT_SYMBOL(ax25_rebuild_header);
 EXPORT_SYMBOL(ax25_findbyuid);
 EXPORT_SYMBOL(ax25_find_cb);
+EXPORT_SYMBOL(ax25_cb_hold);
+EXPORT_SYMBOL(ax25_cb_put);
 EXPORT_SYMBOL(ax25_linkfail_register);
 EXPORT_SYMBOL(ax25_linkfail_release);
 EXPORT_SYMBOL(ax25_listen_register);
diff -ru linux-2.5.74/net/ax25/ax25_ds_subr.c linux-2.5.74.rxq/net/ax25/ax25_ds_subr.c
--- linux-2.5.74/net/ax25/ax25_ds_subr.c	2003-07-06 01:05:13.000000000 +0200
+++ linux-2.5.74.rxq/net/ax25/ax25_ds_subr.c	2003-07-09 18:18:00.000000000 +0200
@@ -40,6 +40,7 @@
 void ax25_ds_enquiry_response(ax25_cb *ax25)
 {
 	ax25_cb *ax25o;
+	struct hlist_node *node;
 
 	/* Please note that neither DK4EG´s nor DG2FEF´s
 	 * DAMA spec mention the following behaviour as seen
@@ -80,7 +81,7 @@
 	ax25_ds_set_timer(ax25->ax25_dev);
 
 	spin_lock_bh(&ax25_list_lock);
-	for (ax25o = ax25_list; ax25o != NULL; ax25o = ax25o->next) {
+	ax25_for_each(ax25o, node, &ax25_list) {
 		if (ax25o == ax25)
 			continue;
 
@@ -160,9 +161,10 @@
 {
 	ax25_cb *ax25;
 	int res = 0;
+	struct hlist_node *node;
 
 	spin_lock_bh(&ax25_list_lock);
-	for (ax25 = ax25_list; ax25 != NULL ; ax25 = ax25->next)
+	ax25_for_each(ax25, node, &ax25_list)
 		if (ax25->ax25_dev == ax25_dev && (ax25->condition & AX25_COND_DAMA_MODE) && ax25->state > AX25_STATE_1) {
 			res = 1;
 			break;
diff -ru linux-2.5.74/net/ax25/ax25_ds_timer.c linux-2.5.74.rxq/net/ax25/ax25_ds_timer.c
--- linux-2.5.74/net/ax25/ax25_ds_timer.c	2003-07-06 01:05:13.000000000 +0200
+++ linux-2.5.74.rxq/net/ax25/ax25_ds_timer.c	2003-07-09 18:18:21.000000000 +0200
@@ -74,6 +74,7 @@
 {
 	ax25_dev *ax25_dev = (struct ax25_dev *) arg;
 	ax25_cb *ax25;
+	struct hlist_node *node;
 
 	if (ax25_dev == NULL || !ax25_dev->dama.slave)
 		return;			/* Yikes! */
@@ -84,7 +85,7 @@
 	}
 
 	spin_lock_bh(&ax25_list_lock);
-	for (ax25=ax25_list; ax25 != NULL; ax25 = ax25->next) {
+	ax25_for_each(ax25, node, &ax25_list) {
 		if (ax25->ax25_dev != ax25_dev || !(ax25->condition & AX25_COND_DAMA_MODE))
 			continue;
 
diff -ru linux-2.5.74/net/ax25/ax25_in.c linux-2.5.74.rxq/net/ax25/ax25_in.c
--- linux-2.5.74/net/ax25/ax25_in.c	2003-07-06 01:05:12.000000000 +0200
+++ linux-2.5.74.rxq/net/ax25/ax25_in.c	2003-07-09 18:15:47.000000000 +0200
@@ -329,6 +329,7 @@
 		if (ax25_process_rx_frame(ax25, skb, type, dama) == 0)
 			kfree_skb(skb);
 
+		ax25_cb_put(ax25);
 		return 0;
 	}
 
@@ -429,7 +430,7 @@
 
 	ax25->state = AX25_STATE_3;
 
-	ax25_insert_socket(ax25);
+	ax25_cb_add(ax25);
 
 	ax25_start_heartbeat(ax25);
 	ax25_start_t3timer(ax25);
diff -ru linux-2.5.74/net/ax25/ax25_ip.c linux-2.5.74.rxq/net/ax25/ax25_ip.c
--- linux-2.5.74/net/ax25/ax25_ip.c	2003-07-06 01:05:13.000000000 +0200
+++ linux-2.5.74.rxq/net/ax25/ax25_ip.c	2003-07-09 18:16:33.000000000 +0200
@@ -107,6 +107,7 @@
 	ax25_address *src, *dst;
 	ax25_dev *ax25_dev;
 	ax25_route _route, *route = &_route;
+	ax25_cb *ax25;
 
 	dst = (ax25_address *)(bp + 1);
 	src = (ax25_address *)(bp + 8);
@@ -167,9 +168,14 @@
 			skb_pull(ourskb, AX25_HEADER_LEN - 1);	/* Keep PID */
 			ourskb->nh.raw = ourskb->data;
 
-			ax25_send_frame(ourskb, ax25_dev->values[AX25_VALUES_PACLEN], &src_c,
-&dst_c, route->digipeat, dev);
-
+			ax25=ax25_send_frame(
+			    ourskb, 
+			    ax25_dev->values[AX25_VALUES_PACLEN], 
+			    &src_c,
+			    &dst_c, route->digipeat, dev);
+			if (ax25) {
+				ax25_cb_put(ax25);
+			}
 			goto put;
 		}
 	}
diff -ru linux-2.5.74/net/ax25/ax25_out.c linux-2.5.74.rxq/net/ax25/ax25_out.c
--- linux-2.5.74/net/ax25/ax25_out.c	2003-07-06 01:05:13.000000000 +0200
+++ linux-2.5.74.rxq/net/ax25/ax25_out.c	2003-07-09 18:17:26.000000000 +0200
@@ -71,7 +71,7 @@
 
 	if (digi != NULL) {
 		if ((ax25->digipeat = kmalloc(sizeof(ax25_digi), GFP_ATOMIC)) == NULL) {
-			ax25_free_cb(ax25);
+			ax25_cb_put(ax25);
 			return NULL;
 		}
 		memcpy(ax25->digipeat, digi, sizeof(ax25_digi));
@@ -93,7 +93,7 @@
 #endif
 	}
 
-	ax25_insert_socket(ax25);
+	ax25_cb_add(ax25);
 
 	ax25->state = AX25_STATE_1;
 
diff -ru linux-2.5.74/net/ax25/ax25_subr.c linux-2.5.74.rxq/net/ax25/ax25_subr.c
--- linux-2.5.74/net/ax25/ax25_subr.c	2003-07-06 01:05:13.000000000 +0200
+++ linux-2.5.74.rxq/net/ax25/ax25_subr.c	2003-07-07 22:30:04.000000000 +0200
@@ -158,10 +158,10 @@
 	struct sk_buff *skb;
 	unsigned char  *dptr;
 
-	if ((skb = alloc_skb(AX25_BPQ_HEADER_LEN + ax25_addr_size(ax25->digipeat) + 2, GFP_ATOMIC)) == NULL)
+	if ((skb = alloc_skb(ax25->ax25_dev->dev->hard_header_len + 2, GFP_ATOMIC)) == NULL)
 		return;
 
-	skb_reserve(skb, AX25_BPQ_HEADER_LEN + ax25_addr_size(ax25->digipeat));
+	skb_reserve(skb, ax25->ax25_dev->dev->hard_header_len);
 
 	skb->nh.raw = skb->data;
 
@@ -202,10 +202,10 @@
 	if (dev == NULL)
 		return;
 
-	if ((skb = alloc_skb(AX25_BPQ_HEADER_LEN + ax25_addr_size(digi) + 1, GFP_ATOMIC)) == NULL)
+	if ((skb = alloc_skb(dev->hard_header_len + 1, GFP_ATOMIC)) == NULL)
 		return;	/* Next SABM will get DM'd */
 
-	skb_reserve(skb, AX25_BPQ_HEADER_LEN + ax25_addr_size(digi));
+	skb_reserve(skb, dev->hard_header_len);
 	skb->nh.raw = skb->data;
 
 	ax25_digi_invert(digi, &retdigi);
diff -ru linux-2.5.74/include/net/ax25.h linux-2.5.74.rxq/include/net/ax25.h
--- linux-2.5.74/include/net/ax25.h	2003-07-06 01:12:50.000000000 +0200
+++ linux-2.5.74.rxq/include/net/ax25.h	2003-07-09 18:09:38.000000000 +0200
@@ -10,6 +10,7 @@
 #include <linux/ax25.h>
 #include <linux/spinlock.h>
 #include <linux/timer.h>
+#include <linux/list.h>
 #include <asm/atomic.h>
 
 #define	AX25_T1CLAMPLO  		1
@@ -180,7 +181,7 @@
 } ax25_dev;
 
 typedef struct ax25_cb {
-	struct ax25_cb		*next;
+	struct hlist_node	ax25_node;
 	ax25_address		source_addr, dest_addr;
 	ax25_digi		*digipeat;
 	ax25_dev		*ax25_dev;
@@ -199,15 +200,20 @@
 	unsigned char		window;
 	struct timer_list	timer;
 	struct sock		*sk;		/* Backlink to socket */
+	int			refcount;
 } ax25_cb;
 
 #define ax25_sk(__sk) ((ax25_cb *)(__sk)->sk_protinfo)
 
+#define ax25_for_each(__ax25, node, list) \
+	hlist_for_each_entry(__ax25, node, list, ax25_node)
+
 /* af_ax25.c */
-extern ax25_cb *ax25_list;
+extern struct hlist_head ax25_list;
 extern spinlock_t ax25_list_lock;
-extern void ax25_free_cb(ax25_cb *);
-extern void ax25_insert_socket(ax25_cb *);
+extern void ax25_cb_hold(ax25_cb *);
+extern void ax25_cb_put(ax25_cb *);
+extern void ax25_cb_add(ax25_cb *);
 struct sock *ax25_find_listener(ax25_address *, int, struct net_device *, int);
 struct sock *ax25_get_socket(ax25_address *, ax25_address *, int);
 extern ax25_cb *ax25_find_cb(ax25_address *, ax25_address *, ax25_digi *, struct net_device *);

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2003-07-09 17:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-07-06 17:27 [PATCH] ax25_cb refcounting & waitqueue usage Jeroen Vreeken
2003-07-07 13:37 ` Arnaldo Carvalho de Melo
2003-07-07 17:23   ` Jeroen Vreeken
2003-07-07 17:25     ` Arnaldo Carvalho de Melo
2003-07-08 22:52       ` Jeroen Vreeken
2003-07-09 17:06         ` Jeroen Vreeken

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox