Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 22/41] CAPI: Use dynamic major for NCCI TTYs by default
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

No need to allocate a fixed major for this TTY, both capifs and udev
make this transparent to the user.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 901b79b..b1de0cb 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -60,10 +60,8 @@ module_param_named(major, capi_major, uint, 0);
 #define CAPINC_NR_PORTS		32
 #define CAPINC_MAX_PORTS	256
 
-static int capi_ttymajor = 191;
 static int capi_ttyminors = CAPINC_NR_PORTS;
 
-module_param_named(ttymajor, capi_ttymajor, uint, 0);
 module_param_named(ttyminors, capi_ttyminors, uint, 0);
 #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
 
@@ -301,6 +299,7 @@ static struct capiminor *capiminor_get(unsigned int minor)
 static void capincci_alloc_minor(struct capidev *cdev, struct capincci *np)
 {
 	struct capiminor *mp;
+	dev_t device;
 
 	if (!(cdev->userflags & CAPIFLAG_HIGHJACKING))
 		return;
@@ -311,9 +310,8 @@ static void capincci_alloc_minor(struct capidev *cdev, struct capincci *np)
 #ifdef _DEBUG_REFCOUNT
 		printk(KERN_DEBUG "set mp->nccip\n");
 #endif
-		mp->capifs_dentry =
-			capifs_new_ncci(mp->minor,
-					MKDEV(capi_ttymajor, mp->minor));
+		device = MKDEV(capinc_tty_driver->major, mp->minor);
+		mp->capifs_dentry = capifs_new_ncci(mp->minor, device);
 	}
 }
 
@@ -1341,7 +1339,7 @@ static int __init capinc_tty_init(void)
 	drv->owner = THIS_MODULE;
 	drv->driver_name = "capi_nc";
 	drv->name = "capi";
-	drv->major = capi_ttymajor;
+	drv->major = 0;
 	drv->minor_start = 0;
 	drv->type = TTY_DRIVER_TYPE_SERIAL;
 	drv->subtype = SERIAL_TYPE_NORMAL;
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH v2 23/41] CAPI: Use kref on capiminor
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Install a reference counter for capiminor objects. Acquire it when
obtaining a capiminor from the array during capinc_tty_open, drop it
when closing the tty again. Another reference is held for the hook-up
with capincci.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   37 ++++++++++++++++++++++++++++---------
 1 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index b1de0cb..732cdb5 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -83,6 +83,8 @@ struct datahandle_queue {
 };
 
 struct capiminor {
+	struct kref kref;
+
 	struct capincci  *nccip;
 	unsigned int      minor;
 	struct dentry *capifs_dentry;
@@ -223,6 +225,8 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 		return NULL;
 	}
 
+	kref_init(&mp->kref);
+
 	mp->ap = ap;
 	mp->ncci = ncci;
 	mp->msgid = 0;
@@ -265,18 +269,11 @@ err_out1:
 	return NULL;
 }
 
-static void capiminor_free(struct capiminor *mp)
+static void capiminor_destroy(struct kref *kref)
 {
-	unsigned long flags;
-
-	tty_unregister_device(capinc_tty_driver, mp->minor);
-
-	write_lock_irqsave(&capiminors_lock, flags);
-	capiminors[mp->minor] = NULL;
-	write_unlock_irqrestore(&capiminors_lock, flags);
+	struct capiminor *mp = container_of(kref, struct capiminor, kref);
 
 	kfree_skb(mp->ttyskb);
-	mp->ttyskb = NULL;
 	skb_queue_purge(&mp->inqueue);
 	skb_queue_purge(&mp->outqueue);
 	capiminor_del_all_ack(mp);
@@ -289,11 +286,31 @@ static struct capiminor *capiminor_get(unsigned int minor)
 
 	read_lock(&capiminors_lock);
 	mp = capiminors[minor];
+	if (mp)
+		kref_get(&mp->kref);
 	read_unlock(&capiminors_lock);
 
 	return mp;
 }
 
+static inline void capiminor_put(struct capiminor *mp)
+{
+	kref_put(&mp->kref, capiminor_destroy);
+}
+
+static void capiminor_free(struct capiminor *mp)
+{
+	unsigned long flags;
+
+	tty_unregister_device(capinc_tty_driver, mp->minor);
+
+	write_lock_irqsave(&capiminors_lock, flags);
+	capiminors[mp->minor] = NULL;
+	write_unlock_irqrestore(&capiminors_lock, flags);
+
+	capiminor_put(mp);
+}
+
 /* -------- struct capincci ----------------------------------------- */
 
 static void capincci_alloc_minor(struct capidev *cdev, struct capincci *np)
@@ -1029,6 +1046,8 @@ static void capinc_tty_close(struct tty_struct * tty, struct file * file)
 #endif
 		if (mp->nccip == NULL)
 			capiminor_free(mp);
+
+		capiminor_put(mp);
 	}
 
 #ifdef _DEBUG_REFCOUNT
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH v2 19/41] CAPI: Switch capiminor list to array
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Using a plain array of pointers simplifies the management of capiminors.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   75 +++++++++++++++++++++------------------------
 1 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 6704b2b..46f85ae 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -85,7 +85,6 @@ struct datahandle_queue {
 };
 
 struct capiminor {
-	struct list_head list;
 	struct capincci  *nccip;
 	unsigned int      minor;
 	struct dentry *capifs_dentry;
@@ -151,8 +150,8 @@ static LIST_HEAD(capidev_list);
 
 #ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
 
-static DEFINE_RWLOCK(capiminor_list_lock);
-static LIST_HEAD(capiminor_list);
+static DEFINE_RWLOCK(capiminors_lock);
+static struct capiminor **capiminors;
 
 /* -------- datahandles --------------------------------------------- */
 
@@ -213,8 +212,8 @@ static void capiminor_del_all_ack(struct capiminor *mp)
 
 static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 {
-	struct capiminor *mp, *p;
-	unsigned int minor = 0;
+	struct capiminor *mp;
+	unsigned int minor;
 	unsigned long flags;
 
 	mp = kzalloc(sizeof(*mp), GFP_KERNEL);
@@ -233,31 +232,23 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 	skb_queue_head_init(&mp->inqueue);
 	skb_queue_head_init(&mp->outqueue);
 
-	/* Allocate the least unused minor number.
-	 */
-	write_lock_irqsave(&capiminor_list_lock, flags);
-	if (list_empty(&capiminor_list))
-		list_add(&mp->list, &capiminor_list);
-	else {
-		list_for_each_entry(p, &capiminor_list, list) {
-			if (p->minor > minor)
-				break;
-			minor++;
-		}
-		
-		if (minor < capi_ttyminors) {
-			mp->minor = minor;
-			list_add(&mp->list, p->list.prev);
+	/* Allocate the least unused minor number. */
+	write_lock_irqsave(&capiminors_lock, flags);
+	for (minor = 0; minor < capi_ttyminors; minor++)
+		if (!capiminors[minor]) {
+			capiminors[minor] = mp;
+			break;
 		}
-	}
-		write_unlock_irqrestore(&capiminor_list_lock, flags);
+	write_unlock_irqrestore(&capiminors_lock, flags);
 
-	if (!(minor < capi_ttyminors)) {
+	if (minor == capi_ttyminors) {
 		printk(KERN_NOTICE "capi: out of minors\n");
-			kfree(mp);
+		kfree(mp);
 		return NULL;
 	}
 
+	mp->minor = minor;
+
 	return mp;
 }
 
@@ -265,9 +256,9 @@ static void capiminor_free(struct capiminor *mp)
 {
 	unsigned long flags;
 
-	write_lock_irqsave(&capiminor_list_lock, flags);
-	list_del(&mp->list);
-	write_unlock_irqrestore(&capiminor_list_lock, flags);
+	write_lock_irqsave(&capiminors_lock, flags);
+	capiminors[mp->minor] = NULL;
+	write_unlock_irqrestore(&capiminors_lock, flags);
 
 	kfree_skb(mp->ttyskb);
 	mp->ttyskb = NULL;
@@ -279,20 +270,16 @@ static void capiminor_free(struct capiminor *mp)
 
 static struct capiminor *capiminor_find(unsigned int minor)
 {
-	struct list_head *l;
-	struct capiminor *p = NULL;
+	struct capiminor *mp;
 
-	read_lock(&capiminor_list_lock);
-	list_for_each(l, &capiminor_list) {
-		p = list_entry(l, struct capiminor, list);
-		if (p->minor == minor)
-			break;
-	}
-	read_unlock(&capiminor_list_lock);
-	if (l == &capiminor_list)
+	if (minor >= capi_ttyminors)
 		return NULL;
 
-	return p;
+	read_lock(&capiminors_lock);
+	mp = capiminors[minor];
+	read_unlock(&capiminors_lock);
+
+	return mp;
 }
 
 /* -------- struct capincci ----------------------------------------- */
@@ -1329,10 +1316,16 @@ static int capinc_tty_init(void)
 	if (capi_ttyminors <= 0)
 		capi_ttyminors = CAPINC_NR_PORTS;
 
-	drv = alloc_tty_driver(capi_ttyminors);
-	if (!drv)
+	capiminors = kzalloc(sizeof(struct capi_minor *) * capi_ttyminors,
+			     GFP_KERNEL);
+	if (!capiminors)
 		return -ENOMEM;
 
+	drv = alloc_tty_driver(capi_ttyminors);
+	if (!drv) {
+		kfree(capiminors);
+		return -ENOMEM;
+	}
 	drv->owner = THIS_MODULE;
 	drv->driver_name = "capi_nc";
 	drv->name = "capi";
@@ -1349,6 +1342,7 @@ static int capinc_tty_init(void)
 	tty_set_operations(drv, &capinc_ops);
 	if (tty_register_driver(drv)) {
 		put_tty_driver(drv);
+		kfree(capiminors);
 		printk(KERN_ERR "Couldn't register capi_nc driver\n");
 		return -1;
 	}
@@ -1363,6 +1357,7 @@ static void capinc_tty_exit(void)
 	if ((retval = tty_unregister_driver(drv)))
 		printk(KERN_ERR "capi: failed to unregister capi_nc driver (%d)\n", retval);
 	put_tty_driver(drv);
+	kfree(capiminors);
 }
 
 #else /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH v2 18/41] CAPI: Switch NCCI list to standard doubly linked list
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Replace open-coded NCCI list management with standard mechanisms.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   52 +++++++++++++++++----------------------------
 1 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 8abec96..6704b2b 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -122,7 +122,7 @@ struct capiminor {
 static DEFINE_SPINLOCK(workaround_lock);
 
 struct capincci {
-	struct capincci *next;
+	struct list_head list;
 	u32		 ncci;
 	struct capidev	*cdev;
 #ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
@@ -139,7 +139,7 @@ struct capidev {
 	struct sk_buff_head recvqueue;
 	wait_queue_head_t recvwait;
 
-	struct capincci *nccis;
+	struct list_head nccis;
 
 	struct mutex lock;
 };
@@ -356,7 +356,7 @@ static inline unsigned int capincci_minor_opencount(struct capincci *np)
 
 static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
 {
-	struct capincci *np, **pp;
+	struct capincci *np;
 
 	np = kzalloc(sizeof(*np), GFP_KERNEL);
 	if (!np)
@@ -366,39 +366,31 @@ static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
 
 	capincci_alloc_minor(cdev, np);
 
-	for (pp=&cdev->nccis; *pp; pp = &(*pp)->next)
-		;
-	*pp = np;
-        return np;
+	list_add_tail(&np->list, &cdev->nccis);
+
+	return np;
 }
 
 static void capincci_free(struct capidev *cdev, u32 ncci)
 {
-	struct capincci *np, **pp;
+	struct capincci *np, *tmp;
 
-	pp=&cdev->nccis;
-	while (*pp) {
-		np = *pp;
+	list_for_each_entry_safe(np, tmp, &cdev->nccis, list)
 		if (ncci == 0xffffffff || np->ncci == ncci) {
-			*pp = (*pp)->next;
 			capincci_free_minor(np);
+			list_del(&np->list);
 			kfree(np);
-			if (*pp == NULL) return;
-		} else {
-			pp = &(*pp)->next;
 		}
-	}
 }
 
 static struct capincci *capincci_find(struct capidev *cdev, u32 ncci)
 {
-	struct capincci *p;
+	struct capincci *np;
 
-	for (p=cdev->nccis; p ; p = p->next) {
-		if (p->ncci == ncci)
-			break;
-	}
-	return p;
+	list_for_each_entry(np, &cdev->nccis, list)
+		if (np->ncci == ncci)
+			return np;
+	return NULL;
 }
 
 #ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
@@ -955,6 +947,7 @@ static int capi_open(struct inode *inode, struct file *file)
 	mutex_init(&cdev->lock);
 	skb_queue_head_init(&cdev->recvqueue);
 	init_waitqueue_head(&cdev->recvwait);
+	INIT_LIST_HEAD(&cdev->nccis);
 	file->private_data = cdev;
 
 	mutex_lock(&capidev_list_lock);
@@ -1427,19 +1420,14 @@ static const struct file_operations capi20_proc_fops = {
  */
 static int capi20ncci_proc_show(struct seq_file *m, void *v)
 {
-        struct capidev *cdev;
-        struct capincci *np;
-	struct list_head *l;
+	struct capidev *cdev;
+	struct capincci *np;
 
 	mutex_lock(&capidev_list_lock);
-	list_for_each(l, &capidev_list) {
-		cdev = list_entry(l, struct capidev, list);
+	list_for_each_entry(cdev, &capidev_list, list) {
 		mutex_lock(&cdev->lock);
-		for (np=cdev->nccis; np; np = np->next) {
-			seq_printf(m, "%d 0x%x\n",
-				       cdev->ap.applid,
-				       np->ncci);
-		}
+		list_for_each_entry(np, &cdev->nccis, list)
+			seq_printf(m, "%d 0x%x\n", cdev->ap.applid, np->ncci);
 		mutex_unlock(&cdev->lock);
 	}
 	mutex_unlock(&capidev_list_lock);
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH v2 14/41] CAPI: Clean up capi_open/release
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Fold capidev_alloc and capidev_free into capi_open and capi_release -
there are no other users. Someone pushed a lock_kernel into capi_open.
Drop it, we don't need it. Also remove the useless test from open that
checks for private_data == NULL.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   93 +++++++++++++++++-----------------------------
 1 files changed, 34 insertions(+), 59 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 623412e..9d7c369 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -401,46 +401,6 @@ static struct capincci *capincci_find(struct capidev *cdev, u32 ncci)
 	return p;
 }
 
-/* -------- struct capidev ------------------------------------------ */
-
-static struct capidev *capidev_alloc(void)
-{
-	struct capidev *cdev;
-
-	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
-	if (!cdev)
-		return NULL;
-
-	mutex_init(&cdev->ncci_list_mtx);
-	skb_queue_head_init(&cdev->recvqueue);
-	init_waitqueue_head(&cdev->recvwait);
-
-	mutex_lock(&capidev_list_lock);
-	list_add_tail(&cdev->list, &capidev_list);
-	mutex_unlock(&capidev_list_lock);
-
-        return cdev;
-}
-
-static void capidev_free(struct capidev *cdev)
-{
-	mutex_lock(&capidev_list_lock);
-	list_del(&cdev->list);
-	mutex_unlock(&capidev_list_lock);
-
-	if (cdev->ap.applid) {
-		capi20_release(&cdev->ap);
-		cdev->ap.applid = 0;
-	}
-	skb_queue_purge(&cdev->recvqueue);
-
-	mutex_lock(&cdev->ncci_list_mtx);
-	capincci_free(cdev, 0xffffffff);
-	mutex_unlock(&cdev->ncci_list_mtx);
-
-	kfree(cdev);
-}
-
 #ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
 /* -------- handle data queue --------------------------------------- */
 
@@ -991,30 +951,45 @@ capi_ioctl(struct inode *inode, struct file *file,
 	return -EINVAL;
 }
 
-static int
-capi_open(struct inode *inode, struct file *file)
+static int capi_open(struct inode *inode, struct file *file)
 {
-	int ret;
-	
-	lock_kernel();
-	if (file->private_data)
-		ret = -EEXIST;
-	else if ((file->private_data = capidev_alloc()) == NULL)
-		ret = -ENOMEM;
-	else
-		ret = nonseekable_open(inode, file);
-	unlock_kernel();
-	return ret;
+	struct capidev *cdev;
+
+	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
+	if (!cdev)
+		return -ENOMEM;
+
+	mutex_init(&cdev->ncci_list_mtx);
+	skb_queue_head_init(&cdev->recvqueue);
+	init_waitqueue_head(&cdev->recvwait);
+	file->private_data = cdev;
+
+	mutex_lock(&capidev_list_lock);
+	list_add_tail(&cdev->list, &capidev_list);
+	mutex_unlock(&capidev_list_lock);
+
+	return nonseekable_open(inode, file);
 }
 
-static int
-capi_release(struct inode *inode, struct file *file)
+static int capi_release(struct inode *inode, struct file *file)
 {
-	struct capidev *cdev = (struct capidev *)file->private_data;
+	struct capidev *cdev = file->private_data;
 
-	capidev_free(cdev);
-	file->private_data = NULL;
-	
+	mutex_lock(&capidev_list_lock);
+	list_del(&cdev->list);
+	mutex_unlock(&capidev_list_lock);
+
+	if (cdev->ap.applid) {
+		capi20_release(&cdev->ap);
+		cdev->ap.applid = 0;
+	}
+	skb_queue_purge(&cdev->recvqueue);
+
+	mutex_lock(&cdev->ncci_list_mtx);
+	capincci_free(cdev, 0xffffffff);
+	mutex_unlock(&cdev->ncci_list_mtx);
+
+	kfree(cdev);
 	return 0;
 }
 
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH v2 13/41] CAPI: Convert capidev_list_lock into a mutex
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

No need for anything "harder" here (specifically no need for
irqsave...). Also, make the list removal the first operation of
capidev_free to avoid dumping half-released devices via /proc.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 7d2ca6b..623412e 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -146,7 +146,7 @@ struct capidev {
 
 /* -------- global variables ---------------------------------------- */
 
-static DEFINE_RWLOCK(capidev_list_lock);
+static DEFINE_MUTEX(capidev_list_lock);
 static LIST_HEAD(capidev_list);
 
 #ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
@@ -406,7 +406,6 @@ static struct capincci *capincci_find(struct capidev *cdev, u32 ncci)
 static struct capidev *capidev_alloc(void)
 {
 	struct capidev *cdev;
-	unsigned long flags;
 
 	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
 	if (!cdev)
@@ -415,15 +414,19 @@ static struct capidev *capidev_alloc(void)
 	mutex_init(&cdev->ncci_list_mtx);
 	skb_queue_head_init(&cdev->recvqueue);
 	init_waitqueue_head(&cdev->recvwait);
-	write_lock_irqsave(&capidev_list_lock, flags);
+
+	mutex_lock(&capidev_list_lock);
 	list_add_tail(&cdev->list, &capidev_list);
-	write_unlock_irqrestore(&capidev_list_lock, flags);
+	mutex_unlock(&capidev_list_lock);
+
         return cdev;
 }
 
 static void capidev_free(struct capidev *cdev)
 {
-	unsigned long flags;
+	mutex_lock(&capidev_list_lock);
+	list_del(&cdev->list);
+	mutex_unlock(&capidev_list_lock);
 
 	if (cdev->ap.applid) {
 		capi20_release(&cdev->ap);
@@ -435,9 +438,6 @@ static void capidev_free(struct capidev *cdev)
 	capincci_free(cdev, 0xffffffff);
 	mutex_unlock(&cdev->ncci_list_mtx);
 
-	write_lock_irqsave(&capidev_list_lock, flags);
-	list_del(&cdev->list);
-	write_unlock_irqrestore(&capidev_list_lock, flags);
 	kfree(cdev);
 }
 
@@ -1431,7 +1431,7 @@ static int capi20_proc_show(struct seq_file *m, void *v)
         struct capidev *cdev;
 	struct list_head *l;
 
-	read_lock(&capidev_list_lock);
+	mutex_lock(&capidev_list_lock);
 	list_for_each(l, &capidev_list) {
 		cdev = list_entry(l, struct capidev, list);
 		seq_printf(m, "0 %d %lu %lu %lu %lu\n",
@@ -1441,7 +1441,7 @@ static int capi20_proc_show(struct seq_file *m, void *v)
 			cdev->ap.nsentctlpkt,
 			cdev->ap.nsentdatapkt);
 	}
-	read_unlock(&capidev_list_lock);
+	mutex_unlock(&capidev_list_lock);
 	return 0;
 }
 
@@ -1468,7 +1468,7 @@ static int capi20ncci_proc_show(struct seq_file *m, void *v)
         struct capincci *np;
 	struct list_head *l;
 
-	read_lock(&capidev_list_lock);
+	mutex_lock(&capidev_list_lock);
 	list_for_each(l, &capidev_list) {
 		cdev = list_entry(l, struct capidev, list);
 		for (np=cdev->nccis; np; np = np->next) {
@@ -1477,7 +1477,7 @@ static int capi20ncci_proc_show(struct seq_file *m, void *v)
 				       np->ncci);
 		}
 	}
-	read_unlock(&capidev_list_lock);
+	mutex_unlock(&capidev_list_lock);
 	return 0;
 }
 
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH v2 10/41] CAPI: Rework locking of controller data structures
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

This patch applies the mutex so far only protecting the controller list
to (almost) all accesses of controller data structures. It also reworks
waiting on state changes in old_capi_manufacturer so that it no longer
poll and holds a module reference to the controller owner while waiting
(the latter was partly done already). Modification and checking of the
blocked state remains racy by design, the caller is responsible for
dealing with this.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/kcapi.c      |  291 ++++++++++++++++++++++++++++------------
 drivers/isdn/capi/kcapi.h      |    5 +-
 drivers/isdn/capi/kcapi_proc.c |    5 +
 include/linux/isdn/capilli.h   |    5 +-
 4 files changed, 217 insertions(+), 89 deletions(-)

diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index e08914d..a99f7e3 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -61,11 +61,12 @@ static char capi_manufakturer[64] = "AVM Berlin";
 LIST_HEAD(capi_drivers);
 DEFINE_MUTEX(capi_drivers_lock);
 
+struct capi_ctr *capi_controller[CAPI_MAXCONTR];
+DEFINE_MUTEX(capi_controller_lock);
+
 static DEFINE_RWLOCK(application_lock);
-static DEFINE_MUTEX(controller_mutex);
 
 struct capi20_appl *capi_applications[CAPI_MAXAPPL];
-struct capi_ctr *capi_controller[CAPI_MAXCONTR];
 
 static int ncontrollers;
 
@@ -171,13 +172,15 @@ static void notify_up(u32 contr)
 	struct capi_ctr *ctr;
 	u16 applid;
 
+	mutex_lock(&capi_controller_lock);
+
 	if (showcapimsgs & 1)
 	        printk(KERN_DEBUG "kcapi: notify up contr %d\n", contr);
 
 	ctr = get_capi_ctr_by_nr(contr);
 	if (ctr) {
 		if (ctr->state == CAPI_CTR_RUNNING)
-			return;
+			goto unlock_out;
 
 		ctr->state = CAPI_CTR_RUNNING;
 
@@ -187,19 +190,24 @@ static void notify_up(u32 contr)
 				continue;
 			register_appl(ctr, applid, &ap->rparam);
 		}
+
+		wake_up_interruptible_all(&ctr->state_wait_queue);
 	} else
 		printk(KERN_WARNING "%s: invalid contr %d\n", __func__, contr);
+
+unlock_out:
+	mutex_unlock(&capi_controller_lock);
 }
 
-static void ctr_down(struct capi_ctr *ctr)
+static void ctr_down(struct capi_ctr *ctr, int new_state)
 {
 	struct capi20_appl *ap;
 	u16 applid;
 
-	if (ctr->state == CAPI_CTR_DETECTED)
+	if (ctr->state == CAPI_CTR_DETECTED || ctr->state == CAPI_CTR_DETACHED)
 		return;
 
-	ctr->state = CAPI_CTR_DETECTED;
+	ctr->state = new_state;
 
 	memset(ctr->manu, 0, sizeof(ctr->manu));
 	memset(&ctr->version, 0, sizeof(ctr->version));
@@ -211,20 +219,26 @@ static void ctr_down(struct capi_ctr *ctr)
 		if (ap && !ap->release_in_progress)
 			capi_ctr_put(ctr);
 	}
+
+	wake_up_interruptible_all(&ctr->state_wait_queue);
 }
 
 static void notify_down(u32 contr)
 {
 	struct capi_ctr *ctr;
 
+	mutex_lock(&capi_controller_lock);
+
 	if (showcapimsgs & 1)
 		printk(KERN_DEBUG "kcapi: notify down contr %d\n", contr);
 
 	ctr = get_capi_ctr_by_nr(contr);
 	if (ctr)
-		ctr_down(ctr);
+		ctr_down(ctr, CAPI_CTR_DETECTED);
 	else
 		printk(KERN_WARNING "%s: invalid contr %d\n", __func__, contr);
+
+	mutex_unlock(&capi_controller_lock);
 }
 
 static int
