Re: [opensuse-isdn-de] Fragen zu AVM FCDSL2 Freeze (speziell an Karsten Keil)

11 Aug 2008

Ich habe mich jetzt nochmal an den Source gewagt - und wie es scheint 
mit Erfolg. Habe jetzt seit ein paar Tagen keine "not on queue" Messages 
mehr und ein Hang ist bislang auch noch nicht aufgetreten. Allerdings 
bin ich mir nicht sicher ob die Loesung so optimal ist. Ich haenge den 
Patch mal an, vielleicht hat ja jemand Lust/Zeit/Knowhow um 
drueberzuschauen bzw. ihn zu testen.

Gerade an Karsten haette ich da noch eine Frage zum Codeteil:

  #if defined (DRIVER_TYPE_DSL)
-       if (ctrl->cnr != card->ctrl2) {
+       if (ctrl->cnr == card->ctrl2) {
                 return;
         }

was auch hier (http://osdir.com/ml/isdn.i4l.user/2005-11/msg00025.html) 
besprochen wurde. Ich habe das bei beiden Auftreten im Code entsprechend 
gaendert. Allerdings ist mir nicht klar was es bewirkt. Waere da eine 
kurze Erklaerung moeglich was die Aenderung bewirkt und ob das in beiden 
Faellen sinnvoll ist?

Danke


Karsten Keil schrieb:
...
On Wed, Jul 30, 2008 at 09:57:39AM +0200, Fabian Urhausen wrote:
...
Danke fuer deine Bestaetigung. Habe das Problem mittlerweile jetzt
schon von mehreren Personen bestaetigt bekommen. Es wundert mich nur
das es (scheinbar) bislang keinen gestoehrt hat und man auch sonst zu
dem Thema nichts findet.
Das Problem ist, das der core des Treibers nur binary only vorliegt,
damit kann man leider solche Sachen schlecht untersuchen und fixen.
...
Bitte lasst die Betroffenen nicht dumm sterben. Keiner eine Idee oder
einen Hinweis?
Eventuell bringt eine Portierung der erwähnten Patches etwas, aber sicher
ist das nicht, die Sachen die  das zusaetzliche locking brauchen, sind im
open source Teil.
diff -NurpP --minimal fritz/src/common.h fritz.patched/src/common.h

--- fritz/src/common.h	2006-01-05 00:00:00.000000000 +0100
+++ fritz.patched/src/common.h	2008-08-07 11:12:32.332267907 +0200
@@ -44,7 +44,6 @@ typedef unsigned (__attr * xfer_func_t) 
 /*---------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
 typedef	long			intptr_t;
-typedef	unsigned long		uintptr_t;
 
 /*---------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
diff -NurpP --minimal fritz/src/devif.c fritz.patched/src/devif.c
--- fritz/src/devif.c	2006-01-05 00:00:00.000000000 +0100
+++ fritz.patched/src/devif.c	2008-08-07 11:12:32.336268487 +0200
@@ -233,7 +233,7 @@ int dma_setup (dma_struct_p pdma, unsign
 	assert (buf_size > 0);
 	assert (pdma != NULL);
 	assert (pdma->virt_addr == 0UL);
-	if (NULL == (buf1 = kmalloc (buf_size + DMA_ALIGNMENT, GFP_ATOMIC))) {
+	if (NULL == (buf1 = kmalloc (buf_size + DMA_ALIGNMENT, GFP_DMA | GFP_ATOMIC))) {
 		pdma->virt_addr = 0UL;
 		return FALSE;
 	}
@@ -748,12 +748,11 @@ static void xfer_task (unsigned long dat
 
 /*---------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
-irqreturn_t device_interrupt (int irq, void * args, struct pt_regs * regs) {
+irqreturn_t device_interrupt (int irq, void * args) {
 	unsigned long	intpins;
 	card_p		cp	= (card_p) args;
 
 	UNUSED_ARG (irq);
-	UNUSED_ARG (regs);
 	assert (capi_card == cp);
 
 	intpins = PINL (cp->mmio_base + C6205_PCI_HSR_OFFSET);
diff -NurpP --minimal fritz/src/devif.h fritz.patched/src/devif.h
--- fritz/src/devif.h	2006-01-05 00:00:00.000000000 +0100
+++ fritz.patched/src/devif.h	2008-08-07 11:12:32.336268487 +0200
@@ -80,7 +80,7 @@ extern __attr void dif_xfer_requirements
 extern void set_interrupt_callback (irq_callback_t, void *);
 extern void clear_interrupt_callback (void);
 
-extern irqreturn_t device_interrupt (int, void *, struct pt_regs *);
+extern irqreturn_t device_interrupt (int, void *);
 
 /*---------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
diff -NurpP --minimal fritz/src/driver.c fritz.patched/src/driver.c
--- fritz/src/driver.c	2006-01-05 00:00:00.000000000 +0100
+++ fritz.patched/src/driver.c	2008-08-07 11:12:32.336268487 +0200
@@ -18,6 +18,10 @@
  * http://www.opensource.org/licenses/lgpl-license.html
  * 
  * Contact: AVM GmbH, Alt-Moabit 95, 10559 Berlin, Germany, email: info@avm.de
+ *
+ * Thu Aug 7 11:08:10 2008
+ * Modified by Fabian Urhausen to fix freezes and improve locking
+ * For more questions contact me at fabian@hostbone.eu
  */
 
 #include <asm/io.h>
@@ -56,6 +60,8 @@
 #include "devif.h"
 #include "driver.h"
 
+#undef SINGLE_LOCK
+
 #define	KILOBYTE		1024
 #define	MEGABYTE		(1024*KILOBYTE)
 #define	_2_MEGS_		(2*MEGABYTE)
@@ -117,7 +123,7 @@ static DECLARE_WAIT_QUEUE_HEAD(wait);
 static DECLARE_WAIT_QUEUE_HEAD(capi_wait);
 static DECLARE_WAIT_QUEUE_HEAD(dbg_wait);
 
-static DECLARE_MUTEX_LOCKED(thread_sync);
+static DECLARE_COMPLETION(thread_sync);
 
 #define SCHED_WAKEUP_CAPI       { atomic_set (&thread_capi_flag, 1); wake_up_interruptible (&capi_wait); }
 #define SCHED_WAKEUP            { atomic_set (&got_kicked, 1); wake_up_interruptible (&wait); }
@@ -488,7 +494,7 @@ int start (card_p cp) {
 	res = request_irq (
 		cp->irq, 
 		&device_interrupt, 
-		SA_INTERRUPT | SA_SHIRQ, 
+		IRQF_DISABLED | IRQF_SHARED, 
 		TARGET, 
 		cp
 	);
@@ -1023,8 +1029,9 @@ static void __kcapi reset_ctrl (struct c
 		if (cp->appls != NULL) {
 			appp = first_appl (cp->appls);
 			while (appp != NULL) {
+				appl_t *next_appp = next_appl (cp->appls, appp);
 				free_ncci (appp->id, (unsigned) -1);
-				appp = next_appl (cp->appls, appp);
+				appp = next_appp;
 			}
 		}
 	}
@@ -1270,7 +1277,6 @@ int msg2stack (unsigned char * msg) {
 			queue_park (capi_card->queue, skb, appl, ncci, hand);
 		} else {
 			lib_memcpy (msg, mptr, mlen);
-			assert (skb->list == NULL);
 			kfree_skb (skb);
 		}
 	}
@@ -1361,6 +1367,7 @@ void msg2capi (unsigned char * msg) {
 /*-S-------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
 static int sched_thread (void * arg) {
+	unsigned long	flags;
 
 	UNUSED_ARG (arg);
 	daemonize (TARGET);
@@ -1396,6 +1403,7 @@ static int sched_thread (void * arg) {
 		}
 
 		/* Body of thread, invoke scheduler */
+		local_irq_save(flags);
 		if (!test_and_set_bit (0, &stack_lock)) {
 			os_timer_poll ();
 			assert (capi_lib->cm_schedule);
@@ -1404,9 +1412,10 @@ static int sched_thread (void * arg) {
 			}
 			clear_bit (0, &stack_lock);
 		}
+		local_irq_restore(flags);
 	}
 	LOG("Scheduler thread stopped.\n");
-	up (&thread_sync);
+	complete(&thread_sync);
 	return 0;
 } /* sched_thread */
 
@@ -1440,7 +1449,7 @@ static void kill_thread (void) {
 			SCHED_WAKEUP;
 		}
 		LOG("Thread signalled, waiting for termination...\n");
-		down (&thread_sync);
+		wait_for_completion(&thread_sync);
 		LOG("Thread[%d] terminated.\n", thread_pid);
 	}
 	thread_pid = -1;
