You must never, ever, under penalty of complete lockup, sleep with
spin-lock held. The fact, that the spinlock is an irqsave/irqrestore
one only makes that WORSE and will crash even on UP! (If you disabled
interrupts, who is going to schedule you back?)
It is also imperative, that the flags for irqsave/irqrestore live on the
stack!
It seems you don't get what _irqsave/_irqrestore means. spin_lock_irqsave
saves CPU flags to provided variable and clear interrupt flag. The
spin_unlock_irqrestore restores CPU flags from that variable.
I suggest you read some book about linux kernel programming -- the
locking is tricky.
On Wed, Jun 09, 2004 at 20:29:17 +0530, aksingh@hss.hns.com wrote:
> I am giving here the mechanism I intend to use in my module to avoid
> race conditions. I use kernel 2.4.20-8, on a uniprocessor system. Please
> suggest if you find any problems with it.
>
> [snap]
>
> /* two global variables */
> spinlock_t my_lock=SPIN_LOCK_UNLOCKED;
> unsigned long my_flags;
>
> pre_routing() {
>
> spin_lock_irqsave(&my_lock, flags);
>
> /****** PROCESS WHATEVER *******/
>
> spin_unlock_irqrestore(&my_lock, flags);
> }
>
> local_out() {
>
> /*** I USE THE SAME lock and flags variable in al functions ****/
Bad, bad, very bad!
> spin_lock_irqsave(&my_lock, flags);
>
> /****** PROCESS WHATEVER ******/
>
> spin_unlock_irqrestore(&my_lock, flags);
> }
>
> Timer1_expiry(){
>
> spin_lock_irqsave(&my_lock, flags);
>
> /****** PROCESS WHATEVER ******/
>
> spin_unlock_irqrestore(&my_lock, flags);
>
>
> }
>
> timer2_expiry() {
>
> spin_lock_irqsave(&my_lock, flags);
>
> /****** PROCESS WHATEVER ******/
>
> spin_unlock_irqrestore(&my_lock, flags);
>
>
> }
>
> read() {
> spin_lock_irqsave(&my_lock, flags);
>
> /****** PROCESS WHATEVER, uses copy_to_user ******/
> /*** the current process might go to sleep here when theres nothing to
> read, should not matter since I am using irqsave ***/
It does a HELL LOT matter. It's a bug. A fatal one. With spin_lock often
fatal, with spin_lock_irqsave always fatal.
> spin_unlock_irqrestore(&my_lock, flags);
> }
>
> block() {
> /*... the block call for the char device(called by read), when there is
> nothing to read */
> return wait_event_interruptible(...);
> }
>
> write(){
> spin_lock_irqsave(&my_lock, flags);
>
> /****** PROCESS WHATEVER, uses copy_from_user .. ******/
> /*** process might go to sleep if the user variable from which data is
> being written is swapped out, shld not matter since I am using irqsave
> ****/
>
> spin_unlock_irqrestore(&my_lock, flags);
> }
>
> poll() {
> havent thought about this ;
> }
>
>
> So How does it look, any obvious flaws ?, something I can do better.
>
> Any suggestions are welcome.
-------------------------------------------------------------------------------
Jan 'Bulb' Hudec <bulb@ucw.cz>
Attachment:
signature.asc
Description: Digital signature