@@ -436,6 +450,9 @@ EXPORT_SYMBOL(capi_ctr_down);
  * @ctr:	controller descriptor structure.
  *
  * Called by hardware driver to stop data flow.
+ *
+ * Note: The caller is responsible for synchronizing concurrent state changes
+ * as well as invocations of capi_ctr_handle_message.
  */
 
 void capi_ctr_suspend_output(struct capi_ctr *ctr)
@@ -454,6 +471,9 @@ EXPORT_SYMBOL(capi_ctr_suspend_output);
  * @ctr:	controller descriptor structure.
  *
  * Called by hardware driver to resume data flow.
+ *
+ * Note: The caller is responsible for synchronizing concurrent state changes
+ * as well as invocations of capi_ctr_handle_message.
  */
 
 void capi_ctr_resume_output(struct capi_ctr *ctr)
@@ -481,21 +501,19 @@ int attach_capi_ctr(struct capi_ctr *ctr)
 {
 	int i;
 
-	mutex_lock(&controller_mutex);
+	mutex_lock(&capi_controller_lock);
 
 	for (i = 0; i < CAPI_MAXCONTR; i++) {
 		if (!capi_controller[i])
 			break;
 	}
 	if (i == CAPI_MAXCONTR) {
-		mutex_unlock(&controller_mutex);
+		mutex_unlock(&capi_controller_lock);
 		printk(KERN_ERR "kcapi: out of controller slots\n");
 	   	return -EBUSY;
 	}
 	capi_controller[i] = ctr;
 
-	mutex_unlock(&controller_mutex);
-
 	ctr->nrecvctlpkt = 0;
 	ctr->nrecvdatapkt = 0;
 	ctr->nsentctlpkt = 0;
@@ -504,11 +522,15 @@ int attach_capi_ctr(struct capi_ctr *ctr)
 	ctr->state = CAPI_CTR_DETECTED;
 	ctr->blocked = 0;
 	ctr->traceflag = showcapimsgs;
+	init_waitqueue_head(&ctr->state_wait_queue);
 
 	sprintf(ctr->procfn, "capi/controllers/%d", ctr->cnr);
 	ctr->procent = proc_create_data(ctr->procfn, 0, NULL, ctr->proc_fops, ctr);
 
 	ncontrollers++;
+
+	mutex_unlock(&capi_controller_lock);
+
 	printk(KERN_NOTICE "kcapi: controller [%03d]: %s attached\n",
 			ctr->cnr, ctr->name);
 	return 0;
@@ -527,19 +549,29 @@ EXPORT_SYMBOL(attach_capi_ctr);
 
 int detach_capi_ctr(struct capi_ctr *ctr)
 {
-	ctr_down(ctr);
+	int err = 0;
 
-	ncontrollers--;
+	mutex_lock(&capi_controller_lock);
 
-	if (ctr->procent) {
-		remove_proc_entry(ctr->procfn, NULL);
-		ctr->procent = NULL;
+	ctr_down(ctr, CAPI_CTR_DETACHED);
+
+	if (capi_controller[ctr->cnr - 1] != ctr) {
+		err = -EINVAL;
+		goto unlock_out;
 	}
 	capi_controller[ctr->cnr - 1] = NULL;
+	ncontrollers--;
+
+	if (ctr->procent)
+		remove_proc_entry(ctr->procfn, NULL);
+
 	printk(KERN_NOTICE "kcapi: controller [%03d]: %s unregistered\n",
 	       ctr->cnr, ctr->name);
 
-	return 0;
+unlock_out:
+	mutex_unlock(&capi_controller_lock);
+
+	return err;
 }
 
 EXPORT_SYMBOL(detach_capi_ctr);
@@ -589,13 +621,21 @@ EXPORT_SYMBOL(unregister_capi_driver);
 
 u16 capi20_isinstalled(void)
 {
+	u16 ret = CAPI_REGNOTINSTALLED;
 	int i;
-	for (i = 0; i < CAPI_MAXCONTR; i++) {
+
+	mutex_lock(&capi_controller_lock);
+
+	for (i = 0; i < CAPI_MAXCONTR; i++)
 		if (capi_controller[i] &&
-		    capi_controller[i]->state == CAPI_CTR_RUNNING)
-			return CAPI_NOERROR;
-	}
-	return CAPI_REGNOTINSTALLED;
+		    capi_controller[i]->state == CAPI_CTR_RUNNING) {
+			ret = CAPI_NOERROR;
+			break;
+		}
+
+	mutex_unlock(&capi_controller_lock);
+
+	return ret;
 }
 
 EXPORT_SYMBOL(capi20_isinstalled);
@@ -648,14 +688,16 @@ u16 capi20_register(struct capi20_appl *ap)
 
 	write_unlock_irqrestore(&application_lock, flags);
 	
-	mutex_lock(&controller_mutex);
+	mutex_lock(&capi_controller_lock);
+
 	for (i = 0; i < CAPI_MAXCONTR; i++) {
 		if (!capi_controller[i] ||
 		    capi_controller[i]->state != CAPI_CTR_RUNNING)
 			continue;
 		register_appl(capi_controller[i], applid, &ap->rparam);
 	}
-	mutex_unlock(&controller_mutex);
+
+	mutex_unlock(&capi_controller_lock);
 
 	if (showcapimsgs & 1) {
 		printk(KERN_DEBUG "kcapi: appl %d up\n", applid);
@@ -688,14 +730,16 @@ u16 capi20_release(struct capi20_appl *ap)
 	capi_applications[ap->applid - 1] = NULL;
 	write_unlock_irqrestore(&application_lock, flags);
 
-	mutex_lock(&controller_mutex);
+	mutex_lock(&capi_controller_lock);
+
 	for (i = 0; i < CAPI_MAXCONTR; i++) {
 		if (!capi_controller[i] ||
 		    capi_controller[i]->state != CAPI_CTR_RUNNING)
 			continue;
 		release_appl(capi_controller[i], ap->applid);
 	}
-	mutex_unlock(&controller_mutex);
+
+	mutex_unlock(&capi_controller_lock);
 
 	flush_scheduled_work();
 	skb_queue_purge(&ap->recv_queue);
@@ -734,6 +778,12 @@ u16 capi20_put_message(struct capi20_appl *ap, struct sk_buff *skb)
 	    || !capi_cmd_valid(CAPIMSG_COMMAND(skb->data))
 	    || !capi_subcmd_valid(CAPIMSG_SUBCOMMAND(skb->data)))
 		return CAPI_ILLCMDORSUBCMDORMSGTOSMALL;
+
+	/*
+	 * The controller reference is protected by the existence of the
+	 * application passed to us. We assume that the caller properly
+	 * synchronizes this service with capi20_release.
+	 */
 	ctr = get_capi_ctr_by_nr(CAPIMSG_CONTROLLER(skb->data));
 	if (!ctr || ctr->state != CAPI_CTR_RUNNING) {
 		ctr = get_capi_ctr_by_nr(1); /* XXX why? */
@@ -798,16 +848,24 @@ EXPORT_SYMBOL(capi20_put_message);
 u16 capi20_get_manufacturer(u32 contr, u8 *buf)
 {
 	struct capi_ctr *ctr;
+	u16 ret;
 
 	if (contr == 0) {
 		strlcpy(buf, capi_manufakturer, CAPI_MANUFACTURER_LEN);
 		return CAPI_NOERROR;
 	}
+
+	mutex_lock(&capi_controller_lock);
+
 	ctr = get_capi_ctr_by_nr(contr);
-	if (!ctr || ctr->state != CAPI_CTR_RUNNING)
-		return CAPI_REGNOTINSTALLED;
-	strlcpy(buf, ctr->manu, CAPI_MANUFACTURER_LEN);
-	return CAPI_NOERROR;
+	if (ctr && ctr->state == CAPI_CTR_RUNNING) {
+		strlcpy(buf, ctr->manu, CAPI_MANUFACTURER_LEN);
+		ret = CAPI_NOERROR;
+	} else
+		ret = CAPI_REGNOTINSTALLED;
+
+	mutex_unlock(&capi_controller_lock);
+	return ret;
 }
 
 EXPORT_SYMBOL(capi20_get_manufacturer);
@@ -825,17 +883,24 @@ EXPORT_SYMBOL(capi20_get_manufacturer);
 u16 capi20_get_version(u32 contr, struct capi_version *verp)
 {
 	struct capi_ctr *ctr;
+	u16 ret;
 
 	if (contr == 0) {
 		*verp = driver_version;
 		return CAPI_NOERROR;
 	}
+
+	mutex_lock(&capi_controller_lock);
+
 	ctr = get_capi_ctr_by_nr(contr);
-	if (!ctr || ctr->state != CAPI_CTR_RUNNING)
-		return CAPI_REGNOTINSTALLED;
+	if (ctr && ctr->state == CAPI_CTR_RUNNING) {
+		memcpy(verp, &ctr->version, sizeof(capi_version));
+		ret = CAPI_NOERROR;
+	} else
+		ret = CAPI_REGNOTINSTALLED;
 
-	memcpy(verp, &ctr->version, sizeof(capi_version));
-	return CAPI_NOERROR;
+	mutex_unlock(&capi_controller_lock);
+	return ret;
 }
 
 EXPORT_SYMBOL(capi20_get_version);
@@ -853,17 +918,24 @@ EXPORT_SYMBOL(capi20_get_version);
 u16 capi20_get_serial(u32 contr, u8 *serial)
 {
 	struct capi_ctr *ctr;
+	u16 ret;
 
 	if (contr == 0) {
 		strlcpy(serial, driver_serial, CAPI_SERIAL_LEN);
 		return CAPI_NOERROR;
 	}
+
+	mutex_lock(&capi_controller_lock);
+
 	ctr = get_capi_ctr_by_nr(contr);
-	if (!ctr || ctr->state != CAPI_CTR_RUNNING)
-		return CAPI_REGNOTINSTALLED;
+	if (ctr && ctr->state == CAPI_CTR_RUNNING) {
+		strlcpy(serial, ctr->serial, CAPI_SERIAL_LEN);
+		ret = CAPI_NOERROR;
+	} else
+		ret = CAPI_REGNOTINSTALLED;
 
-	strlcpy(serial, ctr->serial, CAPI_SERIAL_LEN);
-	return CAPI_NOERROR;
+	mutex_unlock(&capi_controller_lock);
+	return ret;
 }
 
 EXPORT_SYMBOL(capi20_get_serial);
@@ -881,21 +953,64 @@ EXPORT_SYMBOL(capi20_get_serial);
 u16 capi20_get_profile(u32 contr, struct capi_profile *profp)
 {
 	struct capi_ctr *ctr;
+	u16 ret;
 
 	if (contr == 0) {
 		profp->ncontroller = ncontrollers;
 		return CAPI_NOERROR;
 	}
+
+	mutex_lock(&capi_controller_lock);
+
 	ctr = get_capi_ctr_by_nr(contr);
-	if (!ctr || ctr->state != CAPI_CTR_RUNNING)
-		return CAPI_REGNOTINSTALLED;
+	if (ctr && ctr->state == CAPI_CTR_RUNNING) {
+		memcpy(profp, &ctr->profile, sizeof(struct capi_profile));
+		ret = CAPI_NOERROR;
+	} else
+		ret = CAPI_REGNOTINSTALLED;
 
-	memcpy(profp, &ctr->profile, sizeof(struct capi_profile));
-	return CAPI_NOERROR;
+	mutex_unlock(&capi_controller_lock);
+	return ret;
 }
 
 EXPORT_SYMBOL(capi20_get_profile);
 
+/* Must be called with capi_controller_lock held. */
+static int wait_on_ctr_state(struct capi_ctr *ctr, unsigned int state)
+{
+	DEFINE_WAIT(wait);
+	int retval = 0;
+
+	ctr = capi_ctr_get(ctr);
+	if (!ctr)
+		return -ESRCH;
+
+	for (;;) {
+		prepare_to_wait(&ctr->state_wait_queue, &wait,
+				TASK_INTERRUPTIBLE);
+
+		if (ctr->state == state)
+			break;
+		if (ctr->state == CAPI_CTR_DETACHED) {
+			retval = -ESRCH;
+			break;
+		}
+		if (signal_pending(current)) {
+			retval = -EINTR;
+			break;
+		}
+
+		mutex_unlock(&capi_controller_lock);
+		schedule();
+		mutex_lock(&capi_controller_lock);
+	}
+	finish_wait(&ctr->state_wait_queue, &wait);
+
+	capi_ctr_put(ctr);
+
+	return retval;
+}
+
 #ifdef AVMB1_COMPAT
 static int old_capi_manufacturer(unsigned int cmd, void __user *data)
 {
@@ -973,27 +1088,30 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
 					   sizeof(avmb1_loadandconfigdef)))
 				return -EFAULT;
 		}
