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

drivers/isdn/divert/divert_init.c 2004-11-05 08:50:02.000000000 +0100 renamed from drivers/isdn/divert/divert_init.c 2004-08-14 12:55:47.000000000 +0200 +8 -6

@@ -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

drivers/isdn/divert/divert_procfs.c 2004-11-05 08:49:57.000000000 +0100 renamed from drivers/isdn/divert/divert_procfs.c 2004-08-14 12:56:22.000000000 +0200 +4 -3

@@ -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

drivers/isdn/divert/isdn_divert.c 2004-11-05 08:49:20.000000000 +0100 renamed from drivers/isdn/divert/isdn_divert.c 2004-08-14 12:55:34.000000000 +0200 +53 -55

@@ -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

drivers/isdn/i4l/Kconfig 2004-11-17 13:34:09.983567648 +0100 renamed from drivers/isdn/i4l/Kconfig 2004-08-14 12:55:10.000000000 +0200 +0 -0

@@ -99,7 +99,7 @@ config ISDN_DRV_LOOP
 

config ISDN_DIVERSION
tristate "Support isdn diversion services"

drivers/isdn/i4l/Kconfig 2004-11-17 13:34:09.983567648 +0100 renamed from drivers/isdn/i4l/Kconfig 2004-08-14 12:55:10.000000000 +0200 +1 -1

-       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

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

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.

Quoted 7 lines

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.

Dear Linus and Bartlomiej,

Quoted 11 lines

Quoted 7 lines

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]>

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.

Quoted 2 lines

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).

Thanks.

Bartlomiej

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

Salut,

Quoted 2 lines

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

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.

Quoted 3 lines

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.