@@ -1564,19 +1573,19 @@ void __stack init (unsigned len, void (_
 
 /*---------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
-int driver_init (void) {
+int avm_driver_init (void) {
 
 	return (NULL != (capi_lib = link_library (&capi_card)));
-} /* driver_init */
+} /* avm_driver_init */
 
 /*---------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
-void driver_exit (void) {
+void avm_driver_exit (void) {
 
 	assert (capi_lib);
 	free_library ();
 	capi_lib = NULL;
-} /* driver_exit */
+} /* avm_driver_exit */
 
 /*---------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
diff -NurpP --minimal fritz/src/driver.h fritz.patched/src/driver.h
--- fritz/src/driver.h	2006-01-05 00:00:00.000000000 +0100
+++ fritz.patched/src/driver.h	2008-08-07 11:12:32.336268487 +0200
@@ -24,7 +24,7 @@
 #define __have_driver_h__
 
 #include <asm/atomic.h>
-#include <linux/config.h>
+#include <linux/autoconf.h>
 #include <linux/skbuff.h>
 #include <linux/pci.h>
 #include <linux/spinlock.h>
@@ -161,8 +161,8 @@ extern void init (unsigned, void (__attr
 
 /*---------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
-extern int driver_init (void);
-extern void driver_exit (void);
+extern int avm_driver_init (void);
+extern void avm_driver_exit (void);
 
 #endif
 
diff -NurpP --minimal fritz/src/main.c fritz.patched/src/main.c
--- fritz/src/main.c	2006-01-05 00:00:00.000000000 +0100
+++ fritz.patched/src/main.c	2008-08-07 11:12:32.336268487 +0200
@@ -23,7 +23,7 @@
 #include <stdarg.h>
 #include <asm/uaccess.h>
 #include <linux/pci.h>
-#include <linux/config.h>
+#include <linux/autoconf.h>
 #include <linux/version.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -116,14 +116,14 @@ static int __devinit fcdsl2_probe (
 		return -ENODEV;
 	}
 	NOTE("Loading...\n");
-	if (!driver_init ()) {
+	if (!avm_driver_init ()) {
 		ERROR("Error: Driver library not available.\n");
 		NOTE("Not loaded.\n");
 		return -ENOSYS;
 	}
 	if (0 != (res = add_card (dev))) {
 		NOTE("Not loaded.\n");
-		driver_exit ();
+		avm_driver_exit ();
 		return res;
 	}
 	NOTE("Loaded.\n");
@@ -144,7 +144,7 @@ static void __devexit fcdsl2_remove (str
 	NOTE("Removing...\n");
 	remove_ctrls (cp);
 	NOTE("Removed.\n");
-	driver_exit ();
+	avm_driver_exit ();
 #ifndef NDEBUG
 	if (hallocated() != 0) {
 		ERROR("%u bytes leaked.\n", hallocated());
@@ -195,7 +195,7 @@ static int __init fcdsl2_init (void) {
 	NOTE("-- 32 bit CAPI driver --\n");
 #endif
 
-	if (0 == (err = pci_module_init (&fcdsl2_driver))) {
+	if (0 == (err = pci_register_driver (&fcdsl2_driver))) {
 		LOG("PCI driver registered.\n");
 		register_capi_driver (&fcdsl2_capi_driver);
 		LOG("CAPI driver registered.\n");
diff -NurpP --minimal fritz/src/Makefile fritz.patched/src/Makefile
--- fritz/src/Makefile	2006-01-05 00:00:00.000000000 +0100
+++ fritz.patched/src/Makefile	2008-08-07 11:12:32.336268487 +0200
@@ -12,7 +12,7 @@ EXTRA_CFLAGS	+= -D__$(CARD)__ -DTARGET=\
 ifndef DEBUG
 EXTRA_CFLAGS	+= -DNDEBUG
 endif
-EXTRA_LDFLAGS	+= $(LIBDIR)/$(CARD)-lib.o
+EXTRA_LDFLAGS	+= $(LIBDIR)/$(CARD)-lib.o -d
 
 obj-m		:= $(CARD).o
 $(CARD)-objs	:= $(OBJECTS)
diff -NurpP --minimal fritz/src/queue.c fritz.patched/src/queue.c
--- fritz/src/queue.c	2006-01-05 00:00:00.000000000 +0100
+++ fritz.patched/src/queue.c	2008-08-07 11:12:32.336268487 +0200
@@ -30,7 +30,7 @@
 
 /*---------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
-#define	MAKE_KEY(a,n,h)	(((tag_t)(a)<<48)+((tag_t)(n)<<16)+((tag_t)(h)))
+#define	MAKE_KEY(a,n,h) (((avm_tag_t)(a)<<48)+((avm_tag_t)(n)<<16)+((avm_tag_t)(h)))
 #define	IS_EMPTY(q)	((q)->get==NULL)
 
 /*---------------------------------------------------------------------------*\
@@ -79,14 +79,12 @@ void queue_exit (queue_t ** q) { 
 	assert (*q != NULL);
 	while ((*q)->get != NULL) {
 		item = (*q)->get->succ;
-		assert ((*q)->get->msg->list == NULL);
 		kfree_skb ((*q)->get->msg);
 		hfree ((*q)->get);
 		(*q)->get = item;
 	}
 	while ((*q)->noconf != NULL) {
 		item = (*q)->noconf->succ;
-		assert ((*q)->noconf->msg->list == NULL);
 		kfree_skb ((*q)->noconf->msg);
 		hfree ((*q)->noconf);
 		(*q)->noconf = item;
@@ -108,7 +106,6 @@ static int enqueue (queue_t * q, struct 
 
 	assert (q != NULL);
 	assert (msg != NULL);
-	assert (msg->list == NULL);
 	if (NULL == (item = alloc_item ())) {
 		ERROR("Not enough memory for queue item.\n");
 		ERROR("Message lost.\n");
@@ -141,7 +138,6 @@ static struct sk_buff * dequeue (queue_t
 	tmp = q->get;
 	q->get = item;
 	free_item (tmp);
-	assert (res->list == NULL);
 	return res;
 } /* dequeue */
 