+
+		mutex_lock(&capi_controller_lock);
+
 		ctr = get_capi_ctr_by_nr(ldef.contr);
-		if (!ctr)
-			return -EINVAL;
-		ctr = capi_ctr_get(ctr);
-		if (!ctr)
-			return -ESRCH;
+		if (!ctr) {
+			retval = -EINVAL;
+			goto load_unlock_out;
+		}
+
 		if (ctr->load_firmware == NULL) {
 			printk(KERN_DEBUG "kcapi: load: no load function\n");
-			capi_ctr_put(ctr);
-			return -ESRCH;
+			retval = -ESRCH;
+			goto load_unlock_out;
 		}
 
 		if (ldef.t4file.len <= 0) {
 			printk(KERN_DEBUG "kcapi: load: invalid parameter: length of t4file is %d ?\n", ldef.t4file.len);
-			capi_ctr_put(ctr);
-			return -EINVAL;
+			retval = -EINVAL;
+			goto load_unlock_out;
 		}
 		if (ldef.t4file.data == NULL) {
 			printk(KERN_DEBUG "kcapi: load: invalid parameter: dataptr is 0\n");
-			capi_ctr_put(ctr);
-			return -EINVAL;
+			retval = -EINVAL;
+			goto load_unlock_out;
 		}
 
 		ldata.firmware.user = 1;
@@ -1005,52 +1123,47 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
 
 		if (ctr->state != CAPI_CTR_DETECTED) {
 			printk(KERN_INFO "kcapi: load: contr=%d not in detect state\n", ldef.contr);
-			capi_ctr_put(ctr);
-			return -EBUSY;
+			retval = -EBUSY;
+			goto load_unlock_out;
 		}
 		ctr->state = CAPI_CTR_LOADING;
 
 		retval = ctr->load_firmware(ctr, &ldata);
-
 		if (retval) {
 			ctr->state = CAPI_CTR_DETECTED;
-			capi_ctr_put(ctr);
-			return retval;
+			goto load_unlock_out;
 		}
 
-		while (ctr->state != CAPI_CTR_RUNNING) {
-
-			msleep_interruptible(100);	/* 0.1 sec */
+		retval = wait_on_ctr_state(ctr, CAPI_CTR_RUNNING);
 
-			if (signal_pending(current)) {
-				capi_ctr_put(ctr);
-				return -EINTR;
-			}
-		}
-		capi_ctr_put(ctr);
-		return 0;
+load_unlock_out:
+		mutex_unlock(&capi_controller_lock);
+		return retval;
 
 	case AVMB1_RESETCARD:
 		if (copy_from_user(&rdef, data, sizeof(avmb1_resetdef)))
 			return -EFAULT;
+
+		retval = 0;
+
+		mutex_lock(&capi_controller_lock);
+
 		ctr = get_capi_ctr_by_nr(rdef.contr);
-		if (!ctr)
-			return -ESRCH;
+		if (!ctr) {
+			retval = -ESRCH;
+			goto reset_unlock_out;
+		}
 
 		if (ctr->state == CAPI_CTR_DETECTED)
-			return 0;
+			goto reset_unlock_out;
 
 		ctr->reset_ctr(ctr);
 
-		while (ctr->state > CAPI_CTR_DETECTED) {
-
-			msleep_interruptible(100);	/* 0.1 sec */
-
-			if (signal_pending(current))
-				return -EINTR;
-		}
-		return 0;
+		retval = wait_on_ctr_state(ctr, CAPI_CTR_DETECTED);
 
+reset_unlock_out:
+		mutex_unlock(&capi_controller_lock);
+		return retval;
 	}
 	return -EINVAL;
 }
@@ -1068,6 +1181,7 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
 int capi20_manufacturer(unsigned int cmd, void __user *data)
 {
 	struct capi_ctr *ctr;
+	int retval;
 
 	switch (cmd) {
 #ifdef AVMB1_COMPAT
@@ -1085,14 +1199,20 @@ int capi20_manufacturer(unsigned int cmd, void __user *data)
 		if (copy_from_user(&fdef, data, sizeof(kcapi_flagdef)))
 			return -EFAULT;
 
+		mutex_lock(&capi_controller_lock);
+
 		ctr = get_capi_ctr_by_nr(fdef.contr);
-		if (!ctr)
-			return -ESRCH;
+		if (ctr) {
+			ctr->traceflag = fdef.flag;
+			printk(KERN_INFO "kcapi: contr [%03d] set trace=%d\n",
+			       ctr->cnr, ctr->traceflag);
+			retval = 0;
+		} else
+			retval = -ESRCH;
+
+		mutex_unlock(&capi_controller_lock);
 
-		ctr->traceflag = fdef.flag;
-		printk(KERN_INFO "kcapi: contr [%03d] set trace=%d\n",
-		       ctr->cnr, ctr->traceflag);
-		return 0;
+		return retval;
 	}
 	case KCAPI_CMD_ADDCARD:
 	{
@@ -1100,7 +1220,6 @@ int capi20_manufacturer(unsigned int cmd, void __user *data)
 		struct capi_driver *driver = NULL;
 		capicardparams cparams;
 		kcapi_carddef cdef;
-		int retval;
 
 		if ((retval = copy_from_user(&cdef, data, sizeof(cdef))))
 			return retval;
diff --git a/drivers/isdn/capi/kcapi.h b/drivers/isdn/capi/kcapi.h
index 07c5850..f4620b3 100644
--- a/drivers/isdn/capi/kcapi.h
+++ b/drivers/isdn/capi/kcapi.h
@@ -24,6 +24,7 @@ printk(KERN_DEBUG "%s: " format "\n" , __func__ , ## arg); \
 #endif
 
 enum {
+	CAPI_CTR_DETACHED = 0,
 	CAPI_CTR_DETECTED = 1,
 	CAPI_CTR_LOADING  = 2,
 	CAPI_CTR_RUNNING  = 3,
@@ -32,8 +33,10 @@ enum {
 extern struct list_head capi_drivers;
 extern struct mutex capi_drivers_lock;
 
-extern struct capi20_appl *capi_applications[CAPI_MAXAPPL];
 extern struct capi_ctr *capi_controller[CAPI_MAXCONTR];
+extern struct mutex capi_controller_lock;
+
+extern struct capi20_appl *capi_applications[CAPI_MAXAPPL];
 
 #ifdef CONFIG_PROC_FS
 
diff --git a/drivers/isdn/capi/kcapi_proc.c b/drivers/isdn/capi/kcapi_proc.c
index 71b0761..3e6e17a 100644
--- a/drivers/isdn/capi/kcapi_proc.c
+++ b/drivers/isdn/capi/kcapi_proc.c
@@ -35,7 +35,10 @@ static char *state2str(unsigned short state)
 // ---------------------------------------------------------------------------
 
 static void *controller_start(struct seq_file *seq, loff_t *pos)
+	__acquires(capi_controller_lock)
 {
+	mutex_lock(&capi_controller_lock);
+
 	if (*pos < CAPI_MAXCONTR)
 		return &capi_controller[*pos];
 
@@ -52,7 +55,9 @@ static void *controller_next(struct seq_file *seq, void *v, loff_t *pos)
 }
 
 static void controller_stop(struct seq_file *seq, void *v)
+	__releases(capi_controller_lock)
 {
+	mutex_unlock(&capi_controller_lock);
 }
 
 static int controller_show(struct seq_file *seq, void *v)
diff --git a/include/linux/isdn/capilli.h b/include/linux/isdn/capilli.h
index 856f38e..11b57c4 100644
--- a/include/linux/isdn/capilli.h
+++ b/include/linux/isdn/capilli.h
@@ -66,9 +66,10 @@ struct capi_ctr {
 	unsigned long nsentdatapkt;
 
 	int cnr;				/* controller number */
-	volatile unsigned short state;		/* controller state */
-	volatile int blocked;			/* output blocked */
+	unsigned short state;			/* controller state */
+	int blocked;				/* output blocked */
 	int traceflag;				/* capi trace */
+	wait_queue_head_t state_wait_queue;
 
 	struct proc_dir_entry *procent;
         char procfn[128];
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH v2 09/41] CAPI: Rework controller state notifier
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Another step towards proper locking: Rework the callback provided to
capidrv for controller state changes. This is so far attached to an
application, which would require us to hold the corresponding lock
across notification calls.

But there is no direct relation between a controller up/down event and
an application, so let's decouple them and provide a notifier call chain
for those events instead. This notifier chain is first of all used
internally. Here we request the highest priority to unsure that
housekeeping work is done before any other notifications. The chain is
exported via [un]register_capictr_notifier to our only user, capidrv, to
replace the racy and unfixable capi20_set_callback.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capidrv.c |   22 ++++++--
 drivers/isdn/capi/kcapi.c   |  112 +++++++++++++++++++-----------------------
 include/linux/kernelcapi.h  |   17 ++-----
 3 files changed, 72 insertions(+), 79 deletions(-)

diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
index 7d8899a..bf55ed5 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -2210,19 +2210,24 @@ static int capidrv_delcontr(u16 contr)
 }
 
 
-static void lower_callback(unsigned int cmd, u32 contr, void *data)
+static int
+lower_callback(struct notifier_block *nb, unsigned long val, void *v)
 {
+	capi_profile profile;
+	u32 contr = (long)v;
 
-	switch (cmd) {
-	case KCI_CONTRUP:
+	switch (val) {
+	case CAPICTR_UP:
 		printk(KERN_INFO "capidrv: controller %hu up\n", contr);
-		(void) capidrv_addcontr(contr, (capi_profile *) data);
+		if (capi20_get_profile(contr, &profile) == CAPI_NOERROR)
+			(void) capidrv_addcontr(contr, &profile);
 		break;
-	case KCI_CONTRDOWN:
+	case CAPICTR_DOWN:
 		printk(KERN_INFO "capidrv: controller %hu down\n", contr);
 		(void) capidrv_delcontr(contr);
 		break;
 	}
+	return NOTIFY_OK;
 }
 
 /*
@@ -2262,6 +2267,10 @@ static void __exit proc_exit(void)
 	remove_proc_entry("capi/capidrv", NULL);
 }
 
+static struct notifier_block capictr_nb = {
+	.notifier_call = lower_callback,
+};
+
 static int __init capidrv_init(void)
 {
 	capi_profile profile;
@@ -2278,7 +2287,7 @@ static int __init capidrv_init(void)
 		return -EIO;
 	}
 
-	capi20_set_callback(&global.ap, lower_callback);
+	register_capictr_notifier(&capictr_nb);
 
 	errcode = capi20_get_profile(0, &profile);
 	if (errcode != CAPI_NOERROR) {
@@ -2300,6 +2309,7 @@ static int __init capidrv_init(void)
 
 static void __exit capidrv_exit(void)
 {
+	unregister_capictr_notifier(&capictr_nb);
 	capi20_release(&global.ap);
 
 	proc_exit();
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index 9362a7a..e08914d 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -44,12 +44,10 @@ module_param(showcapimsgs, uint, 0);
 
 /* ------------------------------------------------------------- */
 
-struct capi_notifier {
+struct capictr_event {
 	struct work_struct work;
-	unsigned int cmd;
+	unsigned int type;
 	u32 controller;
-	u16 applid;
-	u32 ncci;
 };
 
 /* ------------------------------------------------------------- */
@@ -71,6 +69,8 @@ struct capi_ctr *capi_controller[CAPI_MAXCONTR];
 
 static int ncontrollers;
 
+static BLOCKING_NOTIFIER_HEAD(ctr_notifier_list);
+
 /* -------- controller ref counting -------------------------------------- */
 
 static inline struct capi_ctr *
@@ -165,8 +165,6 @@ static void release_appl(struct capi_ctr *ctr, u16 applid)
 	capi_ctr_put(ctr);
 }
 
-/* -------- KCI_CONTRUP --------------------------------------- */
-
 static void notify_up(u32 contr)
 {
 	struct capi20_appl *ap;
@@ -188,16 +186,11 @@ static void notify_up(u32 contr)
 			if (!ap || ap->release_in_progress)
 				continue;
 			register_appl(ctr, applid, &ap->rparam);
-			if (ap->callback && !ap->release_in_progress)
-				ap->callback(KCI_CONTRUP, contr,
-					     &ctr->profile);
 		}
 	} else
 		printk(KERN_WARNING "%s: invalid contr %d\n", __func__, contr);
 }
 
-/* -------- KCI_CONTRDOWN ------------------------------------- */
-
 static void ctr_down(struct capi_ctr *ctr)
 {
 	struct capi20_appl *ap;
@@ -215,11 +208,8 @@ static void ctr_down(struct capi_ctr *ctr)
 
 	for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
 		ap = get_capi_appl_by_nr(applid);
-		if (ap && !ap->release_in_progress) {
-			if (ap->callback)
-				ap->callback(KCI_CONTRDOWN, ctr->cnr, NULL);
+		if (ap && !ap->release_in_progress)
 			capi_ctr_put(ctr);
-		}
 	}
 }
 
@@ -237,45 +227,63 @@ static void notify_down(u32 contr)
 		printk(KERN_WARNING "%s: invalid contr %d\n", __func__, contr);
 }
 
-static void notify_handler(struct work_struct *work)
+static int
+notify_handler(struct notifier_block *nb, unsigned long val, void *v)
 {
-	struct capi_notifier *np =
-		container_of(work, struct capi_notifier, work);
+	u32 contr = (long)v;
 
-	switch (np->cmd) {
-	case KCI_CONTRUP:
-		notify_up(np->controller);
+	switch (val) {
+	case CAPICTR_UP:
+		notify_up(contr);
 		break;
-	case KCI_CONTRDOWN:
-		notify_down(np->controller);
+	case CAPICTR_DOWN:
+		notify_down(contr);
 		break;
 	}
+	return NOTIFY_OK;
+}
+
+static void do_notify_work(struct work_struct *work)
+{
+	struct capictr_event *event =
+		container_of(work, struct capictr_event, work);
 
-	kfree(np);
+	blocking_notifier_call_chain(&ctr_notifier_list, event->type,
+				     (void *)(long)event->controller);
+	kfree(event);
 }
 
 /*
  * The notifier will result in adding/deleteing of devices. Devices can
  * only removed in user process, not in bh.
  */
-static int notify_push(unsigned int cmd, u32 controller, u16 applid, u32 ncci)
+static int notify_push(unsigned int event_type, u32 controller)
 {
-	struct capi_notifier *np = kmalloc(sizeof(*np), GFP_ATOMIC);
+	struct capictr_event *event = kmalloc(sizeof(*event), GFP_ATOMIC);
 
-	if (!np)
+	if (!event)
 		return -ENOMEM;
 
-	INIT_WORK(&np->work, notify_handler);
-	np->cmd = cmd;
-	np->controller = controller;
-	np->applid = applid;
-	np->ncci = ncci;
+	INIT_WORK(&event->work, do_notify_work);
+	event->type = event_type;
+	event->controller = controller;
 
-	schedule_work(&np->work);
+	schedule_work(&event->work);
 	return 0;
 }
 
-	
+int register_capictr_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&ctr_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(register_capictr_notifier);
+
+int unregister_capictr_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&ctr_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_capictr_notifier);
+
 /* -------- Receiver ------------------------------------------ */
 
 static void recv_handler(struct work_struct *work)
@@ -401,7 +409,7 @@ void capi_ctr_ready(struct capi_ctr *ctr)
 	printk(KERN_NOTICE "kcapi: controller [%03d] \"%s\" ready.\n",
 	       ctr->cnr, ctr->name);
 
-	notify_push(KCI_CONTRUP, ctr->cnr, 0, 0);
+	notify_push(CAPICTR_UP, ctr->cnr);
 }
 
 EXPORT_SYMBOL(capi_ctr_ready);
