[PARPORT] Re: lp interrupt patch


Andrea Arcangeli (arcangeli@mbox.queen.it)
Sun, 3 May 1998 15:32:22 +0200 (CEST)


On Wed, 29 Apr 1998, Andrea Arcangeli wrote:

>I also fixed a possible race conditions in lp_open in SMP system due not
>atomic operation. To do that I added a spinlock to lp_struct. Since

The spinlock was not needed since we can more simply use
test_and_set_bit().

This patch that remove the spinlock, it' s against my latest lp patch.

--- linux/drivers/char/lp.c 1998/04/28 22:57:05 1.12
+++ linux/drivers/char/lp.c 1998/05/03 11:00:27
@@ -84,7 +84,6 @@
 #include <asm/irq.h>
 #include <asm/uaccess.h>
 #include <asm/system.h>
-#include <asm/spinlock.h>
 
 /* if you have more than 3 printers, remember to increase LP_NO */
 #define LP_NO 3
@@ -96,13 +95,7 @@
 #ifdef LP_STATS
                            0, 0, {0},
 #endif
- NULL, 0, 0,
- /*
- * We must initialize the spinlock_t by
- * hand at runtime to avoid gcc 2.7.2.3
- * segfaults.
- */
- /* SPIN_LOCK_UNLOCKED */}
+ NULL, 0, 0}
 };
 
 /* Test if printer is ready (and optionally has no error conditions) */
@@ -580,14 +573,8 @@
                 return -ENXIO;
         if ((LP_F(minor) & LP_EXIST) == 0)
                 return -ENXIO;
- spin_lock(&lp_table[minor].lock);
- if (LP_F(minor) & LP_BUSY)
- {
- spin_unlock(&lp_table[minor].lock);
+ if (test_and_set_bit(LP_BUSY_BIT_POS, &LP_F(minor)) & LP_BUSY)
                 return -EBUSY;
- }
- LP_F(minor) |= LP_BUSY;
- spin_unlock(&lp_table[minor].lock);
 
         MOD_INC_USE_COUNT;
 
@@ -633,8 +620,8 @@
 
         kfree_s(lp_table[minor].lp_buffer, LP_BUFFER_SIZE);
         lp_table[minor].lp_buffer = NULL;
- LP_F(minor) &= ~LP_BUSY;
         MOD_DEC_USE_COUNT;
+ LP_F(minor) &= ~LP_BUSY;
         return 0;
 }
 
@@ -794,7 +781,6 @@
         if (lp_table[nr].dev == NULL)
                 return 1;
         lp_table[nr].flags |= LP_EXIST;
- spin_lock_init(&lp_table[nr].lock);
 
         if (reset)
                 lp_reset(nr);
--- linux/include/linux/lp.h 1998/05/03 11:03:52 1.5
+++ linux/include/linux/lp.h 1998/05/03 11:04:28
@@ -7,8 +7,6 @@
  * Interrupt support added 1993 Nigel Gamble
  */
 
-#include <asm/spinlock.h>
-
 /* Magic numbers for defining port-device mappings */
 #define LP_PARPORT_UNSPEC -4
 #define LP_PARPORT_AUTO -3
@@ -22,6 +20,7 @@
 #define LP_EXIST 0x0001
 #define LP_SELEC 0x0002
 #define LP_BUSY 0x0004
+#define LP_BUSY_BIT_POS 2
 #define LP_OFFL 0x0008
 #define LP_NOPA 0x0010
 #define LP_ERR 0x0020
@@ -131,11 +130,6 @@
         struct wait_queue *wait_q;
         volatile int irq_state;
         unsigned int last_error;
- /*
- * This must be the last field since must be
- * initialized by hand due a gcc 2.7.2 bug.
- */
- spinlock_t lock;
 };
 
 /*

>If all is OK next days I will forward the patch to Linus.

It seems that all is OK but I' d like if people can send me the output of
these tests with my latest patch applyed using lp interrupt driven to
understand if in some cases we really need to poll inside lp_char() as my
latest lp patch does to get best performance.

/usr/bin/time dd if=/dev/zero of=/dev/lp0 count=10
tunelp /dev/lp0 -c 1
/usr/bin/time dd if=/dev/zero of=/dev/lp0 count=10
tunelp /dev/lp0 -c 1000
/usr/bin/time dd if=hereafiletxt of=/dev/lp0 count=10
tunelp /dev/lp0 -c 1
/usr/bin/time dd if=herethesamefiletxt of=/dev/lp0 count=10
tunelp /dev/lp0 -c 1000

With my hardware I get:

root@dragon:~# /usr/bin/time dd if=/dev/zero of=/dev/lp0 count=10
10+0 records in
10+0 records out
0.00user 5.79system 0:08.69elapsed 66%CPU (0avgtext+0avgdata 0maxresident)k
                    ^^^^^^^^^^^^^^^^^^^^^
0inputs+0outputs (81major+11minor)pagefaults 0swaps

root@dragon:~# tunelp /dev/lp0 -c 1
/dev/lp0 using IRQ 7
root@dragon:~# /usr/bin/time dd if=/dev/zero of=/dev/lp0 count=10
10+0 records in
10+0 records out
0.00user 0.06system 0:08.71elapsed 0%CPU (0avgtext+0avgdata 0maxresident)k
                    ^^^^^^^^^^^^^^^^^^^^
0inputs+0outputs (81major+11minor)pagefaults 0swaps

My hardware for example doesn' t need the polling stage in lp_char().

Andrea[s] Arcangeli

-- To unsubscribe, send mail to: linux-parport-request@torque.net --
-- with the single word "unsubscribe" in the body of the message. --



This archive was generated by hypermail 2.0b3 on Wed 30 Dec 1998 - 10:17:41 EST