@@ -215,7 +211,7 @@ void queue_park (
 \*---------------------------------------------------------------------------*/
 void queue_conf (queue_t * q, unsigned appl, NCCI_t ncci, unsigned hand) {
 	qitem_t *		item;
-	tag_t			key = MAKE_KEY (appl, ncci, hand);
+	avm_tag_t		key = MAKE_KEY (appl, ncci, hand);
 	struct sk_buff *	tmp = NULL;
 
 	assert (q != NULL);
@@ -241,7 +237,6 @@ void queue_conf (queue_t * q, unsigned a
 	}
 	unlock (q->lock);
 	if (tmp != NULL) {
-		assert (tmp->list == NULL);
 		kfree_skb (tmp);
 	}
 } /* queue_conf */
diff -NurpP --minimal fritz/src/queue.h fritz.patched/src/queue.h
--- fritz/src/queue.h	2006-01-05 00:00:00.000000000 +0100
+++ fritz.patched/src/queue.h	2008-08-07 11:12:32.336268487 +0200
@@ -29,14 +29,14 @@
 
 /*---------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
-typedef long long		tag_t;
+typedef long long		avm_tag_t;
 typedef __u32			NCCI_t;
 
 /*---------------------------------------------------------------------------*\
 \*---------------------------------------------------------------------------*/
 typedef struct __qitem {
 
-	tag_t			key;
+	avm_tag_t		key;
 	struct sk_buff *	msg;
 	struct __qitem *	succ;
 	struct __qitem *	pred;
diff -NurpP --minimal fritz/src/tables.c fritz.patched/src/tables.c
--- fritz/src/tables.c	2006-01-05 00:00:00.000000000 +0100
+++ fritz.patched/src/tables.c	2008-08-07 16:35:03.240267776 +0200
@@ -96,7 +96,9 @@ static void remove_appl (appltab_t * tab
 		remove_ncci (tab, appp, nccip);
 		nccip = tmp;
 	}
+	lock (capi_card->appls->lock);
 	capilib_release_appl (&tab->ncci_head, appp->id);
+	unlock (capi_card->appls->lock);
 	hfree (appp);
 } /* remove_appl */
 
@@ -234,6 +236,7 @@ static capiinfo_t handle_message (
 		ci = CAPI_ILLAPPNR;
 		goto done;
 	}
+	lock (capi_card->appls->lock);
 	switch (CAPIMSG_CMD(msg->data)) {
 
 	case CAPI_DATA_B3_REQ:
@@ -258,6 +261,7 @@ static capiinfo_t handle_message (
 		}
 	}
 done:
+	unlock (capi_card->appls->lock);
 	return ci;
 } /* handle_message */
 
