2004-11-17 14:56:21

by Steffen A. Mork

[permalink] [raw]
Subject: [PATCH] dss1_divert ISDN module compile fix for kernel 2.6.8.1

Dear kernel developer,

when I switched my installation from kernel 2.4 to 2.6 I
recognized that the ISDN module dss1_divert was marked
incompilable (config option CONFIG_CLEAN_COMPILE must
be turned off). The compile problem was the obsolete using
of kernel 2.4 critical sections. I replaced the cli() stuff
with spinlocks as explained in the Documentation/spinlocks.txt
file. After that the module compiles and runs as expected.

The patch is applied to kernel 2.6.8.1 and should work for
all 2.6 versions. You may also download the patch via http, too:

http://ls7-www.cs.uni-dortmund.de/~mork/dss1_divert.diff

Signed-off-by: Steffen A. Mork <[email protected]>


diff -uprN -X /home/sm/dontdiff linux-vanilla/drivers/isdn/divert/divert_init.c linux-2.6.8.1/drivers/isdn/divert/divert_init.c
--- linux-vanilla/drivers/isdn/divert/divert_init.c 2004-08-14 12:55:47.000000000 +0200
+++ linux-2.6.8.1/drivers/isdn/divert/divert_init.c 2004-11-05 08:50:02.000000000 +0100
@@ -12,6 +12,7 @@
#include <linux/module.h>
#include <linux/version.h>
#include <linux/init.h>
+
#include "isdn_divert.h"

