Here is the second take of my CAPI rework. I tried to address all
feedback on v1, so this one comes with the following changes:
o rebased over net-next
o reworked locking of the NCCI TTY using latest and greatest tty_port
features and kref, hopefully in the right way
o more fine-grained patch steps for the tricky TTY rework
o dynamic major for NCCI TTYs, ie. CAPI no longer claims 191
o dynamic TTY minor registrations, completely obsoleting capifs
o schedule capifs for removal
o some small additional cleanups
I kept my capifs fixes though this thing should die in the future. The
work is done, and people may still use it due to their distro
configuration (and the hard-wired loading by capiinit - needs fixing).
You can pull this series from
git://git.kiszka.org/linux-2.6.git capi
Jan
Jan Kiszka (41):
CAPI: Fix leaks in capifs_new_ncci
CAPI: Sanitize capifs API
CAPI: Eliminate capifs_root variable
CAPI: Pin capifs instead of mounting it
CAPI: Reduce chattiness during module loading/removal
CAPI: Call a controller 'controller', not 'card'
CAPI: Convert capi drivers rwlock into mutex
CAPI: Rework capi_ctr_ready/down
CAPI: Rework controller state notifier
CAPI: Rework locking of controller data structures
CAPI: Rework application locking
CAPI: Reduce #ifdef mess around CONFIG_ISDN_CAPI_MIDDLEWARE
CAPI: Convert capidev_list_lock into a mutex
CAPI: Clean up capi_open/release
CAPI: Rework locking of capidev members
CAPI: Use non-atomic allocation during NCCI setup
CAPI: Fix racy capi_read
CAPI: Switch NCCI list to standard doubly linked list
CAPI: Switch capiminor list to array
CAPI: Clean up capinc_tty_init/exit
CAPI: Dynamically register minor devices
CAPI: Use dynamic major for NCCI TTYs by default
CAPI: Use kref on capiminor
CAPI: Establish install/cleanup handlers for capiminor TTYs
CAPI: Use tty_port to keep track of capiminor's tty
CAPI: Drop remaining NULL checks on tty->driver_data
CAPI: Issue synchronous hangup on capincci_free_minor
CAPI: Drop obsolete nccip from capiminor struct
CAPI: Clean up capiminors_lock
CAPI: Drop atomic ttyopencount
CAPI: Drop handle_minor_recv from capinc_tty_write
CAPI: Rework capiminor RX handler
CAPI: Rename datahandle_queue -> ackqueue_entry
CAPI: Use atomics for capiminor's datahandle and msgid
CAPI: Drop capiminor's unused inbytes counter
CAPI: Fix locking around capiminor's output queue and drop
workaround_lock
CAPI: Clean up capiminor_*_ack
CAPI: Drop return value of handle_minor_send
CAPI: Drop special controller lookup from capi20_put_message
CAPI: Schedule capifs for removal
CAPI: Remove experimental tag from middleware feature
Documentation/feature-removal-schedule.txt | 10 +
drivers/isdn/capi/Kconfig | 16 +-
drivers/isdn/capi/capi.c | 1108 ++++++++++++++--------------
drivers/isdn/capi/capidrv.c | 48 +-
drivers/isdn/capi/capifs.c | 126 ++--
drivers/isdn/capi/capifs.h | 21 +-
drivers/isdn/capi/kcapi.c | 805 +++++++++++---------
drivers/isdn/capi/kcapi.h | 13 +-
drivers/isdn/capi/kcapi_proc.c | 41 +-
include/linux/isdn/capilli.h | 5 +-
include/linux/kernelcapi.h | 17 +-
11 files changed, 1151 insertions(+), 1059 deletions(-)
The CVS revisions dumped by all CAPI modules are meaningless today. And
that some CAPI module is loaded or removed does not necessarily deserve
a message. Just keep the message of the central module, capi.ko, drop
the rest.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 19 +++----------------
drivers/isdn/capi/capidrv.c | 26 --------------------------
drivers/isdn/capi/capifs.c | 20 +-------------------
drivers/isdn/capi/kcapi.c | 27 +++++----------------------
4 files changed, 9 insertions(+), 83 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index dc5ac52..3b07797 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -45,8 +45,6 @@
#include "capifs.h"
-static char *revision = "$Revision: 1.1.2.7 $";
-
MODULE_DESCRIPTION("CAPI4Linux: Userspace /dev/capi20 interface");
MODULE_AUTHOR("Carsten Paeth");
MODULE_LICENSE("GPL");
@@ -1489,21 +1487,11 @@ static void __exit proc_exit(void)
/* -------- init function and module interface ---------------------- */
-static char rev[32];
-
static int __init capi_init(void)
{
- char *p;
- char *compileinfo;
+ const char *compileinfo;
int major_ret;
- if ((p = strchr(revision, ':')) != NULL && p[1]) {
- strlcpy(rev, p + 2, sizeof(rev));
- if ((p = strchr(rev, '$')) != NULL && p > rev)
- *(p-1) = 0;
- } else
- strcpy(rev, "1.0");
-
major_ret = register_chrdev(capi_major, "capi20", &capi_fops);
if (major_ret < 0) {
printk(KERN_ERR "capi20: unable to get major %d\n", capi_major);
@@ -1537,8 +1525,8 @@ static int __init capi_init(void)
#else
compileinfo = " (no middleware)";
#endif
- printk(KERN_NOTICE "capi20: Rev %s: started up with major %d%s\n",
- rev, capi_major, compileinfo);
+ printk(KERN_NOTICE "CAPI 2.0 started up with major %d%s\n",
+ capi_major, compileinfo);
return 0;
}
@@ -1554,7 +1542,6 @@ static void __exit capi_exit(void)
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
capinc_tty_exit();
#endif
- printk(KERN_NOTICE "capi: Rev %s: unloaded\n", rev);
}
module_init(capi_init);
diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
index bb45015..7d8899a 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -35,7 +35,6 @@
#include <linux/isdn/capicmd.h>
#include "capidrv.h"
-static char *revision = "$Revision: 1.1.2.2 $";
static int debugmode = 0;
MODULE_DESCRIPTION("CAPI4Linux: Interface to ISDN4Linux");
@@ -2266,19 +2265,9 @@ static void __exit proc_exit(void)
static int __init capidrv_init(void)
{
capi_profile profile;
- char rev[32];
- char *p;
u32 ncontr, contr;
u16 errcode;
- if ((p = strchr(revision, ':')) != NULL && p[1]) {
- strncpy(rev, p + 2, sizeof(rev));
- rev[sizeof(rev)-1] = 0;
- if ((p = strchr(rev, '$')) != NULL && p > rev)
- *(p-1) = 0;
- } else
- strcpy(rev, "1.0");
-
global.ap.rparam.level3cnt = -2; /* number of bchannels twice */
global.ap.rparam.datablkcnt = 16;
global.ap.rparam.datablklen = 2048;
@@ -2306,29 +2295,14 @@ static int __init capidrv_init(void)
}
proc_init();
- printk(KERN_NOTICE "capidrv: Rev %s: loaded\n", rev);
return 0;
}
static void __exit capidrv_exit(void)
{
- char rev[32];
- char *p;
-
- if ((p = strchr(revision, ':')) != NULL) {
- strncpy(rev, p + 1, sizeof(rev));
- rev[sizeof(rev)-1] = 0;
- if ((p = strchr(rev, '$')) != NULL)
- *p = 0;
- } else {
- strcpy(rev, " ??? ");
- }
-
capi20_release(&global.ap);
proc_exit();
-
- printk(KERN_NOTICE "capidrv: Rev%s: unloaded\n", rev);
}
module_init(capidrv_init);
diff --git a/drivers/isdn/capi/capifs.c b/drivers/isdn/capi/capifs.c
index 51c01ef..8596bd1 100644
--- a/drivers/isdn/capi/capifs.c
+++ b/drivers/isdn/capi/capifs.c
@@ -25,10 +25,6 @@ MODULE_LICENSE("GPL");
/* ------------------------------------------------------------------ */
-static char *revision = "$Revision: 1.1.2.3 $";
-
-/* ------------------------------------------------------------------ */
-
#define CAPIFS_SUPER_MAGIC (('C'<<8)|'N')
static struct vfsmount *capifs_mnt;
@@ -227,21 +223,7 @@ void capifs_free_ncci(struct dentry *dentry)
static int __init capifs_init(void)
{
- char rev[32];
- char *p;
- int err;
-
- if ((p = strchr(revision, ':')) != NULL && p[1]) {
- strlcpy(rev, p + 2, sizeof(rev));
- if ((p = strchr(rev, '$')) != NULL && p > rev)
- *(p-1) = 0;
- } else
- strcpy(rev, "1.0");
-
- err = register_filesystem(&capifs_fs_type);
- if (!err)
- printk(KERN_NOTICE "capifs: Rev %s\n", rev);
- return err;
+ return register_filesystem(&capifs_fs_type);
}
static void __exit capifs_exit(void)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index b0bacf3..ef564ee 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -35,10 +35,6 @@
#endif
#include <linux/mutex.h>
-static char *revision = "$Revision: 1.1.2.8 $";
-
-/* ------------------------------------------------------------- */
-
static int showcapimsgs = 0;
MODULE_DESCRIPTION("CAPI4Linux: kernel CAPI layer");
@@ -1165,25 +1161,12 @@ EXPORT_SYMBOL(capi20_set_callback);
static int __init kcapi_init(void)
{
- char *p;
- char rev[32];
- int ret;
-
- ret = cdebug_init();
- if (ret)
- return ret;
- kcapi_proc_init();
+ int err;
- if ((p = strchr(revision, ':')) != NULL && p[1]) {
- strlcpy(rev, p + 2, sizeof(rev));
- if ((p = strchr(rev, '$')) != NULL && p > rev)
- *(p-1) = 0;
- } else
- strcpy(rev, "1.0");
-
- printk(KERN_NOTICE "CAPI Subsystem Rev %s\n", rev);
-
- return 0;
+ err = cdebug_init();
+ if (!err)
+ kcapi_proc_init();
+ return err;
}
static void __exit kcapi_exit(void)
--
1.6.0.2
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Despite all its bugs, the middleware support of our CAPI stack was
already in use for many, many moons. And after going through its code,
fixing all issues I found, I feel it deserves to officially become a
non-experimental feature.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/Kconfig | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/isdn/capi/Kconfig b/drivers/isdn/capi/Kconfig
index d607d46..a168e8a 100644
--- a/drivers/isdn/capi/Kconfig
+++ b/drivers/isdn/capi/Kconfig
@@ -17,8 +17,7 @@ config CAPI_TRACE
If unsure, say Y.
config ISDN_CAPI_MIDDLEWARE
- bool "CAPI2.0 Middleware support (EXPERIMENTAL)"
- depends on EXPERIMENTAL
+ bool "CAPI2.0 Middleware support"
help
This option will enhance the capabilities of the /dev/capi20
interface. It will provide a means of moving a data connection,
--
1.6.0.2
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 <[email protected]>
---
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
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 <[email protected]>
---
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 <[email protected]>
+
+----------------------------
+
+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 <[email protected]>
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
capincci_free and, thus, capincci_free_minor runs in process context, so
we can issue the hangup of the associated TTY synchronously.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index acc811b..b382ede 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -351,7 +351,7 @@ static void capincci_free_minor(struct capincci *np)
#ifdef _DEBUG_REFCOUNT
printk(KERN_DEBUG "reset mp->nccip\n");
#endif
- tty_hangup(tty);
+ tty_vhangup(tty);
tty_kref_put(tty);
}
--
1.6.0.2
The inbytes counter was only updated but never read.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 10 +++-------
1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 08d5a8a..be85c8c 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -98,7 +98,6 @@ struct capiminor {
struct sk_buff *ttyskb;
struct sk_buff_head inqueue;
- int inbytes;
struct sk_buff_head outqueue;
int outbytes;
@@ -520,15 +519,12 @@ deref_tty:
static void handle_minor_recv(struct capiminor *mp)
{
struct sk_buff *skb;
- while ((skb = skb_dequeue(&mp->inqueue)) != NULL) {
- unsigned int len = skb->len;
- mp->inbytes -= len;
+
+ while ((skb = skb_dequeue(&mp->inqueue)) != NULL)
if (handle_recv_skb(mp, skb) < 0) {
skb_queue_head(&mp->inqueue, skb);
- mp->inbytes += len;
return;
}
- }
}
static int handle_minor_send(struct capiminor *mp)
@@ -659,7 +655,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
datahandle, skb->len-CAPIMSG_LEN(skb->data));
#endif
skb_queue_tail(&mp->inqueue, skb);
- mp->inbytes += skb->len;
+
handle_minor_recv(mp);
} else if (CAPIMSG_SUBCOMMAND(skb->data) == CAPI_CONF) {
--
1.6.0.2
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Avoid re-queuing skbs unless the error detected in handle_recv_skb is
expected to be recoverable such as lacking memory, a full CAPI queue, a
full TTY input buffer, or a not yet existing TTY.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 60 +++++++++++++++++++++++++++++-----------------
1 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 554fa1b..c5c54fa 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -436,15 +436,13 @@ gen_data_b3_resp_for(struct capiminor *mp, struct sk_buff *skb)
static int handle_recv_skb(struct capiminor *mp, struct sk_buff *skb)
{
+ unsigned int datalen = skb->len - CAPIMSG_LEN(skb->data);
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);
-
tty = tty_port_tty_get(&mp->port);
if (!tty) {
#ifdef _DEBUG_DATAFLOW
@@ -454,50 +452,68 @@ static int handle_recv_skb(struct capiminor *mp, struct sk_buff *skb)
}
ld = tty_ldisc_ref(tty);
- if (!ld)
- goto out1;
+ if (!ld) {
+ /* fatal error, do not requeue */
+ ret = 0;
+ kfree_skb(skb);
+ goto deref_tty;
+ }
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 out2;
+ /* fatal error, do not requeue */
+ goto free_skb;
}
if (mp->ttyinstop) {
#if defined(_DEBUG_DATAFLOW) || defined(_DEBUG_TTYFUNCS)
printk(KERN_DEBUG "capi: recv tty throttled\n");
#endif
- goto out2;
+ goto deref_ldisc;
}
+
if (tty->receive_room < datalen) {
#if defined(_DEBUG_DATAFLOW) || defined(_DEBUG_TTYFUNCS)
printk(KERN_DEBUG "capi: no room in tty\n");
#endif
- goto out2;
+ goto deref_ldisc;
}
- if ((nskb = gen_data_b3_resp_for(mp, skb)) == NULL) {
+
+ nskb = gen_data_b3_resp_for(mp, skb);
+ if (!nskb) {
printk(KERN_ERR "capi: gen_data_b3_resp failed\n");
- goto out2;
+ goto deref_ldisc;
}
- datahandle = CAPIMSG_U16(skb->data,CAPIMSG_BASELEN+4);
+
+ datahandle = CAPIMSG_U16(skb->data, CAPIMSG_BASELEN + 4);
+
errcode = capi20_put_message(mp->ap, nskb);
- if (errcode != CAPI_NOERROR) {
+
+ if (errcode == CAPI_NOERROR) {
+ 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(tty, skb->data, NULL, skb->len);
+ } else {
printk(KERN_ERR "capi: send DATA_B3_RESP failed=%x\n",
errcode);
kfree_skb(nskb);
- goto out2;
+
+ if (errcode == CAPI_SENDQUEUEFULL)
+ goto deref_ldisc;
}
- (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(tty, skb->data, NULL, skb->len);
- kfree_skb(skb);
+
+free_skb:
ret = 0;
-out2:
+ kfree_skb(skb);
+
+deref_ldisc:
tty_ldisc_deref(ld);
-out1:
+
+deref_tty:
tty_kref_put(tty);
return ret;
}
--
1.6.0.2
The capiminor members datahandle and msgid are incremented outside any
lock, so better do this atomically.
Signed-off-by: Jan Kiszka <[email protected]>
---
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
We did not evaluate handle_minor_send's return value, just (void)'ed it
away. Time for a cleanup.
Signed-off-by: Jan Kiszka <[email protected]>
---
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
No need for irqsave acquisition of acklock, bh-safe is sufficient.
Moverover, move kfree out of the lock and do not take acklock at all
in capiminor_del_all_ack as we are the last user of the list here.
Signed-off-by: Jan Kiszka <[email protected]>
---
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 074496f..40b81b4 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -149,7 +149,6 @@ static struct tty_driver *capinc_tty_driver;
static int capiminor_add_ack(struct capiminor *mp, u16 datahandle)
{
struct ackqueue_entry *n;
- unsigned long flags;
n = kmalloc(sizeof(*n), GFP_ATOMIC);
if (unlikely(!n)) {
@@ -158,44 +157,40 @@ static int capiminor_add_ack(struct capiminor *mp, u16 datahandle)
}
n->datahandle = datahandle;
INIT_LIST_HEAD(&n->list);
- spin_lock_irqsave(&mp->ackqlock, flags);
+ spin_lock_bh(&mp->ackqlock);
list_add_tail(&n->list, &mp->ackqueue);
mp->nack++;
- spin_unlock_irqrestore(&mp->ackqlock, flags);
+ spin_unlock_bh(&mp->ackqlock);
return 0;
}
static int capiminor_del_ack(struct capiminor *mp, u16 datahandle)
{
struct ackqueue_entry *p, *tmp;
- unsigned long flags;
- spin_lock_irqsave(&mp->ackqlock, flags);
+ spin_lock_bh(&mp->ackqlock);
list_for_each_entry_safe(p, tmp, &mp->ackqueue, list) {
if (p->datahandle == datahandle) {
list_del(&p->list);
- kfree(p);
mp->nack--;
- spin_unlock_irqrestore(&mp->ackqlock, flags);
+ spin_unlock_bh(&mp->ackqlock);
+ kfree(p);
return 0;
}
}
- spin_unlock_irqrestore(&mp->ackqlock, flags);
+ spin_unlock_bh(&mp->ackqlock);
return -1;
}
static void capiminor_del_all_ack(struct capiminor *mp)
{
struct ackqueue_entry *p, *tmp;
- unsigned long flags;
- spin_lock_irqsave(&mp->ackqlock, flags);
list_for_each_entry_safe(p, tmp, &mp->ackqueue, list) {
list_del(&p->list);
kfree(p);
mp->nack--;
}
- spin_unlock_irqrestore(&mp->ackqlock, flags);
}
@@ -676,7 +671,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
CAPIMSG_U16(skb->data, CAPIMSG_BASELEN+4+2));
#endif
kfree_skb(skb);
- (void)capiminor_del_ack(mp, datahandle);
+ capiminor_del_ack(mp, datahandle);
tty = tty_port_tty_get(&mp->port);
if (tty) {
tty_wakeup(tty);
--
1.6.0.2
When something went wrong during capifs_new_ncci, the looked up dentry
was not properly released. Neither was the allocated inode. Refactor the
function to avoid leaks.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capifs.c | 25 ++++++++++++++++++-------
1 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/isdn/capi/capifs.c b/drivers/isdn/capi/capifs.c
index 9f8f67b..dc68fcb 100644
--- a/drivers/isdn/capi/capifs.c
+++ b/drivers/isdn/capi/capifs.c
@@ -152,22 +152,33 @@ static struct dentry *get_node(int num)
void capifs_new_ncci(unsigned int number, dev_t device)
{
struct dentry *dentry;
- struct inode *inode = new_inode(capifs_mnt->mnt_sb);
- if (!inode)
- return;
- inode->i_ino = number+2;
+ struct inode *inode;
dentry = get_node(number);
+ if (IS_ERR(dentry))
+ goto unlock_out;
+
+ if (dentry->d_inode) {
+ dput(dentry);
+ goto unlock_out;
+ }
+
+ inode = new_inode(capifs_mnt->mnt_sb);
+ if (!inode) {
+ dput(dentry);
+ goto unlock_out;
+ }
/* config contents is protected by root's i_mutex */
inode->i_uid = config.setuid ? config.uid : current_fsuid();
inode->i_gid = config.setgid ? config.gid : current_fsgid();
inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
+ inode->i_ino = number + 2;
init_special_inode(inode, S_IFCHR|config.mode, device);
- //inode->i_op = &capifs_file_inode_operations;
- if (!IS_ERR(dentry) && !dentry->d_inode)
- d_instantiate(dentry, inode);
+ d_instantiate(dentry, inode);
+
+unlock_out:
mutex_unlock(&capifs_root->d_inode->i_mutex);
}
--
1.6.0.2
Properly associate/disassociate a capiminor object with its TTY via the
install/cleanup handlers instead of trying to guess first open and last
close.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 42 ++++++++++++++++++++++++++++--------------
1 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 732cdb5..3e4997a 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -1005,16 +1005,34 @@ static const struct file_operations capi_fops =
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
/* -------- tty_operations for capincci ----------------------------- */
-static int capinc_tty_open(struct tty_struct * tty, struct file * file)
+static int
+capinc_tty_install(struct tty_driver *driver, struct tty_struct *tty)
{
- struct capiminor *mp;
- unsigned long flags;
+ int idx = tty->index;
+ struct capiminor *mp = capiminor_get(idx);
+ int ret = tty_init_termios(tty);
+
+ if (ret == 0) {
+ tty_driver_kref_get(driver);
+ tty->count++;
+ tty->driver_data = mp;
+ driver->ttys[idx] = tty;
+ } else
+ capiminor_put(mp);
+ return ret;
+}
- mp = capiminor_get(iminor(file->f_path.dentry->d_inode));
- if (mp->nccip == NULL)
- return -ENXIO;
+static void capinc_tty_cleanup(struct tty_struct *tty)
+{
+ struct capiminor *mp = tty->driver_data;
+ tty->driver_data = NULL;
+ capiminor_put(mp);
+}
- tty->driver_data = (void *)mp;
+static int capinc_tty_open(struct tty_struct * tty, struct file * file)
+{
+ struct capiminor *mp = tty->driver_data;
+ unsigned long flags;
spin_lock_irqsave(&workaround_lock, flags);
if (atomic_read(&mp->ttyopencount) == 0)
@@ -1030,15 +1048,12 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file)
static void capinc_tty_close(struct tty_struct * tty, struct file * file)
{
- struct capiminor *mp;
+ struct capiminor *mp = tty->driver_data;
- mp = (struct capiminor *)tty->driver_data;
- if (mp) {
if (atomic_dec_and_test(&mp->ttyopencount)) {
#ifdef _DEBUG_REFCOUNT
printk(KERN_DEBUG "capinc_tty_close lastclose\n");
#endif
- tty->driver_data = NULL;
mp->tty = NULL;
}
#ifdef _DEBUG_REFCOUNT
@@ -1047,9 +1062,6 @@ static void capinc_tty_close(struct tty_struct * tty, struct file * file)
if (mp->nccip == NULL)
capiminor_free(mp);
- capiminor_put(mp);
- }
-
#ifdef _DEBUG_REFCOUNT
printk(KERN_DEBUG "capinc_tty_close\n");
#endif
@@ -1333,6 +1345,8 @@ static const struct tty_operations capinc_ops = {
.flush_buffer = capinc_tty_flush_buffer,
.set_ldisc = capinc_tty_set_ldisc,
.send_xchar = capinc_tty_send_xchar,
+ .install = capinc_tty_install,
+ .cleanup = capinc_tty_cleanup,
};
static int __init capinc_tty_init(void)
--
1.6.0.2
Replace open-coded NCCI list management with standard mechanisms.
Signed-off-by: Jan Kiszka <[email protected]>
---
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
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 <[email protected]>
---
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
Both capincci_alloc and capiminor_alloc run in non-atomic context,
update their memory allocations accordingly.
Signed-off-by: Jan Kiszka <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Not needed, tty->count keeps track of this information. At this chance,
drop traces of ancient attempts to debug this logic via _DEBUG_REFCOUNT.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 30 ++++++++++--------------------
1 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 0b264b4..867589a 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -45,7 +45,6 @@ MODULE_DESCRIPTION("CAPI4Linux: Userspace /dev/capi20 interface");
MODULE_AUTHOR("Carsten Paeth");
MODULE_LICENSE("GPL");
-#undef _DEBUG_REFCOUNT /* alloc/free and open/close debug */
#undef _DEBUG_TTYFUNCS /* call to tty_driver */
#undef _DEBUG_DATAFLOW /* data flow */
@@ -97,7 +96,6 @@ struct capiminor {
int ttyinstop;
int ttyoutstop;
struct sk_buff *ttyskb;
- atomic_t ttyopencount;
struct sk_buff_head inqueue;
int inbytes;
@@ -230,7 +228,6 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
mp->ap = ap;
mp->ncci = ncci;
mp->msgid = 0;
- atomic_set(&mp->ttyopencount,0);
INIT_LIST_HEAD(&mp->ackqueue);
spin_lock_init(&mp->ackqlock);
@@ -350,8 +347,17 @@ static void capincci_free_minor(struct capincci *np)
static inline unsigned int capincci_minor_opencount(struct capincci *np)
{
struct capiminor *mp = np->minorp;
+ unsigned int count = 0;
+ struct tty_struct *tty;
- return mp ? atomic_read(&mp->ttyopencount) : 0;
+ if (mp) {
+ tty = tty_port_tty_get(&mp->port);
+ if (tty) {
+ count = tty->count;
+ tty_kref_put(tty);
+ }
+ }
+ return count;
}
#else /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
@@ -1054,10 +1060,6 @@ static int capinc_tty_open(struct tty_struct *tty, struct file *filp)
return err;
spin_lock_irqsave(&workaround_lock, flags);
- atomic_inc(&mp->ttyopencount);
-#ifdef _DEBUG_REFCOUNT
- printk(KERN_DEBUG "capinc_tty_open ocount=%d\n", atomic_read(&mp->ttyopencount));
-#endif
handle_minor_recv(mp);
spin_unlock_irqrestore(&workaround_lock, flags);
return 0;
@@ -1067,18 +1069,6 @@ static void capinc_tty_close(struct tty_struct *tty, struct file *filp)
{
struct capiminor *mp = tty->driver_data;
- if (atomic_dec_and_test(&mp->ttyopencount)) {
-#ifdef _DEBUG_REFCOUNT
- printk(KERN_DEBUG "capinc_tty_close lastclose\n");
-#endif
- }
-#ifdef _DEBUG_REFCOUNT
- printk(KERN_DEBUG "capinc_tty_close ocount=%d\n", atomic_read(&mp->ttyopencount));
-#endif
-
-#ifdef _DEBUG_REFCOUNT
- printk(KERN_DEBUG "capinc_tty_close\n");
-#endif
tty_port_close(&mp->port, tty, filp);
}
--
1.6.0.2
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 <[email protected]>
---
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
This struct is describing a queue entry, not the queue itself.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index c5c54fa..9d4750a 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -76,7 +76,7 @@ struct capidev;
struct capincci;
struct capiminor;
-struct datahandle_queue {
+struct ackqueue_entry {
struct list_head list;
u16 datahandle;
};
@@ -156,7 +156,7 @@ static struct tty_driver *capinc_tty_driver;
static int capiminor_add_ack(struct capiminor *mp, u16 datahandle)
{
- struct datahandle_queue *n;
+ struct ackqueue_entry *n;
unsigned long flags;
n = kmalloc(sizeof(*n), GFP_ATOMIC);
@@ -175,7 +175,7 @@ static int capiminor_add_ack(struct capiminor *mp, u16 datahandle)
static int capiminor_del_ack(struct capiminor *mp, u16 datahandle)
{
- struct datahandle_queue *p, *tmp;
+ struct ackqueue_entry *p, *tmp;
unsigned long flags;
spin_lock_irqsave(&mp->ackqlock, flags);
@@ -194,7 +194,7 @@ static int capiminor_del_ack(struct capiminor *mp, u16 datahandle)
static void capiminor_del_all_ack(struct capiminor *mp)
{
- struct datahandle_queue *p, *tmp;
+ struct ackqueue_entry *p, *tmp;
unsigned long flags;
spin_lock_irqsave(&mp->ackqlock, flags);
--
1.6.0.2
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Instead of looking up the dentry of an NCCI node again in
capifs_free_ncci pass the pointer via the capifs user.
This patch also reduces the #ifdef mess in capi.c a bit as far as capifs
was causing it.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 14 +++++-------
drivers/isdn/capi/capifs.c | 50 +++++++++++++++++++++++++------------------
drivers/isdn/capi/capifs.h | 21 ++++++++++++++++-
3 files changed, 54 insertions(+), 31 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 79f9364..dc5ac52 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -42,9 +42,8 @@
#include <linux/moduleparam.h>
#include <linux/isdn/capiutil.h>
#include <linux/isdn/capicmd.h>
-#if defined(CONFIG_ISDN_CAPI_CAPIFS) || defined(CONFIG_ISDN_CAPI_CAPIFS_MODULE)
+
#include "capifs.h"
-#endif
static char *revision = "$Revision: 1.1.2.7 $";
@@ -96,6 +95,7 @@ struct capiminor {
struct list_head list;
struct capincci *nccip;
unsigned int minor;
+ struct dentry *capifs_dentry;
struct capi20_appl *ap;
u32 ncci;
@@ -328,9 +328,9 @@ static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
#ifdef _DEBUG_REFCOUNT
printk(KERN_DEBUG "set mp->nccip\n");
#endif
-#if defined(CONFIG_ISDN_CAPI_CAPIFS) || defined(CONFIG_ISDN_CAPI_CAPIFS_MODULE)
- capifs_new_ncci(mp->minor, MKDEV(capi_ttymajor, mp->minor));
-#endif
+ mp->capifs_dentry =
+ capifs_new_ncci(mp->minor,
+ MKDEV(capi_ttymajor, mp->minor));
}
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
for (pp=&cdev->nccis; *pp; pp = &(*pp)->next)
@@ -353,9 +353,7 @@ static void capincci_free(struct capidev *cdev, u32 ncci)
*pp = (*pp)->next;
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
if ((mp = np->minorp) != NULL) {
-#if defined(CONFIG_ISDN_CAPI_CAPIFS) || defined(CONFIG_ISDN_CAPI_CAPIFS_MODULE)
- capifs_free_ncci(mp->minor);
-#endif
+ capifs_free_ncci(mp->capifs_dentry);
if (mp->tty) {
mp->nccip = NULL;
#ifdef _DEBUG_REFCOUNT
diff --git a/drivers/isdn/capi/capifs.c b/drivers/isdn/capi/capifs.c
index dc68fcb..91aafad 100644
--- a/drivers/isdn/capi/capifs.c
+++ b/drivers/isdn/capi/capifs.c
@@ -141,31 +141,32 @@ static struct file_system_type capifs_fs_type = {
.kill_sb = kill_anon_super,
};
-static struct dentry *get_node(int num)
-{
- char s[10];
- struct dentry *root = capifs_root;
- mutex_lock(&root->d_inode->i_mutex);
- return lookup_one_len(s, root, sprintf(s, "%d", num));
-}
-
-void capifs_new_ncci(unsigned int number, dev_t device)
+struct dentry *capifs_new_ncci(unsigned int number, dev_t device)
{
struct dentry *dentry;
struct inode *inode;
+ char name[10];
+ int namelen;
- dentry = get_node(number);
- if (IS_ERR(dentry))
+ mutex_lock(&capifs_root->d_inode->i_mutex);
+
+ namelen = sprintf(name, "%d", number);
+ dentry = lookup_one_len(name, capifs_root, namelen);
+ if (IS_ERR(dentry)) {
+ dentry = NULL;
goto unlock_out;
+ }
if (dentry->d_inode) {
dput(dentry);
+ dentry = NULL;
goto unlock_out;
}
inode = new_inode(capifs_mnt->mnt_sb);
if (!inode) {
dput(dentry);
+ dentry = NULL;
goto unlock_out;
}
@@ -177,24 +178,31 @@ void capifs_new_ncci(unsigned int number, dev_t device)
init_special_inode(inode, S_IFCHR|config.mode, device);
d_instantiate(dentry, inode);
+ dget(dentry);
unlock_out:
mutex_unlock(&capifs_root->d_inode->i_mutex);
+
+ return dentry;
}
-void capifs_free_ncci(unsigned int number)
+void capifs_free_ncci(struct dentry *dentry)
{
- struct dentry *dentry = get_node(number);
-
- if (!IS_ERR(dentry)) {
- struct inode *inode = dentry->d_inode;
- if (inode) {
- inode->i_nlink--;
- d_delete(dentry);
- dput(dentry);
- }
+ struct inode *inode;
+
+ if (!dentry)
+ return;
+
+ mutex_lock(&capifs_root->d_inode->i_mutex);
+
+ inode = dentry->d_inode;
+ if (inode) {
+ drop_nlink(inode);
+ d_delete(dentry);
dput(dentry);
}
+ dput(dentry);
+
mutex_unlock(&capifs_root->d_inode->i_mutex);
}
diff --git a/drivers/isdn/capi/capifs.h b/drivers/isdn/capi/capifs.h
index d0bd4c3..e193d11 100644
--- a/drivers/isdn/capi/capifs.h
+++ b/drivers/isdn/capi/capifs.h
@@ -7,5 +7,22 @@
*
*/
-void capifs_new_ncci(unsigned int num, dev_t device);
-void capifs_free_ncci(unsigned int num);
+#include <linux/dcache.h>
+
+#if defined(CONFIG_ISDN_CAPI_CAPIFS) || defined(CONFIG_ISDN_CAPI_CAPIFS_MODULE)
+
+struct dentry *capifs_new_ncci(unsigned int num, dev_t device);
+void capifs_free_ncci(struct dentry *dentry);
+
+#else
+
+static inline struct dentry *capifs_new_ncci(unsigned int num, dev_t device)
+{
+ return NULL;
+}
+
+static inline void capifs_free_ncci(struct dentry *dentry)
+{
+}
+
+#endif
--
1.6.0.2
Using a plain array of pointers simplifies the management of capiminors.
Signed-off-by: Jan Kiszka <[email protected]>
---
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
tty_struct's driver_data cannot be NULL, no need to test for it.
Signed-off-by: Jan Kiszka <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
At least for our internal use, fix the misnomers that refer to a CAPI
controller as 'card'. No functional changes.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/kcapi.c | 320 +++++++++++++++++++++-------------------
drivers/isdn/capi/kcapi.h | 8 +-
drivers/isdn/capi/kcapi_proc.c | 17 +-
include/linux/isdn/capilli.h | 2 +-
4 files changed, 178 insertions(+), 169 deletions(-)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index ef564ee..f37c13b 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -67,24 +67,24 @@ static DEFINE_RWLOCK(application_lock);
static DEFINE_MUTEX(controller_mutex);
struct capi20_appl *capi_applications[CAPI_MAXAPPL];
-struct capi_ctr *capi_cards[CAPI_MAXCONTR];
+struct capi_ctr *capi_controller[CAPI_MAXCONTR];
-static int ncards;
+static int ncontrollers;
/* -------- controller ref counting -------------------------------------- */
static inline struct capi_ctr *
-capi_ctr_get(struct capi_ctr *card)
+capi_ctr_get(struct capi_ctr *ctr)
{
- if (!try_module_get(card->owner))
+ if (!try_module_get(ctr->owner))
return NULL;
- return card;
+ return ctr;
}
static inline void
-capi_ctr_put(struct capi_ctr *card)
+capi_ctr_put(struct capi_ctr *ctr)
{
- module_put(card->owner);
+ module_put(ctr->owner);
}
/* ------------------------------------------------------------- */
@@ -94,7 +94,7 @@ static inline struct capi_ctr *get_capi_ctr_by_nr(u16 contr)
if (contr - 1 >= CAPI_MAXCONTR)
return NULL;
- return capi_cards[contr - 1];
+ return capi_controller[contr - 1];
}
static inline struct capi20_appl *get_capi_appl_by_nr(u16 applid)
@@ -144,46 +144,48 @@ static inline int capi_subcmd_valid(u8 subcmd)
/* ------------------------------------------------------------ */
-static void register_appl(struct capi_ctr *card, u16 applid, capi_register_params *rparam)
+static void
+register_appl(struct capi_ctr *ctr, u16 applid, capi_register_params *rparam)
{
- card = capi_ctr_get(card);
+ ctr = capi_ctr_get(ctr);
- if (card)
- card->register_appl(card, applid, rparam);
+ if (ctr)
+ ctr->register_appl(ctr, applid, rparam);
else
- printk(KERN_WARNING "%s: cannot get card resources\n", __func__);
+ printk(KERN_WARNING "%s: cannot get controller resources\n",
+ __func__);
}
-static void release_appl(struct capi_ctr *card, u16 applid)
+static void release_appl(struct capi_ctr *ctr, u16 applid)
{
DBG("applid %#x", applid);
- card->release_appl(card, applid);
- capi_ctr_put(card);
+ ctr->release_appl(ctr, applid);
+ capi_ctr_put(ctr);
}
/* -------- KCI_CONTRUP --------------------------------------- */
static void notify_up(u32 contr)
{
- struct capi_ctr *card = get_capi_ctr_by_nr(contr);
+ struct capi_ctr *ctr = get_capi_ctr_by_nr(contr);
struct capi20_appl *ap;
u16 applid;
if (showcapimsgs & 1) {
printk(KERN_DEBUG "kcapi: notify up contr %d\n", contr);
}
- if (!card) {
+ if (!ctr) {
printk(KERN_WARNING "%s: invalid contr %d\n", __func__, contr);
return;
}
for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
ap = get_capi_appl_by_nr(applid);
if (!ap || ap->release_in_progress) continue;
- register_appl(card, applid, &ap->rparam);
+ register_appl(ctr, applid, &ap->rparam);
if (ap->callback && !ap->release_in_progress)
- ap->callback(KCI_CONTRUP, contr, &card->profile);
+ ap->callback(KCI_CONTRUP, contr, &ctr->profile);
}
}
@@ -269,14 +271,15 @@ static void recv_handler(struct work_struct *work)
/**
* capi_ctr_handle_message() - handle incoming CAPI message
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
* @appl: application ID.
* @skb: message.
*
* Called by hardware driver to pass a CAPI message to the application.
*/
-void capi_ctr_handle_message(struct capi_ctr * card, u16 appl, struct sk_buff *skb)
+void capi_ctr_handle_message(struct capi_ctr *ctr, u16 appl,
+ struct sk_buff *skb)
{
struct capi20_appl *ap;
int showctl = 0;
@@ -284,43 +287,45 @@ void capi_ctr_handle_message(struct capi_ctr * card, u16 appl, struct sk_buff *s
unsigned long flags;
_cdebbuf *cdb;
- if (card->cardstate != CARD_RUNNING) {
+ if (ctr->state != CAPI_CTR_RUNNING) {
cdb = capi_message2str(skb->data);
if (cdb) {
printk(KERN_INFO "kcapi: controller [%03d] not active, got: %s",
- card->cnr, cdb->buf);
+ ctr->cnr, cdb->buf);
cdebbuf_free(cdb);
} else
printk(KERN_INFO "kcapi: controller [%03d] not active, cannot trace\n",
- card->cnr);
+ ctr->cnr);
goto error;
}
cmd = CAPIMSG_COMMAND(skb->data);
subcmd = CAPIMSG_SUBCOMMAND(skb->data);
if (cmd == CAPI_DATA_B3 && subcmd == CAPI_IND) {
- card->nrecvdatapkt++;
- if (card->traceflag > 2) showctl |= 2;
+ ctr->nrecvdatapkt++;
+ if (ctr->traceflag > 2)
+ showctl |= 2;
} else {
- card->nrecvctlpkt++;
- if (card->traceflag) showctl |= 2;
+ ctr->nrecvctlpkt++;
+ if (ctr->traceflag)
+ showctl |= 2;
}
- showctl |= (card->traceflag & 1);
+ showctl |= (ctr->traceflag & 1);
if (showctl & 2) {
if (showctl & 1) {
printk(KERN_DEBUG "kcapi: got [%03d] id#%d %s len=%u\n",
- card->cnr, CAPIMSG_APPID(skb->data),
+ ctr->cnr, CAPIMSG_APPID(skb->data),
capi_cmd2str(cmd, subcmd),
CAPIMSG_LEN(skb->data));
} else {
cdb = capi_message2str(skb->data);
if (cdb) {
printk(KERN_DEBUG "kcapi: got [%03d] %s\n",
- card->cnr, cdb->buf);
+ ctr->cnr, cdb->buf);
cdebbuf_free(cdb);
} else
printk(KERN_DEBUG "kcapi: got [%03d] id#%d %s len=%u, cannot trace\n",
- card->cnr, CAPIMSG_APPID(skb->data),
+ ctr->cnr, CAPIMSG_APPID(skb->data),
capi_cmd2str(cmd, subcmd),
CAPIMSG_LEN(skb->data));
}
@@ -356,74 +361,75 @@ EXPORT_SYMBOL(capi_ctr_handle_message);
/**
* capi_ctr_ready() - signal CAPI controller ready
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to signal that the controller is up and running.
*/
-void capi_ctr_ready(struct capi_ctr * card)
+void capi_ctr_ready(struct capi_ctr *ctr)
{
- card->cardstate = CARD_RUNNING;
+ ctr->state = CAPI_CTR_RUNNING;
- printk(KERN_NOTICE "kcapi: card [%03d] \"%s\" ready.\n",
- card->cnr, card->name);
+ printk(KERN_NOTICE "kcapi: controller [%03d] \"%s\" ready.\n",
+ ctr->cnr, ctr->name);
- notify_push(KCI_CONTRUP, card->cnr, 0, 0);
+ notify_push(KCI_CONTRUP, ctr->cnr, 0, 0);
}
EXPORT_SYMBOL(capi_ctr_ready);
/**
* capi_ctr_down() - signal CAPI controller not ready
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to signal that the controller is down and
* unavailable for use.
*/
-void capi_ctr_down(struct capi_ctr * card)
+void capi_ctr_down(struct capi_ctr *ctr)
{
u16 appl;
DBG("");
- if (card->cardstate == CARD_DETECTED)
+ if (ctr->state == CAPI_CTR_DETECTED)
return;
- card->cardstate = CARD_DETECTED;
+ ctr->state = CAPI_CTR_DETECTED;
- memset(card->manu, 0, sizeof(card->manu));
- memset(&card->version, 0, sizeof(card->version));
- memset(&card->profile, 0, sizeof(card->profile));
- memset(card->serial, 0, sizeof(card->serial));
+ memset(ctr->manu, 0, sizeof(ctr->manu));
+ memset(&ctr->version, 0, sizeof(ctr->version));
+ memset(&ctr->profile, 0, sizeof(ctr->profile));
+ memset(ctr->serial, 0, sizeof(ctr->serial));
for (appl = 1; appl <= CAPI_MAXAPPL; appl++) {
struct capi20_appl *ap = get_capi_appl_by_nr(appl);
if (!ap || ap->release_in_progress)
continue;
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
}
- printk(KERN_NOTICE "kcapi: card [%03d] down.\n", card->cnr);
+ printk(KERN_NOTICE "kcapi: controller [%03d] down.\n", ctr->cnr);
- notify_push(KCI_CONTRDOWN, card->cnr, 0, 0);
+ notify_push(KCI_CONTRDOWN, ctr->cnr, 0, 0);
}
EXPORT_SYMBOL(capi_ctr_down);
/**
* capi_ctr_suspend_output() - suspend controller
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to stop data flow.
*/
-void capi_ctr_suspend_output(struct capi_ctr *card)
+void capi_ctr_suspend_output(struct capi_ctr *ctr)
{
- if (!card->blocked) {
- printk(KERN_DEBUG "kcapi: card [%03d] suspend\n", card->cnr);
- card->blocked = 1;
+ if (!ctr->blocked) {
+ printk(KERN_DEBUG "kcapi: controller [%03d] suspend\n",
+ ctr->cnr);
+ ctr->blocked = 1;
}
}
@@ -431,16 +437,17 @@ EXPORT_SYMBOL(capi_ctr_suspend_output);
/**
* capi_ctr_resume_output() - resume controller
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to resume data flow.
*/
-void capi_ctr_resume_output(struct capi_ctr *card)
+void capi_ctr_resume_output(struct capi_ctr *ctr)
{
- if (card->blocked) {
- printk(KERN_DEBUG "kcapi: card [%03d] resume\n", card->cnr);
- card->blocked = 0;
+ if (ctr->blocked) {
+ printk(KERN_DEBUG "kcapi: controller [%03d] resumed\n",
+ ctr->cnr);
+ ctr->blocked = 0;
}
}
@@ -450,21 +457,20 @@ EXPORT_SYMBOL(capi_ctr_resume_output);
/**
* attach_capi_ctr() - register CAPI controller
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to register a controller with the CAPI subsystem.
* Return value: 0 on success, error code < 0 on error
*/
-int
-attach_capi_ctr(struct capi_ctr *card)
+int attach_capi_ctr(struct capi_ctr *ctr)
{
int i;
mutex_lock(&controller_mutex);
for (i = 0; i < CAPI_MAXCONTR; i++) {
- if (capi_cards[i] == NULL)
+ if (!capi_controller[i])
break;
}
if (i == CAPI_MAXCONTR) {
@@ -472,25 +478,25 @@ attach_capi_ctr(struct capi_ctr *card)
printk(KERN_ERR "kcapi: out of controller slots\n");
return -EBUSY;
}
- capi_cards[i] = card;
+ capi_controller[i] = ctr;
mutex_unlock(&controller_mutex);
- card->nrecvctlpkt = 0;
- card->nrecvdatapkt = 0;
- card->nsentctlpkt = 0;
- card->nsentdatapkt = 0;
- card->cnr = i + 1;
- card->cardstate = CARD_DETECTED;
- card->blocked = 0;
- card->traceflag = showcapimsgs;
-
- sprintf(card->procfn, "capi/controllers/%d", card->cnr);
- card->procent = proc_create_data(card->procfn, 0, NULL, card->proc_fops, card);
-
- ncards++;
- printk(KERN_NOTICE "kcapi: Controller [%03d]: %s attached\n",
- card->cnr, card->name);
+ ctr->nrecvctlpkt = 0;
+ ctr->nrecvdatapkt = 0;
+ ctr->nsentctlpkt = 0;
+ ctr->nsentdatapkt = 0;
+ ctr->cnr = i + 1;
+ ctr->state = CAPI_CTR_DETECTED;
+ ctr->blocked = 0;
+ ctr->traceflag = showcapimsgs;
+
+ sprintf(ctr->procfn, "capi/controllers/%d", ctr->cnr);
+ ctr->procent = proc_create_data(ctr->procfn, 0, NULL, ctr->proc_fops, ctr);
+
+ ncontrollers++;
+ printk(KERN_NOTICE "kcapi: controller [%03d]: %s attached\n",
+ ctr->cnr, ctr->name);
return 0;
}
@@ -498,27 +504,27 @@ EXPORT_SYMBOL(attach_capi_ctr);
/**
* detach_capi_ctr() - unregister CAPI controller
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to remove the registration of a controller
* with the CAPI subsystem.
* Return value: 0 on success, error code < 0 on error
*/
-int detach_capi_ctr(struct capi_ctr *card)
+int detach_capi_ctr(struct capi_ctr *ctr)
{
- if (card->cardstate != CARD_DETECTED)
- capi_ctr_down(card);
+ if (ctr->state != CAPI_CTR_DETECTED)
+ capi_ctr_down(ctr);
- ncards--;
+ ncontrollers--;
- if (card->procent) {
- remove_proc_entry(card->procfn, NULL);
- card->procent = NULL;
+ if (ctr->procent) {
+ remove_proc_entry(ctr->procfn, NULL);
+ ctr->procent = NULL;
}
- capi_cards[card->cnr - 1] = NULL;
- printk(KERN_NOTICE "kcapi: Controller [%03d]: %s unregistered\n",
- card->cnr, card->name);
+ capi_controller[ctr->cnr - 1] = NULL;
+ printk(KERN_NOTICE "kcapi: controller [%03d]: %s unregistered\n",
+ ctr->cnr, ctr->name);
return 0;
}
@@ -576,7 +582,8 @@ u16 capi20_isinstalled(void)
{
int i;
for (i = 0; i < CAPI_MAXCONTR; i++) {
- if (capi_cards[i] && capi_cards[i]->cardstate == CARD_RUNNING)
+ if (capi_controller[i] &&
+ capi_controller[i]->state == CAPI_CTR_RUNNING)
return CAPI_NOERROR;
}
return CAPI_REGNOTINSTALLED;
@@ -635,9 +642,10 @@ u16 capi20_register(struct capi20_appl *ap)
mutex_lock(&controller_mutex);
for (i = 0; i < CAPI_MAXCONTR; i++) {
- if (!capi_cards[i] || capi_cards[i]->cardstate != CARD_RUNNING)
+ if (!capi_controller[i] ||
+ capi_controller[i]->state != CAPI_CTR_RUNNING)
continue;
- register_appl(capi_cards[i], applid, &ap->rparam);
+ register_appl(capi_controller[i], applid, &ap->rparam);
}
mutex_unlock(&controller_mutex);
@@ -674,9 +682,10 @@ u16 capi20_release(struct capi20_appl *ap)
mutex_lock(&controller_mutex);
for (i = 0; i < CAPI_MAXCONTR; i++) {
- if (!capi_cards[i] || capi_cards[i]->cardstate != CARD_RUNNING)
+ if (!capi_controller[i] ||
+ capi_controller[i]->state != CAPI_CTR_RUNNING)
continue;
- release_appl(capi_cards[i], ap->applid);
+ release_appl(capi_controller[i], ap->applid);
}
mutex_unlock(&controller_mutex);
@@ -703,13 +712,13 @@ EXPORT_SYMBOL(capi20_release);
u16 capi20_put_message(struct capi20_appl *ap, struct sk_buff *skb)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
int showctl = 0;
u8 cmd, subcmd;
DBG("applid %#x", ap->applid);
- if (ncards == 0)
+ if (ncontrollers == 0)
return CAPI_REGNOTINSTALLED;
if ((ap->applid == 0) || ap->release_in_progress)
return CAPI_ILLAPPNR;
@@ -717,28 +726,30 @@ 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;
- card = get_capi_ctr_by_nr(CAPIMSG_CONTROLLER(skb->data));
- if (!card || card->cardstate != CARD_RUNNING) {
- card = get_capi_ctr_by_nr(1); // XXX why?
- if (!card || card->cardstate != CARD_RUNNING)
+ 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 (card->blocked)
+ if (ctr->blocked)
return CAPI_SENDQUEUEFULL;
cmd = CAPIMSG_COMMAND(skb->data);
subcmd = CAPIMSG_SUBCOMMAND(skb->data);
if (cmd == CAPI_DATA_B3 && subcmd== CAPI_REQ) {
- card->nsentdatapkt++;
+ ctr->nsentdatapkt++;
ap->nsentdatapkt++;
- if (card->traceflag > 2) showctl |= 2;
+ if (ctr->traceflag > 2)
+ showctl |= 2;
} else {
- card->nsentctlpkt++;
+ ctr->nsentctlpkt++;
ap->nsentctlpkt++;
- if (card->traceflag) showctl |= 2;
+ if (ctr->traceflag)
+ showctl |= 2;
}
- showctl |= (card->traceflag & 1);
+ showctl |= (ctr->traceflag & 1);
if (showctl & 2) {
if (showctl & 1) {
printk(KERN_DEBUG "kcapi: put [%03d] id#%d %s len=%u\n",
@@ -761,7 +772,7 @@ u16 capi20_put_message(struct capi20_appl *ap, struct sk_buff *skb)
CAPIMSG_LEN(skb->data));
}
}
- return card->send_message(card, skb);
+ return ctr->send_message(ctr, skb);
}
EXPORT_SYMBOL(capi20_put_message);
@@ -778,16 +789,16 @@ EXPORT_SYMBOL(capi20_put_message);
u16 capi20_get_manufacturer(u32 contr, u8 *buf)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
if (contr == 0) {
strlcpy(buf, capi_manufakturer, CAPI_MANUFACTURER_LEN);
return CAPI_NOERROR;
}
- card = get_capi_ctr_by_nr(contr);
- if (!card || card->cardstate != CARD_RUNNING)
+ ctr = get_capi_ctr_by_nr(contr);
+ if (!ctr || ctr->state != CAPI_CTR_RUNNING)
return CAPI_REGNOTINSTALLED;
- strlcpy(buf, card->manu, CAPI_MANUFACTURER_LEN);
+ strlcpy(buf, ctr->manu, CAPI_MANUFACTURER_LEN);
return CAPI_NOERROR;
}
@@ -805,17 +816,17 @@ EXPORT_SYMBOL(capi20_get_manufacturer);
u16 capi20_get_version(u32 contr, struct capi_version *verp)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
if (contr == 0) {
*verp = driver_version;
return CAPI_NOERROR;
}
- card = get_capi_ctr_by_nr(contr);
- if (!card || card->cardstate != CARD_RUNNING)
+ ctr = get_capi_ctr_by_nr(contr);
+ if (!ctr || ctr->state != CAPI_CTR_RUNNING)
return CAPI_REGNOTINSTALLED;
- memcpy((void *) verp, &card->version, sizeof(capi_version));
+ memcpy(verp, &ctr->version, sizeof(capi_version));
return CAPI_NOERROR;
}
@@ -833,17 +844,17 @@ EXPORT_SYMBOL(capi20_get_version);
u16 capi20_get_serial(u32 contr, u8 *serial)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
if (contr == 0) {
strlcpy(serial, driver_serial, CAPI_SERIAL_LEN);
return CAPI_NOERROR;
}
- card = get_capi_ctr_by_nr(contr);
- if (!card || card->cardstate != CARD_RUNNING)
+ ctr = get_capi_ctr_by_nr(contr);
+ if (!ctr || ctr->state != CAPI_CTR_RUNNING)
return CAPI_REGNOTINSTALLED;
- strlcpy((void *) serial, card->serial, CAPI_SERIAL_LEN);
+ strlcpy(serial, ctr->serial, CAPI_SERIAL_LEN);
return CAPI_NOERROR;
}
@@ -861,18 +872,17 @@ EXPORT_SYMBOL(capi20_get_serial);
u16 capi20_get_profile(u32 contr, struct capi_profile *profp)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
if (contr == 0) {
- profp->ncontroller = ncards;
+ profp->ncontroller = ncontrollers;
return CAPI_NOERROR;
}
- card = get_capi_ctr_by_nr(contr);
- if (!card || card->cardstate != CARD_RUNNING)
+ ctr = get_capi_ctr_by_nr(contr);
+ if (!ctr || ctr->state != CAPI_CTR_RUNNING)
return CAPI_REGNOTINSTALLED;
- memcpy((void *) profp, &card->profile,
- sizeof(struct capi_profile));
+ memcpy(profp, &ctr->profile, sizeof(struct capi_profile));
return CAPI_NOERROR;
}
@@ -885,7 +895,7 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
avmb1_extcarddef cdef;
avmb1_resetdef rdef;
capicardparams cparams;
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
struct capi_driver *driver = NULL;
capiloaddata ldata;
struct list_head *l;
@@ -958,26 +968,26 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
sizeof(avmb1_loadandconfigdef)))
return -EFAULT;
}
- card = get_capi_ctr_by_nr(ldef.contr);
- if (!card)
+ ctr = get_capi_ctr_by_nr(ldef.contr);
+ if (!ctr)
return -EINVAL;
- card = capi_ctr_get(card);
- if (!card)
+ ctr = capi_ctr_get(ctr);
+ if (!ctr)
return -ESRCH;
- if (card->load_firmware == NULL) {
+ if (ctr->load_firmware == NULL) {
printk(KERN_DEBUG "kcapi: load: no load function\n");
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return -ESRCH;
}
if (ldef.t4file.len <= 0) {
printk(KERN_DEBUG "kcapi: load: invalid parameter: length of t4file is %d ?\n", ldef.t4file.len);
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return -EINVAL;
}
if (ldef.t4file.data == NULL) {
printk(KERN_DEBUG "kcapi: load: invalid parameter: dataptr is 0\n");
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return -EINVAL;
}
@@ -988,46 +998,46 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
ldata.configuration.data = ldef.t4config.data;
ldata.configuration.len = ldef.t4config.len;
- if (card->cardstate != CARD_DETECTED) {
+ if (ctr->state != CAPI_CTR_DETECTED) {
printk(KERN_INFO "kcapi: load: contr=%d not in detect state\n", ldef.contr);
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return -EBUSY;
}
- card->cardstate = CARD_LOADING;
+ ctr->state = CAPI_CTR_LOADING;
- retval = card->load_firmware(card, &ldata);
+ retval = ctr->load_firmware(ctr, &ldata);
if (retval) {
- card->cardstate = CARD_DETECTED;
- capi_ctr_put(card);
+ ctr->state = CAPI_CTR_DETECTED;
+ capi_ctr_put(ctr);
return retval;
}
- while (card->cardstate != CARD_RUNNING) {
+ while (ctr->state != CAPI_CTR_RUNNING) {
msleep_interruptible(100); /* 0.1 sec */
if (signal_pending(current)) {
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return -EINTR;
}
}
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return 0;
case AVMB1_RESETCARD:
if (copy_from_user(&rdef, data, sizeof(avmb1_resetdef)))
return -EFAULT;
- card = get_capi_ctr_by_nr(rdef.contr);
- if (!card)
+ ctr = get_capi_ctr_by_nr(rdef.contr);
+ if (!ctr)
return -ESRCH;
- if (card->cardstate == CARD_DETECTED)
+ if (ctr->state == CAPI_CTR_DETECTED)
return 0;
- card->reset_ctr(card);
+ ctr->reset_ctr(ctr);
- while (card->cardstate > CARD_DETECTED) {
+ while (ctr->state > CAPI_CTR_DETECTED) {
msleep_interruptible(100); /* 0.1 sec */
@@ -1052,7 +1062,7 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
int capi20_manufacturer(unsigned int cmd, void __user *data)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
switch (cmd) {
#ifdef AVMB1_COMPAT
@@ -1070,13 +1080,13 @@ int capi20_manufacturer(unsigned int cmd, void __user *data)
if (copy_from_user(&fdef, data, sizeof(kcapi_flagdef)))
return -EFAULT;
- card = get_capi_ctr_by_nr(fdef.contr);
- if (!card)
+ ctr = get_capi_ctr_by_nr(fdef.contr);
+ if (!ctr)
return -ESRCH;
- card->traceflag = fdef.flag;
+ ctr->traceflag = fdef.flag;
printk(KERN_INFO "kcapi: contr [%03d] set trace=%d\n",
- card->cnr, card->traceflag);
+ ctr->cnr, ctr->traceflag);
return 0;
}
case KCAPI_CMD_ADDCARD:
diff --git a/drivers/isdn/capi/kcapi.h b/drivers/isdn/capi/kcapi.h
index 244711f..f62c53b 100644
--- a/drivers/isdn/capi/kcapi.h
+++ b/drivers/isdn/capi/kcapi.h
@@ -24,16 +24,16 @@ printk(KERN_DEBUG "%s: " format "\n" , __func__ , ## arg); \
#endif
enum {
- CARD_DETECTED = 1,
- CARD_LOADING = 2,
- CARD_RUNNING = 3,
+ CAPI_CTR_DETECTED = 1,
+ CAPI_CTR_LOADING = 2,
+ CAPI_CTR_RUNNING = 3,
};
extern struct list_head capi_drivers;
extern rwlock_t capi_drivers_list_lock;
extern struct capi20_appl *capi_applications[CAPI_MAXAPPL];
-extern struct capi_ctr *capi_cards[CAPI_MAXCONTR];
+extern struct capi_ctr *capi_controller[CAPI_MAXCONTR];
#ifdef CONFIG_PROC_FS
diff --git a/drivers/isdn/capi/kcapi_proc.c b/drivers/isdn/capi/kcapi_proc.c
index 09d4db7..59fd16a 100644
--- a/drivers/isdn/capi/kcapi_proc.c
+++ b/drivers/isdn/capi/kcapi_proc.c
@@ -15,13 +15,12 @@
#include <linux/seq_file.h>
#include <linux/init.h>
-static char *
-cardstate2str(unsigned short cardstate)
+static char *state2str(unsigned short state)
{
- switch (cardstate) {
- case CARD_DETECTED: return "detected";
- case CARD_LOADING: return "loading";
- case CARD_RUNNING: return "running";
+ switch (state) {
+ case CAPI_CTR_DETECTED: return "detected";
+ case CAPI_CTR_LOADING: return "loading";
+ case CAPI_CTR_RUNNING: return "running";
default: return "???";
}
}
@@ -38,7 +37,7 @@ cardstate2str(unsigned short cardstate)
static void *controller_start(struct seq_file *seq, loff_t *pos)
{
if (*pos < CAPI_MAXCONTR)
- return &capi_cards[*pos];
+ return &capi_controller[*pos];
return NULL;
}
@@ -47,7 +46,7 @@ static void *controller_next(struct seq_file *seq, void *v, loff_t *pos)
{
++*pos;
if (*pos < CAPI_MAXCONTR)
- return &capi_cards[*pos];
+ return &capi_controller[*pos];
return NULL;
}
@@ -65,7 +64,7 @@ static int controller_show(struct seq_file *seq, void *v)
seq_printf(seq, "%d %-10s %-8s %-16s %s\n",
ctr->cnr, ctr->driver_name,
- cardstate2str(ctr->cardstate),
+ state2str(ctr->state),
ctr->name,
ctr->procinfo ? ctr->procinfo(ctr) : "");
diff --git a/include/linux/isdn/capilli.h b/include/linux/isdn/capilli.h
index d3e5e9d..856f38e 100644
--- a/include/linux/isdn/capilli.h
+++ b/include/linux/isdn/capilli.h
@@ -66,7 +66,7 @@ struct capi_ctr {
unsigned long nsentdatapkt;
int cnr; /* controller number */
- volatile unsigned short cardstate; /* controller state */
+ volatile unsigned short state; /* controller state */
volatile int blocked; /* output blocked */
int traceflag; /* capi trace */
--
1.6.0.2
Drop the application rw-lock in favour of RCU. This synchronizes
capi20_release against capi_ctr_handle_message which may dereference an
application from (soft-)IRQ context. Any other access to the application
list is now protected by the capi_controller_lock as well. This also
allows to safely inspect applications for /proc dumping by holding
capi_controller_lock.
At this chance, drop some useless release_in_progress checks where we
obtained the application pointer from the list (which becomes NULL on
release_in_progress).
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/kcapi.c | 52 +++++++++++++++++-----------------------
drivers/isdn/capi/kcapi_proc.c | 11 +++++---
2 files changed, 29 insertions(+), 34 deletions(-)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index a99f7e3..0b4c8a7 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -34,6 +34,7 @@
#include <linux/b1lli.h>
#endif
#include <linux/mutex.h>
+#include <linux/rcupdate.h>
static int showcapimsgs = 0;
@@ -64,8 +65,6 @@ DEFINE_MUTEX(capi_drivers_lock);
struct capi_ctr *capi_controller[CAPI_MAXCONTR];
DEFINE_MUTEX(capi_controller_lock);
-static DEFINE_RWLOCK(application_lock);
-
struct capi20_appl *capi_applications[CAPI_MAXAPPL];
static int ncontrollers;
@@ -103,7 +102,7 @@ static inline struct capi20_appl *get_capi_appl_by_nr(u16 applid)
if (applid - 1 >= CAPI_MAXAPPL)
return NULL;
- return capi_applications[applid - 1];
+ return rcu_dereference(capi_applications[applid - 1]);
}
/* -------- util functions ------------------------------------ */
@@ -186,7 +185,7 @@ static void notify_up(u32 contr)
for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
ap = get_capi_appl_by_nr(applid);
- if (!ap || ap->release_in_progress)
+ if (!ap)
continue;
register_appl(ctr, applid, &ap->rparam);
}
@@ -216,7 +215,7 @@ static void ctr_down(struct capi_ctr *ctr, int new_state)
for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
ap = get_capi_appl_by_nr(applid);
- if (ap && !ap->release_in_progress)
+ if (ap)
capi_ctr_put(ctr);
}
@@ -336,7 +335,6 @@ void capi_ctr_handle_message(struct capi_ctr *ctr, u16 appl,
struct capi20_appl *ap;
int showctl = 0;
u8 cmd, subcmd;
- unsigned long flags;
_cdebbuf *cdb;
if (ctr->state != CAPI_CTR_RUNNING) {
@@ -384,10 +382,10 @@ void capi_ctr_handle_message(struct capi_ctr *ctr, u16 appl,
}
- read_lock_irqsave(&application_lock, flags);
+ rcu_read_lock();
ap = get_capi_appl_by_nr(CAPIMSG_APPID(skb->data));
- if ((!ap) || (ap->release_in_progress)) {
- read_unlock_irqrestore(&application_lock, flags);
+ if (!ap) {
+ rcu_read_unlock();
cdb = capi_message2str(skb->data);
if (cdb) {
printk(KERN_ERR "kcapi: handle_message: applid %d state released (%s)\n",
@@ -401,7 +399,7 @@ void capi_ctr_handle_message(struct capi_ctr *ctr, u16 appl,
}
skb_queue_tail(&ap->recv_queue, skb);
schedule_work(&ap->recv_work);
- read_unlock_irqrestore(&application_lock, flags);
+ rcu_read_unlock();
return;
@@ -656,40 +654,35 @@ u16 capi20_register(struct capi20_appl *ap)
{
int i;
u16 applid;
- unsigned long flags;
DBG("");
if (ap->rparam.datablklen < 128)
return CAPI_LOGBLKSIZETOSMALL;
- write_lock_irqsave(&application_lock, flags);
+ ap->nrecvctlpkt = 0;
+ ap->nrecvdatapkt = 0;
+ ap->nsentctlpkt = 0;
+ ap->nsentdatapkt = 0;
+ mutex_init(&ap->recv_mtx);
+ skb_queue_head_init(&ap->recv_queue);
+ INIT_WORK(&ap->recv_work, recv_handler);
+ ap->release_in_progress = 0;
+
+ mutex_lock(&capi_controller_lock);
for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
if (capi_applications[applid - 1] == NULL)
break;
}
if (applid > CAPI_MAXAPPL) {
- write_unlock_irqrestore(&application_lock, flags);
+ mutex_unlock(&capi_controller_lock);
return CAPI_TOOMANYAPPLS;
}
ap->applid = applid;
capi_applications[applid - 1] = ap;
- ap->nrecvctlpkt = 0;
- ap->nrecvdatapkt = 0;
- ap->nsentctlpkt = 0;
- ap->nsentdatapkt = 0;
- mutex_init(&ap->recv_mtx);
- skb_queue_head_init(&ap->recv_queue);
- INIT_WORK(&ap->recv_work, recv_handler);
- ap->release_in_progress = 0;
-
- write_unlock_irqrestore(&application_lock, flags);
-
- mutex_lock(&capi_controller_lock);
-
for (i = 0; i < CAPI_MAXCONTR; i++) {
if (!capi_controller[i] ||
capi_controller[i]->state != CAPI_CTR_RUNNING)
@@ -721,16 +714,15 @@ EXPORT_SYMBOL(capi20_register);
u16 capi20_release(struct capi20_appl *ap)
{
int i;
- unsigned long flags;
DBG("applid %#x", ap->applid);
- write_lock_irqsave(&application_lock, flags);
+ mutex_lock(&capi_controller_lock);
+
ap->release_in_progress = 1;
capi_applications[ap->applid - 1] = NULL;
- write_unlock_irqrestore(&application_lock, flags);
- mutex_lock(&capi_controller_lock);
+ synchronize_rcu();
for (i = 0; i < CAPI_MAXCONTR; i++) {
if (!capi_controller[i] ||
diff --git a/drivers/isdn/capi/kcapi_proc.c b/drivers/isdn/capi/kcapi_proc.c
index 3e6e17a..ea2dff6 100644
--- a/drivers/isdn/capi/kcapi_proc.c
+++ b/drivers/isdn/capi/kcapi_proc.c
@@ -139,9 +139,11 @@ static const struct file_operations proc_contrstats_ops = {
// applid nrecvctlpkt nrecvdatapkt nsentctlpkt nsentdatapkt
// ---------------------------------------------------------------------------
-static void *
-applications_start(struct seq_file *seq, loff_t *pos)
+static void *applications_start(struct seq_file *seq, loff_t *pos)
+ __acquires(capi_controller_lock)
{
+ mutex_lock(&capi_controller_lock);
+
if (*pos < CAPI_MAXAPPL)
return &capi_applications[*pos];
@@ -158,9 +160,10 @@ applications_next(struct seq_file *seq, void *v, loff_t *pos)
return NULL;
}
-static void
-applications_stop(struct seq_file *seq, void *v)
+static void applications_stop(struct seq_file *seq, void *v)
+ __releases(capi_controller_lock)
{
+ mutex_unlock(&capi_controller_lock);
}
static int
--
1.6.0.2
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 <[email protected]>
---
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
Turn the lock protecting registered capi drivers into a mutex and apply
it consistently.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/kcapi.c | 49 ++++++++++++++++++----------------------
drivers/isdn/capi/kcapi.h | 2 +-
drivers/isdn/capi/kcapi_proc.c | 8 +++---
3 files changed, 27 insertions(+), 32 deletions(-)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index f37c13b..c46964f 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -61,7 +61,7 @@ static char capi_manufakturer[64] = "AVM Berlin";
#define NCCI2CTRL(ncci) (((ncci) >> 24) & 0x7f)
LIST_HEAD(capi_drivers);
-DEFINE_RWLOCK(capi_drivers_list_lock);
+DEFINE_MUTEX(capi_drivers_lock);
static DEFINE_RWLOCK(application_lock);
static DEFINE_MUTEX(controller_mutex);
@@ -540,11 +540,9 @@ EXPORT_SYMBOL(detach_capi_ctr);
void register_capi_driver(struct capi_driver *driver)
{
- unsigned long flags;
-
- write_lock_irqsave(&capi_drivers_list_lock, flags);
+ mutex_lock(&capi_drivers_lock);
list_add_tail(&driver->list, &capi_drivers);
- write_unlock_irqrestore(&capi_drivers_list_lock, flags);
+ mutex_unlock(&capi_drivers_lock);
}
EXPORT_SYMBOL(register_capi_driver);
@@ -558,11 +556,9 @@ EXPORT_SYMBOL(register_capi_driver);
void unregister_capi_driver(struct capi_driver *driver)
{
- unsigned long flags;
-
- write_lock_irqsave(&capi_drivers_list_lock, flags);
+ mutex_lock(&capi_drivers_lock);
list_del(&driver->list);
- write_unlock_irqrestore(&capi_drivers_list_lock, flags);
+ mutex_unlock(&capi_drivers_lock);
}
EXPORT_SYMBOL(unregister_capi_driver);
@@ -899,7 +895,6 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
struct capi_driver *driver = NULL;
capiloaddata ldata;
struct list_head *l;
- unsigned long flags;
int retval;
switch (cmd) {
@@ -919,7 +914,8 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
cparams.irq = cdef.irq;
cparams.cardnr = cdef.cardnr;
- read_lock_irqsave(&capi_drivers_list_lock, flags);
+ mutex_lock(&capi_drivers_lock);
+
switch (cdef.cardtype) {
case AVM_CARDTYPE_B1:
list_for_each(l, &capi_drivers) {
@@ -940,18 +936,15 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
break;
}
if (!driver) {
- read_unlock_irqrestore(&capi_drivers_list_lock, flags);
printk(KERN_ERR "kcapi: driver not loaded.\n");
- return -EIO;
- }
- if (!driver->add_card) {
- read_unlock_irqrestore(&capi_drivers_list_lock, flags);
+ retval = -EIO;
+ } else if (!driver->add_card) {
printk(KERN_ERR "kcapi: driver has no add card function.\n");
- return -EIO;
- }
+ retval = -EIO;
+ } else
+ retval = driver->add_card(driver, &cparams);
- retval = driver->add_card(driver, &cparams);
- read_unlock_irqrestore(&capi_drivers_list_lock, flags);
+ mutex_unlock(&capi_drivers_lock);
return retval;
case AVMB1_LOAD:
@@ -1107,6 +1100,8 @@ int capi20_manufacturer(unsigned int cmd, void __user *data)
cparams.cardtype = 0;
cdef.driver[sizeof(cdef.driver)-1] = 0;
+ mutex_lock(&capi_drivers_lock);
+
list_for_each(l, &capi_drivers) {
driver = list_entry(l, struct capi_driver, list);
if (strcmp(driver->name, cdef.driver) == 0)
@@ -1115,15 +1110,15 @@ int capi20_manufacturer(unsigned int cmd, void __user *data)
if (driver == NULL) {
printk(KERN_ERR "kcapi: driver \"%s\" not loaded.\n",
cdef.driver);
- return -ESRCH;
- }
-
- if (!driver->add_card) {
+ retval = -ESRCH;
+ } else if (!driver->add_card) {
printk(KERN_ERR "kcapi: driver \"%s\" has no add card function.\n", cdef.driver);
- return -EIO;
- }
+ retval = -EIO;
+ } else
+ retval = driver->add_card(driver, &cparams);
- return driver->add_card(driver, &cparams);
+ mutex_unlock(&capi_drivers_lock);
+ return retval;
}
default:
diff --git a/drivers/isdn/capi/kcapi.h b/drivers/isdn/capi/kcapi.h
index f62c53b..07c5850 100644
--- a/drivers/isdn/capi/kcapi.h
+++ b/drivers/isdn/capi/kcapi.h
@@ -30,7 +30,7 @@ enum {
};
extern struct list_head capi_drivers;
-extern rwlock_t capi_drivers_list_lock;
+extern struct mutex capi_drivers_lock;
extern struct capi20_appl *capi_applications[CAPI_MAXAPPL];
extern struct capi_ctr *capi_controller[CAPI_MAXCONTR];
diff --git a/drivers/isdn/capi/kcapi_proc.c b/drivers/isdn/capi/kcapi_proc.c
index 59fd16a..71b0761 100644
--- a/drivers/isdn/capi/kcapi_proc.c
+++ b/drivers/isdn/capi/kcapi_proc.c
@@ -238,9 +238,9 @@ static const struct file_operations proc_applstats_ops = {
// ---------------------------------------------------------------------------
static void *capi_driver_start(struct seq_file *seq, loff_t *pos)
- __acquires(&capi_drivers_list_lock)
+ __acquires(&capi_drivers_lock)
{
- read_lock(&capi_drivers_list_lock);
+ mutex_lock(&capi_drivers_lock);
return seq_list_start(&capi_drivers, *pos);
}
@@ -250,9 +250,9 @@ static void *capi_driver_next(struct seq_file *seq, void *v, loff_t *pos)
}
static void capi_driver_stop(struct seq_file *seq, void *v)
- __releases(&capi_drivers_list_lock)
+ __releases(&capi_drivers_lock)
{
- read_unlock(&capi_drivers_list_lock);
+ mutex_unlock(&capi_drivers_lock);
}
static int capi_driver_show(struct seq_file *seq, void *v)
--
1.6.0.2
This step prepares the application of proper controller locking: Push
all state changing work into the notify handler that are called by
capi_ctr_ready and capi_ctr_down, switch detach_capi_ctr to issue a
synchronous ctr_down. Also ensure that we do not go through any action
if the state did not change.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/kcapi.c | 95 +++++++++++++++++++++++---------------------
1 files changed, 50 insertions(+), 45 deletions(-)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index c46964f..9362a7a 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -169,44 +169,74 @@ static void release_appl(struct capi_ctr *ctr, u16 applid)
static void notify_up(u32 contr)
{
- struct capi_ctr *ctr = get_capi_ctr_by_nr(contr);
struct capi20_appl *ap;
+ struct capi_ctr *ctr;
u16 applid;
- if (showcapimsgs & 1) {
+ if (showcapimsgs & 1)
printk(KERN_DEBUG "kcapi: notify up contr %d\n", contr);
- }
- if (!ctr) {
+
+ ctr = get_capi_ctr_by_nr(contr);
+ if (ctr) {
+ if (ctr->state == CAPI_CTR_RUNNING)
+ return;
+
+ ctr->state = CAPI_CTR_RUNNING;
+
+ for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
+ ap = get_capi_appl_by_nr(applid);
+ 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);
- return;
- }
- for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
- ap = get_capi_appl_by_nr(applid);
- 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);
- }
}
/* -------- KCI_CONTRDOWN ------------------------------------- */
-static void notify_down(u32 contr)
+static void ctr_down(struct capi_ctr *ctr)
{
struct capi20_appl *ap;
u16 applid;
- if (showcapimsgs & 1) {
- printk(KERN_DEBUG "kcapi: notify down contr %d\n", contr);
- }
+ if (ctr->state == CAPI_CTR_DETECTED)
+ return;
+
+ ctr->state = CAPI_CTR_DETECTED;
+
+ memset(ctr->manu, 0, sizeof(ctr->manu));
+ memset(&ctr->version, 0, sizeof(ctr->version));
+ memset(&ctr->profile, 0, sizeof(ctr->profile));
+ memset(ctr->serial, 0, sizeof(ctr->serial));
for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
ap = get_capi_appl_by_nr(applid);
- if (ap && ap->callback && !ap->release_in_progress)
- ap->callback(KCI_CONTRDOWN, contr, NULL);
+ if (ap && !ap->release_in_progress) {
+ if (ap->callback)
+ ap->callback(KCI_CONTRDOWN, ctr->cnr, NULL);
+ capi_ctr_put(ctr);
+ }
}
}
+static void notify_down(u32 contr)
+{
+ struct capi_ctr *ctr;
+
+ 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);
+ else
+ printk(KERN_WARNING "%s: invalid contr %d\n", __func__, contr);
+}
+
static void notify_handler(struct work_struct *work)
{
struct capi_notifier *np =
@@ -368,8 +398,6 @@ EXPORT_SYMBOL(capi_ctr_handle_message);
void capi_ctr_ready(struct capi_ctr *ctr)
{
- ctr->state = CAPI_CTR_RUNNING;
-
printk(KERN_NOTICE "kcapi: controller [%03d] \"%s\" ready.\n",
ctr->cnr, ctr->name);
@@ -388,28 +416,6 @@ EXPORT_SYMBOL(capi_ctr_ready);
void capi_ctr_down(struct capi_ctr *ctr)
{
- u16 appl;
-
- DBG("");
-
- if (ctr->state == CAPI_CTR_DETECTED)
- return;
-
- ctr->state = CAPI_CTR_DETECTED;
-
- memset(ctr->manu, 0, sizeof(ctr->manu));
- memset(&ctr->version, 0, sizeof(ctr->version));
- memset(&ctr->profile, 0, sizeof(ctr->profile));
- memset(ctr->serial, 0, sizeof(ctr->serial));
-
- for (appl = 1; appl <= CAPI_MAXAPPL; appl++) {
- struct capi20_appl *ap = get_capi_appl_by_nr(appl);
- if (!ap || ap->release_in_progress)
- continue;
-
- capi_ctr_put(ctr);
- }
-
printk(KERN_NOTICE "kcapi: controller [%03d] down.\n", ctr->cnr);
notify_push(KCI_CONTRDOWN, ctr->cnr, 0, 0);
@@ -513,8 +519,7 @@ EXPORT_SYMBOL(attach_capi_ctr);
int detach_capi_ctr(struct capi_ctr *ctr)
{
- if (ctr->state != CAPI_CTR_DETECTED)
- capi_ctr_down(ctr);
+ ctr_down(ctr);
ncontrollers--;
--
1.6.0.2
Make the code a bit more readable be providing stub functions for the
!CONFIG_ISDN_CAPI_MIDDLEWARE case. Though a few lines are moved around,
this comes with no functional changes.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 150 +++++++++++++++++++++++++--------------------
1 files changed, 83 insertions(+), 67 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 3b07797..7d2ca6b 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -23,14 +23,10 @@
#include <linux/smp_lock.h>
#include <linux/timer.h>
#include <linux/wait.h>
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
#include <linux/tty.h>
-#ifdef CONFIG_PPP
#include <linux/netdevice.h>
#include <linux/ppp_defs.h>
#include <linux/if_ppp.h>
-#endif /* CONFIG_PPP */
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
#include <linux/skbuff.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
@@ -56,17 +52,17 @@ MODULE_LICENSE("GPL");
/* -------- driver information -------------------------------------- */
static struct class *capi_class;
-
static int capi_major = 68; /* allocated */
+
+module_param_named(major, capi_major, uint, 0);
+
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
-#define CAPINC_NR_PORTS 32
+#define CAPINC_NR_PORTS 32
#define CAPINC_MAX_PORTS 256
+
static int capi_ttymajor = 191;
static int capi_ttyminors = CAPINC_NR_PORTS;
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
-module_param_named(major, capi_major, uint, 0);
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
module_param_named(ttymajor, capi_ttymajor, uint, 0);
module_param_named(ttyminors, capi_ttyminors, uint, 0);
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
@@ -81,7 +77,6 @@ module_param_named(ttyminors, capi_ttyminors, uint, 0);
struct capidev;
struct capincci;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
struct capiminor;
struct datahandle_queue {
@@ -116,7 +111,6 @@ struct capiminor {
int nack;
spinlock_t ackqlock;
};
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
/* FIXME: The following lock is a sledgehammer-workaround to a
* locking issue with the capiminor (and maybe other) data structure(s).
@@ -156,14 +150,13 @@ static DEFINE_RWLOCK(capidev_list_lock);
static LIST_HEAD(capidev_list);
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
+
static DEFINE_RWLOCK(capiminor_list_lock);
static LIST_HEAD(capiminor_list);
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
/* -------- datahandles --------------------------------------------- */
-static int capincci_add_ack(struct capiminor *mp, u16 datahandle)
+static int capiminor_add_ack(struct capiminor *mp, u16 datahandle)
{
struct datahandle_queue *n;
unsigned long flags;
@@ -301,26 +294,17 @@ static struct capiminor *capiminor_find(unsigned int minor)
return p;
}
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
/* -------- struct capincci ----------------------------------------- */
-static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
+static void capincci_alloc_minor(struct capidev *cdev, struct capincci *np)
{
- struct capincci *np, **pp;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- struct capiminor *mp = NULL;
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+ struct capiminor *mp;
- np = kzalloc(sizeof(*np), GFP_ATOMIC);
- if (!np)
- return NULL;
- np->ncci = ncci;
- np->cdev = cdev;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- mp = NULL;
- if (cdev->userflags & CAPIFLAG_HIGHJACKING)
- mp = np->minorp = capiminor_alloc(&cdev->ap, ncci);
+ if (!(cdev->userflags & CAPIFLAG_HIGHJACKING))
+ return;
+
+ mp = np->minorp = capiminor_alloc(&cdev->ap, np->ncci);
if (mp) {
mp->nccip = np;
#ifdef _DEBUG_REFCOUNT
@@ -330,7 +314,58 @@ static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
capifs_new_ncci(mp->minor,
MKDEV(capi_ttymajor, mp->minor));
}
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+}
+
+static void capincci_free_minor(struct capincci *np)
+{
+ struct capiminor *mp = np->minorp;
+
+ if (mp) {
+ capifs_free_ncci(mp->capifs_dentry);
+ if (mp->tty) {
+ mp->nccip = NULL;
+#ifdef _DEBUG_REFCOUNT
+ printk(KERN_DEBUG "reset mp->nccip\n");
+#endif
+ tty_hangup(mp->tty);
+ } else {
+ capiminor_free(mp);
+ }
+ }
+}
+
+static inline unsigned int capincci_minor_opencount(struct capincci *np)
+{
+ struct capiminor *mp = np->minorp;
+
+ return mp ? atomic_read(&mp->ttyopencount) : 0;
+}
+
+#else /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
+
+static inline void
+capincci_alloc_minor(struct capidev *cdev, struct capincci *np) { }
+static inline void capincci_free_minor(struct capincci *np) { }
+
+static inline unsigned int capincci_minor_opencount(struct capincci *np)
+{
+ return 0;
+}
+
+#endif /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
+
+static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
+{
+ struct capincci *np, **pp;
+
+ np = kzalloc(sizeof(*np), GFP_ATOMIC);
+ if (!np)
+ return NULL;
+ np->ncci = ncci;
+ np->cdev = cdev;
+
+ capincci_alloc_minor(cdev, np);
+
for (pp=&cdev->nccis; *pp; pp = &(*pp)->next)
;
*pp = np;
@@ -340,29 +375,13 @@ static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
static void capincci_free(struct capidev *cdev, u32 ncci)
{
struct capincci *np, **pp;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- struct capiminor *mp;
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
pp=&cdev->nccis;
while (*pp) {
np = *pp;
if (ncci == 0xffffffff || np->ncci == ncci) {
*pp = (*pp)->next;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- if ((mp = np->minorp) != NULL) {
- capifs_free_ncci(mp->capifs_dentry);
- if (mp->tty) {
- mp->nccip = NULL;
-#ifdef _DEBUG_REFCOUNT
- printk(KERN_DEBUG "reset mp->nccip\n");
-#endif
- tty_hangup(mp->tty);
- } else {
- capiminor_free(mp);
- }
- }
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+ capincci_free_minor(np);
kfree(np);
if (*pp == NULL) return;
} else {
@@ -552,7 +571,7 @@ static int handle_minor_send(struct capiminor *mp)
capimsg_setu16(skb->data, 18, datahandle);
capimsg_setu16(skb->data, 20, 0); /* Flags */
- if (capincci_add_ack(mp, datahandle) < 0) {
+ if (capiminor_add_ack(mp, datahandle) < 0) {
skb_pull(skb, CAPI_DATA_B3_REQ_LEN);
skb_queue_head(&mp->outqueue, skb);
return count;
@@ -628,10 +647,13 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
spin_unlock_irqrestore(&workaround_lock, flags);
return;
}
+
#ifndef CONFIG_ISDN_CAPI_MIDDLEWARE
skb_queue_tail(&cdev->recvqueue, skb);
wake_up_interruptible(&cdev->recvwait);
+
#else /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+
mp = np->minorp;
if (!mp) {
skb_queue_tail(&cdev->recvqueue, skb);
@@ -639,10 +661,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
spin_unlock_irqrestore(&workaround_lock, flags);
return;
}
-
-
if (CAPIMSG_SUBCOMMAND(skb->data) == CAPI_IND) {
-
datahandle = CAPIMSG_U16(skb->data, CAPIMSG_BASELEN+4+4+2);
#ifdef _DEBUG_DATAFLOW
printk(KERN_DEBUG "capi_signal: DATA_B3_IND %u len=%d\n",
@@ -672,6 +691,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
wake_up_interruptible(&cdev->recvwait);
}
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+
spin_unlock_irqrestore(&workaround_lock, flags);
}
@@ -929,9 +949,6 @@ capi_ioctl(struct inode *inode, struct file *file,
case CAPI_NCCI_OPENCOUNT:
{
struct capincci *nccip;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- struct capiminor *mp;
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
unsigned ncci;
int count = 0;
if (copy_from_user(&ncci, argp, sizeof(ncci)))
@@ -942,11 +959,7 @@ capi_ioctl(struct inode *inode, struct file *file,
mutex_unlock(&cdev->ncci_list_mtx);
return 0;
}
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- if ((mp = nccip->minorp) != NULL) {
- count += atomic_read(&mp->ttyopencount);
- }
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+ count += capincci_minor_opencount(nccip);
mutex_unlock(&cdev->ncci_list_mtx);
return count;
}
@@ -1396,7 +1409,16 @@ static void capinc_tty_exit(void)
put_tty_driver(drv);
}
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+#else /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
+
+static inline int capinc_tty_init(void)
+{
+ return 0;
+}
+
+static inline void capinc_tty_exit(void) { }
+
+#endif /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
/* -------- /proc functions ----------------------------------------- */
@@ -1505,23 +1527,19 @@ static int __init capi_init(void)
device_create(capi_class, NULL, MKDEV(capi_major, 0), NULL, "capi");
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
if (capinc_tty_init() < 0) {
device_destroy(capi_class, MKDEV(capi_major, 0));
class_destroy(capi_class);
unregister_chrdev(capi_major, "capi20");
return -ENOMEM;
}
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
proc_init();
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
#if defined(CONFIG_ISDN_CAPI_CAPIFS) || defined(CONFIG_ISDN_CAPI_CAPIFS_MODULE)
compileinfo = " (middleware+capifs)";
-#else
+#elif defined(CONFIG_ISDN_CAPI_MIDDLEWARE)
compileinfo = " (no capifs)";
-#endif
#else
compileinfo = " (no middleware)";
#endif
@@ -1539,9 +1557,7 @@ static void __exit capi_exit(void)
class_destroy(capi_class);
unregister_chrdev(capi_major, "capi20");
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
capinc_tty_exit();
-#endif
}
module_init(capi_init);
--
1.6.0.2
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 <[email protected]>
---
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
capifs_mnt->mnt_sb->s_root already contains what we need.
Signed-off-by: Jan Kiszka <[email protected]>
---
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
> o reworked locking of the NCCI TTY using latest and greatest tty_port
> features and kref, hopefully in the right way
Looks sane to me
Alan
On Montag, 8. Februar 2010 21:12:04 Jan Kiszka wrote:
> Here is the second take of my CAPI rework. I tried to address all
> feedback on v1, so this one comes with the following changes:
>
> o rebased over net-next
> o reworked locking of the NCCI TTY using latest and greatest tty_port
> features and kref, hopefully in the right way
> o more fine-grained patch steps for the tricky TTY rework
> o dynamic major for NCCI TTYs, ie. CAPI no longer claims 191
> o dynamic TTY minor registrations, completely obsoleting capifs
> o schedule capifs for removal
> o some small additional cleanups
>
> I kept my capifs fixes though this thing should die in the future. The
> work is done, and people may still use it due to their distro
> configuration (and the hard-wired loading by capiinit - needs fixing).
>
> You can pull this series from
>
> git://git.kiszka.org/linux-2.6.git capi
>
Thank you for that effort, it looks very good for me.
I will acknowlegde them, after a few tests.
> Jan
>
> Jan Kiszka (41):
> CAPI: Fix leaks in capifs_new_ncci
> CAPI: Sanitize capifs API
> CAPI: Eliminate capifs_root variable
> CAPI: Pin capifs instead of mounting it
> CAPI: Reduce chattiness during module loading/removal
> CAPI: Call a controller 'controller', not 'card'
> CAPI: Convert capi drivers rwlock into mutex
> CAPI: Rework capi_ctr_ready/down
> CAPI: Rework controller state notifier
> CAPI: Rework locking of controller data structures
> CAPI: Rework application locking
> CAPI: Reduce #ifdef mess around CONFIG_ISDN_CAPI_MIDDLEWARE
> CAPI: Convert capidev_list_lock into a mutex
> CAPI: Clean up capi_open/release
> CAPI: Rework locking of capidev members
> CAPI: Use non-atomic allocation during NCCI setup
> CAPI: Fix racy capi_read
> CAPI: Switch NCCI list to standard doubly linked list
> CAPI: Switch capiminor list to array
> CAPI: Clean up capinc_tty_init/exit
> CAPI: Dynamically register minor devices
> CAPI: Use dynamic major for NCCI TTYs by default
> CAPI: Use kref on capiminor
> CAPI: Establish install/cleanup handlers for capiminor TTYs
> CAPI: Use tty_port to keep track of capiminor's tty
> CAPI: Drop remaining NULL checks on tty->driver_data
> CAPI: Issue synchronous hangup on capincci_free_minor
> CAPI: Drop obsolete nccip from capiminor struct
> CAPI: Clean up capiminors_lock
> CAPI: Drop atomic ttyopencount
> CAPI: Drop handle_minor_recv from capinc_tty_write
> CAPI: Rework capiminor RX handler
> CAPI: Rename datahandle_queue -> ackqueue_entry
> CAPI: Use atomics for capiminor's datahandle and msgid
> CAPI: Drop capiminor's unused inbytes counter
> CAPI: Fix locking around capiminor's output queue and drop
> workaround_lock
> CAPI: Clean up capiminor_*_ack
> CAPI: Drop return value of handle_minor_send
> CAPI: Drop special controller lookup from capi20_put_message
> CAPI: Schedule capifs for removal
> CAPI: Remove experimental tag from middleware feature
>
> Documentation/feature-removal-schedule.txt | 10 +
> drivers/isdn/capi/Kconfig | 16 +-
> drivers/isdn/capi/capi.c | 1108
> ++++++++++++++-------------- drivers/isdn/capi/capidrv.c |
> 48 +-
> drivers/isdn/capi/capifs.c | 126 ++--
> drivers/isdn/capi/capifs.h | 21 +-
> drivers/isdn/capi/kcapi.c | 805 +++++++++++---------
> drivers/isdn/capi/kcapi.h | 13 +-
> drivers/isdn/capi/kcapi_proc.c | 41 +-
> include/linux/isdn/capilli.h | 5 +-
> include/linux/kernelcapi.h | 17 +-
> 11 files changed, 1151 insertions(+), 1059 deletions(-)
>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Am 2010-02-08 21:12 schrieb Jan Kiszka:
> Here is the second take of my CAPI rework. I tried to address all
> feedback on v1 [...]
> I kept my capifs fixes though this thing should die in the future. The
> work is done, and people may still use it due to their distro
> configuration (and the hard-wired loading by capiinit - needs fixing).
Great work. Many thanks for going through that effort!
T.
- --
Tilman Schmidt E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge?ffnet mindestens haltbar bis: (siehe R?ckseite)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.12 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAktyy6YACgkQQ3+did9BuFvj7QCeO+qncLR2yGECEadwnWcVDo8Q
piEAnj6ZzuZ4z68SmzI20YVV6AQN9LE9
=m+sN
-----END PGP SIGNATURE-----
From: Karsten Keil <[email protected]>
Date: Tue, 9 Feb 2010 13:54:23 +0100
> Thank you for that effort, it looks very good for me.
>
> I will acknowlegde them, after a few tests.
It's been more than a week later, will you be able to
ever find time to do this "ACK"?
From: Jan Kiszka <[email protected]>
Date: Mon, 8 Feb 2010 21:12:04 +0100
> Here is the second take of my CAPI rework. I tried to address all
> feedback on v1, so this one comes with the following changes:
>
> o rebased over net-next
> o reworked locking of the NCCI TTY using latest and greatest tty_port
> features and kref, hopefully in the right way
> o more fine-grained patch steps for the tricky TTY rework
> o dynamic major for NCCI TTYs, ie. CAPI no longer claims 191
> o dynamic TTY minor registrations, completely obsoleting capifs
> o schedule capifs for removal
> o some small additional cleanups
>
> I kept my capifs fixes though this thing should die in the future. The
> work is done, and people may still use it due to their distro
> configuration (and the hard-wired loading by capiinit - needs fixing).
Thank you for all of your hard work.
I've applied your entire series to net-next-2.6
Thanks again!