@@ -418,7 +426,7 @@ void capi_ctr_down(struct capi_ctr *ctr)
 {
 	printk(KERN_NOTICE "kcapi: controller [%03d] down.\n", ctr->cnr);
 
-	notify_push(KCI_CONTRDOWN, ctr->cnr, 0, 0);
+	notify_push(CAPICTR_DOWN, ctr->cnr);
 }
 
 EXPORT_SYMBOL(capi_ctr_down);
@@ -633,7 +641,6 @@ u16 capi20_register(struct capi20_appl *ap)
 	ap->nrecvdatapkt = 0;
 	ap->nsentctlpkt = 0;
 	ap->nsentdatapkt = 0;
-	ap->callback = NULL;
 	mutex_init(&ap->recv_mtx);
 	skb_queue_head_init(&ap->recv_queue);
 	INIT_WORK(&ap->recv_work, recv_handler);
@@ -1137,30 +1144,6 @@ int capi20_manufacturer(unsigned int cmd, void __user *data)
 
 EXPORT_SYMBOL(capi20_manufacturer);
 
-/* temporary hack */
-
-/**
- * capi20_set_callback() - set CAPI application notification callback function
- * @ap:		CAPI application descriptor structure.
- * @callback:	callback function (NULL to remove).
- *
- * If not NULL, the callback function will be called to notify the
- * application of the addition or removal of a controller.
- * The first argument (cmd) will tell whether the controller was added
- * (KCI_CONTRUP) or removed (KCI_CONTRDOWN).
- * The second argument (contr) will be the controller number.
- * For cmd==KCI_CONTRUP the third argument (data) will be a pointer to the
- * new controller's capability profile structure.
- */
-
-void capi20_set_callback(struct capi20_appl *ap,
-			 void (*callback) (unsigned int cmd, __u32 contr, void *data))
-{
-	ap->callback = callback;
-}
-
-EXPORT_SYMBOL(capi20_set_callback);
-
 /* ------------------------------------------------------------- */
 /* -------- Init & Cleanup ------------------------------------- */
 /* ------------------------------------------------------------- */
@@ -1169,10 +1152,17 @@ EXPORT_SYMBOL(capi20_set_callback);
  * init / exit functions
  */
 
+static struct notifier_block capictr_nb = {
+	.notifier_call = notify_handler,
+	.priority = INT_MAX,
+};
+
 static int __init kcapi_init(void)
 {
 	int err;
 
+	register_capictr_notifier(&capictr_nb);
+
 	err = cdebug_init();
 	if (!err)
 		kcapi_proc_init();
diff --git a/include/linux/kernelcapi.h b/include/linux/kernelcapi.h
index a53e932..9c26839 100644
--- a/include/linux/kernelcapi.h
+++ b/include/linux/kernelcapi.h
@@ -48,9 +48,7 @@ typedef struct kcapi_carddef {
 #include <linux/list.h>
 #include <linux/skbuff.h>
 #include <linux/workqueue.h>
-
-#define	KCI_CONTRUP	0	/* arg: struct capi_profile */
-#define	KCI_CONTRDOWN	1	/* arg: NULL */
+#include <linux/notifier.h>
 
 struct capi20_appl {
 	u16 applid;
@@ -67,11 +65,6 @@ struct capi20_appl {
 	struct sk_buff_head recv_queue;
 	struct work_struct recv_work;
 	int release_in_progress;
-
-	/* ugly hack to allow for notification of added/removed
-	 * controllers. The Right Way (tm) is known. XXX
-	 */
-	void (*callback) (unsigned int cmd, __u32 contr, void *data);
 };
 
 u16 capi20_isinstalled(void);
@@ -84,11 +77,11 @@ u16 capi20_get_serial(u32 contr, u8 serial[CAPI_SERIAL_LEN]);
 u16 capi20_get_profile(u32 contr, struct capi_profile *profp);
 int capi20_manufacturer(unsigned int cmd, void __user *data);
 
-/* temporary hack XXX */
-void capi20_set_callback(struct capi20_appl *ap, 
-			 void (*callback) (unsigned int cmd, __u32 contr, void *data));
-
+#define CAPICTR_UP			0
+#define CAPICTR_DOWN			1
 
+int register_capictr_notifier(struct notifier_block *nb);
+int unregister_capictr_notifier(struct notifier_block *nb);
 
 #define CAPI_NOERROR                      0x0000
 
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH v2 04/41] CAPI: Pin capifs instead of mounting it
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Auto-mounting the capifs during module init prevents unloading its
module. Instead, pin the filesystem as long as some NCCI node exists.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capifs.c |   27 ++++++++++++++++++---------
 1 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/isdn/capi/capifs.c b/drivers/isdn/capi/capifs.c
index 6ae0edf..51c01ef 100644
--- a/drivers/isdn/capi/capifs.c
+++ b/drivers/isdn/capi/capifs.c
@@ -32,6 +32,7 @@ static char *revision = "$Revision: 1.1.2.3 $";
 #define CAPIFS_SUPER_MAGIC (('C'<<8)|'N')
 
 static struct vfsmount *capifs_mnt;
+static int capifs_mnt_count;
 
 static struct {
 	int setuid;
@@ -140,7 +141,7 @@ static struct file_system_type capifs_fs_type = {
 	.kill_sb	= kill_anon_super,
 };
 
-struct dentry *capifs_new_ncci(unsigned int number, dev_t device)
+static struct dentry *new_ncci(unsigned int number, dev_t device)
 {
 	struct super_block *s = capifs_mnt->mnt_sb;
 	struct dentry *root = s->s_root;
@@ -187,6 +188,20 @@ unlock_out:
 	return dentry;
 }
 
+struct dentry *capifs_new_ncci(unsigned int number, dev_t device)
+{
+	struct dentry *dentry;
+
+	if (simple_pin_fs(&capifs_fs_type, &capifs_mnt, &capifs_mnt_count) < 0)
+		return NULL;
+
+	dentry = new_ncci(number, device);
+	if (!dentry)
+		simple_release_fs(&capifs_mnt, &capifs_mnt_count);
+
+	return dentry;
+}
+
 void capifs_free_ncci(struct dentry *dentry)
 {
 	struct dentry *root = capifs_mnt->mnt_sb->s_root;
@@ -206,6 +221,8 @@ void capifs_free_ncci(struct dentry *dentry)
 	dput(dentry);
 
 	mutex_unlock(&root->d_inode->i_mutex);
+
+	simple_release_fs(&capifs_mnt, &capifs_mnt_count);
 }
 
 static int __init capifs_init(void)
@@ -222,13 +239,6 @@ static int __init capifs_init(void)
 		strcpy(rev, "1.0");
 
 	err = register_filesystem(&capifs_fs_type);
-	if (!err) {
-		capifs_mnt = kern_mount(&capifs_fs_type);
-		if (IS_ERR(capifs_mnt)) {
-			err = PTR_ERR(capifs_mnt);
-			unregister_filesystem(&capifs_fs_type);
-		}
-	}
 	if (!err)
 		printk(KERN_NOTICE "capifs: Rev %s\n", rev);
 	return err;
@@ -237,7 +247,6 @@ static int __init capifs_init(void)
 static void __exit capifs_exit(void)
 {
 	unregister_filesystem(&capifs_fs_type);
-	mntput(capifs_mnt);
 }
 
 EXPORT_SYMBOL(capifs_new_ncci);
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH v2 03/41] CAPI: Eliminate capifs_root variable
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

capifs_mnt->mnt_sb->s_root already contains what we need.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capifs.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/isdn/capi/capifs.c b/drivers/isdn/capi/capifs.c
index 91aafad..6ae0edf 100644
--- a/drivers/isdn/capi/capifs.c
+++ b/drivers/isdn/capi/capifs.c
@@ -32,7 +32,6 @@ static char *revision = "$Revision: 1.1.2.3 $";
 #define CAPIFS_SUPER_MAGIC (('C'<<8)|'N')
 
 static struct vfsmount *capifs_mnt;
-static struct dentry *capifs_root;
 
 static struct {
 	int setuid;
@@ -118,7 +117,7 @@ capifs_fill_super(struct super_block *s, void *data, int silent)
 	inode->i_fop = &simple_dir_operations;
 	inode->i_nlink = 2;
 
-	capifs_root = s->s_root = d_alloc_root(inode);
+	s->s_root = d_alloc_root(inode);
 	if (s->s_root)
 		return 0;
 	
@@ -143,15 +142,17 @@ static struct file_system_type capifs_fs_type = {
 
 struct dentry *capifs_new_ncci(unsigned int number, dev_t device)
 {
+	struct super_block *s = capifs_mnt->mnt_sb;
+	struct dentry *root = s->s_root;
 	struct dentry *dentry;
 	struct inode *inode;
 	char name[10];
 	int namelen;
 
-	mutex_lock(&capifs_root->d_inode->i_mutex);
+	mutex_lock(&root->d_inode->i_mutex);
 
 	namelen = sprintf(name, "%d", number);
-	dentry = lookup_one_len(name, capifs_root, namelen);
+	dentry = lookup_one_len(name, root, namelen);
 	if (IS_ERR(dentry)) {
 		dentry = NULL;
 		goto unlock_out;
@@ -163,7 +164,7 @@ struct dentry *capifs_new_ncci(unsigned int number, dev_t device)
 		goto unlock_out;
 	}
 
-	inode = new_inode(capifs_mnt->mnt_sb);
+	inode = new_inode(s);
 	if (!inode) {
 		dput(dentry);
 		dentry = NULL;
@@ -181,19 +182,20 @@ struct dentry *capifs_new_ncci(unsigned int number, dev_t device)
 	dget(dentry);
 
 unlock_out:
-	mutex_unlock(&capifs_root->d_inode->i_mutex);
+	mutex_unlock(&root->d_inode->i_mutex);
 
 	return dentry;
 }
 
 void capifs_free_ncci(struct dentry *dentry)
 {
+	struct dentry *root = capifs_mnt->mnt_sb->s_root;
 	struct inode *inode;
 
 	if (!dentry)
 		return;
 
-	mutex_lock(&capifs_root->d_inode->i_mutex);
+	mutex_lock(&root->d_inode->i_mutex);
 
 	inode = dentry->d_inode;
 	if (inode) {
@@ -203,7 +205,7 @@ void capifs_free_ncci(struct dentry *dentry)
 	}
 	dput(dentry);
 
-	mutex_unlock(&capifs_root->d_inode->i_mutex);
+	mutex_unlock(&root->d_inode->i_mutex);
 }
 
 static int __init capifs_init(void)
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH v2 40/41] CAPI: Schedule capifs for removal
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

With dynamic TTY nodes and the help of udev, we no longer need this
special filesystem. Schedule it for removal in one year from now.

As a last duty to this feature, move its help to right option so that
users can read the rationale.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 Documentation/feature-removal-schedule.txt |   10 ++++++++++
 drivers/isdn/capi/Kconfig                  |   13 +++++++------
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index 2f93ac0..721a2aa 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -521,3 +521,13 @@ Why:	Duplicate functionality with the gspca_zc3xx driver, zc0301 only
 	sensors) wich are also supported by the gspca_zc3xx driver
 	(which supports 53 USB-ID's in total)
 Who:	Hans de Goede <hdegoede@redhat.com>
+
+----------------------------
+
+What:	capifs
+When:	February 2011
+Files:	drivers/isdn/capi/capifs.*
+Why:	udev fully replaces this special file system that only contains CAPI
+	NCCI TTY device nodes. User space (pppdcapiplugin) works without
+	noticing the difference.
+Who:	Jan Kiszka <jan.kiszka@web.de>
diff --git a/drivers/isdn/capi/Kconfig b/drivers/isdn/capi/Kconfig
index b2a0475..d607d46 100644
--- a/drivers/isdn/capi/Kconfig
+++ b/drivers/isdn/capi/Kconfig
@@ -35,18 +35,19 @@ config ISDN_CAPI_CAPI20
 	  Y/M here.
 
 config ISDN_CAPI_CAPIFS_BOOL
-	bool "CAPI2.0 filesystem support"
+	bool "CAPI2.0 filesystem support (DEPRECATED)"
 	depends on ISDN_CAPI_MIDDLEWARE && ISDN_CAPI_CAPI20
+	help
+	  This option provides a special file system, similar to /dev/pts with
+	  device nodes for the special ttys established by using the
+	  middleware extension above.
+	  You no longer need this, udev fully replaces it. This feature is
+	  scheduled for removal.
 
 config ISDN_CAPI_CAPIFS
 	tristate
 	depends on ISDN_CAPI_CAPIFS_BOOL
 	default ISDN_CAPI_CAPI20
-	help
-	  This option provides a special file system, similar to /dev/pts with
-	  device nodes for the special ttys established by using the
-	  middleware extension above. If you want to use pppd with
-	  pppdcapiplugin to dial up to your ISP, say Y here.
 
 config ISDN_CAPI_CAPIDRV
 	tristate "CAPI2.0 capidrv interface support"
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 39/41] CAPI: Drop special controller lookup from capi20_put_message
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

This strange special rule to fall back to controller 1 cannot be derived
from the CAPI specs and looks a lot like it was once dedicated to some
out-of-tree driver, probably AVM's broken fcdsl2 (FRITZ!Card DSL v2.0).
I found no in-tree user that needs this check, and I'm now taking care
of the fcdsl2. So drop these bits from our stack.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/kcapi.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index 0b4c8a7..ce9b05b 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -777,11 +777,8 @@ u16 capi20_put_message(struct capi20_appl *ap, struct sk_buff *skb)
 	 * synchronizes this service with capi20_release.
 	 */
 	ctr = get_capi_ctr_by_nr(CAPIMSG_CONTROLLER(skb->data));
-	if (!ctr || ctr->state != CAPI_CTR_RUNNING) {
-		ctr = get_capi_ctr_by_nr(1); /* XXX why? */
-		if (!ctr || ctr->state != CAPI_CTR_RUNNING)
-			return CAPI_REGNOTINSTALLED;
-	}
+	if (!ctr || ctr->state != CAPI_CTR_RUNNING)
+		return CAPI_REGNOTINSTALLED;
 	if (ctr->blocked)
 		return CAPI_SENDQUEUEFULL;
 
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 38/41] CAPI: Drop return value of handle_minor_send
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

We did not evaluate handle_minor_send's return value, just (void)'ed it
away. Time for a cleanup.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 40b81b4..ee58375 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -516,25 +516,24 @@ static void handle_minor_recv(struct capiminor *mp)
 		}
 }
 
-static int handle_minor_send(struct capiminor *mp)
+static void handle_minor_send(struct capiminor *mp)
 {
 	struct tty_struct *tty;
 	struct sk_buff *skb;
 	u16 len;
-	int count = 0;
 	u16 errcode;
 	u16 datahandle;
 
 	tty = tty_port_tty_get(&mp->port);
 	if (!tty)
-		return 0;
+		return;
 
 	if (mp->ttyoutstop) {
 #if defined(_DEBUG_DATAFLOW) || defined(_DEBUG_TTYFUNCS)
 		printk(KERN_DEBUG "capi: send: tty stopped\n");
 #endif
 		tty_kref_put(tty);
-		return 0;
+		return;
 	}
 
 	while (1) {
@@ -570,12 +569,10 @@ static int handle_minor_send(struct capiminor *mp)
 			mp->outbytes += len;
 			spin_unlock_bh(&mp->outlock);
 
-			tty_kref_put(tty);
-			return count;
+			break;
 		}
 		errcode = capi20_put_message(mp->ap, skb);
 		if (errcode == CAPI_NOERROR) {
-			count++;
 #ifdef _DEBUG_DATAFLOW
 			printk(KERN_DEBUG "capi: DATA_B3_REQ %u len=%u\n",
 							datahandle, len);
@@ -600,7 +597,6 @@ static int handle_minor_send(struct capiminor *mp)
 		kfree_skb(skb);
 	}
 	tty_kref_put(tty);
-	return count;
 }
 
 #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
@@ -677,7 +673,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 			tty_wakeup(tty);
 			tty_kref_put(tty);
 		}
-		(void)handle_minor_send(mp);
+		handle_minor_send(mp);
 
 	} else {
 		/* ups, let capi application handle it :-) */
@@ -1114,7 +1110,7 @@ static int capinc_tty_write(struct tty_struct *tty,
 	mp->outbytes += skb->len;
 	spin_unlock_bh(&mp->outlock);
 
-	(void)handle_minor_send(mp);
+	handle_minor_send(mp);
 
 	return count;
 }
@@ -1157,7 +1153,7 @@ unlock_out:
 	spin_unlock_bh(&mp->outlock);
 
 	if (invoke_send)
-		(void)handle_minor_send(mp);
+		handle_minor_send(mp);
 
 	return ret;
 }
@@ -1179,7 +1175,7 @@ static void capinc_tty_flush_chars(struct tty_struct *tty)
 		mp->outbytes += skb->len;
 		spin_unlock_bh(&mp->outlock);
 
-		(void)handle_minor_send(mp);
+		handle_minor_send(mp);
 	} else
 		spin_unlock_bh(&mp->outlock);
 
@@ -1269,7 +1265,7 @@ static void capinc_tty_start(struct tty_struct *tty)
 	printk(KERN_DEBUG "capinc_tty_start\n");
 #endif
 	mp->ttyoutstop = 0;
-	(void)handle_minor_send(mp);
+	handle_minor_send(mp);
 }
 
 static void capinc_tty_hangup(struct tty_struct *tty)
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 36/41] CAPI: Fix locking around capiminor's output queue and drop workaround_lock
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Introduce outlock as a spin lock that protects capiminor's outqueue,
outbytes and outskb (formerly known as ttyskb). outlock can be acquired
from soft-IRQ context via capinc_write, so make it bh-safe.

