Skip to content

Commit

Permalink
CAPI: Rework controller state notifier
Browse files Browse the repository at this point in the history 
Another step towards proper locking: Rework the callback provided to
capidrv for controller state changes. This is so far attached to an
application, which would require us to hold the corresponding lock
across notification calls.

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

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Jan Kiszka authored and David S. Miller committed Feb 17, 2010
1 parent 3efecf7 commit ef69bb2
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 79 deletions.
22 changes: 16 additions & 6 deletions drivers/isdn/capi/capidrv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2210,19 +2210,24 @@ static int capidrv_delcontr(u16 contr)
}


static void lower_callback(unsigned int cmd, u32 contr, void *data)
static int
lower_callback(struct notifier_block *nb, unsigned long val, void *v)
{
capi_profile profile;
u32 contr = (long)v;

switch (cmd) {
case KCI_CONTRUP:
switch (val) {
case CAPICTR_UP:
printk(KERN_INFO "capidrv: controller %hu up\n", contr);
(void) capidrv_addcontr(contr, (capi_profile *) data);
if (capi20_get_profile(contr, &profile) == CAPI_NOERROR)
(void) capidrv_addcontr(contr, &profile);
break;
case KCI_CONTRDOWN:
case CAPICTR_DOWN:
printk(KERN_INFO "capidrv: controller %hu down\n", contr);
(void) capidrv_delcontr(contr);
break;
}
return NOTIFY_OK;
}

/*
Expand Down Expand Up @@ -2262,6 +2267,10 @@ static void __exit proc_exit(void)
remove_proc_entry("capi/capidrv", NULL);
}

static struct notifier_block capictr_nb = {
.notifier_call = lower_callback,
};

static int __init capidrv_init(void)
{
capi_profile profile;
Expand All @@ -2278,7 +2287,7 @@ static int __init capidrv_init(void)
return -EIO;
}

capi20_set_callback(&global.ap, lower_callback);
register_capictr_notifier(&capictr_nb);

errcode = capi20_get_profile(0, &profile);
if (errcode != CAPI_NOERROR) {
Expand All @@ -2300,6 +2309,7 @@ static int __init capidrv_init(void)

static void __exit capidrv_exit(void)
{
unregister_capictr_notifier(&capictr_nb);
capi20_release(&global.ap);

proc_exit();
Expand Down
112 changes: 51 additions & 61 deletions drivers/isdn/capi/kcapi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

/* ------------------------------------------------------------- */
Expand All @@ -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 *
Expand Down Expand Up @@ -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;
Expand All @@ -188,16 +186,11 @@ static void notify_up(u32 contr)
if (!ap || ap->release_in_progress)
continue;
register_appl(ctr, applid, &ap->rparam);
if (ap->callback && !ap->release_in_progress)
ap->callback(KCI_CONTRUP, contr,
&ctr->profile);
}
} else
printk(KERN_WARNING "%s: invalid contr %d\n", __func__, contr);
}

/* -------- KCI_CONTRDOWN ------------------------------------- */

static void ctr_down(struct capi_ctr *ctr)
{
struct capi20_appl *ap;
Expand All @@ -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);
}
}
}

Expand All @@ -237,45 +227,63 @@ static void notify_down(u32 contr)
printk(KERN_WARNING "%s: invalid contr %d\n", __func__, contr);
}

static void notify_handler(struct work_struct *work)
static int
notify_handler(struct notifier_block *nb, unsigned long val, void *v)
{
struct capi_notifier *np =
container_of(work, struct capi_notifier, work);
u32 contr = (long)v;

switch (np->cmd) {
case KCI_CONTRUP:
notify_up(np->controller);
switch (val) {
case CAPICTR_UP:
notify_up(contr);
break;
case KCI_CONTRDOWN:
notify_down(np->controller);
case CAPICTR_DOWN:
notify_down(contr);
break;
}
return NOTIFY_OK;
}

static void do_notify_work(struct work_struct *work)
{
struct capictr_event *event =
container_of(work, struct capictr_event, work);

kfree(np);
blocking_notifier_call_chain(&ctr_notifier_list, event->type,
(void *)(long)event->controller);
kfree(event);
}

/*
* The notifier will result in adding/deleteing of devices. Devices can
* only removed in user process, not in bh.
*/
static int notify_push(unsigned int cmd, u32 controller, u16 applid, u32 ncci)
static int notify_push(unsigned int event_type, u32 controller)
{
struct capi_notifier *np = kmalloc(sizeof(*np), GFP_ATOMIC);
struct capictr_event *event = kmalloc(sizeof(*event), GFP_ATOMIC);

if (!np)
if (!event)
return -ENOMEM;

INIT_WORK(&np->work, notify_handler);
np->cmd = cmd;
np->controller = controller;
np->applid = applid;
np->ncci = ncci;
INIT_WORK(&event->work, do_notify_work);
event->type = event_type;
event->controller = controller;

schedule_work(&np->work);
schedule_work(&event->work);
return 0;
}


int register_capictr_notifier(struct notifier_block *nb)
{
return blocking_notifier_chain_register(&ctr_notifier_list, nb);
}
EXPORT_SYMBOL_GPL(register_capictr_notifier);

int unregister_capictr_notifier(struct notifier_block *nb)
{
return blocking_notifier_chain_unregister(&ctr_notifier_list, nb);
}
EXPORT_SYMBOL_GPL(unregister_capictr_notifier);

/* -------- Receiver ------------------------------------------ */

static void recv_handler(struct work_struct *work)
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -633,7 +641,6 @@ u16 capi20_register(struct capi20_appl *ap)
ap->nrecvdatapkt = 0;
ap->nsentctlpkt = 0;
ap->nsentdatapkt = 0;
ap->callback = NULL;
mutex_init(&ap->recv_mtx);
skb_queue_head_init(&ap->recv_queue);
INIT_WORK(&ap->recv_work, recv_handler);
Expand Down Expand Up @@ -1137,30 +1144,6 @@ int capi20_manufacturer(unsigned int cmd, void __user *data)

EXPORT_SYMBOL(capi20_manufacturer);

/* temporary hack */

/**
* capi20_set_callback() - set CAPI application notification callback function
* @ap: CAPI application descriptor structure.
* @callback: callback function (NULL to remove).
*
* If not NULL, the callback function will be called to notify the
* application of the addition or removal of a controller.
* The first argument (cmd) will tell whether the controller was added
* (KCI_CONTRUP) or removed (KCI_CONTRDOWN).
* The second argument (contr) will be the controller number.
* For cmd==KCI_CONTRUP the third argument (data) will be a pointer to the
* new controller's capability profile structure.
*/

void capi20_set_callback(struct capi20_appl *ap,
void (*callback) (unsigned int cmd, __u32 contr, void *data))
{
ap->callback = callback;
}

EXPORT_SYMBOL(capi20_set_callback);

/* ------------------------------------------------------------- */
/* -------- Init & Cleanup ------------------------------------- */
/* ------------------------------------------------------------- */
Expand All @@ -1169,10 +1152,17 @@ EXPORT_SYMBOL(capi20_set_callback);
* init / exit functions
*/

static struct notifier_block capictr_nb = {
.notifier_call = notify_handler,
.priority = INT_MAX,
};

static int __init kcapi_init(void)
{
int err;

register_capictr_notifier(&capictr_nb);

err = cdebug_init();
if (!err)
kcapi_proc_init();
Expand Down
17 changes: 5 additions & 12 deletions include/linux/kernelcapi.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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

Expand Down

0 comments on commit ef69bb2

Please sign in to comment.