MODULE_DESCRIPTION("ISDN4Linux: Call diversion support");
@@ -59,23 +60,24 @@ static int __init divert_init(void)
/* Module deinit code */
/**********************/
static void __exit divert_exit(void)
-{ unsigned long flags;
+{
+ unsigned long flags;
+ spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
int i;

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
divert_if.cmd = DIVERT_CMD_REL; /* release */
if ((i = DIVERT_REG_NAME(&divert_if)) != DIVERT_NO_ERR)
{ printk(KERN_WARNING "dss1_divert: error %d releasing module\n",i);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
return;
}
if (divert_dev_deinit())
{ printk(KERN_WARNING "dss1_divert: device busy, remove cancelled\n");
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
return;
}
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
deleterule(-1); /* delete all rules and free mem */
deleteprocs();
printk(KERN_INFO "dss1_divert module successfully removed \n");
diff -uprN -X /home/sm/dontdiff linux-vanilla/drivers/isdn/divert/divert_procfs.c linux-2.6.8.1/drivers/isdn/divert/divert_procfs.c
--- linux-vanilla/drivers/isdn/divert/divert_procfs.c 2004-08-14 12:56:22.000000000 +0200
+++ linux-2.6.8.1/drivers/isdn/divert/divert_procfs.c 2004-11-05 08:49:57.000000000 +0100
@@ -22,6 +22,7 @@
#include <linux/isdnif.h>
#include "isdn_divert.h"

+
/*********************************/
/* Variables for interface queue */
/*********************************/
@@ -181,6 +182,7 @@ isdn_divert_ioctl(struct inode *inode, s
divert_ioctl dioctl;
int i;
unsigned long flags;
+ spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
divert_rule *rulep;
char *cp;

@@ -215,10 +217,9 @@ isdn_divert_ioctl(struct inode *inode, s
case IIOCMODRULE:
if (!(rulep = getruleptr(dioctl.getsetrule.ruleidx)))
return (-EINVAL);
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
*rulep = dioctl.getsetrule.rule; /* copy data */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
return (0); /* no copy required */
break;

diff -uprN -X /home/sm/dontdiff linux-vanilla/drivers/isdn/divert/isdn_divert.c linux-2.6.8.1/drivers/isdn/divert/isdn_divert.c
--- linux-vanilla/drivers/isdn/divert/isdn_divert.c 2004-08-14 12:55:34.000000000 +0200
+++ linux-2.6.8.1/drivers/isdn/divert/isdn_divert.c 2004-11-05 08:49:20.000000000 +0100
@@ -11,6 +11,7 @@

#include <linux/version.h>
#include <linux/proc_fs.h>
+
#include "isdn_divert.h"

/**********************************/
@@ -51,50 +52,48 @@ static unsigned char extern_wait_max = 4
/* timer callback function */
/***************************/
static void deflect_timer_expire(ulong arg)
-{ unsigned long flags;
+{
+ unsigned long flags;
+ spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
struct call_struc *cs = (struct call_struc *) arg;

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
del_timer(&cs->timer); /* delete active timer */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);

switch(cs->akt_state)
{ case DEFLECT_PROCEED:
cs->ics.command = ISDN_CMD_HANGUP; /* cancel action */
divert_if.ll_cmd(&cs->ics);
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
break;

case DEFLECT_ALERT:
cs->ics.command = ISDN_CMD_REDIR; /* protocol */
strcpy(cs->ics.parm.setup.phone,cs->deflect_dest);
strcpy(cs->ics.parm.setup.eazmsn,"Testtext delayed");
- divert_if.ll_cmd(&cs->ics);
- save_flags(flags);
- cli();
+ divert_if.ll_cmd(&cs->ics);
+ spin_lock_irqsave(&divert_lock, flags);
cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
break;

case DEFLECT_AUTODEL:
default:
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
if (cs->prev)
cs->prev->next = cs->next; /* forward link */
else
divert_head = cs->next;
if (cs->next)
cs->next->prev = cs->prev; /* back link */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
kfree(cs);
return;

@@ -110,6 +109,7 @@ int cf_command(int drvid, int mode,
u_char proc, char *msn,
u_char service, char *fwd_nr, ulong *procid)
{ unsigned long flags;
+ spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
int retval,msnlen;
int fwd_len;
char *p,*ielenp,tmp[60];
@@ -166,10 +166,9 @@ int cf_command(int drvid, int mode,
cs->ics.parm.dss1_io.datalen = p - tmp; /* total len */
cs->ics.parm.dss1_io.data = tmp; /* start of buffer */

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs->ics.parm.dss1_io.ll_id = next_id++; /* id for callback */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
*procid = cs->ics.parm.dss1_io.ll_id;

sprintf(cs->info,"%d 0x%lx %s%s 0 %s %02x %d%s%s\n",
@@ -187,11 +186,10 @@ int cf_command(int drvid, int mode,

if (!retval)
{ cs->prev = NULL;
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs->next = divert_head;
divert_head = cs;
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
}
else
kfree(cs);
@@ -206,6 +204,7 @@ int deflect_extern_action(u_char cmd, ul
{ struct call_struc *cs;
isdn_ctrl ic;
unsigned long flags;
+ spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
int i;

if ((cmd & 0x7F) > 2) return(-EINVAL); /* invalid command */
@@ -224,13 +223,12 @@ int deflect_extern_action(u_char cmd, ul
{ case 0: /* hangup */
del_timer(&cs->timer);
ic.command = ISDN_CMD_HANGUP;
- i = divert_if.ll_cmd(&ic);
- save_flags(flags);
- cli();
+ i = divert_if.ll_cmd(&ic);
+ spin_lock_irqsave(&divert_lock, flags);
cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
break;

case 1: /* alert */
@@ -239,12 +237,12 @@ int deflect_extern_action(u_char cmd, ul
del_timer(&cs->timer);
ic.command = ISDN_CMD_ALERT;
if ((i = divert_if.ll_cmd(&ic)))
- { save_flags(flags);
- cli();
+ {
+ spin_lock_irqsave(&divert_lock, flags);
cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
}
else
cs->akt_state = DEFLECT_ALERT;
@@ -256,12 +254,12 @@ int deflect_extern_action(u_char cmd, ul
strcpy(cs->ics.parm.setup.eazmsn, "Testtext manual");
ic.command = ISDN_CMD_REDIR;
if ((i = divert_if.ll_cmd(&ic)))
- { save_flags(flags);
- cli();
+ {
+ spin_lock_irqsave(&divert_lock, flags);
cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
}
else
cs->akt_state = DEFLECT_ALERT;
@@ -277,6 +275,7 @@ int deflect_extern_action(u_char cmd, ul
int insertrule(int idx, divert_rule *newrule)
{ struct deflect_struc *ds,*ds1=NULL;
unsigned long flags;
+ spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;

if (!(ds = (struct deflect_struc *) kmalloc(sizeof(struct deflect_struc),
GFP_KERNEL)))
@@ -284,8 +283,7 @@ int insertrule(int idx, divert_rule *new

ds->rule = *newrule; /* set rule */

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);

if (idx >= 0)
{ ds1 = table_head;
@@ -313,7 +311,7 @@ int insertrule(int idx, divert_rule *new
table_head = ds; /* first element */
}

- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
return(0);
} /* insertrule */

@@ -323,14 +321,14 @@ int insertrule(int idx, divert_rule *new
int deleterule(int idx)
{ struct deflect_struc *ds,*ds1;
unsigned long flags;
+ spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;

if (idx < 0)
- { save_flags(flags);
- cli();
+ { spin_lock_irqsave(&divert_lock, flags);
ds = table_head;
table_head = NULL;
table_tail = NULL;
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
while (ds)
{ ds1 = ds;
ds = ds->next;
@@ -339,8 +337,7 @@ int deleterule(int idx)
return(0);
}

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
ds = table_head;

while ((ds) && (idx > 0))
@@ -349,7 +346,8 @@ int deleterule(int idx)
}

if (!ds)
- { restore_flags(flags);
+ {
+ spin_unlock_irqrestore(&divert_lock, flags);
return(-EINVAL);
}

@@ -363,7 +361,7 @@ int deleterule(int idx)
else
table_head = ds->next; /* start of chain */

- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
kfree(ds);
return(0);
} /* deleterule */
@@ -391,6 +389,7 @@ divert_rule *getruleptr(int idx)
int isdn_divert_icall(isdn_ctrl *ic)
{ int retval = 0;
unsigned long flags;
+ spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
struct call_struc *cs = NULL;
struct deflect_struc *dv;
char *p,*p1;
@@ -474,10 +473,9 @@ int isdn_divert_icall(isdn_ctrl *ic)
else
cs->timer.expires = 0;
cs->akt_state = dv->rule.action;
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs->divert_id = next_id++; /* new sequence number */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
cs->prev = NULL;
if (cs->akt_state == DEFLECT_ALERT)
{ strcpy(cs->deflect_dest,dv->rule.to_nr);
@@ -525,12 +523,11 @@ int isdn_divert_icall(isdn_ctrl *ic)

if (cs)
{ cs->prev = NULL;
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs->next = divert_head;
divert_head = cs;
if (cs->timer.expires) add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);

put_info_buffer(cs->info);
return(retval);
@@ -543,9 +540,9 @@ int isdn_divert_icall(isdn_ctrl *ic)
void deleteprocs(void)
{ struct call_struc *cs, *cs1;
unsigned long flags;
+ spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs = divert_head;
divert_head = NULL;
while (cs)
@@ -554,7 +551,7 @@ void deleteprocs(void)
cs = cs->next;
kfree(cs1);
}
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
} /* deleteprocs */

/****************************************************/
@@ -701,6 +698,7 @@ int prot_stat_callback(isdn_ctrl *ic)
{ struct call_struc *cs, *cs1;
int i;
unsigned long flags;
+ spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;

cs = divert_head; /* start of list */
cs1 = NULL;
@@ -769,8 +767,8 @@ int prot_stat_callback(isdn_ctrl *ic)
}

if (cs1->ics.driver == -1)
- { save_flags(flags);
- cli();
+ {
+ spin_lock_irqsave(&divert_lock, flags);
del_timer(&cs1->timer);
if (cs1->prev)
cs1->prev->next = cs1->next; /* forward link */
@@ -778,7 +776,7 @@ int prot_stat_callback(isdn_ctrl *ic)
divert_head = cs1->next;
if (cs1->next)
cs1->next->prev = cs1->prev; /* back link */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
kfree(cs1);
}

@@ -792,6 +790,7 @@ int prot_stat_callback(isdn_ctrl *ic)
int isdn_divert_stat_callback(isdn_ctrl *ic)
{ struct call_struc *cs, *cs1;
unsigned long flags;
+ spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
int retval;

retval = -1;
@@ -826,15 +825,14 @@ int isdn_divert_stat_callback(isdn_ctrl
cs = cs->next;
if (cs1->ics.driver == -1)
{
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
if (cs1->prev)
cs1->prev->next = cs1->next; /* forward link */
else
divert_head = cs1->next;
if (cs1->next)
cs1->next->prev = cs1->prev; /* back link */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
kfree(cs1);
}
}
diff -uprN -X /home/sm/dontdiff linux-vanilla/drivers/isdn/i4l/Kconfig linux-2.6.8.1/drivers/isdn/i4l/Kconfig
--- linux-vanilla/drivers/isdn/i4l/Kconfig 2004-08-14 12:55:10.000000000 +0200
+++ linux-2.6.8.1/drivers/isdn/i4l/Kconfig 2004-11-17 13:34:09.983567648 +0100
@@ -99,7 +99,7 @@ config ISDN_DRV_LOOP

config ISDN_DIVERSION
tristate "Support isdn diversion services"
- depends on BROKEN && BROKEN_ON_SMP
+ depends on ISDN && ISDN_I4L
help
This option allows you to use some supplementary diversion
services in conjunction with the HiSax driver on an EURO/DSS1




Subject: Re: [PATCH] dss1_divert ISDN module compile fix for kernel 2.6.8.1

On Wed, 17 Nov 2004 15:54:37 +0100, Steffen A. Mork
<[email protected]> wrote:
> Dear kernel developer,
>
> when I switched my installation from kernel 2.4 to 2.6 I
> recognized that the ISDN module dss1_divert was marked
> incompilable (config option CONFIG_CLEAN_COMPILE must
> be turned off). The compile problem was the obsolete using
> of kernel 2.4 critical sections. I replaced the cli() stuff
> with spinlocks as explained in the Documentation/spinlocks.txt
> file. After that the module compiles and runs as expected.

This looks wrong, you are using many private spinlocks instead
of one global spinlock.

> The patch is applied to kernel 2.6.8.1 and should work for
> all 2.6 versions. You may also download the patch via http, too:
>
> http://ls7-www.cs.uni-dortmund.de/~mork/dss1_divert.diff
>
> Signed-off-by: Steffen A. Mork <[email protected]>
>
> diff -uprN -X /home/sm/dontdiff linux-vanilla/drivers/isdn/divert/divert_init.c linux-2.6.8.1/drivers/isdn/divert/divert_init.c
> --- linux-vanilla/drivers/isdn/divert/divert_init.c 2004-08-14 12:55:47.000000000 +0200
> +++ linux-2.6.8.1/drivers/isdn/divert/divert_init.c 2004-11-05 08:50:02.000000000 +0100
> @@ -12,6 +12,7 @@
> #include <linux/module.h>
> #include <linux/version.h>
> #include <linux/init.h>
> +
> #include "isdn_divert.h"
>
> MODULE_DESCRIPTION("ISDN4Linux: Call diversion support");
> @@ -59,23 +60,24 @@ static int __init divert_init(void)
> /* Module deinit code */
> /**********************/
> static void __exit divert_exit(void)
> -{ unsigned long flags;
> +{
> + unsigned long flags;
> + spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
> int i;
>
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> divert_if.cmd = DIVERT_CMD_REL; /* release */
> if ((i = DIVERT_REG_NAME(&divert_if)) != DIVERT_NO_ERR)
> { printk(KERN_WARNING "dss1_divert: error %d releasing module\n",i);
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> return;
> }
> if (divert_dev_deinit())
> { printk(KERN_WARNING "dss1_divert: device busy, remove cancelled\n");
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> return;
> }
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> deleterule(-1); /* delete all rules and free mem */
> deleteprocs();
> printk(KERN_INFO "dss1_divert module successfully removed \n");
> diff -uprN -X /home/sm/dontdiff linux-vanilla/drivers/isdn/divert/divert_procfs.c linux-2.6.8.1/drivers/isdn/divert/divert_procfs.c
> --- linux-vanilla/drivers/isdn/divert/divert_procfs.c 2004-08-14 12:56:22.000000000 +0200
> +++ linux-2.6.8.1/drivers/isdn/divert/divert_procfs.c 2004-11-05 08:49:57.000000000 +0100
> @@ -22,6 +22,7 @@
> #include <linux/isdnif.h>
> #include "isdn_divert.h"
>
> +
> /*********************************/
> /* Variables for interface queue */
> /*********************************/
> @@ -181,6 +182,7 @@ isdn_divert_ioctl(struct inode *inode, s
> divert_ioctl dioctl;
> int i;
> unsigned long flags;
> + spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
> divert_rule *rulep;
> char *cp;
>
> @@ -215,10 +217,9 @@ isdn_divert_ioctl(struct inode *inode, s
> case IIOCMODRULE:
> if (!(rulep = getruleptr(dioctl.getsetrule.ruleidx)))
> return (-EINVAL);
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> *rulep = dioctl.getsetrule.rule; /* copy data */
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> return (0); /* no copy required */
> break;
>
> diff -uprN -X /home/sm/dontdiff linux-vanilla/drivers/isdn/divert/isdn_divert.c linux-2.6.8.1/drivers/isdn/divert/isdn_divert.c
> --- linux-vanilla/drivers/isdn/divert/isdn_divert.c 2004-08-14 12:55:34.000000000 +0200
> +++ linux-2.6.8.1/drivers/isdn/divert/isdn_divert.c 2004-11-05 08:49:20.000000000 +0100
> @@ -11,6 +11,7 @@
>
> #include <linux/version.h>
> #include <linux/proc_fs.h>
> +
> #include "isdn_divert.h"
>
> /**********************************/
> @@ -51,50 +52,48 @@ static unsigned char extern_wait_max = 4
> /* timer callback function */
> /***************************/
> static void deflect_timer_expire(ulong arg)
> -{ unsigned long flags;
> +{
> + unsigned long flags;
> + spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
> struct call_struc *cs = (struct call_struc *) arg;
>
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> del_timer(&cs->timer); /* delete active timer */
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
>
> switch(cs->akt_state)
> { case DEFLECT_PROCEED:
> cs->ics.command = ISDN_CMD_HANGUP; /* cancel action */
> divert_if.ll_cmd(&cs->ics);
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
> cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
> add_timer(&cs->timer);
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> break;
>
> case DEFLECT_ALERT:
> cs->ics.command = ISDN_CMD_REDIR; /* protocol */
> strcpy(cs->ics.parm.setup.phone,cs->deflect_dest);
> strcpy(cs->ics.parm.setup.eazmsn,"Testtext delayed");
> - divert_if.ll_cmd(&cs->ics);
> - save_flags(flags);
> - cli();
> + divert_if.ll_cmd(&cs->ics);
> + spin_lock_irqsave(&divert_lock, flags);
> cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
> cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
> add_timer(&cs->timer);
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> break;
>
> case DEFLECT_AUTODEL:
> default:
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> if (cs->prev)
> cs->prev->next = cs->next; /* forward link */
> else
> divert_head = cs->next;
> if (cs->next)
> cs->next->prev = cs->prev; /* back link */
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> kfree(cs);
> return;
>
> @@ -110,6 +109,7 @@ int cf_command(int drvid, int mode,
> u_char proc, char *msn,
> u_char service, char *fwd_nr, ulong *procid)
> { unsigned long flags;
> + spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
> int retval,msnlen;
> int fwd_len;
> char *p,*ielenp,tmp[60];
> @@ -166,10 +166,9 @@ int cf_command(int drvid, int mode,
> cs->ics.parm.dss1_io.datalen = p - tmp; /* total len */
> cs->ics.parm.dss1_io.data = tmp; /* start of buffer */
>
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> cs->ics.parm.dss1_io.ll_id = next_id++; /* id for callback */
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> *procid = cs->ics.parm.dss1_io.ll_id;
>
> sprintf(cs->info,"%d 0x%lx %s%s 0 %s %02x %d%s%s\n",
> @@ -187,11 +186,10 @@ int cf_command(int drvid, int mode,
>
> if (!retval)
> { cs->prev = NULL;
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> cs->next = divert_head;
> divert_head = cs;
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> }
> else
> kfree(cs);
> @@ -206,6 +204,7 @@ int deflect_extern_action(u_char cmd, ul
> { struct call_struc *cs;
> isdn_ctrl ic;
> unsigned long flags;
> + spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
> int i;
>
> if ((cmd & 0x7F) > 2) return(-EINVAL); /* invalid command */
> @@ -224,13 +223,12 @@ int deflect_extern_action(u_char cmd, ul
> { case 0: /* hangup */
> del_timer(&cs->timer);
> ic.command = ISDN_CMD_HANGUP;
> - i = divert_if.ll_cmd(&ic);
> - save_flags(flags);
> - cli();
> + i = divert_if.ll_cmd(&ic);
> + spin_lock_irqsave(&divert_lock, flags);
> cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
> cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
> add_timer(&cs->timer);
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> break;
>
> case 1: /* alert */
> @@ -239,12 +237,12 @@ int deflect_extern_action(u_char cmd, ul
> del_timer(&cs->timer);
> ic.command = ISDN_CMD_ALERT;
> if ((i = divert_if.ll_cmd(&ic)))
> - { save_flags(flags);
> - cli();
> + {
> + spin_lock_irqsave(&divert_lock, flags);
> cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
> cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
> add_timer(&cs->timer);
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> }
> else
> cs->akt_state = DEFLECT_ALERT;
> @@ -256,12 +254,12 @@ int deflect_extern_action(u_char cmd, ul
> strcpy(cs->ics.parm.setup.eazmsn, "Testtext manual");
> ic.command = ISDN_CMD_REDIR;
> if ((i = divert_if.ll_cmd(&ic)))
> - { save_flags(flags);
> - cli();
> + {
> + spin_lock_irqsave(&divert_lock, flags);
> cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
> cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
> add_timer(&cs->timer);
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> }
> else
> cs->akt_state = DEFLECT_ALERT;
> @@ -277,6 +275,7 @@ int deflect_extern_action(u_char cmd, ul
> int insertrule(int idx, divert_rule *newrule)
> { struct deflect_struc *ds,*ds1=NULL;
> unsigned long flags;
> + spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
>
> if (!(ds = (struct deflect_struc *) kmalloc(sizeof(struct deflect_struc),
> GFP_KERNEL)))
> @@ -284,8 +283,7 @@ int insertrule(int idx, divert_rule *new
>
> ds->rule = *newrule; /* set rule */
>
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
>
> if (idx >= 0)
> { ds1 = table_head;
> @@ -313,7 +311,7 @@ int insertrule(int idx, divert_rule *new
> table_head = ds; /* first element */
> }
>
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> return(0);
> } /* insertrule */
>
> @@ -323,14 +321,14 @@ int insertrule(int idx, divert_rule *new
> int deleterule(int idx)
> { struct deflect_struc *ds,*ds1;
> unsigned long flags;
> + spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
>
> if (idx < 0)
> - { save_flags(flags);
> - cli();
> + { spin_lock_irqsave(&divert_lock, flags);
> ds = table_head;
> table_head = NULL;
> table_tail = NULL;
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> while (ds)
> { ds1 = ds;
> ds = ds->next;
> @@ -339,8 +337,7 @@ int deleterule(int idx)
> return(0);
> }
>
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> ds = table_head;
>
> while ((ds) && (idx > 0))
> @@ -349,7 +346,8 @@ int deleterule(int idx)
> }
>
> if (!ds)
> - { restore_flags(flags);
> + {
> + spin_unlock_irqrestore(&divert_lock, flags);
> return(-EINVAL);
> }
>
> @@ -363,7 +361,7 @@ int deleterule(int idx)
> else
> table_head = ds->next; /* start of chain */
>
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> kfree(ds);
> return(0);
> } /* deleterule */
> @@ -391,6 +389,7 @@ divert_rule *getruleptr(int idx)
> int isdn_divert_icall(isdn_ctrl *ic)
> { int retval = 0;
> unsigned long flags;
> + spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
> struct call_struc *cs = NULL;
> struct deflect_struc *dv;
> char *p,*p1;
> @@ -474,10 +473,9 @@ int isdn_divert_icall(isdn_ctrl *ic)
> else
> cs->timer.expires = 0;
> cs->akt_state = dv->rule.action;
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> cs->divert_id = next_id++; /* new sequence number */
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> cs->prev = NULL;
> if (cs->akt_state == DEFLECT_ALERT)
> { strcpy(cs->deflect_dest,dv->rule.to_nr);
> @@ -525,12 +523,11 @@ int isdn_divert_icall(isdn_ctrl *ic)
>
> if (cs)
> { cs->prev = NULL;
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> cs->next = divert_head;
> divert_head = cs;
> if (cs->timer.expires) add_timer(&cs->timer);
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
>
> put_info_buffer(cs->info);
> return(retval);
> @@ -543,9 +540,9 @@ int isdn_divert_icall(isdn_ctrl *ic)
> void deleteprocs(void)
> { struct call_struc *cs, *cs1;
> unsigned long flags;
> + spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
>
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> cs = divert_head;
> divert_head = NULL;
> while (cs)
> @@ -554,7 +551,7 @@ void deleteprocs(void)
> cs = cs->next;
> kfree(cs1);
> }
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> } /* deleteprocs */
>
> /****************************************************/
> @@ -701,6 +698,7 @@ int prot_stat_callback(isdn_ctrl *ic)
> { struct call_struc *cs, *cs1;
> int i;
> unsigned long flags;
> + spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
>
> cs = divert_head; /* start of list */
> cs1 = NULL;
> @@ -769,8 +767,8 @@ int prot_stat_callback(isdn_ctrl *ic)
> }
>
> if (cs1->ics.driver == -1)
> - { save_flags(flags);
> - cli();
> + {
> + spin_lock_irqsave(&divert_lock, flags);
> del_timer(&cs1->timer);
> if (cs1->prev)
> cs1->prev->next = cs1->next; /* forward link */
> @@ -778,7 +776,7 @@ int prot_stat_callback(isdn_ctrl *ic)
> divert_head = cs1->next;
> if (cs1->next)
> cs1->next->prev = cs1->prev; /* back link */
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> kfree(cs1);
> }
>
> @@ -792,6 +790,7 @@ int prot_stat_callback(isdn_ctrl *ic)
> int isdn_divert_stat_callback(isdn_ctrl *ic)
> { struct call_struc *cs, *cs1;
> unsigned long flags;
> + spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
> int retval;
>
> retval = -1;
> @@ -826,15 +825,14 @@ int isdn_divert_stat_callback(isdn_ctrl
> cs = cs->next;
> if (cs1->ics.driver == -1)
> {
> - save_flags(flags);
> - cli();
> + spin_lock_irqsave(&divert_lock, flags);
> if (cs1->prev)
> cs1->prev->next = cs1->next; /* forward link */
> else
> divert_head = cs1->next;
> if (cs1->next)
> cs1->next->prev = cs1->prev; /* back link */
> - restore_flags(flags);
> + spin_unlock_irqrestore(&divert_lock, flags);
> kfree(cs1);
> }
> }
> diff -uprN -X /home/sm/dontdiff linux-vanilla/drivers/isdn/i4l/Kconfig linux-2.6.8.1/drivers/isdn/i4l/Kconfig
> --- linux-vanilla/drivers/isdn/i4l/Kconfig 2004-08-14 12:55:10.000000000 +0200
> +++ linux-2.6.8.1/drivers/isdn/i4l/Kconfig 2004-11-17 13:34:09.983567648 +0100
> @@ -99,7 +99,7 @@ config ISDN_DRV_LOOP
>
> config ISDN_DIVERSION
> tristate "Support isdn diversion services"
> - depends on BROKEN && BROKEN_ON_SMP
> + depends on ISDN && ISDN_I4L
> help
> This option allows you to use some supplementary diversion
> services in conjunction with the HiSax driver on an EURO/DSS1

2004-11-17 17:27:14

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] dss1_divert ISDN module compile fix for kernel 2.6.8.1



On Wed, 17 Nov 2004, Bartlomiej Zolnierkiewicz wrote:
>
> This looks wrong, you are using many private spinlocks instead
> of one global spinlock.

Good catch, I didn't look closely enough.

Steffen, can you make that "divert_lock" one global one, and re-test? I'd
do it myself, but it really does need testing, since there might be a
deadlock lurking there that the local lock bug hid.

Linus

2004-11-17 18:58:55

by Steffen A. Mork

[permalink] [raw]
Subject: Re: [PATCH] dss1_divert ISDN module compile fix for kernel 2.6.8.1

Dear Linus and Bartlomiej,

>>when I switched my installation from kernel 2.4 to 2.6 I
>>recognized that the ISDN module dss1_divert was marked
>>incompilable (config option CONFIG_CLEAN_COMPILE must
>>be turned off). The compile problem was the obsolete using
>>of kernel 2.4 critical sections. I replaced the cli() stuff
>>with spinlocks as explained in the Documentation/spinlocks.txt
>>file. After that the module compiles and runs as expected.
>
>
> This looks wrong, you are using many private spinlocks instead
> of one global spinlock.
OK, thank you. I went into the copy/paste trap. I corrected the patch to
a global spinlock and tested it successfully.

The corrected patch is applied to kernel 2.6.8.1 and should work for
all 2.6 versions. You may also download the patch via http, too:

http://ls7-www.cs.uni-dortmund.de/~mork/dss1_divert.diff

Signed-off-by: Steffen A. Mork <[email protected]>

diff -uprN -X dontdiff linux-vanilla/drivers/isdn/divert/divert_init.c linux-2.6.8.1/drivers/isdn/divert/divert_init.c
--- linux-vanilla/drivers/isdn/divert/divert_init.c 2004-08-14 12:55:47.000000000 +0200
+++ linux-2.6.8.1/drivers/isdn/divert/divert_init.c 2004-11-17 17:50:38.000000000 +0100
@@ -12,6 +12,7 @@
#include <linux/module.h>
#include <linux/version.h>
#include <linux/init.h>
+
#include "isdn_divert.h"

MODULE_DESCRIPTION("ISDN4Linux: Call diversion support");
@@ -59,23 +60,23 @@ static int __init divert_init(void)
/* Module deinit code */
/**********************/
static void __exit divert_exit(void)
-{ unsigned long flags;
+{
+ unsigned long flags;
int i;

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
divert_if.cmd = DIVERT_CMD_REL; /* release */
if ((i = DIVERT_REG_NAME(&divert_if)) != DIVERT_NO_ERR)
{ printk(KERN_WARNING "dss1_divert: error %d releasing module\n",i);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
return;
}
if (divert_dev_deinit())
{ printk(KERN_WARNING "dss1_divert: device busy, remove cancelled\n");
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
return;
}
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
deleterule(-1); /* delete all rules and free mem */
deleteprocs();
printk(KERN_INFO "dss1_divert module successfully removed \n");
diff -uprN -X dontdiff linux-vanilla/drivers/isdn/divert/divert_procfs.c linux-2.6.8.1/drivers/isdn/divert/divert_procfs.c
--- linux-vanilla/drivers/isdn/divert/divert_procfs.c 2004-08-14 12:56:22.000000000 +0200
+++ linux-2.6.8.1/drivers/isdn/divert/divert_procfs.c 2004-11-17 17:42:00.000000000 +0100
@@ -22,6 +22,7 @@
#include <linux/isdnif.h>
#include "isdn_divert.h"

+
/*********************************/
/* Variables for interface queue */
/*********************************/
@@ -215,10 +216,9 @@ isdn_divert_ioctl(struct inode *inode, s
case IIOCMODRULE:
if (!(rulep = getruleptr(dioctl.getsetrule.ruleidx)))
return (-EINVAL);
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
*rulep = dioctl.getsetrule.rule; /* copy data */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
return (0); /* no copy required */
break;

diff -uprN -X dontdiff linux-vanilla/drivers/isdn/divert/isdn_divert.c linux-2.6.8.1/drivers/isdn/divert/isdn_divert.c
--- linux-vanilla/drivers/isdn/divert/isdn_divert.c 2004-08-14 12:55:34.000000000 +0200
+++ linux-2.6.8.1/drivers/isdn/divert/isdn_divert.c 2004-11-17 19:39:32.000000000 +0100
@@ -11,6 +11,7 @@

#include <linux/version.h>
#include <linux/proc_fs.h>
+
#include "isdn_divert.h"

/**********************************/
@@ -47,54 +48,53 @@ static struct deflect_struc *table_head
static struct deflect_struc *table_tail = NULL;
static unsigned char extern_wait_max = 4; /* maximum wait in s for external process */

+spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
+
/***************************/
/* timer callback function */
/***************************/
static void deflect_timer_expire(ulong arg)
-{ unsigned long flags;
+{
+ unsigned long flags;
struct call_struc *cs = (struct call_struc *) arg;

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
del_timer(&cs->timer); /* delete active timer */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);

switch(cs->akt_state)
{ case DEFLECT_PROCEED:
cs->ics.command = ISDN_CMD_HANGUP; /* cancel action */
divert_if.ll_cmd(&cs->ics);
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
break;

case DEFLECT_ALERT:
cs->ics.command = ISDN_CMD_REDIR; /* protocol */
strcpy(cs->ics.parm.setup.phone,cs->deflect_dest);
strcpy(cs->ics.parm.setup.eazmsn,"Testtext delayed");
- divert_if.ll_cmd(&cs->ics);
- save_flags(flags);
- cli();
+ divert_if.ll_cmd(&cs->ics);
+ spin_lock_irqsave(&divert_lock, flags);
cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
break;

case DEFLECT_AUTODEL:
default:
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
if (cs->prev)
cs->prev->next = cs->next; /* forward link */
else
divert_head = cs->next;
if (cs->next)
cs->next->prev = cs->prev; /* back link */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
kfree(cs);
return;

@@ -166,10 +166,9 @@ int cf_command(int drvid, int mode,
cs->ics.parm.dss1_io.datalen = p - tmp; /* total len */
cs->ics.parm.dss1_io.data = tmp; /* start of buffer */

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs->ics.parm.dss1_io.ll_id = next_id++; /* id for callback */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
*procid = cs->ics.parm.dss1_io.ll_id;

sprintf(cs->info,"%d 0x%lx %s%s 0 %s %02x %d%s%s\n",
@@ -187,11 +186,10 @@ int cf_command(int drvid, int mode,

if (!retval)
{ cs->prev = NULL;
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs->next = divert_head;
divert_head = cs;
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
}
else
kfree(cs);
@@ -224,13 +222,12 @@ int deflect_extern_action(u_char cmd, ul
{ case 0: /* hangup */
del_timer(&cs->timer);
ic.command = ISDN_CMD_HANGUP;
- i = divert_if.ll_cmd(&ic);
- save_flags(flags);
- cli();
+ i = divert_if.ll_cmd(&ic);
+ spin_lock_irqsave(&divert_lock, flags);
cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
break;

case 1: /* alert */
@@ -239,12 +236,12 @@ int deflect_extern_action(u_char cmd, ul
del_timer(&cs->timer);
ic.command = ISDN_CMD_ALERT;
if ((i = divert_if.ll_cmd(&ic)))
- { save_flags(flags);
- cli();
+ {
+ spin_lock_irqsave(&divert_lock, flags);
cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
}
else
cs->akt_state = DEFLECT_ALERT;
@@ -256,12 +253,12 @@ int deflect_extern_action(u_char cmd, ul
strcpy(cs->ics.parm.setup.eazmsn, "Testtext manual");
ic.command = ISDN_CMD_REDIR;
if ((i = divert_if.ll_cmd(&ic)))
- { save_flags(flags);
- cli();
+ {
+ spin_lock_irqsave(&divert_lock, flags);
cs->akt_state = DEFLECT_AUTODEL; /* delete after timeout */
cs->timer.expires = jiffies + (HZ * AUTODEL_TIME);
add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
}
else
cs->akt_state = DEFLECT_ALERT;
@@ -284,8 +281,7 @@ int insertrule(int idx, divert_rule *new

ds->rule = *newrule; /* set rule */

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);

if (idx >= 0)
{ ds1 = table_head;
@@ -313,7 +309,7 @@ int insertrule(int idx, divert_rule *new
table_head = ds; /* first element */
}

- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
return(0);
} /* insertrule */

@@ -325,12 +321,11 @@ int deleterule(int idx)
unsigned long flags;

if (idx < 0)
- { save_flags(flags);
- cli();
+ { spin_lock_irqsave(&divert_lock, flags);
ds = table_head;
table_head = NULL;
table_tail = NULL;
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
while (ds)
{ ds1 = ds;
ds = ds->next;
@@ -339,8 +334,7 @@ int deleterule(int idx)
return(0);
}

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
ds = table_head;

while ((ds) && (idx > 0))
@@ -349,7 +343,8 @@ int deleterule(int idx)
}

if (!ds)
- { restore_flags(flags);
+ {
+ spin_unlock_irqrestore(&divert_lock, flags);
return(-EINVAL);
}

@@ -363,7 +358,7 @@ int deleterule(int idx)
else
table_head = ds->next; /* start of chain */

- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
kfree(ds);
return(0);
} /* deleterule */
@@ -474,10 +469,9 @@ int isdn_divert_icall(isdn_ctrl *ic)
else
cs->timer.expires = 0;
cs->akt_state = dv->rule.action;
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs->divert_id = next_id++; /* new sequence number */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
cs->prev = NULL;
if (cs->akt_state == DEFLECT_ALERT)
{ strcpy(cs->deflect_dest,dv->rule.to_nr);
@@ -525,12 +519,11 @@ int isdn_divert_icall(isdn_ctrl *ic)

if (cs)
{ cs->prev = NULL;
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs->next = divert_head;
divert_head = cs;
if (cs->timer.expires) add_timer(&cs->timer);
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);

put_info_buffer(cs->info);
return(retval);
@@ -544,8 +537,7 @@ void deleteprocs(void)
{ struct call_struc *cs, *cs1;
unsigned long flags;

- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
cs = divert_head;
divert_head = NULL;
while (cs)
@@ -554,7 +546,7 @@ void deleteprocs(void)
cs = cs->next;
kfree(cs1);
}
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
} /* deleteprocs */

/****************************************************/
@@ -769,8 +761,8 @@ int prot_stat_callback(isdn_ctrl *ic)
}

if (cs1->ics.driver == -1)
- { save_flags(flags);
- cli();
+ {
+ spin_lock_irqsave(&divert_lock, flags);
del_timer(&cs1->timer);
if (cs1->prev)
cs1->prev->next = cs1->next; /* forward link */
@@ -778,7 +770,7 @@ int prot_stat_callback(isdn_ctrl *ic)
divert_head = cs1->next;
if (cs1->next)
cs1->next->prev = cs1->prev; /* back link */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
kfree(cs1);
}

@@ -826,15 +818,14 @@ int isdn_divert_stat_callback(isdn_ctrl
cs = cs->next;
if (cs1->ics.driver == -1)
{
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&divert_lock, flags);
if (cs1->prev)
cs1->prev->next = cs1->next; /* forward link */
else
divert_head = cs1->next;
if (cs1->next)
cs1->next->prev = cs1->prev; /* back link */
- restore_flags(flags);
+ spin_unlock_irqrestore(&divert_lock, flags);
kfree(cs1);
}
}
diff -uprN -X dontdiff linux-vanilla/drivers/isdn/divert/isdn_divert.h linux-2.6.8.1/drivers/isdn/divert/isdn_divert.h
--- linux-vanilla/drivers/isdn/divert/isdn_divert.h 2004-08-14 12:54:51.000000000 +0200
+++ linux-2.6.8.1/drivers/isdn/divert/isdn_divert.h 2004-11-17 18:59:16.000000000 +0100
@@ -114,6 +114,8 @@ struct divert_info
/**************/
/* Prototypes */
/**************/
+extern spinlock_t divert_lock;
+
extern ulong if_used; /* number of interface users */
extern int divert_dev_deinit(void);
extern int divert_dev_init(void);
diff -uprN -X dontdiff linux-vanilla/drivers/isdn/i4l/Kconfig linux-2.6.8.1/drivers/isdn/i4l/Kconfig
--- linux-vanilla/drivers/isdn/i4l/Kconfig 2004-08-14 12:55:10.000000000 +0200
+++ linux-2.6.8.1/drivers/isdn/i4l/Kconfig 2004-11-17 13:34:09.983567648 +0100
@@ -99,7 +99,7 @@ config ISDN_DRV_LOOP

config ISDN_DIVERSION
tristate "Support isdn diversion services"
- depends on BROKEN && BROKEN_ON_SMP
+ depends on ISDN && ISDN_I4L
help
This option allows you to use some supplementary diversion
services in conjunction with the HiSax driver on an EURO/DSS1



2004-11-17 18:05:07

by Jan Engelhardt

[permalink] [raw]
Subject: Re: dss1_divert ISDN module compile fix for kernel 2.6.8.1

>when I switched my installation from kernel 2.4 to 2.6 I
>recognized that the ISDN module dss1_divert was marked
>incompilable (config option CONFIG_CLEAN_COMPILE must
>be turned off). The compile problem was the obsolete using
>of kernel 2.4 critical sections. I replaced the cli() stuff
>with spinlocks as explained in the Documentation/spinlocks.txt
>file. After that the module compiles and runs as expected.

I had a similar issue, but for me, it only returned a warning rather than a
compile error.


I posted something similar earlier:
http://marc.theaimsgroup.com/?l=linux-kernel&m=109937866706186&w=2 and
http://marc.theaimsgroup.com/?l=linux-kernel&m=109943458323994&w=2 plus
the latest message I wrote a few secs ago to this list.



Jan Engelhardt
--
Gesellschaft für Wissenschaftliche Datenverarbeitung
Am Fassberg, 37077 Göttingen, http://www.gwdg.de

2004-11-17 21:18:24

by Steffen A. Mork

[permalink] [raw]
Subject: Re: [PATCH] dss1_divert ISDN module compile fix for kernel 2.6.8.1

Dear Linus and Bartlomiej,

> Moreover, you must make sure that there is no other code (ie. in generic
> ISDN code) doing cli() stuff on objects that you are protecting by a spinlock
> etc., I've checked divert driver and it looks OK.
>
>
>>OK, thank you. I went into the copy/paste trap. I corrected the patch to
>>a global spinlock and tested it successfully.
>
>
> Great.
>
> Please do "interdiff" between the old and the new patch
> and send it to Linus (he has already merged the old patch).
the interdiff patch is appended to this mail and the link is:

http://ls7-www.cs.uni-dortmund.de/~mork/dss1_divert-patch1-2-patch2.diff

Signed-off-by: Steffen A. Mork <[email protected]>

> Thanks.
Thank you.



> Bartlomiej
Steffen

diff -uprN -X dontdiff linux-2.6.8.1-patch1/drivers/isdn/divert/divert_init.c linux-2.6.8.1-patch2/drivers/isdn/divert/divert_init.c
--- linux-2.6.8.1-patch1/drivers/isdn/divert/divert_init.c 2004-11-05 08:50:02.000000000 +0100
+++ linux-2.6.8.1-patch2/drivers/isdn/divert/divert_init.c 2004-11-17 17:50:38.000000000 +0100
@@ -62,7 +62,6 @@ static int __init divert_init(void)
static void __exit divert_exit(void)
{
unsigned long flags;
- spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
int i;

spin_lock_irqsave(&divert_lock, flags);
diff -uprN -X dontdiff linux-2.6.8.1-patch1/drivers/isdn/divert/divert_procfs.c linux-2.6.8.1-patch2/drivers/isdn/divert/divert_procfs.c
--- linux-2.6.8.1-patch1/drivers/isdn/divert/divert_procfs.c 2004-11-05 08:49:57.000000000 +0100
+++ linux-2.6.8.1-patch2/drivers/isdn/divert/divert_procfs.c 2004-11-17 17:42:00.000000000 +0100
@@ -182,7 +182,6 @@ isdn_divert_ioctl(struct inode *inode, s
divert_ioctl dioctl;
int i;
unsigned long flags;
- spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
divert_rule *rulep;
char *cp;

diff -uprN -X dontdiff linux-2.6.8.1-patch1/drivers/isdn/divert/isdn_divert.c linux-2.6.8.1-patch2/drivers/isdn/divert/isdn_divert.c
--- linux-2.6.8.1-patch1/drivers/isdn/divert/isdn_divert.c 2004-11-05 08:49:20.000000000 +0100
+++ linux-2.6.8.1-patch2/drivers/isdn/divert/isdn_divert.c 2004-11-17 19:39:32.000000000 +0100
@@ -48,13 +48,14 @@ static struct deflect_struc *table_head
static struct deflect_struc *table_tail = NULL;
static unsigned char extern_wait_max = 4; /* maximum wait in s for external process */

+spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
+
/***************************/
/* timer callback function */
/***************************/
static void deflect_timer_expire(ulong arg)
{
unsigned long flags;
- spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
struct call_struc *cs = (struct call_struc *) arg;

spin_lock_irqsave(&divert_lock, flags);
@@ -109,7 +110,6 @@ int cf_command(int drvid, int mode,
u_char proc, char *msn,
u_char service, char *fwd_nr, ulong *procid)
{ unsigned long flags;
- spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
int retval,msnlen;
int fwd_len;
char *p,*ielenp,tmp[60];
@@ -204,7 +204,6 @@ int deflect_extern_action(u_char cmd, ul
{ struct call_struc *cs;
isdn_ctrl ic;
unsigned long flags;
- spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
int i;

if ((cmd & 0x7F) > 2) return(-EINVAL); /* invalid command */
@@ -275,7 +274,6 @@ int deflect_extern_action(u_char cmd, ul
int insertrule(int idx, divert_rule *newrule)
{ struct deflect_struc *ds,*ds1=NULL;
unsigned long flags;
- spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;

if (!(ds = (struct deflect_struc *) kmalloc(sizeof(struct deflect_struc),
GFP_KERNEL)))
@@ -321,7 +319,6 @@ int insertrule(int idx, divert_rule *new
int deleterule(int idx)
{ struct deflect_struc *ds,*ds1;
unsigned long flags;
- spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;

if (idx < 0)
{ spin_lock_irqsave(&divert_lock, flags);
@@ -389,7 +386,6 @@ divert_rule *getruleptr(int idx)
int isdn_divert_icall(isdn_ctrl *ic)
{ int retval = 0;
unsigned long flags;
- spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
struct call_struc *cs = NULL;
struct deflect_struc *dv;
char *p,*p1;
@@ -540,7 +536,6 @@ int isdn_divert_icall(isdn_ctrl *ic)
void deleteprocs(void)
{ struct call_struc *cs, *cs1;
unsigned long flags;
- spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;

spin_lock_irqsave(&divert_lock, flags);
cs = divert_head;
@@ -698,7 +693,6 @@ int prot_stat_callback(isdn_ctrl *ic)
{ struct call_struc *cs, *cs1;
int i;
unsigned long flags;
- spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;

cs = divert_head; /* start of list */
cs1 = NULL;
@@ -790,7 +784,6 @@ int prot_stat_callback(isdn_ctrl *ic)
int isdn_divert_stat_callback(isdn_ctrl *ic)
{ struct call_struc *cs, *cs1;
unsigned long flags;
- spinlock_t divert_lock = SPIN_LOCK_UNLOCKED;
int retval;

retval = -1;
diff -uprN -X dontdiff linux-2.6.8.1-patch1/drivers/isdn/divert/isdn_divert.h linux-2.6.8.1-patch2/drivers/isdn/divert/isdn_divert.h
--- linux-2.6.8.1-patch1/drivers/isdn/divert/isdn_divert.h 2004-08-14 12:54:51.000000000 +0200
+++ linux-2.6.8.1-patch2/drivers/isdn/divert/isdn_divert.h 2004-11-17 18:59:16.000000000 +0100
@@ -114,6 +114,8 @@ struct divert_info
/**************/
/* Prototypes */
/**************/
+extern spinlock_t divert_lock;
+
extern ulong if_used; /* number of interface users */
extern int divert_dev_deinit(void);
extern int divert_dev_init(void);



2004-11-17 22:47:37

by Tonnerre

[permalink] [raw]
Subject: Re: [PATCH] dss1_divert ISDN module compile fix for kernel 2.6.8.1

Salut,

On Wed, Nov 17, 2004 at 03:54:37PM +0100, Steffen A. Mork wrote:
> http://ls7-www.cs.uni-dortmund.de/~mork/dss1_divert.diff

This patch is wrong. You're using a spinlock you declare on the stack
of a function, so it's completely pointless.

Please look at http://users.thundrix.ch/~tonnerre/kernel/2.6.1-tch1/
where I gathered lots of ISDN patches a while ago. It might be a start
for you.

Tonnerre


Attachments:
(No filename) (419.00 B)
(No filename) (189.00 B)
Download all attachments
Subject: Re: [PATCH] dss1_divert ISDN module compile fix for kernel 2.6.8.1

On Wed, 17 Nov 2004 19:52:54 +0100, Steffen A. Mork
<[email protected]> wrote:
> Dear Linus and Bartlomiej,
>
>
>
> >>when I switched my installation from kernel 2.4 to 2.6 I
> >>recognized that the ISDN module dss1_divert was marked
> >>incompilable (config option CONFIG_CLEAN_COMPILE must
> >>be turned off). The compile problem was the obsolete using
> >>of kernel 2.4 critical sections. I replaced the cli() stuff
> >>with spinlocks as explained in the Documentation/spinlocks.txt
> >>file. After that the module compiles and runs as expected.
> >
> >
> > This looks wrong, you are using many private spinlocks instead
> > of one global spinlock.

Moreover, you must make sure that there is no other code (ie. in generic
ISDN code) doing cli() stuff on objects that you are protecting by a spinlock
etc., I've checked divert driver and it looks OK.

> OK, thank you. I went into the copy/paste trap. I corrected the patch to
> a global spinlock and tested it successfully.

Great.

> The corrected patch is applied to kernel 2.6.8.1 and should work for
> all 2.6 versions. You may also download the patch via http, too:
>
> http://ls7-www.cs.uni-dortmund.de/~mork/dss1_divert.diff
>
> Signed-off-by: Steffen A. Mork <[email protected]>

Please do "interdiff" between the old and the new patch
and send it to Linus (he has already merged the old patch).

Thanks.
Bartlomiej

2004-11-18 05:54:44

by Steffen A. Mork

[permalink] [raw]
Subject: Re: [PATCH] dss1_divert ISDN module compile fix for kernel 2.6.8.1

> Salut,
Hallo,

> On Wed, Nov 17, 2004 at 03:54:37PM +0100, Steffen A. Mork wrote:
>
>>http://ls7-www.cs.uni-dortmund.de/~mork/dss1_divert.diff
>
>
> This patch is wrong. You're using a spinlock you declare on the stack
> of a function, so it's completely pointless.
You are right. I've corrected my mistake already. Look at

http://ls7-www.cs.uni-dortmund.de/~mork/dss1_divert-patch1-2-patch2.diff

to correct that patch or

http://ls7-www.cs.uni-dortmund.de/~mork/dss1_divert-patch2.diff

for the complete correct patch.

> Please look at http://users.thundrix.ch/~tonnerre/kernel/2.6.1-tch1/
> where I gathered lots of ISDN patches a while ago. It might be a start
> for you.
Thank you.


> Tonnerre
Steffen


2004-11-18 05:58:40

by Tonnerre

[permalink] [raw]
Subject: Re: [PATCH] dss1_divert ISDN module compile fix for kernel 2.6.8.1

Salut,

On Thu, Nov 18, 2004 at 06:53:35AM +0100, Steffen A. Mork wrote:
> You are right. I've corrected my mistake already. Look at
>
> http://ls7-www.cs.uni-dortmund.de/~mork/dss1_divert-patch1-2-patch2.diff
>
> to correct that patch or
>
> http://ls7-www.cs.uni-dortmund.de/~mork/dss1_divert-patch2.diff
>
> for the complete correct patch.

Thanks, these patches hadn't yet made it to me at that point in time.

> >Please look at http://users.thundrix.ch/~tonnerre/kernel/2.6.1-tch1/
> >where I gathered lots of ISDN patches a while ago. It might be a start
> >for you.
>
> Thank you.

Please notify me on your results!

Tonnerre


Attachments:
(No filename) (648.00 B)
(No filename) (189.00 B)
Download all attachments