This finally removes the last reason for keeping the workaround lock
around (which was incomplete and partly broken anyway). And as we no
longer call handle_recv_skb in atomic context, gen_data_b3_resp_for can
use non-atomic allocation now.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |  126 ++++++++++++++++++++++++---------------------
 1 files changed, 67 insertions(+), 59 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index be85c8c..074496f 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -95,11 +95,13 @@ struct capiminor {
 	struct tty_port port;
 	int                ttyinstop;
 	int                ttyoutstop;
-	struct sk_buff    *ttyskb;
 
-	struct sk_buff_head inqueue;
-	struct sk_buff_head outqueue;
-	int                 outbytes;
+	struct sk_buff_head	inqueue;
+
+	struct sk_buff_head	outqueue;
+	int			outbytes;
+	struct sk_buff		*outskb;
+	spinlock_t		outlock;
 
 	/* transmit path */
 	struct list_head ackqueue;
@@ -107,15 +109,6 @@ struct capiminor {
 	spinlock_t ackqlock;
 };
 
-/* FIXME: The following lock is a sledgehammer-workaround to a
- * locking issue with the capiminor (and maybe other) data structure(s).
- * Access to this data is done in a racy way and crashes the machine with
- * a FritzCard DSL driver; sooner or later. This is a workaround
- * which trades scalability vs stability, so it doesn't crash the kernel anymore.
- * The correct (and scalable) fix for the issue seems to require
- * an API change to the drivers... . */
-static DEFINE_SPINLOCK(workaround_lock);
-
 struct capincci {
 	struct list_head list;
 	u32		 ncci;
@@ -231,6 +224,7 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 
 	skb_queue_head_init(&mp->inqueue);
 	skb_queue_head_init(&mp->outqueue);
+	spin_lock_init(&mp->outlock);
 
 	tty_port_init(&mp->port);
 	mp->port.ops = &capiminor_port_ops;
@@ -271,7 +265,7 @@ static void capiminor_destroy(struct kref *kref)
 {
 	struct capiminor *mp = container_of(kref, struct capiminor, kref);
 
-	kfree_skb(mp->ttyskb);
+	kfree_skb(mp->outskb);
 	skb_queue_purge(&mp->inqueue);
 	skb_queue_purge(&mp->outqueue);
 	capiminor_del_all_ack(mp);
@@ -417,7 +411,7 @@ static struct sk_buff *
 gen_data_b3_resp_for(struct capiminor *mp, struct sk_buff *skb)
 {
 	struct sk_buff *nskb;
-	nskb = alloc_skb(CAPI_DATA_B3_RESP_LEN, GFP_ATOMIC);
+	nskb = alloc_skb(CAPI_DATA_B3_RESP_LEN, GFP_KERNEL);
 	if (nskb) {
 		u16 datahandle = CAPIMSG_U16(skb->data,CAPIMSG_BASELEN+4+4+2);
 		unsigned char *s = skb_put(nskb, CAPI_DATA_B3_RESP_LEN);
@@ -548,9 +542,18 @@ static int handle_minor_send(struct capiminor *mp)
 		return 0;
 	}
 
-	while ((skb = skb_dequeue(&mp->outqueue)) != NULL) {
-		datahandle = atomic_inc_return(&mp->datahandle);
+	while (1) {
+		spin_lock_bh(&mp->outlock);
+		skb = __skb_dequeue(&mp->outqueue);
+		if (!skb) {
+			spin_unlock_bh(&mp->outlock);
+			break;
+		}
 		len = (u16)skb->len;
+		mp->outbytes -= len;
+		spin_unlock_bh(&mp->outlock);
+
+		datahandle = atomic_inc_return(&mp->datahandle);
 		skb_push(skb, CAPI_DATA_B3_REQ_LEN);
 		memset(skb->data, 0, CAPI_DATA_B3_REQ_LEN);
 		capimsg_setu16(skb->data, 0, CAPI_DATA_B3_REQ_LEN);
@@ -566,14 +569,18 @@ static int handle_minor_send(struct capiminor *mp)
 
 		if (capiminor_add_ack(mp, datahandle) < 0) {
 			skb_pull(skb, CAPI_DATA_B3_REQ_LEN);
-			skb_queue_head(&mp->outqueue, skb);
+
+			spin_lock_bh(&mp->outlock);
+			__skb_queue_head(&mp->outqueue, skb);
+			mp->outbytes += len;
+			spin_unlock_bh(&mp->outlock);
+
 			tty_kref_put(tty);
 			return count;
 		}
 		errcode = capi20_put_message(mp->ap, skb);
 		if (errcode == CAPI_NOERROR) {
 			count++;
-			mp->outbytes -= len;
 #ifdef _DEBUG_DATAFLOW
 			printk(KERN_DEBUG "capi: DATA_B3_REQ %u len=%u\n",
 							datahandle, len);
@@ -584,13 +591,17 @@ static int handle_minor_send(struct capiminor *mp)
 
 		if (errcode == CAPI_SENDQUEUEFULL) {
 			skb_pull(skb, CAPI_DATA_B3_REQ_LEN);
-			skb_queue_head(&mp->outqueue, skb);
+
+			spin_lock_bh(&mp->outlock);
+			__skb_queue_head(&mp->outqueue, skb);
+			mp->outbytes += len;
+			spin_unlock_bh(&mp->outlock);
+
 			break;
 		}
 
 		/* ups, drop packet */
 		printk(KERN_ERR "capi: put_message = %x\n", errcode);
-		mp->outbytes -= len;
 		kfree_skb(skb);
 	}
 	tty_kref_put(tty);
@@ -609,7 +620,6 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 	u16 datahandle;
 #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
 	struct capincci *np;
-	unsigned long flags;
 
 	mutex_lock(&cdev->lock);
 
@@ -621,7 +631,6 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 	if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_IND)
 		capincci_alloc(cdev, CAPIMSG_NCCI(skb->data));
 
-	spin_lock_irqsave(&workaround_lock, flags);
 	if (CAPIMSG_COMMAND(skb->data) != CAPI_DATA_B3) {
 		skb_queue_tail(&cdev->recvqueue, skb);
 		wake_up_interruptible(&cdev->recvwait);
@@ -683,7 +692,6 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
 
 unlock_out:
-	spin_unlock_irqrestore(&workaround_lock, flags);
 	mutex_unlock(&cdev->lock);
 }
 
@@ -1062,16 +1070,13 @@ static void capinc_tty_cleanup(struct tty_struct *tty)
 static int capinc_tty_open(struct tty_struct *tty, struct file *filp)
 {
 	struct capiminor *mp = tty->driver_data;
-	unsigned long flags;
 	int err;
 
 	err = tty_port_open(&mp->port, tty, filp);
 	if (err)
 		return err;
 
-	spin_lock_irqsave(&workaround_lock, flags);
 	handle_minor_recv(mp);
-	spin_unlock_irqrestore(&workaround_lock, flags);
 	return 0;
 }
 
@@ -1087,71 +1092,78 @@ static int capinc_tty_write(struct tty_struct *tty,
 {
 	struct capiminor *mp = tty->driver_data;
 	struct sk_buff *skb;
-	unsigned long flags;
 
 #ifdef _DEBUG_TTYFUNCS
 	printk(KERN_DEBUG "capinc_tty_write(count=%d)\n", count);
 #endif
 
-	spin_lock_irqsave(&workaround_lock, flags);
-	skb = mp->ttyskb;
+	spin_lock_bh(&mp->outlock);
+	skb = mp->outskb;
 	if (skb) {
-		mp->ttyskb = NULL;
-		skb_queue_tail(&mp->outqueue, skb);
+		mp->outskb = NULL;
+		__skb_queue_tail(&mp->outqueue, skb);
 		mp->outbytes += skb->len;
 	}
 
 	skb = alloc_skb(CAPI_DATA_B3_REQ_LEN+count, GFP_ATOMIC);
 	if (!skb) {
 		printk(KERN_ERR "capinc_tty_write: alloc_skb failed\n");
-		spin_unlock_irqrestore(&workaround_lock, flags);
+		spin_unlock_bh(&mp->outlock);
 		return -ENOMEM;
 	}
 
 	skb_reserve(skb, CAPI_DATA_B3_REQ_LEN);
 	memcpy(skb_put(skb, count), buf, count);
 
-	skb_queue_tail(&mp->outqueue, skb);
+	__skb_queue_tail(&mp->outqueue, skb);
 	mp->outbytes += skb->len;
+	spin_unlock_bh(&mp->outlock);
+
 	(void)handle_minor_send(mp);
-	spin_unlock_irqrestore(&workaround_lock, flags);
+
 	return count;
 }
 
 static int capinc_tty_put_char(struct tty_struct *tty, unsigned char ch)
 {
 	struct capiminor *mp = tty->driver_data;
+	bool invoke_send = false;
 	struct sk_buff *skb;
-	unsigned long flags;
 	int ret = 1;
 
 #ifdef _DEBUG_TTYFUNCS
 	printk(KERN_DEBUG "capinc_put_char(%u)\n", ch);
 #endif
 
-	spin_lock_irqsave(&workaround_lock, flags);
-	skb = mp->ttyskb;
+	spin_lock_bh(&mp->outlock);
+	skb = mp->outskb;
 	if (skb) {
 		if (skb_tailroom(skb) > 0) {
 			*(skb_put(skb, 1)) = ch;
-			spin_unlock_irqrestore(&workaround_lock, flags);
-			return 1;
+			goto unlock_out;
 		}
-		mp->ttyskb = NULL;
-		skb_queue_tail(&mp->outqueue, skb);
+		mp->outskb = NULL;
+		__skb_queue_tail(&mp->outqueue, skb);
 		mp->outbytes += skb->len;
-		(void)handle_minor_send(mp);
+		invoke_send = true;
 	}
+
 	skb = alloc_skb(CAPI_DATA_B3_REQ_LEN+CAPI_MAX_BLKSIZE, GFP_ATOMIC);
 	if (skb) {
 		skb_reserve(skb, CAPI_DATA_B3_REQ_LEN);
 		*(skb_put(skb, 1)) = ch;
-		mp->ttyskb = skb;
+		mp->outskb = skb;
 	} else {
 		printk(KERN_ERR "capinc_put_char: char %u lost\n", ch);
 		ret = 0;
 	}
-	spin_unlock_irqrestore(&workaround_lock, flags);
+
+unlock_out:
+	spin_unlock_bh(&mp->outlock);
+
+	if (invoke_send)
+		(void)handle_minor_send(mp);
+
 	return ret;
 }
 
@@ -1159,22 +1171,24 @@ static void capinc_tty_flush_chars(struct tty_struct *tty)
 {
 	struct capiminor *mp = tty->driver_data;
 	struct sk_buff *skb;
-	unsigned long flags;
 
 #ifdef _DEBUG_TTYFUNCS
 	printk(KERN_DEBUG "capinc_tty_flush_chars\n");
 #endif
 
-	spin_lock_irqsave(&workaround_lock, flags);
-	skb = mp->ttyskb;
+	spin_lock_bh(&mp->outlock);
+	skb = mp->outskb;
 	if (skb) {
-		mp->ttyskb = NULL;
-		skb_queue_tail(&mp->outqueue, skb);
+		mp->outskb = NULL;
+		__skb_queue_tail(&mp->outqueue, skb);
 		mp->outbytes += skb->len;
+		spin_unlock_bh(&mp->outlock);
+
 		(void)handle_minor_send(mp);
-	}
-	(void)handle_minor_recv(mp);
-	spin_unlock_irqrestore(&workaround_lock, flags);
+	} else
+		spin_unlock_bh(&mp->outlock);
+
+	handle_minor_recv(mp);
 }
 
 static int capinc_tty_write_room(struct tty_struct *tty)
@@ -1234,15 +1248,12 @@ static void capinc_tty_throttle(struct tty_struct *tty)
 static void capinc_tty_unthrottle(struct tty_struct *tty)
 {
 	struct capiminor *mp = tty->driver_data;
-	unsigned long flags;
 
 #ifdef _DEBUG_TTYFUNCS
 	printk(KERN_DEBUG "capinc_tty_unthrottle\n");
 #endif
-	spin_lock_irqsave(&workaround_lock, flags);
 	mp->ttyinstop = 0;
 	handle_minor_recv(mp);
-	spin_unlock_irqrestore(&workaround_lock, flags);
 }
 
 static void capinc_tty_stop(struct tty_struct *tty)
@@ -1258,15 +1269,12 @@ static void capinc_tty_stop(struct tty_struct *tty)
 static void capinc_tty_start(struct tty_struct *tty)
 {
 	struct capiminor *mp = tty->driver_data;
-	unsigned long flags;
 
 #ifdef _DEBUG_TTYFUNCS
 	printk(KERN_DEBUG "capinc_tty_start\n");
 #endif
-	spin_lock_irqsave(&workaround_lock, flags);
 	mp->ttyoutstop = 0;
 	(void)handle_minor_send(mp);
-	spin_unlock_irqrestore(&workaround_lock, flags);
 }
 
 static void capinc_tty_hangup(struct tty_struct *tty)
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 34/41] CAPI: Use atomics for capiminor's datahandle and msgid
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

The capiminor members datahandle and msgid are incremented outside any
lock, so better do this atomically.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 9d4750a..08d5a8a 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -87,10 +87,10 @@ struct capiminor {
 	unsigned int      minor;
 	struct dentry *capifs_dentry;
 
-	struct capi20_appl *ap;
-	u32		 ncci;
-	u16		 datahandle;
-	u16		 msgid;
+	struct capi20_appl	*ap;
+	u32			ncci;
+	atomic_t		datahandle;
+	atomic_t		msgid;
 
 	struct tty_port port;
 	int                ttyinstop;
@@ -227,7 +227,6 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 
 	mp->ap = ap;
 	mp->ncci = ncci;
-	mp->msgid = 0;
 	INIT_LIST_HEAD(&mp->ackqueue);
 	spin_lock_init(&mp->ackqlock);
 
@@ -427,7 +426,7 @@ gen_data_b3_resp_for(struct capiminor *mp, struct sk_buff *skb)
 		capimsg_setu16(s, 2, mp->ap->applid);
 		capimsg_setu8 (s, 4, CAPI_DATA_B3);
 		capimsg_setu8 (s, 5, CAPI_RESP);
-		capimsg_setu16(s, 6, mp->msgid++);
+		capimsg_setu16(s, 6, atomic_inc_return(&mp->msgid));
 		capimsg_setu32(s, 8, mp->ncci);
 		capimsg_setu16(s, 12, datahandle);
 	}
@@ -554,7 +553,7 @@ static int handle_minor_send(struct capiminor *mp)
 	}
 
 	while ((skb = skb_dequeue(&mp->outqueue)) != NULL) {
-		datahandle = mp->datahandle;
+		datahandle = atomic_inc_return(&mp->datahandle);
 		len = (u16)skb->len;
 		skb_push(skb, CAPI_DATA_B3_REQ_LEN);
 		memset(skb->data, 0, CAPI_DATA_B3_REQ_LEN);
@@ -562,7 +561,7 @@ static int handle_minor_send(struct capiminor *mp)
 		capimsg_setu16(skb->data, 2, mp->ap->applid);
 		capimsg_setu8 (skb->data, 4, CAPI_DATA_B3);
 		capimsg_setu8 (skb->data, 5, CAPI_REQ);
-		capimsg_setu16(skb->data, 6, mp->msgid++);
+		capimsg_setu16(skb->data, 6, atomic_inc_return(&mp->msgid));
 		capimsg_setu32(skb->data, 8, mp->ncci);	/* NCCI */
 		capimsg_setu32(skb->data, 12, (u32)(long)skb->data);/* Data32 */
 		capimsg_setu16(skb->data, 16, len);	/* Data length */
@@ -577,7 +576,6 @@ static int handle_minor_send(struct capiminor *mp)
 		}
 		errcode = capi20_put_message(mp->ap, skb);
 		if (errcode == CAPI_NOERROR) {
-			mp->datahandle++;
 			count++;
 			mp->outbytes -= len;
 #ifdef _DEBUG_DATAFLOW
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 31/41] CAPI: Drop handle_minor_recv from capinc_tty_write
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Sending a message down the CAPI stack may trigger the reception of an
answer, but this will go through capi_recv_message and call
handle_minor_recv from there. There is no need to walk the receive queue
on capinc_tty_write.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 867589a..554fa1b 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -1104,7 +1104,6 @@ static int capinc_tty_write(struct tty_struct *tty,
 	skb_queue_tail(&mp->outqueue, skb);
 	mp->outbytes += skb->len;
 	(void)handle_minor_send(mp);
-	(void)handle_minor_recv(mp);
 	spin_unlock_irqrestore(&workaround_lock, flags);
 	return count;
 }
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 29/41] CAPI: Clean up capiminors_lock
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Use a plain spin lock for capiminors_lock, drop inconsistent irqsafe
acquisitions (it's only used in process context anyway).

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index cf5e996..0b264b4 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -149,7 +149,7 @@ static LIST_HEAD(capidev_list);
 
 #ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
 
-static DEFINE_RWLOCK(capiminors_lock);
+static DEFINE_SPINLOCK(capiminors_lock);
 static struct capiminor **capiminors;
 
 static struct tty_driver *capinc_tty_driver;
@@ -218,7 +218,6 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 	struct capiminor *mp;
 	struct device *dev;
 	unsigned int minor;
-	unsigned long flags;
 
 	mp = kzalloc(sizeof(*mp), GFP_KERNEL);
   	if (!mp) {
@@ -242,13 +241,13 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 	mp->port.ops = &capiminor_port_ops;
 
 	/* Allocate the least unused minor number. */
-	write_lock_irqsave(&capiminors_lock, flags);
+	spin_lock(&capiminors_lock);
 	for (minor = 0; minor < capi_ttyminors; minor++)
 		if (!capiminors[minor]) {
 			capiminors[minor] = mp;
 			break;
 		}
-	write_unlock_irqrestore(&capiminors_lock, flags);
+	spin_unlock(&capiminors_lock);
 
 	if (minor == capi_ttyminors) {
 		printk(KERN_NOTICE "capi: out of minors\n");
@@ -264,9 +263,9 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 	return mp;
 
 err_out2:
-	write_lock_irqsave(&capiminors_lock, flags);
+	spin_lock(&capiminors_lock);
 	capiminors[minor] = NULL;
-	write_unlock_irqrestore(&capiminors_lock, flags);
+	spin_unlock(&capiminors_lock);
 
 err_out1:
 	kfree(mp);
@@ -288,11 +287,11 @@ static struct capiminor *capiminor_get(unsigned int minor)
 {
 	struct capiminor *mp;
 
-	read_lock(&capiminors_lock);
+	spin_lock(&capiminors_lock);
 	mp = capiminors[minor];
 	if (mp)
 		kref_get(&mp->kref);
-	read_unlock(&capiminors_lock);
+	spin_unlock(&capiminors_lock);
 
 	return mp;
 }
@@ -304,13 +303,11 @@ static inline void capiminor_put(struct capiminor *mp)
 
 static void capiminor_free(struct capiminor *mp)
 {
-	unsigned long flags;
-
 	tty_unregister_device(capinc_tty_driver, mp->minor);
 
-	write_lock_irqsave(&capiminors_lock, flags);
+	spin_lock(&capiminors_lock);
 	capiminors[mp->minor] = NULL;
-	write_unlock_irqrestore(&capiminors_lock, flags);
+	spin_unlock(&capiminors_lock);
 
 	capiminor_put(mp);
 }
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 28/41] CAPI: Drop obsolete nccip from capiminor struct
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

The nccip in capiminor used to serve as an indicator that the NCCI was
close. But we don't need this, we issue a hangup on capincci_free_minor.
So drop this legacy.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   44 +-------------------------------------------
 1 files changed, 1 insertions(+), 43 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index b382ede..cf5e996 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -85,7 +85,6 @@ struct datahandle_queue {
 struct capiminor {
 	struct kref kref;
 
-	struct capincci  *nccip;
 	unsigned int      minor;
 	struct dentry *capifs_dentry;
 
@@ -328,10 +327,6 @@ static void capincci_alloc_minor(struct capidev *cdev, struct capincci *np)
 
 	mp = np->minorp = capiminor_alloc(&cdev->ap, np->ncci);
 	if (mp) {
-		mp->nccip = np;
-#ifdef _DEBUG_REFCOUNT
-		printk(KERN_DEBUG "set mp->nccip\n");
-#endif
 		device = MKDEV(capinc_tty_driver->major, mp->minor);
 		mp->capifs_dentry = capifs_new_ncci(mp->minor, device);
 	}
@@ -347,10 +342,6 @@ static void capincci_free_minor(struct capincci *np)
 
 		tty = tty_port_tty_get(&mp->port);
 		if (tty) {
-			mp->nccip = NULL;
-#ifdef _DEBUG_REFCOUNT
-			printk(KERN_DEBUG "reset mp->nccip\n");
-#endif
 			tty_vhangup(tty);
 			tty_kref_put(tty);
 		}
@@ -1094,7 +1085,7 @@ static void capinc_tty_close(struct tty_struct *tty, struct file *filp)
 	tty_port_close(&mp->port, tty, filp);
 }
 
-static int capinc_tty_write(struct tty_struct * tty,
+static int capinc_tty_write(struct tty_struct *tty,
 			    const unsigned char *buf, int count)
 {
 	struct capiminor *mp = tty->driver_data;
@@ -1105,13 +1096,6 @@ static int capinc_tty_write(struct tty_struct * tty,
 	printk(KERN_DEBUG "capinc_tty_write(count=%d)\n", count);
 #endif
 
-	if (!mp->nccip) {
-#ifdef _DEBUG_TTYFUNCS
-		printk(KERN_DEBUG "capinc_tty_write: mp or mp->ncci NULL\n");
-#endif
-		return 0;
-	}
-
 	spin_lock_irqsave(&workaround_lock, flags);
 	skb = mp->ttyskb;
 	if (skb) {
@@ -1149,13 +1133,6 @@ static int capinc_tty_put_char(struct tty_struct *tty, unsigned char ch)
 	printk(KERN_DEBUG "capinc_put_char(%u)\n", ch);
 #endif
 
-	if (!mp->nccip) {
-#ifdef _DEBUG_TTYFUNCS
-		printk(KERN_DEBUG "capinc_tty_put_char: mp or mp->ncci NULL\n");
-#endif
-		return 0;
-	}
-
 	spin_lock_irqsave(&workaround_lock, flags);
 	skb = mp->ttyskb;
 	if (skb) {
@@ -1192,13 +1169,6 @@ static void capinc_tty_flush_chars(struct tty_struct *tty)
 	printk(KERN_DEBUG "capinc_tty_flush_chars\n");
 #endif
 
-	if (!mp->nccip) {
-#ifdef _DEBUG_TTYFUNCS
-		printk(KERN_DEBUG "capinc_tty_flush_chars: mp or mp->ncci NULL\n");
-#endif
-		return;
-	}
-
 	spin_lock_irqsave(&workaround_lock, flags);
 	skb = mp->ttyskb;
 	if (skb) {
@@ -1216,12 +1186,6 @@ static int capinc_tty_write_room(struct tty_struct *tty)
 	struct capiminor *mp = tty->driver_data;
 	int room;
 
-	if (!mp->nccip) {
-#ifdef _DEBUG_TTYFUNCS
-		printk(KERN_DEBUG "capinc_tty_write_room: mp or mp->ncci NULL\n");
-#endif
-		return 0;
-	}
 	room = CAPINC_MAX_SENDQUEUE-skb_queue_len(&mp->outqueue);
 	room *= CAPI_MAX_BLKSIZE;
 #ifdef _DEBUG_TTYFUNCS
@@ -1234,12 +1198,6 @@ static int capinc_tty_chars_in_buffer(struct tty_struct *tty)
 {
 	struct capiminor *mp = tty->driver_data;
 
-	if (!mp->nccip) {
-#ifdef _DEBUG_TTYFUNCS
-		printk(KERN_DEBUG "capinc_tty_chars_in_buffer: mp or mp->ncci NULL\n");
-#endif
-		return 0;
-	}
 #ifdef _DEBUG_TTYFUNCS
 	printk(KERN_DEBUG "capinc_tty_chars_in_buffer = %d nack=%d sq=%d rq=%d\n",
 			mp->outbytes, mp->nack,
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 26/41] CAPI: Drop remaining NULL checks on tty->driver_data
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

tty_struct's driver_data cannot be NULL, no need to test for it.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   64 ++++++++++++++++++++++-----------------------
 1 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index e164a8f..acc811b 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -1097,7 +1097,7 @@ static void capinc_tty_close(struct tty_struct *tty, struct file *filp)
 static int capinc_tty_write(struct tty_struct * tty,
 			    const unsigned char *buf, int count)
 {
-	struct capiminor *mp = (struct capiminor *)tty->driver_data;
+	struct capiminor *mp = tty->driver_data;
 	struct sk_buff *skb;
 	unsigned long flags;
 
@@ -1105,7 +1105,7 @@ static int capinc_tty_write(struct tty_struct * tty,
 	printk(KERN_DEBUG "capinc_tty_write(count=%d)\n", count);
 #endif
 
-	if (!mp || !mp->nccip) {
+	if (!mp->nccip) {
 #ifdef _DEBUG_TTYFUNCS
 		printk(KERN_DEBUG "capinc_tty_write: mp or mp->ncci NULL\n");
 #endif
@@ -1140,7 +1140,7 @@ static int capinc_tty_write(struct tty_struct * tty,
 
 static int capinc_tty_put_char(struct tty_struct *tty, unsigned char ch)
 {
-	struct capiminor *mp = (struct capiminor *)tty->driver_data;
+	struct capiminor *mp = tty->driver_data;
 	struct sk_buff *skb;
 	unsigned long flags;
 	int ret = 1;
@@ -1149,7 +1149,7 @@ static int capinc_tty_put_char(struct tty_struct *tty, unsigned char ch)
 	printk(KERN_DEBUG "capinc_put_char(%u)\n", ch);
 #endif
 
-	if (!mp || !mp->nccip) {
+	if (!mp->nccip) {
 #ifdef _DEBUG_TTYFUNCS
 		printk(KERN_DEBUG "capinc_tty_put_char: mp or mp->ncci NULL\n");
 #endif
@@ -1184,7 +1184,7 @@ static int capinc_tty_put_char(struct tty_struct *tty, unsigned char ch)
 
 static void capinc_tty_flush_chars(struct tty_struct *tty)
 {
-	struct capiminor *mp = (struct capiminor *)tty->driver_data;
+	struct capiminor *mp = tty->driver_data;
 	struct sk_buff *skb;
 	unsigned long flags;
 
@@ -1192,7 +1192,7 @@ static void capinc_tty_flush_chars(struct tty_struct *tty)
 	printk(KERN_DEBUG "capinc_tty_flush_chars\n");
 #endif
 
-	if (!mp || !mp->nccip) {
+	if (!mp->nccip) {
 #ifdef _DEBUG_TTYFUNCS
 		printk(KERN_DEBUG "capinc_tty_flush_chars: mp or mp->ncci NULL\n");
 #endif
@@ -1213,9 +1213,10 @@ static void capinc_tty_flush_chars(struct tty_struct *tty)
 
 static int capinc_tty_write_room(struct tty_struct *tty)
 {
-	struct capiminor *mp = (struct capiminor *)tty->driver_data;
+	struct capiminor *mp = tty->driver_data;
 	int room;
-	if (!mp || !mp->nccip) {
+
+	if (!mp->nccip) {
 #ifdef _DEBUG_TTYFUNCS
 		printk(KERN_DEBUG "capinc_tty_write_room: mp or mp->ncci NULL\n");
 #endif
@@ -1231,8 +1232,9 @@ static int capinc_tty_write_room(struct tty_struct *tty)
 
 static int capinc_tty_chars_in_buffer(struct tty_struct *tty)
 {
-	struct capiminor *mp = (struct capiminor *)tty->driver_data;
-	if (!mp || !mp->nccip) {
+	struct capiminor *mp = tty->driver_data;
+
+	if (!mp->nccip) {
 #ifdef _DEBUG_TTYFUNCS
 		printk(KERN_DEBUG "capinc_tty_chars_in_buffer: mp or mp->ncci NULL\n");
 #endif
@@ -1266,55 +1268,51 @@ static void capinc_tty_set_termios(struct tty_struct *tty, struct ktermios * old
 #endif
 }
 
-static void capinc_tty_throttle(struct tty_struct * tty)
+static void capinc_tty_throttle(struct tty_struct *tty)
 {
-	struct capiminor *mp = (struct capiminor *)tty->driver_data;
+	struct capiminor *mp = tty->driver_data;
 #ifdef _DEBUG_TTYFUNCS
 	printk(KERN_DEBUG "capinc_tty_throttle\n");
 #endif
-	if (mp)
-		mp->ttyinstop = 1;
+	mp->ttyinstop = 1;
 }
 
-static void capinc_tty_unthrottle(struct tty_struct * tty)
+static void capinc_tty_unthrottle(struct tty_struct *tty)
 {
-	struct capiminor *mp = (struct capiminor *)tty->driver_data;
+	struct capiminor *mp = tty->driver_data;
 	unsigned long flags;
+
 #ifdef _DEBUG_TTYFUNCS
 	printk(KERN_DEBUG "capinc_tty_unthrottle\n");
 #endif
-	if (mp) {
-		spin_lock_irqsave(&workaround_lock, flags);
-		mp->ttyinstop = 0;
-		handle_minor_recv(mp);
-		spin_unlock_irqrestore(&workaround_lock, flags);
-	}
+	spin_lock_irqsave(&workaround_lock, flags);
+	mp->ttyinstop = 0;
+	handle_minor_recv(mp);
+	spin_unlock_irqrestore(&workaround_lock, flags);
 }
 
 static void capinc_tty_stop(struct tty_struct *tty)
 {
-	struct capiminor *mp = (struct capiminor *)tty->driver_data;
+	struct capiminor *mp = tty->driver_data;
+
 #ifdef _DEBUG_TTYFUNCS
 	printk(KERN_DEBUG "capinc_tty_stop\n");
 #endif
-	if (mp) {
-		mp->ttyoutstop = 1;
-	}
+	mp->ttyoutstop = 1;
 }
 
 static void capinc_tty_start(struct tty_struct *tty)
 {
-	struct capiminor *mp = (struct capiminor *)tty->driver_data;
+	struct capiminor *mp = tty->driver_data;
 	unsigned long flags;
+
 #ifdef _DEBUG_TTYFUNCS
 	printk(KERN_DEBUG "capinc_tty_start\n");
 #endif
-	if (mp) {
-		spin_lock_irqsave(&workaround_lock, flags);
-		mp->ttyoutstop = 0;
-		(void)handle_minor_send(mp);
-		spin_unlock_irqrestore(&workaround_lock, flags);
-	}
+	spin_lock_irqsave(&workaround_lock, flags);
+	mp->ttyoutstop = 0;
+	(void)handle_minor_send(mp);
+	spin_unlock_irqrestore(&workaround_lock, flags);
 }
 
 static void capinc_tty_hangup(struct tty_struct *tty)
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 25/41] CAPI: Use tty_port to keep track of capiminor's tty
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Use the reference management features of tty_port to look up and drop
again the tty_struct associated with a capiminor.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   94 ++++++++++++++++++++++++++++++----------------
 1 files changed, 62 insertions(+), 32 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 3e4997a..e164a8f 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -94,7 +94,7 @@ struct capiminor {
 	u16		 datahandle;
 	u16		 msgid;
 
-	struct tty_struct *tty;
+	struct tty_port port;
 	int                ttyinstop;
 	int                ttyoutstop;
 	struct sk_buff    *ttyskb;
@@ -212,6 +212,8 @@ static void capiminor_del_all_ack(struct capiminor *mp)
 
 /* -------- struct capiminor ---------------------------------------- */
 
+static const struct tty_port_operations capiminor_port_ops; /* we have none */
+
 static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 {
 	struct capiminor *mp;
@@ -237,6 +239,9 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 	skb_queue_head_init(&mp->inqueue);
 	skb_queue_head_init(&mp->outqueue);
 
+	tty_port_init(&mp->port);
+	mp->port.ops = &capiminor_port_ops;
+
 	/* Allocate the least unused minor number. */
 	write_lock_irqsave(&capiminors_lock, flags);
 	for (minor = 0; minor < capi_ttyminors; minor++)
@@ -335,18 +340,22 @@ static void capincci_alloc_minor(struct capidev *cdev, struct capincci *np)
 static void capincci_free_minor(struct capincci *np)
 {
 	struct capiminor *mp = np->minorp;
+	struct tty_struct *tty;
 
 	if (mp) {
 		capifs_free_ncci(mp->capifs_dentry);
-		if (mp->tty) {
+
+		tty = tty_port_tty_get(&mp->port);
+		if (tty) {
 			mp->nccip = NULL;
 #ifdef _DEBUG_REFCOUNT
 			printk(KERN_DEBUG "reset mp->nccip\n");
 #endif
-			tty_hangup(mp->tty);
-		} else {
-			capiminor_free(mp);
+			tty_hangup(tty);
+			tty_kref_put(tty);
 		}
+
+		capiminor_free(mp);
 	}
 }
 
@@ -433,44 +442,48 @@ gen_data_b3_resp_for(struct capiminor *mp, struct sk_buff *skb)
 
 static int handle_recv_skb(struct capiminor *mp, struct sk_buff *skb)
 {
+	struct tty_struct *tty;
 	struct sk_buff *nskb;
 	int datalen;
 	u16 errcode, datahandle;
 	struct tty_ldisc *ld;
-	
+	int ret = -1;
+
 	datalen = skb->len - CAPIMSG_LEN(skb->data);
-	if (mp->tty == NULL)
-	{
+
+	tty = tty_port_tty_get(&mp->port);
+	if (!tty) {
 #ifdef _DEBUG_DATAFLOW
 		printk(KERN_DEBUG "capi: currently no receiver\n");
 #endif
 		return -1;
 	}
 	
-	ld = tty_ldisc_ref(mp->tty);
-	if (ld == NULL)
-		return -1;
+	ld = tty_ldisc_ref(tty);
+	if (!ld)
+		goto out1;
+
 	if (ld->ops->receive_buf == NULL) {
 #if defined(_DEBUG_DATAFLOW) || defined(_DEBUG_TTYFUNCS)
 		printk(KERN_DEBUG "capi: ldisc has no receive_buf function\n");
 #endif
-		goto bad;
+		goto out2;
 	}
 	if (mp->ttyinstop) {
 #if defined(_DEBUG_DATAFLOW) || defined(_DEBUG_TTYFUNCS)
 		printk(KERN_DEBUG "capi: recv tty throttled\n");
 #endif
-		goto bad;
+		goto out2;
 	}
-	if (mp->tty->receive_room < datalen) {
+	if (tty->receive_room < datalen) {
 #if defined(_DEBUG_DATAFLOW) || defined(_DEBUG_TTYFUNCS)
 		printk(KERN_DEBUG "capi: no room in tty\n");
 #endif
-		goto bad;
+		goto out2;
 	}
 	if ((nskb = gen_data_b3_resp_for(mp, skb)) == NULL) {
 		printk(KERN_ERR "capi: gen_data_b3_resp failed\n");
-		goto bad;
+		goto out2;
 	}
 	datahandle = CAPIMSG_U16(skb->data,CAPIMSG_BASELEN+4);
 	errcode = capi20_put_message(mp->ap, nskb);
@@ -478,20 +491,21 @@ static int handle_recv_skb(struct capiminor *mp, struct sk_buff *skb)
 		printk(KERN_ERR "capi: send DATA_B3_RESP failed=%x\n",
 				errcode);
 		kfree_skb(nskb);
-		goto bad;
+		goto out2;
 	}
 	(void)skb_pull(skb, CAPIMSG_LEN(skb->data));
 #ifdef _DEBUG_DATAFLOW
 	printk(KERN_DEBUG "capi: DATA_B3_RESP %u len=%d => ldisc\n",
 				datahandle, skb->len);
 #endif
-	ld->ops->receive_buf(mp->tty, skb->data, NULL, skb->len);
+	ld->ops->receive_buf(tty, skb->data, NULL, skb->len);
 	kfree_skb(skb);
+	ret = 0;
+out2:
 	tty_ldisc_deref(ld);
-	return 0;
-bad:
-	tty_ldisc_deref(ld);
-	return -1;
+out1:
+	tty_kref_put(tty);
+	return ret;
 }
 
 static void handle_minor_recv(struct capiminor *mp)
@@ -510,16 +524,22 @@ static void handle_minor_recv(struct capiminor *mp)
 
 static int handle_minor_send(struct capiminor *mp)
 {
+	struct tty_struct *tty;
 	struct sk_buff *skb;
 	u16 len;
 	int count = 0;
 	u16 errcode;
 	u16 datahandle;
 
-	if (mp->tty && mp->ttyoutstop) {
+	tty = tty_port_tty_get(&mp->port);
+	if (!tty)
+		return 0;
+
+	if (mp->ttyoutstop) {
 #if defined(_DEBUG_DATAFLOW) || defined(_DEBUG_TTYFUNCS)
 		printk(KERN_DEBUG "capi: send: tty stopped\n");
 #endif
+		tty_kref_put(tty);
 		return 0;
 	}
 
@@ -542,6 +562,7 @@ static int handle_minor_send(struct capiminor *mp)
 		if (capiminor_add_ack(mp, datahandle) < 0) {
 			skb_pull(skb, CAPI_DATA_B3_REQ_LEN);
 			skb_queue_head(&mp->outqueue, skb);
+			tty_kref_put(tty);
 			return count;
 		}
 		errcode = capi20_put_message(mp->ap, skb);
@@ -568,6 +589,7 @@ static int handle_minor_send(struct capiminor *mp)
 		mp->outbytes -= len;
 		kfree_skb(skb);
 	}
+	tty_kref_put(tty);
 	return count;
 }
 
@@ -578,6 +600,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 {
 	struct capidev *cdev = ap->private;
 #ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
+	struct tty_struct *tty;
 	struct capiminor *mp;
 	u16 datahandle;
 #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
@@ -641,8 +664,11 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 #endif
 		kfree_skb(skb);
 		(void)capiminor_del_ack(mp, datahandle);
-		if (mp->tty)
-			tty_wakeup(mp->tty);
+		tty = tty_port_tty_get(&mp->port);
+		if (tty) {
+			tty_wakeup(tty);
+			tty_kref_put(tty);
+		}
 		(void)handle_minor_send(mp);
 
 	} else {
@@ -1029,14 +1055,17 @@ static void capinc_tty_cleanup(struct tty_struct *tty)
 	capiminor_put(mp);
 }
 
-static int capinc_tty_open(struct tty_struct * tty, struct file * file)
+static int capinc_tty_open(struct tty_struct *tty, struct file *filp)
 {
 	struct capiminor *mp = tty->driver_data;
 	unsigned long flags;
+	int err;
+
+	err = tty_port_open(&mp->port, tty, filp);
+	if (err)
+		return err;
 
 	spin_lock_irqsave(&workaround_lock, flags);
-	if (atomic_read(&mp->ttyopencount) == 0)
-		mp->tty = tty;
 	atomic_inc(&mp->ttyopencount);
 #ifdef _DEBUG_REFCOUNT
 	printk(KERN_DEBUG "capinc_tty_open ocount=%d\n", atomic_read(&mp->ttyopencount));
@@ -1046,7 +1075,7 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file)
 	return 0;
 }
 
-static void capinc_tty_close(struct tty_struct * tty, struct file * file)
+static void capinc_tty_close(struct tty_struct *tty, struct file *filp)
 {
 	struct capiminor *mp = tty->driver_data;
 
@@ -1054,17 +1083,15 @@ static void capinc_tty_close(struct tty_struct * tty, struct file * file)
 #ifdef _DEBUG_REFCOUNT
 			printk(KERN_DEBUG "capinc_tty_close lastclose\n");
 #endif
-			mp->tty = NULL;
 		}
 #ifdef _DEBUG_REFCOUNT
 		printk(KERN_DEBUG "capinc_tty_close ocount=%d\n", atomic_read(&mp->ttyopencount));
 #endif
-		if (mp->nccip == NULL)
-			capiminor_free(mp);
 
 #ifdef _DEBUG_REFCOUNT
 	printk(KERN_DEBUG "capinc_tty_close\n");
 #endif
+	tty_port_close(&mp->port, tty, filp);
 }
 
 static int capinc_tty_write(struct tty_struct * tty,
@@ -1292,9 +1319,12 @@ static void capinc_tty_start(struct tty_struct *tty)
 
 static void capinc_tty_hangup(struct tty_struct *tty)
 {
+	struct capiminor *mp = tty->driver_data;
+
 #ifdef _DEBUG_TTYFUNCS
 	printk(KERN_DEBUG "capinc_tty_hangup\n");
 #endif
+	tty_port_hangup(&mp->port);
 }
 
 static int capinc_tty_break_ctl(struct tty_struct *tty, int state)
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 21/41] CAPI: Dynamically register minor devices
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Register capiminors dynamically with the TTY core so that udev can make
them show up as the NCCIs appear or disappear. This removes the need to
check if the capiminor requested in capinc_tty_open actually exists.

And this completely obsoletes capifs which will be scheduled for removal
in a later patch.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   35 ++++++++++++++++++++++++-----------
 1 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index c22b349..901b79b 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -153,6 +153,8 @@ static LIST_HEAD(capidev_list);
 static DEFINE_RWLOCK(capiminors_lock);
 static struct capiminor **capiminors;
 
+static struct tty_driver *capinc_tty_driver;
+
 /* -------- datahandles --------------------------------------------- */
 
 static int capiminor_add_ack(struct capiminor *mp, u16 datahandle)
@@ -213,6 +215,7 @@ static void capiminor_del_all_ack(struct capiminor *mp)
 static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 {
 	struct capiminor *mp;
+	struct device *dev;
 	unsigned int minor;
 	unsigned long flags;
 
@@ -243,19 +246,33 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 
 	if (minor == capi_ttyminors) {
 		printk(KERN_NOTICE "capi: out of minors\n");
-		kfree(mp);
-		return NULL;
+		goto err_out1;
 	}
 
 	mp->minor = minor;
 
+	dev = tty_register_device(capinc_tty_driver, minor, NULL);
+	if (IS_ERR(dev))
+		goto err_out2;
+
 	return mp;
+
+err_out2:
+	write_lock_irqsave(&capiminors_lock, flags);
+	capiminors[minor] = NULL;
+	write_unlock_irqrestore(&capiminors_lock, flags);
+
+err_out1:
+	kfree(mp);
+	return NULL;
 }
 
 static void capiminor_free(struct capiminor *mp)
 {
 	unsigned long flags;
 
+	tty_unregister_device(capinc_tty_driver, mp->minor);
+
 	write_lock_irqsave(&capiminors_lock, flags);
 	capiminors[mp->minor] = NULL;
 	write_unlock_irqrestore(&capiminors_lock, flags);
@@ -268,13 +285,10 @@ static void capiminor_free(struct capiminor *mp)
 	kfree(mp);
 }
 
-static struct capiminor *capiminor_find(unsigned int minor)
+static struct capiminor *capiminor_get(unsigned int minor)
 {
 	struct capiminor *mp;
 
-	if (minor >= capi_ttyminors)
-		return NULL;
-
 	read_lock(&capiminors_lock);
 	mp = capiminors[minor];
 	read_unlock(&capiminors_lock);
@@ -981,8 +995,7 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file)
 	struct capiminor *mp;
 	unsigned long flags;
 
-	if ((mp = capiminor_find(iminor(file->f_path.dentry->d_inode))) == NULL)
-		return -ENXIO;
+	mp = capiminor_get(iminor(file->f_path.dentry->d_inode));
 	if (mp->nccip == NULL)
 		return -ENXIO;
 
@@ -1284,8 +1297,6 @@ static void capinc_tty_send_xchar(struct tty_struct *tty, char ch)
 #endif
 }
 
-static struct tty_driver *capinc_tty_driver;
-
 static const struct tty_operations capinc_ops = {
 	.open = capinc_tty_open,
 	.close = capinc_tty_close,
@@ -1339,7 +1350,9 @@ static int __init capinc_tty_init(void)
 	drv->init_termios.c_oflag = OPOST | ONLCR;
 	drv->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL | CLOCAL;
 	drv->init_termios.c_lflag = 0;
-	drv->flags = TTY_DRIVER_REAL_RAW|TTY_DRIVER_RESET_TERMIOS;
+	drv->flags =
+		TTY_DRIVER_REAL_RAW | TTY_DRIVER_RESET_TERMIOS |
+		TTY_DRIVER_DYNAMIC_DEV;
 	tty_set_operations(drv, &capinc_ops);
 
 	err = tty_register_driver(drv);
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 20/41] CAPI: Clean up capinc_tty_init/exit
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Return proper error code if tty_register_driver fails. In contrast,
tty_unregister_driver cannot practically fail, so drop that error
handling. Finally, mark capinc_tty_init/exit with __init/__exit.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 46f85ae..c22b349 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -1307,10 +1307,11 @@ static const struct tty_operations capinc_ops = {
 	.send_xchar = capinc_tty_send_xchar,
 };
 
-static int capinc_tty_init(void)
+static int __init capinc_tty_init(void)
 {
 	struct tty_driver *drv;
-	
+	int err;
+
 	if (capi_ttyminors > CAPINC_MAX_PORTS)
 		capi_ttyminors = CAPINC_MAX_PORTS;
 	if (capi_ttyminors <= 0)
@@ -1340,23 +1341,22 @@ static int capinc_tty_init(void)
 	drv->init_termios.c_lflag = 0;
 	drv->flags = TTY_DRIVER_REAL_RAW|TTY_DRIVER_RESET_TERMIOS;
 	tty_set_operations(drv, &capinc_ops);
-	if (tty_register_driver(drv)) {
+
+	err = tty_register_driver(drv);
+	if (err) {
 		put_tty_driver(drv);
 		kfree(capiminors);
 		printk(KERN_ERR "Couldn't register capi_nc driver\n");
-		return -1;
+		return err;
 	}
 	capinc_tty_driver = drv;
 	return 0;
 }
 
-static void capinc_tty_exit(void)
+static void __exit capinc_tty_exit(void)
 {
-	struct tty_driver *drv = capinc_tty_driver;
-	int retval;
-	if ((retval = tty_unregister_driver(drv)))
-		printk(KERN_ERR "capi: failed to unregister capi_nc driver (%d)\n", retval);
-	put_tty_driver(drv);
+	tty_unregister_driver(capinc_tty_driver);
+	put_tty_driver(capinc_tty_driver);
 	kfree(capiminors);
 }
 
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 17/41] CAPI: Fix racy capi_read
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

capi_read still used interruptible_sleep_on, risking to miss a wakeup
this way. Convert it to wait_event_interruptible.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |   19 +++++++------------
 1 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index f8f8660..8abec96 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -657,24 +657,19 @@ capi_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	struct capidev *cdev = (struct capidev *)file->private_data;
 	struct sk_buff *skb;
 	size_t copied;
+	int err;
 
 	if (!cdev->ap.applid)
 		return -ENODEV;
 
-	if ((skb = skb_dequeue(&cdev->recvqueue)) == NULL) {
-
+	skb = skb_dequeue(&cdev->recvqueue);
+	if (!skb) {
 		if (file->f_flags & O_NONBLOCK)
 			return -EAGAIN;
-
-		for (;;) {
-			interruptible_sleep_on(&cdev->recvwait);
-			if ((skb = skb_dequeue(&cdev->recvqueue)) != NULL)
-				break;
-			if (signal_pending(current))
-				break;
-		}
-		if (skb == NULL)
-			return -ERESTARTNOHAND;
+		err = wait_event_interruptible(cdev->recvwait,
+				(skb = skb_dequeue(&cdev->recvqueue)));
+		if (err)
+			return err;
 	}
 	if (skb->len > count) {
 		skb_queue_head(&cdev->recvqueue, skb);
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 16/41] CAPI: Use non-atomic allocation during NCCI setup
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Both capincci_alloc and capiminor_alloc run in non-atomic context,
update their memory allocations accordingly.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 403bf8f..f8f8660 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -217,7 +217,7 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 	unsigned int minor = 0;
 	unsigned long flags;
 
-	mp = kzalloc(sizeof(*mp), GFP_ATOMIC);
+	mp = kzalloc(sizeof(*mp), GFP_KERNEL);
   	if (!mp) {
   		printk(KERN_ERR "capi: can't alloc capiminor\n");
 		return NULL;
@@ -358,7 +358,7 @@ static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
 {
 	struct capincci *np, **pp;
 
-	np = kzalloc(sizeof(*np), GFP_ATOMIC);
+	np = kzalloc(sizeof(*np), GFP_KERNEL);
 	if (!np)
 		return NULL;
 	np->ncci = ncci;
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH v2 15/41] CAPI: Rework locking of capidev members
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
  To: David Miller, Karsten Keil
  Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
	Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>

Rename 'ncci_list_mtx' to 'lock', expressing that it now protects a
larger set of capidev members: the NCCI list, ap.applid (ie. the
registration of the application), and modifications of userflags.

We do not need to protect each and every check for ap.applid because,
once an application is registered, it will stay for the whole lifetime
of the device.

Also, there is no need to apply the capidev mutex during release (if
there could be concurrent users, we would crash them anyway by freeing
the device at the end of capi_release).

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 drivers/isdn/capi/capi.c |  181 ++++++++++++++++++++++------------------------
 1 files changed, 88 insertions(+), 93 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 9d7c369..403bf8f 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -141,7 +141,7 @@ struct capidev {
 
 	struct capincci *nccis;
 
-	struct mutex ncci_list_mtx;
+	struct mutex lock;
 };
 
 /* -------- global variables ---------------------------------------- */
@@ -574,38 +574,31 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 	u16 datahandle;
 #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
 	struct capincci *np;
-	u32 ncci;
 	unsigned long flags;
 
+	mutex_lock(&cdev->lock);
+
 	if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_CONF) {
 		u16 info = CAPIMSG_U16(skb->data, 12); // Info field
-		if ((info & 0xff00) == 0) {
-			mutex_lock(&cdev->ncci_list_mtx);
+		if ((info & 0xff00) == 0)
 			capincci_alloc(cdev, CAPIMSG_NCCI(skb->data));
-			mutex_unlock(&cdev->ncci_list_mtx);
-		}
 	}
-	if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_IND) {
-		mutex_lock(&cdev->ncci_list_mtx);
+	if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_IND)
 		capincci_alloc(cdev, CAPIMSG_NCCI(skb->data));
-		mutex_unlock(&cdev->ncci_list_mtx);
-	}
+
 	spin_lock_irqsave(&workaround_lock, flags);
 	if (CAPIMSG_COMMAND(skb->data) != CAPI_DATA_B3) {
 		skb_queue_tail(&cdev->recvqueue, skb);
 		wake_up_interruptible(&cdev->recvwait);
-		spin_unlock_irqrestore(&workaround_lock, flags);
-		return;
+		goto unlock_out;
 	}
-	ncci = CAPIMSG_CONTROL(skb->data);
-	for (np = cdev->nccis; np && np->ncci != ncci; np = np->next)
-		;
+
+	np = capincci_find(cdev, CAPIMSG_CONTROL(skb->data));
 	if (!np) {
 		printk(KERN_ERR "BUG: capi_signal: ncci not found\n");
 		skb_queue_tail(&cdev->recvqueue, skb);
 		wake_up_interruptible(&cdev->recvwait);
-		spin_unlock_irqrestore(&workaround_lock, flags);
-		return;
+		goto unlock_out;
 	}
 
 #ifndef CONFIG_ISDN_CAPI_MIDDLEWARE
@@ -618,8 +611,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 	if (!mp) {
 		skb_queue_tail(&cdev->recvqueue, skb);
 		wake_up_interruptible(&cdev->recvwait);
-		spin_unlock_irqrestore(&workaround_lock, flags);
-		return;
+		goto unlock_out;
 	}
 	if (CAPIMSG_SUBCOMMAND(skb->data) == CAPI_IND) {
 		datahandle = CAPIMSG_U16(skb->data, CAPIMSG_BASELEN+4+4+2);
@@ -652,7 +644,9 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 	}
 #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
 
+unlock_out:
 	spin_unlock_irqrestore(&workaround_lock, flags);
+	mutex_unlock(&cdev->lock);
 }
 
 /* -------- file_operations for capidev ----------------------------- */
@@ -730,9 +724,9 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos
 	CAPIMSG_SETAPPID(skb->data, cdev->ap.applid);
 
 	if (CAPIMSG_CMD(skb->data) == CAPI_DISCONNECT_B3_RESP) {
-		mutex_lock(&cdev->ncci_list_mtx);
+		mutex_lock(&cdev->lock);
 		capincci_free(cdev, CAPIMSG_NCCI(skb->data));
-		mutex_unlock(&cdev->ncci_list_mtx);
+		mutex_unlock(&cdev->lock);
 	}
 
 	cdev->errcode = capi20_put_message(&cdev->ap, skb);
@@ -765,30 +759,35 @@ capi_ioctl(struct inode *inode, struct file *file,
 	   unsigned int cmd, unsigned long arg)
 {
 	struct capidev *cdev = file->private_data;
-	struct capi20_appl *ap = &cdev->ap;
 	capi_ioctl_struct data;
 	int retval = -EINVAL;
 	void __user *argp = (void __user *)arg;
 
 	switch (cmd) {
 	case CAPI_REGISTER:
-		{
-			if (ap->applid)
-				return -EEXIST;
+		mutex_lock(&cdev->lock);
 
-			if (copy_from_user(&cdev->ap.rparam, argp,
-					   sizeof(struct capi_register_params)))
-				return -EFAULT;
-			
-			cdev->ap.private = cdev;
-			cdev->ap.recv_message = capi_recv_message;
-			cdev->errcode = capi20_register(ap);
-			if (cdev->errcode) {
-				ap->applid = 0;
-				return -EIO;
-			}
+		if (cdev->ap.applid) {
+			retval = -EEXIST;
+			goto register_out;
+		}
+		if (copy_from_user(&cdev->ap.rparam, argp,
+				   sizeof(struct capi_register_params))) {
+			retval = -EFAULT;
+			goto register_out;
+		}
+		cdev->ap.private = cdev;
+		cdev->ap.recv_message = capi_recv_message;
+		cdev->errcode = capi20_register(&cdev->ap);
+		retval = (int)cdev->ap.applid;
+		if (cdev->errcode) {
+			cdev->ap.applid = 0;
+			retval = -EIO;
 		}
-		return (int)ap->applid;
+
+register_out:
+		mutex_unlock(&cdev->lock);
+		return retval;
 
 	case CAPI_GET_VERSION:
 		{
@@ -887,68 +886,67 @@ capi_ioctl(struct inode *inode, struct file *file,
 		return 0;
 
 	case CAPI_SET_FLAGS:
-	case CAPI_CLR_FLAGS:
-		{
-			unsigned userflags;
-			if (copy_from_user(&userflags, argp,
-					   sizeof(userflags)))
-				return -EFAULT;
-			if (cmd == CAPI_SET_FLAGS)
-				cdev->userflags |= userflags;
-			else
-				cdev->userflags &= ~userflags;
-		}
-		return 0;
+	case CAPI_CLR_FLAGS: {
+		unsigned userflags;
+
+		if (copy_from_user(&userflags, argp, sizeof(userflags)))
+			return -EFAULT;
 
+		mutex_lock(&cdev->lock);
+		if (cmd == CAPI_SET_FLAGS)
+			cdev->userflags |= userflags;
+		else
+			cdev->userflags &= ~userflags;
+		mutex_unlock(&cdev->lock);
+		return 0;
+	}
 	case CAPI_GET_FLAGS:
 		if (copy_to_user(argp, &cdev->userflags,
 				 sizeof(cdev->userflags)))
 			return -EFAULT;
 		return 0;
 
-	case CAPI_NCCI_OPENCOUNT:
-		{
-			struct capincci *nccip;
-			unsigned ncci;
-			int count = 0;
-			if (copy_from_user(&ncci, argp, sizeof(ncci)))
-				return -EFAULT;
+	case CAPI_NCCI_OPENCOUNT: {
+		struct capincci *nccip;
+		unsigned ncci;
+		int count = 0;
 
-			mutex_lock(&cdev->ncci_list_mtx);
-			if ((nccip = capincci_find(cdev, (u32) ncci)) == NULL) {
-				mutex_unlock(&cdev->ncci_list_mtx);
-				return 0;
-			}
-			count += capincci_minor_opencount(nccip);
-			mutex_unlock(&cdev->ncci_list_mtx);
-			return count;
-		}
-		return 0;
+		if (copy_from_user(&ncci, argp, sizeof(ncci)))
+			return -EFAULT;
+
+		mutex_lock(&cdev->lock);
+		nccip = capincci_find(cdev, (u32)ncci);
+		if (nccip)
+			count = capincci_minor_opencount(nccip);
+		mutex_unlock(&cdev->lock);
+		return count;
+	}
 
 #ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
-	case CAPI_NCCI_GETUNIT:
-		{
-			struct capincci *nccip;
-			struct capiminor *mp;
-			unsigned ncci;
-			int unit = 0;
-			if (copy_from_user(&ncci, argp,
-					   sizeof(ncci)))
-				return -EFAULT;
-			mutex_lock(&cdev->ncci_list_mtx);
-			nccip = capincci_find(cdev, (u32) ncci);
-			if (!nccip || (mp = nccip->minorp) == NULL) {
-				mutex_unlock(&cdev->ncci_list_mtx);
-				return -ESRCH;
-			}
-			unit = mp->minor;
-			mutex_unlock(&cdev->ncci_list_mtx);
-			return unit;
+	case CAPI_NCCI_GETUNIT: {
+		struct capincci *nccip;
+		struct capiminor *mp;
+		unsigned ncci;
+		int unit = -ESRCH;
+
+		if (copy_from_user(&ncci, argp, sizeof(ncci)))
+			return -EFAULT;
+
+		mutex_lock(&cdev->lock);
+		nccip = capincci_find(cdev, (u32)ncci);
+		if (nccip) {
+			mp = nccip->minorp;
+			if (mp)
+				unit = mp->minor;
 		}
-		return 0;
+		mutex_unlock(&cdev->lock);
+		return unit;
+	}
 #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+
+	default:
+		return -EINVAL;
 	}
-	return -EINVAL;
 }
 
 static int capi_open(struct inode *inode, struct file *file)
@@ -959,7 +957,7 @@ static int capi_open(struct inode *inode, struct file *file)
 	if (!cdev)
 		return -ENOMEM;
 
-	mutex_init(&cdev->ncci_list_mtx);
+	mutex_init(&cdev->lock);
 	skb_queue_head_init(&cdev->recvqueue);
 	init_waitqueue_head(&cdev->recvwait);
 	file->private_data = cdev;
@@ -979,15 +977,10 @@ static int capi_release(struct inode *inode, struct file *file)
 	list_del(&cdev->list);
 	mutex_unlock(&capidev_list_lock);
 
-	if (cdev->ap.applid) {
+	if (cdev->ap.applid)
 		capi20_release(&cdev->ap);
-		cdev->ap.applid = 0;
-	}
 	skb_queue_purge(&cdev->recvqueue);
-
-	mutex_lock(&cdev->ncci_list_mtx);
 	capincci_free(cdev, 0xffffffff);
-	mutex_unlock(&cdev->ncci_list_mtx);
 
 	kfree(cdev);
 	return 0;
@@ -1446,11 +1439,13 @@ static int capi20ncci_proc_show(struct seq_file *m, void *v)
 	mutex_lock(&capidev_list_lock);
 	list_for_each(l, &capidev_list) {
 		cdev = list_entry(l, struct capidev, list);
+		mutex_lock(&cdev->lock);
 		for (np=cdev->nccis; np; np = np->next) {
 			seq_printf(m, "%d 0x%x\n",
 				       cdev->ap.applid,
 				       np->ncci);
 		}
+		mutex_unlock(&cdev->lock);
 	}
 	mutex_unlock(&capidev_list_lock);
 	return 0;
-- 
1.6.0.2

^ permalink raw reply related


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