public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Steffen A. Mork" <linux-dev@morknet.de>
To: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: linux-kernel@vger.kernel.org, trivial@rustcorp.com.au, torvalds@osdl.org
Subject: Re: [PATCH] dss1_divert ISDN module compile fix for kernel 2.6.8.1
Date: Wed, 17 Nov 2004 19:52:54 +0100	[thread overview]
Message-ID: <419B9E06.6020100@morknet.de> (raw)
In-Reply-To: <58cb370e0411170828365d1982@mail.gmail.com>

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 <linux-dev@morknet.de>

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




  parent reply	other threads:[~2004-11-17 18:58 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-11-17 14:54 [PATCH] dss1_divert ISDN module compile fix for kernel 2.6.8.1 Steffen A. Mork
2004-11-17 16:28 ` Bartlomiej Zolnierkiewicz
2004-11-17 16:55   ` Linus Torvalds
2004-11-17 18:52   ` Steffen A. Mork [this message]
2004-11-17 20:07     ` Bartlomiej Zolnierkiewicz
2004-11-17 21:14       ` Steffen A. Mork
2004-11-17 16:39 ` Jan Engelhardt
2004-11-17 22:43 ` [PATCH] " Tonnerre
2004-11-18  5:53   ` Steffen A. Mork
2004-11-18  5:57     ` Tonnerre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=419B9E06.6020100@morknet.de \
    --to=linux-dev@morknet.de \
    --cc=bzolnier@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    --cc=trivial@rustcorp.com.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox