First of all sorry if I'm breaking the correct flow of ISDN patches, but
I'm under the impression that things currently get merged directly via
Dave, right?
This series can also be pulled from
git://git.kiszka.org/linux-2.6.git capi
As one of the (presumably) few remaining users of an AVM FRITZ!Card DSL
v2.0, I originally just wanted to finally get that broken proprietary
driver into a more reliable state. But I actually ended up fixing also
wide parts of the in-kernel CAPI stack - a job that turned out to be
almost as "interesting" as hacking on that driver.
The major issue of the CAPI stack were incomplete or broken locking
schemes. That caused sporadic crashes, specifically during
setup/shutdown phases of connections, devices, or whatever dynamic
objects. But it also affected some code that was executed during normal
operation. I think I fixed most of those issue, at least all I found via
code inspection, lockdep runs, and testing. This series in production
use on my home gateway for two weeks now (under CONFIG_PREEMPT - the
original stack and driver code only worked acceptably under
CONFIG_PREEMPT_NONE). I also applied some cleanups, and polished the old
capifs a bit.
So the series concludes with removing the experimental tag from the CAPI
middleware, primarily the kernel's CAPI-speaking gate to user space
(eg. for use with Asterisk). There also patch to officially claim the
char major number 191 that CAPI (and likely only CAPI) used in the past
decade.
Note that, although I touched some small parts of it, I did _not_ fix
any locking issue of the capidrv, the interface to the legacy ISDN
stack. It is _surely_ broken in similar ways. But while it may also be
fixable, I rather decided that I'm going to invest a bit time in a
replacement of the last local user, isdnlog.
Looking forward to feedback,
Jan
PS: If anyone is interested in the fcdsl2 FRITZ!Card driver rework, you
can find the current version at
git://git.kiszka.org/fcdsl2.git
Jan Kiszka (31):
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: Fix locking around glueing capiminor and capidev
CAPI: Drop atomic ttyopencount
CAPI: Remove useless checks for tty->driver_data or hung up state
CAPI: Rework tty locking in RX and TX path of capiminor
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
CAPI: Clean up capiminor_*_ack
CAPI: Drop handle_minor_send return value
CAPI: Drop special controller lookup from capi20_put_message
CAPI: Remove experimental tag from middleware feature
CAPI: Officially claim char major 191
Documentation/devices.txt | 7 +-
drivers/isdn/capi/Kconfig | 3 +-
drivers/isdn/capi/capi.c | 937 ++++++++++++++++++----------------------
drivers/isdn/capi/capidrv.c | 48 +--
drivers/isdn/capi/capifs.c | 126 +++---
drivers/isdn/capi/capifs.h | 21 +-
drivers/isdn/capi/kcapi.c | 797 ++++++++++++++++++----------------
drivers/isdn/capi/kcapi.h | 12 +-
drivers/isdn/capi/kcapi_proc.c | 41 +-
include/linux/isdn/capilli.h | 2 +-
include/linux/kernelcapi.h | 17 +-
11 files changed, 1005 insertions(+), 1006 deletions(-)
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 %sn", 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
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 58504ef..c456f61 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -44,8 +44,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");
@@ -1518,21 +1516,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 %dn", capi_major);
@@ -1566,8 +1554,8 @@ static int __init capi_init(void)
#else
compileinfo = " (no middleware)";
#endif
- printk(KERN_NOTICE "capi20: Rev %s: started up with major %d%sn",
- rev, capi_major, compileinfo);
+ printk(KERN_NOTICE "CAPI 2.0 started up with major %d%sn",
+ capi_major, compileinfo);
return 0;
}
@@ -1583,7 +1571,6 @@ static void __exit capi_exit(void)
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
capinc_tty_exit();
#endif
- printk(KERN_NOTICE "capi: Rev %s: unloadedn", rev);
}
module_init(capi_init);
diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
index 66b7d7a..52118a9 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -34,7 +34,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");
@@ -2287,19 +2286,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;
@@ -2327,29 +2316,14 @@ static int __init capidrv_init(void)
}
proc_init();
- printk(KERN_NOTICE "capidrv: Rev %s: loadedn", 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: unloadedn", 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 %sn", 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 dc506ab..4a75234 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");
@@ -1171,25 +1167,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 %sn", rev);
-
- return 0;
+ err = cdebug_init();
+ if (!err)
+ kcapi_proc_init();
+ return err;
}
static void __exit kcapi_exit(void)
--
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 | 326 ++++++++++++++++++++-------------------
drivers/isdn/capi/kcapi.h | 8 +-
drivers/isdn/capi/kcapi_proc.c | 17 +-
include/linux/isdn/capilli.h | 2 +-
4 files changed, 180 insertions(+), 173 deletions(-)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index 4a75234..402a16b 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 resourcesn", __func__);
+ printk(KERN_WARNING "%s: cannot get controller resourcesn",
+ __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 %dn", contr);
}
- if (!card) {
+ if (!ctr) {
printk(KERN_WARNING "%s: invalid contr %dn", __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 tracen",
- 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=%un",
- 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] %sn",
- card->cnr, cdb->buf);
+ ctr->cnr, cdb->buf);
cdebbuf_free(cdb);
} else
printk(KERN_DEBUG "kcapi: got [%03d] id#%d %s len=%u, cannot tracen",
- 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] suspendn", card->cnr);
- card->blocked = 1;
+ if (!ctr->blocked) {
+ printk(KERN_DEBUG "kcapi: controller [%03d] suspendn",
+ 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] resumen", card->cnr);
- card->blocked = 0;
+ if (ctr->blocked) {
+ printk(KERN_DEBUG "kcapi: controller [%03d] resumedn",
+ 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,31 +478,29 @@ attach_capi_ctr(struct capi_ctr *card)
printk(KERN_ERR "kcapi: out of controller slotsn");
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 = create_proc_entry(card->procfn, 0, NULL);
- if (card->procent) {
- card->procent->read_proc =
- (int (*)(char *,char **,off_t,int,int *,void *))
- card->ctr_read_proc;
- card->procent->data = card;
+ 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 = create_proc_entry(ctr->procfn, 0, NULL);
+ if (ctr->procent) {
+ ctr->procent->read_proc = (read_proc_t *)ctr->ctr_read_proc;
+ ctr->procent->data = ctr;
}
- ncards++;
- printk(KERN_NOTICE "kcapi: Controller [%03d]: %s attachedn",
- card->cnr, card->name);
+ ncontrollers++;
+ printk(KERN_NOTICE "kcapi: controller [%03d]: %s attachedn",
+ ctr->cnr, ctr->name);
return 0;
}
@@ -504,27 +508,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 unregisteredn",
- card->cnr, card->name);
+ capi_controller[ctr->cnr - 1] = NULL;
+ printk(KERN_NOTICE "kcapi: controller [%03d]: %s unregisteredn",
+ ctr->cnr, ctr->name);
return 0;
}
@@ -582,7 +586,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;
@@ -641,9 +646,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);
@@ -680,9 +686,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);
@@ -709,13 +716,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;
@@ -723,28 +730,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=%un",
@@ -767,7 +776,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);
@@ -784,16 +793,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;
}
@@ -811,17 +820,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;
}
@@ -839,17 +848,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;
}
@@ -867,18 +876,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;
}
@@ -891,7 +899,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;
@@ -964,26 +972,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 functionn");
- 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 0n");
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return -EINVAL;
}
@@ -994,46 +1002,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 staten", 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 */
@@ -1058,7 +1066,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
@@ -1076,13 +1084,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=%dn",
- 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 %sn",
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 7acb87a..ffede68 100644
--- a/include/linux/isdn/capilli.h
+++ b/include/linux/isdn/capilli.h
@@ -67,7 +67,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
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
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
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 402a16b..476239f 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);
@@ -544,11 +544,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);
@@ -562,11 +560,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);
@@ -903,7 +899,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) {
@@ -923,7 +918,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) {
@@ -944,18 +940,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:
@@ -1111,6 +1104,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)
@@ -1119,15 +1114,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
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 65bf91e..58504ef 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -41,9 +41,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 $";
@@ -95,6 +94,7 @@ struct capiminor {
struct list_head list;
struct capincci *nccip;
unsigned int minor;
+ struct dentry *capifs_dentry;
struct capi20_appl *ap;
u32 ncci;
@@ -327,9 +327,9 @@ static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
#ifdef _DEBUG_REFCOUNT
printk(KERN_DEBUG "set mp->nccipn");
#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)
@@ -352,9 +352,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
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 2af6c05..2af81c8 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -400,46 +400,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 --------------------------------------- */
@@ -990,30 +950,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
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 476239f..3fcc95b 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 %dn", 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 %dn", __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 %dn", 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 %dn", contr);
+
+ ctr = get_capi_ctr_by_nr(contr);
+ if (ctr)
+ ctr_down(ctr);
+ else
+ printk(KERN_WARNING "%s: invalid contr %dn", __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);
@@ -517,8 +523,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
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 52118a9..de11764 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -2209,19 +2209,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 upn", 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 downn", contr);
(void) capidrv_delcontr(contr);
break;
}
+ return NOTIFY_OK;
}
/*
@@ -2283,6 +2288,10 @@ static void __exit proc_exit(void)
}
}
+static struct notifier_block capictr_nb = {
+ .notifier_call = lower_callback,
+};
+
static int __init capidrv_init(void)
{
capi_profile profile;
@@ -2299,7 +2308,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) {
@@ -2321,6 +2330,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 3fcc95b..0fda05d 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 %dn", __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 %dn", __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);
@@ -637,7 +645,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);
@@ -1141,30 +1148,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 ------------------------------------- */
/* ------------------------------------------------------------- */
@@ -1173,10 +1156,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
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 964650a..2c5d1b8 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);
}
@@ -214,7 +213,7 @@ 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)
capi_ctr_put(ctr);
}
}
@@ -332,7 +331,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) {
@@ -380,10 +378,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",
@@ -397,7 +395,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;
@@ -649,40 +647,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)
@@ -714,16 +707,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
This patch applies the mutex so far only protecting the controller list
to (almost) all accesses of controller data structures.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/kcapi.c | 259 +++++++++++++++++++++++++++-------------
drivers/isdn/capi/kcapi.h | 4 +-
drivers/isdn/capi/kcapi_proc.c | 5 +
3 files changed, 186 insertions(+), 82 deletions(-)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index 0fda05d..964650a 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 %dn", 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;
@@ -189,6 +192,9 @@ static void notify_up(u32 contr)
}
} else
printk(KERN_WARNING "%s: invalid contr %dn", __func__, contr);
+
+unlock_out:
+ mutex_unlock(&capi_controller_lock);
}
static void ctr_down(struct capi_ctr *ctr)
@@ -217,6 +223,8 @@ 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 %dn", contr);
@@ -225,6 +233,8 @@ static void notify_down(u32 contr)
ctr_down(ctr);
else
printk(KERN_WARNING "%s: invalid contr %dn", __func__, contr);
+
+ mutex_unlock(&capi_controller_lock);
}
static int
@@ -481,21 +491,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 slotsn");
return -EBUSY;
}
capi_controller[i] = ctr;
- mutex_unlock(&controller_mutex);
-
ctr->nrecvctlpkt = 0;
ctr->nrecvdatapkt = 0;
ctr->nsentctlpkt = 0;
@@ -513,6 +521,9 @@ int attach_capi_ctr(struct capi_ctr *ctr)
}
ncontrollers++;
+
+ mutex_unlock(&capi_controller_lock);
+
printk(KERN_NOTICE "kcapi: controller [%03d]: %s attachedn",
ctr->cnr, ctr->name);
return 0;
@@ -531,19 +542,29 @@ EXPORT_SYMBOL(attach_capi_ctr);
int detach_capi_ctr(struct capi_ctr *ctr)
{
+ int err = 0;
+
+ mutex_lock(&capi_controller_lock);
+
ctr_down(ctr);
+ if (capi_controller[ctr->cnr - 1] != ctr) {
+ err = -EINVAL;
+ goto unlock_out;
+ }
+ capi_controller[ctr->cnr - 1] = NULL;
ncontrollers--;
- if (ctr->procent) {
+ if (ctr->procent)
remove_proc_entry(ctr->procfn, NULL);
- ctr->procent = NULL;
- }
- capi_controller[ctr->cnr - 1] = NULL;
+
printk(KERN_NOTICE "kcapi: controller [%03d]: %s unregisteredn",
ctr->cnr, ctr->name);
- return 0;
+unlock_out:
+ mutex_unlock(&capi_controller_lock);
+
+ return err;
}
EXPORT_SYMBOL(detach_capi_ctr);
@@ -593,13 +614,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);
@@ -652,14 +681,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 upn", applid);
@@ -692,14 +723,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);
@@ -738,6 +771,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? */
@@ -802,16 +841,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);
@@ -829,17 +876,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);
@@ -857,17 +911,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);
@@ -885,21 +946,53 @@ 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)
+{
+ int cnr = ctr->cnr;
+ int retval = 0;
+
+ while (ctr->state != state) {
+ mutex_unlock(&capi_controller_lock);
+
+ msleep_interruptible(100); /* 0.1 sec */
+
+ mutex_lock(&capi_controller_lock);
+
+ if (signal_pending(current)) {
+ retval = -EINTR;
+ break;
+ }
+ if (ctr != get_capi_ctr_by_nr(cnr)) {
+ retval = -ESRCH;
+ break;
+ }
+ }
+ return retval;
+}
+
#ifdef AVMB1_COMPAT
static int old_capi_manufacturer(unsigned int cmd, void __user *data)
{
@@ -977,27 +1070,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 functionn");
- 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 0n");
- capi_ctr_put(ctr);
- return -EINVAL;
+ retval = -EINVAL;
+ goto load_unlock_out;
}
ldata.firmware.user = 1;
@@ -1009,52 +1105,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 staten", 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) {
+ retval = wait_on_ctr_state(ctr, CAPI_CTR_RUNNING);
- msleep_interruptible(100); /* 0.1 sec */
-
- 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;
}
@@ -1072,6 +1163,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
@@ -1089,14 +1181,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=%dn",
+ 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=%dn",
- ctr->cnr, ctr->traceflag);
- return 0;
+ return retval;
}
case KCAPI_CMD_ADDCARD:
{
@@ -1104,7 +1202,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..966c4b2 100644
--- a/drivers/isdn/capi/kcapi.h
+++ b/drivers/isdn/capi/kcapi.h
@@ -32,8 +32,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)
--
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 fa18971..2af6c05 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -145,7 +145,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
@@ -405,7 +405,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)
@@ -414,15 +413,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);
@@ -434,9 +437,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);
}
@@ -1432,7 +1432,7 @@ static int proc_capidev_read_proc(char *page, char **start, off_t off,
struct list_head *l;
int len = 0;
- read_lock(&capidev_list_lock);
+ mutex_lock(&capidev_list_lock);
list_for_each(l, &capidev_list) {
cdev = list_entry(l, struct capidev, list);
len += sprintf(page+len, "0 %d %lu %lu %lu %lun",
@@ -1451,7 +1451,7 @@ static int proc_capidev_read_proc(char *page, char **start, off_t off,
}
endloop:
- read_unlock(&capidev_list_lock);
+ mutex_unlock(&capidev_list_lock);
if (len < count)
*eof = 1;
if (len > count) len = count;
@@ -1471,7 +1471,7 @@ static int proc_capincci_read_proc(char *page, char **start, off_t off,
struct list_head *l;
int len = 0;
- 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) {
@@ -1488,7 +1488,7 @@ static int proc_capincci_read_proc(char *page, char **start, off_t off,
}
}
endloop:
- read_unlock(&capidev_list_lock);
+ mutex_unlock(&capidev_list_lock);
*start = page+off;
if (len < count)
*eof = 1;
--
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 | 187 ++++++++++++++++++++++-----------------------
1 files changed, 91 insertions(+), 96 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 2af81c8..e7830b7 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -140,7 +140,7 @@ struct capidev {
struct capincci *nccis;
- struct mutex ncci_list_mtx;
+ struct mutex lock;
};
/* -------- global variables ---------------------------------------- */
@@ -573,38 +573,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 foundn");
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
@@ -617,8 +610,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);
@@ -651,7 +643,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 ----------------------------- */
@@ -729,9 +723,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);
@@ -764,30 +758,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;
}
- return (int)ap->applid;
+ 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;
+ }
+
+register_out:
+ mutex_unlock(&cdev->lock);
+ return retval;
case CAPI_GET_VERSION:
{
@@ -886,68 +885,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)
@@ -958,7 +956,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;
@@ -978,15 +976,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;
@@ -1449,6 +1442,7 @@ static int proc_capincci_read_proc(char *page, char **start, off_t off,
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) {
len += sprintf(page+len, "%d 0x%xn",
cdev->ap.applid,
@@ -1456,11 +1450,12 @@ static int proc_capincci_read_proc(char *page, char **start, off_t off,
if (len <= off) {
off -= len;
len = 0;
- } else {
- if (len-off > count)
- goto endloop;
+ } else if (len-off > count) {
+ mutex_unlock(&cdev->lock);
+ goto endloop;
}
}
+ mutex_unlock(&cdev->lock);
}
endloop:
mutex_unlock(&capidev_list_lock);
--
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 b71b8b1..90a20fa 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -656,24 +656,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
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 c456f61..fa18971 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/poll.h>
@@ -55,17 +51,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 */
@@ -80,7 +76,6 @@ module_param_named(ttyminors, capi_ttyminors, uint, 0);
struct capidev;
struct capincci;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
struct capiminor;
struct datahandle_queue {
@@ -115,7 +110,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).
@@ -155,14 +149,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;
@@ -300,26 +293,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
@@ -329,7 +313,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->nccipn");
+#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;
@@ -339,29 +374,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->nccipn");
-#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 {
@@ -551,7 +570,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;
@@ -627,10 +646,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);
@@ -638,10 +660,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=%dn",
@@ -671,6 +690,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);
}
@@ -928,9 +948,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)))
@@ -941,11 +958,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;
}
@@ -1395,7 +1408,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 ----------------------------------------- */
@@ -1534,23 +1556,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
@@ -1568,9 +1586,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
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 e7830b7..b71b8b1 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -216,7 +216,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 capiminorn");
return NULL;
@@ -357,7 +357,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
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 6ab496c..40468ac 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -100,7 +100,6 @@ struct capiminor {
struct sk_buff *ttyskb;
struct sk_buff_head inqueue;
- int inbytes;
struct sk_buff_head outqueue;
int outbytes;
@@ -481,15 +480,12 @@ keep:
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)
@@ -611,7 +607,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
Replace open-coded NCCI list management with standard mechanisms.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 47 ++++++++++++++++++---------------------------
1 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 90a20fa..249ff13 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -121,7 +121,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
@@ -138,7 +138,7 @@ struct capidev {
struct sk_buff_head recvqueue;
wait_queue_head_t recvwait;
- struct capincci *nccis;
+ struct list_head nccis;
struct mutex lock;
};
@@ -355,7 +355,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)
@@ -365,39 +365,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
@@ -954,6 +946,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);
@@ -1429,16 +1422,14 @@ endloop:
static int proc_capincci_read_proc(char *page, char **start, off_t off,
int count, int *eof, void *data)
{
- struct capidev *cdev;
- struct capincci *np;
- struct list_head *l;
+ struct capidev *cdev;
+ struct capincci *np;
int len = 0;
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) {
+ list_for_each_entry(np, &cdev->nccis, list) {
len += sprintf(page+len, "%d 0x%xn",
cdev->ap.applid,
np->ncci);
--
1.6.0.2
The CAPI user interface comes with two types of devices: /dev/capi20 is
a character device, instances are represented by capidev objects. Each
established network control connection triggers the creation of a TTY
device (/dev/capi/<ncci>), open instances are managed via capiminor
objects. capincci objects are representing the connections, and they
link the corresponding capiminor to its capidev. Unfortunately, the
lifetimes of an NCCI is not identical to that of a capiminor instance.
So we need proper locking to glue things and also unbind them again.
This patch fixes the totally broken attempts to achieve this via a
rwlock and some atomic counter. It converts the rwlock into a mutex and
applies it to all related critical sections (capinc_tty_open/close as
well as capiminor_alloc/release).
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 82 +++++++++++++++++++++++----------------------
1 files changed, 42 insertions(+), 40 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 249ff13..b53cead 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -150,7 +150,7 @@ static LIST_HEAD(capidev_list);
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
-static DEFINE_RWLOCK(capiminor_list_lock);
+static DEFINE_MUTEX(glue_lock);
static LIST_HEAD(capiminor_list);
/* -------- datahandles --------------------------------------------- */
@@ -214,7 +214,6 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
{
struct capiminor *mp, *p;
unsigned int minor = 0;
- unsigned long flags;
mp = kzalloc(sizeof(*mp), GFP_KERNEL);
if (!mp) {
@@ -234,7 +233,6 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
/* 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 {
@@ -249,7 +247,6 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
list_add(&mp->list, p->list.prev);
}
}
- write_unlock_irqrestore(&capiminor_list_lock, flags);
if (!(minor < capi_ttyminors)) {
printk(KERN_NOTICE "capi: out of minorsn");
@@ -262,36 +259,25 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
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);
kfree_skb(mp->ttyskb);
- mp->ttyskb = NULL;
skb_queue_purge(&mp->inqueue);
skb_queue_purge(&mp->outqueue);
+
capiminor_del_all_ack(mp);
+
kfree(mp);
}
static struct capiminor *capiminor_find(unsigned int minor)
{
- struct list_head *l;
- struct capiminor *p = NULL;
-
- 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)
- return NULL;
+ struct capiminor *mp;
- return p;
+ list_for_each_entry(mp, &capiminor_list, list)
+ if (mp->minor == minor)
+ return mp;
+ return NULL;
}
/* -------- struct capincci ----------------------------------------- */
@@ -303,6 +289,8 @@ static void capincci_alloc_minor(struct capidev *cdev, struct capincci *np)
if (!(cdev->userflags & CAPIFLAG_HIGHJACKING))
return;
+ mutex_lock(&glue_lock);
+
mp = np->minorp = capiminor_alloc(&cdev->ap, np->ncci);
if (mp) {
mp->nccip = np;
@@ -313,12 +301,17 @@ static void capincci_alloc_minor(struct capidev *cdev, struct capincci *np)
capifs_new_ncci(mp->minor,
MKDEV(capi_ttymajor, mp->minor));
}
+
+ mutex_unlock(&glue_lock);
}
static void capincci_free_minor(struct capincci *np)
{
- struct capiminor *mp = np->minorp;
+ struct capiminor *mp;
+ mutex_lock(&glue_lock);
+
+ mp = np->minorp;
if (mp) {
capifs_free_ncci(mp->capifs_dentry);
if (mp->tty) {
@@ -331,6 +324,8 @@ static void capincci_free_minor(struct capincci *np)
capiminor_free(mp);
}
}
+
+ mutex_unlock(&glue_lock);
}
static inline unsigned int capincci_minor_opencount(struct capincci *np)
@@ -992,13 +987,17 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file)
{
struct capiminor *mp;
unsigned long flags;
+ int err = 0;
- if ((mp = capiminor_find(iminor(file->f_path.dentry->d_inode))) == NULL)
- return -ENXIO;
- if (mp->nccip == NULL)
- return -ENXIO;
+ mutex_lock(&glue_lock);
- tty->driver_data = (void *)mp;
+ mp = capiminor_find(iminor(file->f_path.dentry->d_inode));
+ if (!mp || !mp->nccip) {
+ err = -ENXIO;
+ goto unlock_out;
+ }
+
+ tty->driver_data = mp;
spin_lock_irqsave(&workaround_lock, flags);
if (atomic_read(&mp->ttyopencount) == 0)
@@ -1009,28 +1008,31 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file)
#endif
handle_minor_recv(mp);
spin_unlock_irqrestore(&workaround_lock, flags);
- return 0;
+
+unlock_out:
+ mutex_unlock(&glue_lock);
+ return err;
}
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)) {
+ mutex_lock(&glue_lock);
+
+ if (atomic_dec_and_test(&mp->ttyopencount)) {
#ifdef _DEBUG_REFCOUNT
- printk(KERN_DEBUG "capinc_tty_close lastclosen");
+ printk(KERN_DEBUG "capinc_tty_close lastclosen");
#endif
- tty->driver_data = NULL;
- mp->tty = NULL;
- }
+ mp->tty = NULL;
+ }
#ifdef _DEBUG_REFCOUNT
printk(KERN_DEBUG "capinc_tty_close ocount=%dn", atomic_read(&mp->ttyopencount));
#endif
- if (mp->nccip == NULL)
- capiminor_free(mp);
- }
+ if (!mp->nccip)
+ capiminor_free(mp);
+
+ mutex_unlock(&glue_lock);
#ifdef _DEBUG_REFCOUNT
printk(KERN_DEBUG "capinc_tty_closen");
--
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 | 31 +++----------------------------
1 files changed, 3 insertions(+), 28 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index b53cead..7710a66 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -44,7 +44,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 */
@@ -98,7 +97,6 @@ struct capiminor {
int ttyinstop;
int ttyoutstop;
struct sk_buff *ttyskb;
- atomic_t ttyopencount;
struct sk_buff_head inqueue;
int inbytes;
@@ -224,7 +222,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);
@@ -294,9 +291,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->nccipn");
-#endif
mp->capifs_dentry =
capifs_new_ncci(mp->minor,
MKDEV(capi_ttymajor, mp->minor));
@@ -316,9 +310,6 @@ static void capincci_free_minor(struct capincci *np)
capifs_free_ncci(mp->capifs_dentry);
if (mp->tty) {
mp->nccip = NULL;
-#ifdef _DEBUG_REFCOUNT
- printk(KERN_DEBUG "reset mp->nccipn");
-#endif
tty_hangup(mp->tty);
} else {
capiminor_free(mp);
@@ -332,7 +323,7 @@ static inline unsigned int capincci_minor_opencount(struct capincci *np)
{
struct capiminor *mp = np->minorp;
- return mp ? atomic_read(&mp->ttyopencount) : 0;
+ return (mp && mp->tty) ? mp->tty->count : 0;
}
#else /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
@@ -998,14 +989,9 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file)
}
tty->driver_data = mp;
+ mp->tty = tty;
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=%dn", atomic_read(&mp->ttyopencount));
-#endif
handle_minor_recv(mp);
spin_unlock_irqrestore(&workaround_lock, flags);
@@ -1020,23 +1006,12 @@ static void capinc_tty_close(struct tty_struct * tty, struct file * file)
mutex_lock(&glue_lock);
- if (atomic_dec_and_test(&mp->ttyopencount)) {
-#ifdef _DEBUG_REFCOUNT
- printk(KERN_DEBUG "capinc_tty_close lastclosen");
-#endif
+ if (tty->count == 1)
mp->tty = NULL;
- }
-#ifdef _DEBUG_REFCOUNT
- printk(KERN_DEBUG "capinc_tty_close ocount=%dn", atomic_read(&mp->ttyopencount));
-#endif
if (!mp->nccip)
capiminor_free(mp);
mutex_unlock(&glue_lock);
-
-#ifdef _DEBUG_REFCOUNT
- printk(KERN_DEBUG "capinc_tty_closen");
-#endif
}
static int capinc_tty_write(struct tty_struct * tty,
--
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 b2a0475..66647ed 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
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).
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 119 +++++++++++++++++++++++++---------------------
1 files changed, 65 insertions(+), 54 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 40468ac..32a6f04 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -97,11 +97,13 @@ struct capiminor {
struct mutex ttylock;
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;
@@ -109,15 +111,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;
@@ -226,6 +219,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);
mutex_init(&mp->ttylock);
@@ -259,7 +253,7 @@ static void capiminor_free(struct capiminor *mp)
{
list_del(&mp->list);
- kfree_skb(mp->ttyskb);
+ kfree_skb(mp->outskb);
skb_queue_purge(&mp->inqueue);
skb_queue_purge(&mp->outqueue);
@@ -503,9 +497,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);
@@ -521,13 +524,17 @@ 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);
+
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=%un",
datahandle, len);
@@ -538,13 +545,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 = %xn", errcode);
- mp->outbytes -= len;
kfree_skb(skb);
}
return count;
@@ -561,7 +572,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);
@@ -573,7 +583,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);
@@ -634,7 +643,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);
}
@@ -989,7 +997,6 @@ static const struct file_operations capi_fops =
static int capinc_tty_open(struct tty_struct * tty, struct file * file)
{
struct capiminor *mp;
- unsigned long flags;
int err = 0;
mutex_lock(&glue_lock);
@@ -1003,9 +1010,7 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file)
tty->driver_data = mp;
mp->tty = tty;
- spin_lock_irqsave(&workaround_lock, flags);
handle_minor_recv(mp);
- spin_unlock_irqrestore(&workaround_lock, flags);
unlock_out:
mutex_unlock(&glue_lock);
@@ -1035,71 +1040,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 failedn");
- 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 lostn", 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;
}
@@ -1107,21 +1119,22 @@ 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_charsn");
#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);
- }
- spin_unlock_irqrestore(&workaround_lock, flags);
+ } else
+ spin_unlock_bh(&mp->outlock);
handle_minor_recv(mp);
}
@@ -1203,14 +1216,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_startn");
#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
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 | 20 ++++++++------------
1 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 32a6f04..7a04492 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -149,7 +149,6 @@ static LIST_HEAD(capiminor_list);
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);
}
@@ -628,7 +623,8 @@ 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);
+
mutex_lock(&mp->ttylock);
if (mp->tty)
tty_wakeup(mp->tty);
--
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 e880639..b8019a1 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -77,7 +77,7 @@ struct capidev;
struct capincci;
struct capiminor;
-struct datahandle_queue {
+struct ackqueue_entry {
struct list_head list;
u16 datahandle;
};
@@ -156,7 +156,7 @@ static LIST_HEAD(capiminor_list);
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
Introduce a mutex called ttylock to make handle_recv_skb atomic /wrt
parallel invocations as well as capinc_tty_close clearing the tty_struct
reference. Also 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, or a full TTY input buffer.
As we expect to be in thread context in handle_recv_skb, we must neither
call this function from potenially atomic capinc_tty_write nor while
holding the workaround_lock. For the same reason, we do not need to
allocate skbs atomically in gen_data_b3_resp_for.
The ttylock is also used to avoid waking up a non-existent TTY in
capi_recv_message.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 90 ++++++++++++++++++++++++++++------------------
1 files changed, 55 insertions(+), 35 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 77faa4e..e880639 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -93,9 +93,10 @@ struct capiminor {
u16 datahandle;
u16 msgid;
- struct tty_struct *tty;
- int ttyinstop;
- int ttyoutstop;
+ struct tty_struct *tty;
+ struct mutex ttylock;
+ int ttyinstop;
+ int ttyoutstop;
struct sk_buff *ttyskb;
struct sk_buff_head inqueue;
@@ -228,6 +229,8 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
skb_queue_head_init(&mp->inqueue);
skb_queue_head_init(&mp->outqueue);
+ mutex_init(&mp->ttylock);
+
/* Allocate the least unused minor number.
*/
if (list_empty(&capiminor_list))
@@ -385,7 +388,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);
@@ -406,61 +409,74 @@ static int handle_recv_skb(struct capiminor *mp, struct sk_buff *skb)
int datalen;
u16 errcode, datahandle;
struct tty_ldisc *ld;
-
- datalen = skb->len - CAPIMSG_LEN(skb->data);
- if (mp->tty == NULL)
- {
+ int ret = -1;
+
+ mutex_lock(&mp->ttylock);
+
+ if (!mp->tty) {
#ifdef _DEBUG_DATAFLOW
printk(KERN_DEBUG "capi: currently no receivern");
#endif
+ mutex_unlock(&mp->ttylock);
return -1;
}
ld = tty_ldisc_ref(mp->tty);
- if (ld == NULL)
- return -1;
- if (ld->ops->receive_buf == NULL) {
+ if (!ld || !ld->ops->receive_buf) {
#if defined(_DEBUG_DATAFLOW) || defined(_DEBUG_TTYFUNCS)
- printk(KERN_DEBUG "capi: ldisc has no receive_buf functionn");
+ printk(KERN_DEBUG "capi: no ldisc or receive_buf functionn");
#endif
- goto bad;
+ ret = 0;
+ goto drop;
}
if (mp->ttyinstop) {
#if defined(_DEBUG_DATAFLOW) || defined(_DEBUG_TTYFUNCS)
printk(KERN_DEBUG "capi: recv tty throttledn");
#endif
- goto bad;
+ goto keep;
}
+
+ datalen = skb->len - CAPIMSG_LEN(skb->data);
if (mp->tty->receive_room < datalen) {
#if defined(_DEBUG_DATAFLOW) || defined(_DEBUG_TTYFUNCS)
printk(KERN_DEBUG "capi: no room in ttyn");
#endif
- goto bad;
+ goto keep;
}
- 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 failedn");
- goto bad;
+ goto keep;
}
+
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 => ldiscn",
+ datahandle, skb->len);
+#endif
+ ld->ops->receive_buf(mp->tty, skb->data, NULL, skb->len);
+ } else {
printk(KERN_ERR "capi: send DATA_B3_RESP failed=%xn",
errcode);
kfree_skb(nskb);
- goto bad;
+
+ if (errcode == CAPI_SENDQUEUEFULL)
+ goto keep;
}
- (void)skb_pull(skb, CAPIMSG_LEN(skb->data));
-#ifdef _DEBUG_DATAFLOW
- printk(KERN_DEBUG "capi: DATA_B3_RESP %u len=%d => ldiscn",
- datahandle, skb->len);
-#endif
- ld->ops->receive_buf(mp->tty, skb->data, NULL, skb->len);
+ ret = 0;
+
+drop:
kfree_skb(skb);
+keep:
tty_ldisc_deref(ld);
- return 0;
-bad:
- tty_ldisc_deref(ld);
- return -1;
+ mutex_unlock(&mp->ttylock);
+ return ret;
}
static void handle_minor_recv(struct capiminor *mp)
@@ -610,8 +626,10 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
#endif
kfree_skb(skb);
(void)capiminor_del_ack(mp, datahandle);
+ mutex_lock(&mp->ttylock);
if (mp->tty)
tty_wakeup(mp->tty);
+ mutex_unlock(&mp->ttylock);
(void)handle_minor_send(mp);
} else {
@@ -1006,8 +1024,12 @@ static void capinc_tty_close(struct tty_struct * tty, struct file * file)
mutex_lock(&glue_lock);
- if (tty->count == 1)
+ if (tty->count == 1) {
+ mutex_lock(&mp->ttylock);
mp->tty = NULL;
+ mutex_unlock(&mp->ttylock);
+ }
+
if (!mp->nccip)
capiminor_free(mp);
@@ -1046,7 +1068,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;
}
@@ -1106,8 +1127,9 @@ static void capinc_tty_flush_chars(struct tty_struct *tty)
mp->outbytes += skb->len;
(void)handle_minor_send(mp);
}
- (void)handle_minor_recv(mp);
spin_unlock_irqrestore(&workaround_lock, flags);
+
+ handle_minor_recv(mp);
}
static int capinc_tty_write_room(struct tty_struct *tty)
@@ -1167,14 +1189,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_unthrottlen");
#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)
--
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 | 20 +++++++++-----------
1 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 7a04492..a91b9ae 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -477,11 +477,10 @@ 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 sk_buff *skb;
u16 len;
- int count = 0;
u16 errcode;
u16 datahandle;
@@ -489,7 +488,7 @@ static int handle_minor_send(struct capiminor *mp)
#if defined(_DEBUG_DATAFLOW) || defined(_DEBUG_TTYFUNCS)
printk(KERN_DEBUG "capi: send: tty stoppedn");
#endif
- return 0;
+ return;
}
while (1) {
@@ -525,11 +524,10 @@ static int handle_minor_send(struct capiminor *mp)
mp->outbytes += len;
spin_unlock_bh(&mp->outlock);
- 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=%un",
datahandle, len);
@@ -553,7 +551,6 @@ static int handle_minor_send(struct capiminor *mp)
printk(KERN_ERR "capi: put_message = %xn", errcode);
kfree_skb(skb);
}
- return count;
}
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
@@ -629,7 +626,8 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
if (mp->tty)
tty_wakeup(mp->tty);
mutex_unlock(&mp->ttylock);
- (void)handle_minor_send(mp);
+
+ handle_minor_send(mp);
} else {
/* ups, let capi application handle it :-) */
@@ -1063,7 +1061,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;
}
@@ -1106,7 +1104,7 @@ unlock_out:
spin_unlock_bh(&mp->outlock);
if (invoke_send)
- (void)handle_minor_send(mp);
+ handle_minor_send(mp);
return ret;
}
@@ -1128,7 +1126,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);
@@ -1217,7 +1215,7 @@ static void capinc_tty_start(struct tty_struct *tty)
printk(KERN_DEBUG "capinc_tty_startn");
#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
Remove more desperate attempts to fix races in the original code: As
long as the TTY is open, its driver_data cannot become NULL. And we
already call tty_hangup to signal the end of a connection, no need to
check for mp->nccip in addition.
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/isdn/capi/capi.c | 80 ++++++++++++----------------------------------
1 files changed, 21 insertions(+), 59 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 7710a66..77faa4e 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -1017,7 +1017,7 @@ static void capinc_tty_close(struct tty_struct * tty, struct file * file)
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;
@@ -1025,13 +1025,6 @@ static int capinc_tty_write(struct tty_struct * tty,
printk(KERN_DEBUG "capinc_tty_write(count=%d)n", count);
#endif
- if (!mp || !mp->nccip) {
-#ifdef _DEBUG_TTYFUNCS
- printk(KERN_DEBUG "capinc_tty_write: mp or mp->ncci NULLn");
-#endif
- return 0;
- }
-
spin_lock_irqsave(&workaround_lock, flags);
skb = mp->ttyskb;
if (skb) {
@@ -1060,7 +1053,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;
@@ -1069,13 +1062,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 || !mp->nccip) {
-#ifdef _DEBUG_TTYFUNCS
- printk(KERN_DEBUG "capinc_tty_put_char: mp or mp->ncci NULLn");
-#endif
- return 0;
- }
-
spin_lock_irqsave(&workaround_lock, flags);
skb = mp->ttyskb;
if (skb) {
@@ -1104,7 +1090,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;
@@ -1112,13 +1098,6 @@ static void capinc_tty_flush_chars(struct tty_struct *tty)
printk(KERN_DEBUG "capinc_tty_flush_charsn");
#endif
- if (!mp || !mp->nccip) {
-#ifdef _DEBUG_TTYFUNCS
- printk(KERN_DEBUG "capinc_tty_flush_chars: mp or mp->ncci NULLn");
-#endif
- return;
- }
-
spin_lock_irqsave(&workaround_lock, flags);
skb = mp->ttyskb;
if (skb) {
@@ -1133,14 +1112,9 @@ 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) {
-#ifdef _DEBUG_TTYFUNCS
- printk(KERN_DEBUG "capinc_tty_write_room: mp or mp->ncci NULLn");
-#endif
- return 0;
- }
+
room = CAPINC_MAX_SENDQUEUE-skb_queue_len(&mp->outqueue);
room *= CAPI_MAX_BLKSIZE;
#ifdef _DEBUG_TTYFUNCS
@@ -1151,13 +1125,8 @@ 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) {
-#ifdef _DEBUG_TTYFUNCS
- printk(KERN_DEBUG "capinc_tty_chars_in_buffer: mp or mp->ncci NULLn");
-#endif
- return 0;
- }
+ struct capiminor *mp = tty->driver_data;
+
#ifdef _DEBUG_TTYFUNCS
printk(KERN_DEBUG "capinc_tty_chars_in_buffer = %d nack=%d sq=%d rq=%dn",
mp->outbytes, mp->nack,
@@ -1188,53 +1157,46 @@ static void capinc_tty_set_termios(struct tty_struct *tty, struct ktermios * old
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_throttlen");
#endif
- if (mp)
- mp->ttyinstop = 1;
+ mp->ttyinstop = 1;
}
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_unthrottlen");
#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_stopn");
#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_startn");
#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
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 2c5d1b8..0e00028 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -770,11 +770,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
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 b8019a1..6ab496c 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -88,10 +88,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_struct *tty;
struct mutex ttylock;
@@ -222,7 +222,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);
@@ -396,7 +395,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);
}
@@ -509,7 +508,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);
@@ -517,7 +516,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 */
@@ -531,7 +530,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
I found no trace of this mysterious "pcl181" device, neither in-tree nor
out there in the wild. At the same time, the in-tree CAPI middleware is
using major 191 for many years now and obviously without any conflict.
Let's officially claim this major number.
CC: Alan Cox <[email protected]>
Signed-off-by: Jan Kiszka <[email protected]>
---
Documentation/devices.txt | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/devices.txt b/Documentation/devices.txt
index 53d64d3..4dfc2a0 100644
--- a/Documentation/devices.txt
+++ b/Documentation/devices.txt
@@ -403,7 +403,7 @@ Your cooperation is appreciated.
188 = /dev/smbusbios SMBus BIOS
189 = /dev/ussp_ctl User space serial port control
190 = /dev/crash Mission Critical Linux crash dump facility
- 191 = /dev/pcl181 <information missing>
+ 191 = /dev/capi/[0-9]* CAPI 2.0 middleware, NCCI TTYs
192 = /dev/nas_xbus NAS xbus LCD/buttons access
193 = /dev/d7s SPARC 7-segment display
194 = /dev/zkshim Zero-Knowledge network shim control
@@ -2618,7 +2618,10 @@ Your cooperation is appreciated.
1 = /dev/kctt1 Second KCT/T card
...
-191 char Reserved for PCMCIA
+191 char CAPI 2.0 middleware, NCCI TTYs
+ 0 = /dev/capi/0 TTY for NCCI ID 0
+ 1 = /dev/capi/1 TTY for NCCI ID 1
+ ...
192 char Kernel profiling interface
0 = /dev/profile Profiling control device
--
1.6.0.2
From: Jan Kiszka <[email protected]>
Date: Sat, 23 Jan 2010 00:03:26 +0100
> First of all sorry if I'm breaking the correct flow of ISDN patches, but
> I'm under the impression that things currently get merged directly via
> Dave, right?
>
> This series can also be pulled from
>
> git://git.kiszka.org/linux-2.6.git capi
This doesn't pull cleanly into net-next-2.6 which is where I would
be adding it.
There were a bunch of changes to the procfs code here and it conflicts
with your changes.
Please do a merge and sort things out, and then send another
pull request.
Thanks.
On Sun, 10 Jan 2010 14:12:41 +0100
Jan Kiszka <[email protected]> wrote:
> 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.
Slightly NAK this - I'm really glad to see the work getting done, but
until it uses krefs properly for the tty references I'd say it is
experimental still.
I'd like to help you sort out the tty krefs stuff, using tty_port and so
on and get that wart fixed too.
Alan
On Sun, 10 Jan 2010 14:21:31 +0100
Jan Kiszka <[email protected]> wrote:
> I found no trace of this mysterious "pcl181" device, neither in-tree nor
> out there in the wild. At the same time, the in-tree CAPI middleware is
> using major 191 for many years now and obviously without any conflict.
> Let's officially claim this major number.
This is not the way it should have been done but whoever needs spanking
got away with it years ago. Given that this seems the best way forward.
With LANANA hat on
Acked-by: Alan Cox <[email protected]>
Hi Alan,
> > I found no trace of this mysterious "pcl181" device, neither in-tree nor
> > out there in the wild. At the same time, the in-tree CAPI middleware is
> > using major 191 for many years now and obviously without any conflict.
> > Let's officially claim this major number.
>
> This is not the way it should have been done but whoever needs spanking
> got away with it years ago. Given that this seems the best way forward.
>
> With LANANA hat on
actually in the days of udev, the capifs is not really needed anymore.
The right choice would be to remove it. I haven't been enabling it since
years.
Regards
Marcel
Marcel Holtmann wrote:
> Hi Alan,
>
>>> I found no trace of this mysterious "pcl181" device, neither in-tree nor
>>> out there in the wild. At the same time, the in-tree CAPI middleware is
>>> using major 191 for many years now and obviously without any conflict.
>>> Let's officially claim this major number.
>> This is not the way it should have been done but whoever needs spanking
>> got away with it years ago. Given that this seems the best way forward.
>>
>> With LANANA hat on
>
> actually in the days of udev, the capifs is not really needed anymore.
> The right choice would be to remove it. I haven't been enabling it since
> years.
First of all, the capifs story is orthogonal to the major claim.
But basically you are right, capifs is likely not needed anymore. The
only user visible change - and that was holding me back to suggest its
removal - is the time when the NCCI minor ttys show up under /dev/capi/
(or wherever you direct them to). If I didn't miss something about udev,
it will make all possible minors pop up once the major is registered.
However, I'm not sure if there is some userland actually relying on this.
Jan
Alan Cox wrote:
> On Sun, 10 Jan 2010 14:12:41 +0100
> Jan Kiszka <[email protected]> wrote:
>
>> 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.
>
> Slightly NAK this - I'm really glad to see the work getting done, but
> until it uses krefs properly for the tty references I'd say it is
> experimental still.
>
> I'd like to help you sort out the tty krefs stuff, using tty_port and so
> on and get that wart fixed too.
Will look into to this. Some good starting point or reference driver for me?
Thanks,
Jan
On Samstag, 23. Januar 2010 13:48:12 Marcel Holtmann wrote:
> Hi Alan,
>
> > > I found no trace of this mysterious "pcl181" device, neither in-tree
> > > nor out there in the wild. At the same time, the in-tree CAPI
> > > middleware is using major 191 for many years now and obviously without
> > > any conflict. Let's officially claim this major number.
> >
> > This is not the way it should have been done but whoever needs spanking
> > got away with it years ago. Given that this seems the best way forward.
> >
> > With LANANA hat on
>
> actually in the days of udev, the capifs is not really needed anymore.
> The right choice would be to remove it. I haven't been enabling it since
> years.
>
So far I understand, the pppd capiplugin is the only user of it, so it could
be disabled for most users without any problems, as long they are not using
PPP connections via CAPI.
I never understand capifs very well, I think that it can be dropped because of
udev, but maybe need some adjustment in user space as well (make sure that
udev did create the node before open it).
I f I remember correctly, here was some proposal to replace the /dev/capi/
nodes with devpts, this would remove the complete capi_tty device major
as well.
Karsten
On Sat, 23 Jan 2010 14:13:47 +0100
Jan Kiszka <[email protected]> wrote:
> Alan Cox wrote:
> > On Sun, 10 Jan 2010 14:12:41 +0100
> > Jan Kiszka <[email protected]> wrote:
> >
> >> 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.
> >
> > Slightly NAK this - I'm really glad to see the work getting done, but
> > until it uses krefs properly for the tty references I'd say it is
> > experimental still.
> >
> > I'd like to help you sort out the tty krefs stuff, using tty_port and so
> > on and get that wart fixed too.
>
> Will look into to this. Some good starting point or reference driver for me?
Most of the drivers at least use tty_port and the tty_port ref counting
helpers. The most full use of the new interfaces is probably the latest
USB serial code and the drivers/mmc/card/sdio_uart.c code, which uses all
the new facilities to the full to kick all the nasty open/close/hangup
locking and logic out of the low level drivers.
Hi Jan,
> >>> I found no trace of this mysterious "pcl181" device, neither in-tree nor
> >>> out there in the wild. At the same time, the in-tree CAPI middleware is
> >>> using major 191 for many years now and obviously without any conflict.
> >>> Let's officially claim this major number.
> >> This is not the way it should have been done but whoever needs spanking
> >> got away with it years ago. Given that this seems the best way forward.
> >>
> >> With LANANA hat on
> >
> > actually in the days of udev, the capifs is not really needed anymore.
> > The right choice would be to remove it. I haven't been enabling it since
> > years.
>
> First of all, the capifs story is orthogonal to the major claim.
my point here is merely that when using udev, you need to fixed assigned
major number. Dynamic major numbers will just work fine.
> But basically you are right, capifs is likely not needed anymore. The
> only user visible change - and that was holding me back to suggest its
> removal - is the time when the NCCI minor ttys show up under /dev/capi/
> (or wherever you direct them to). If I didn't miss something about udev,
> it will make all possible minors pop up once the major is registered.
> However, I'm not sure if there is some userland actually relying on this.
That is just an issue with the current code. There is no requirement to
create all minors are at the same. You can create/remove minors on
demand as you please. And udev will take care of the device nodes for
you.
Regards
Marcel
Hi Karsten,
> > > > I found no trace of this mysterious "pcl181" device, neither in-tree
> > > > nor out there in the wild. At the same time, the in-tree CAPI
> > > > middleware is using major 191 for many years now and obviously without
> > > > any conflict. Let's officially claim this major number.
> > >
> > > This is not the way it should have been done but whoever needs spanking
> > > got away with it years ago. Given that this seems the best way forward.
> > >
> > > With LANANA hat on
> >
> > actually in the days of udev, the capifs is not really needed anymore.
> > The right choice would be to remove it. I haven't been enabling it since
> > years.
> >
> So far I understand, the pppd capiplugin is the only user of it, so it could
> be disabled for most users without any problems, as long they are not using
> PPP connections via CAPI.
PPP connection via CAPI works just fine without capifs. You just need
udev to create the device nodes.
> I never understand capifs very well, I think that it can be dropped because of
> udev, but maybe need some adjustment in user space as well (make sure that
> udev did create the node before open it).
I am pretty sure that I send a patch for that a long long time ago. I
haven been using CAPI + PPP without capifs.
> I f I remember correctly, here was some proposal to replace the /dev/capi/
> nodes with devpts, this would remove the complete capi_tty device major
> as well.
Don't remember anything like this. However extending the kernel code
with a CAPI PPP channel type would be better actually.
Regards
Marcel
Marcel Holtmann wrote:
> Hi Karsten,
>
>>>>> I found no trace of this mysterious "pcl181" device, neither in-tree
>>>>> nor out there in the wild. At the same time, the in-tree CAPI
>>>>> middleware is using major 191 for many years now and obviously without
>>>>> any conflict. Let's officially claim this major number.
>>>> This is not the way it should have been done but whoever needs spanking
>>>> got away with it years ago. Given that this seems the best way forward.
>>>>
>>>> With LANANA hat on
>>> actually in the days of udev, the capifs is not really needed anymore.
>>> The right choice would be to remove it. I haven't been enabling it since
>>> years.
>>>
>> So far I understand, the pppd capiplugin is the only user of it, so it could
>> be disabled for most users without any problems, as long they are not using
>> PPP connections via CAPI.
>
> PPP connection via CAPI works just fine without capifs. You just need
> udev to create the device nodes.
>
>> I never understand capifs very well, I think that it can be dropped because of
>> udev, but maybe need some adjustment in user space as well (make sure that
>> udev did create the node before open it).
>
> I am pretty sure that I send a patch for that a long long time ago. I
> haven been using CAPI + PPP without capifs.
>
>> I f I remember correctly, here was some proposal to replace the /dev/capi/
>> nodes with devpts, this would remove the complete capi_tty device major
>> as well.
>
> Don't remember anything like this. However extending the kernel code
> with a CAPI PPP channel type would be better actually.
Not sure how much of pppdcapiplugin would have to be moved into the
kernel then, but if it allows us to even drop that thing, it might be
worth it.
In any case, I think we first need a solution for existing user space.
So if pppdcapiplugin can be safely considered the only user of
/dev/capi/*, I will rework my series to use a dynamic major and will
file a patch to first deprecate and then remove capifs. What would be a
reasonable warning period? Something like 3 kernel releases?
Jan