@@ -267,6 +271,7 @@ capiinfo_t handle_data_conf (appltab_t *
 	unsigned	appl, hand;
 	NCCI_t		ncci;
 	
+	lock (capi_card->appls->lock);
 	assert (tab != NULL);
 	assert (q != NULL);
 	assert (m != NULL);
@@ -276,6 +281,7 @@ capiinfo_t handle_data_conf (appltab_t *
 	hand = CAPIMSG_U16(m, 12);
 	queue_conf (q, appl, ncci, hand);
 	capilib_data_b3_conf (&tab->ncci_head, appl, ncci, CAPIMSG_MSGID(m));
+	unlock (capi_card->appls->lock);
 	return CAPI_NOERROR;
 } /* handle_data_conf */
 
@@ -303,7 +309,9 @@ static ncci_t * create_ncci (	
 		return NULL;
 	}
 	LOG("New NCCI(%x), window size %u...\n", ncci, wsize);
+	lock (capi_card->appls->lock);
 	capilib_new_ncci (&tab->ncci_head, appp->id, ncci, wsize);
+	unlock (capi_card->appls->lock);
 	tmp->ncci     = ncci;
 	tmp->appl     = appp->id;
 	tmp->win_size = wsize;
@@ -330,7 +338,9 @@ void remove_ncci (appltab_t * tab, appl_
 	assert (nccip);
 	if (nccip != NULL) {
 		LOG("Remove NCCI(%x), appl %u...\n", nccip->ncci, appp->id);
+		lock (capi_card->appls->lock);
 		capilib_free_ncci (&tab->ncci_head, appp->id, nccip->ncci);
+		unlock (capi_card->appls->lock);
 		lock (tab->lock);
 		for (i = 0; i < appp->blk_count; i++) {
 			if (nccip->data[i] != NULL) {
@@ -554,7 +564,7 @@ void __kcapi release_appl (struct capi_c
 	assert (ctrl != NULL);
 	card = GET_CARD(ctrl);
 #if defined (DRIVER_TYPE_DSL)
-	if (ctrl->cnr != card->ctrl2) {
+	if (ctrl->cnr == card->ctrl2) {
 		return;
 	}
 #endif
@@ -580,7 +590,6 @@ u16 __kcapi send_msg (struct capi_ctr * 
 
 	assert (ctrl != NULL);
 	assert (skb != NULL);
-	assert (skb->list == NULL);
 	card = GET_CARD(ctrl);
 	ci = handle_message (card->appls, card->queue, skb);
 	kick_scheduler ();
@@ -605,7 +614,7 @@ void __kcapi register_appl (
 	assert (args != NULL);
 	card = GET_CARD(ctrl);
 #if defined (DRIVER_TYPE_DSL)
-	if (ctrl->cnr != card->ctrl2) {
+	if (ctrl->cnr == card->ctrl2) {
 		return;
 	}
 #endif
diff -NurpP --minimal fritz/src/tools.c fritz.patched/src/tools.c
--- fritz/src/tools.c	2006-01-05 00:00:00.000000000 +0100
+++ fritz.patched/src/tools.c	2008-08-07 11:12:32.336268487 +0200
@@ -110,7 +110,11 @@ static unsigned		lib_heap_size	= 0;
 #define	FENCE2_OK(h,m)	(*(unsigned *)(((char *) m)+(h)->size)==FENCE_TAG)
 
 static unsigned		alloc_count	= 0;
+#if defined (SINGLE_LOCK)
+# define track_lock qt_lock
+#else
 static spinlock_t	track_lock	= SPIN_LOCK_UNLOCKED;
+#endif
 
 #if !defined (NDEBUG) && defined (LOG_TIMER)
 static struct timeval	zero_time;

    