Re: race conditions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 10, 2004 at 11:14:11 +0530, aksingh@hss.hns.com wrote:
>       In my module's case the situation is different . The read function in
> the module puts to sleep the current process, waiting for some data on the
> char buffer(a global variable in the module). Now this char buffer is
> filled by another function in the same module, that is process independent,
> it is a timer handler, which fills this buffer.
> So, right I shld not say save_irq as it would disable interrupts and my
> timer handler would never be called to fill that char buffer, resulting in
> a deadlock, so i shld do this :
> 
> read(){
> 
> dont take any locks here, as the process might go to sleep using
> wait_event_interruptible;
> and this sleeping process wld then be awakened by timer1_expiry_func(),
> which calls wake_up_interruptible.
> /* process goes to sleep */
> 

But you obviously have to lock the data. On the other hand, since
copy_to/_from_user MAY SLEEP, you MUST NOT hold spinlock over that.

> }
> 
> write(){
> spin_lock not called right at the beginning, called after copy_fom_user has
> returned some usable values.

Right. copy_from_user might sleep...

> spin_lock(&my_lock);
> write to some module global variables; /*so hold this lock only while the
> variable values are being assigned, the user process cannot go to sleep
> here */
> spin_unlock(&my_lock);
> }
> 
> timer1_expiry_func() {
> 1. fill char buffer

Remember that it's data that is protected with locks. So if some data --
buffer in this case -- is to be protected with spinlock, it must always
be so. You still should use _irqsave/_irqrestore here, unless you are
absolutely sure interrupts are disabled here.

> 2. wakeup interruptible

Just wake_up. wake_up wakes up all processes on a waitqueue, no matter
whether they are in TASK_UNINTERRUPTIBLE or TASK_INTERRUPTIBLE state.
You only want wake_up_interruptible when waking up for other reason than
satisfying the condition.

> }
> 
> timer2_expiry_func() {
> 1. does not fill (touch) thatt char buffer, so need not wake anything, but
> in general i should not lock timer1 & timer 2 expiry funcs using spinlocks
> }

No. You are locking access to data. If it touches buffer, it must be
locked.

> netfilter_hook_funcs(){
> 1. module global variables as set in write are used here
> 2. so do a spin_lock(&my_lock) when this starts
> 3. and do a spin_unlock(&my_lock) when this ends
> }

No. For one thing, you must again do _irqsave/_irqrestore variant (flags
as local variable!) unless you are definitely sure interrupts are
disabled here -- and interrupt handlers usualy enable interrupts quite
early, so it's likely interrupts are enabled here.

> I hope this looks better. Thanks for the comments Jan, actually I have
> tried reading rubini, though my understanding is still a bit wobbly now.

I don't know whether it's described better somewhere else.

The key thing to note is, that spinlock does a busy waiting. Something
like "while(lock);" (careful crafting in assembly is necessary for it to
work, of course). So only the other CPU on SMP can ever release it
(that's why spinlocks compile to nothing in non-preemptable UP kernel).

If interrupt handler ever need the lock, you need the
_irqsave/_irqrestore variant, which clears interrupt flag. So the
interrut simply can't happen when the lock is held.

Note however, that many functions are not called from interrupt, but
rather from scheduler -- and that won't run when you are in kernel mode
unless you call schedule(). (In preemptable kernel it might happen, but
holding spinlock prevents preemption.)

On the other hand, semaphores are implemented with wait_queue and
schedule(). So you can safely use semaphores over long blocks of code,
that does sleep. But you can't use semaphore under spinlock and you
can't use semaphore in interrupt context.

-------------------------------------------------------------------------------
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Newbies FAQ]     [Linux Kernel Mentors]     [Linux Kernel Development]     [IETF Annouce]     [Git]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux SCSI]     [Linux ACPI]
  Powered by Linux