Making code non-preemptable

I am working on a device driver where a call to a counter card is either scheduled with a timeout, or called from an interrupt. I have a semaphore at the beginning so that the subroutine cannot interrupt itself. However, I need two commands to be atomic such that they cannot get pre-empted by the kernel. I know that this is not an issue with my kernel, it's a non-preemtive 2.2 kernel, but I'd like to make sure it doesn't get broken in the future.

static inline void getcounts() { struct timeval tv; unsigned long flags;

if (test_and_set_bit(0, &glb_atom))return; // disable interrupts, these need to be atomic save_flags(flags); cli(); do_gettimeofday(&tv); outb_p(0x40, CTRX); // latch counter 1 restore_flags(); // system irqs re-enabled here l = inb(CTR1); h=inb(CTR1); glb_data.t1 = (h

Reply to
Brian K. Michalk
Loading thread data ...

How are they going to be interrupted by anything else if you've disabled interrupts? And if this is potentially runnning in an interrupt handler, I wouldn't use a semaphore if I were you. If you block on it while running in interrupt context, you'll get an oops.

HTH Neil

--
/***************************************************
  *Neil Horman
 Click to see the full signature
Reply to
Neil Horman

Yes, that is what we have spinlocks for. Put spinlock_irq_save and unlock_irq_restore around the critical region (or something like that I might not remember correctly about the names).

--
Kasper Dupont -- der bruger for meget tid paa usenet.
For sending spam use mailto:aaarep@daimi.au.dk
 Click to see the full signature
Reply to
Kasper Dupont

I was reading Rubini about how code needed to be able to handle preempted cases. I know it's not a problem now, but wanted it to work on SMP or pre emptable kernels.

My semaphore is a set and test then return if true. It does not block. If something else is already in the getcounts() routine, there is no need to run it again, I'll get the same data twice.

I don't see how this will generate an oops. At least it not yet. It's been running this way for two days now.

Reply to
Brian K. Michalk

So you made your own semaphore instead of using the one provided by the kernel? How can it do anything usefull if it doesn't block when an execution context comes in and sees that the semaphore is already acquired?

You're probably just getting lucky. If you have a semaphore which doesn't block, then your execution is ok, but your code is very broken, since the semaphore isn't doing its job, and if you're using a semaphore that does block appropriately, then you're in danger, as the first time an interrupt handler sees that semaphore as locked, it will try to reschedule itself. Since rescheduling in an interrupt handler is illegal, you'll get an oops. Kasper is correct, use spin_lock_irqsave and spin_unlock_irqrestore. These will allow to ensure that you don't get an interrupt when someone else is executing your code, and its smp safe.

HTH Neil

--
/***************************************************
  *Neil Horman
 Click to see the full signature
Reply to
Neil Horman

Okay. That helps a lot. I'm not using a real semaphore, I'm using the command: test_and_set_bit()

I do not want to reschedule the ISR, I simply want it to exit. The subroutine getcounts() Should not have more than two processes simultaneously, and that's where the semaphore is.

The psuedocode is something like this: while(1) { wait for .25 seconds if timed out getcounts() do something with counts (supplied by by ISR, or timed out) }

ISR: { getcounts() wake_up_interruptible() }

So the main loop times out, or is woken up by the ISR. getcounts does not need to be rescheduled, because the context of the interrupt and the ISR are identical, they both return the same data to the same destination.

I definately know I don't want to wait on the semaphore becoming available.

Rather than posting back and forth, is there some other documentation you could point me to?

I have Rub> >

Reply to
Brian K. Michalk

If all your after is to ensure that you exit if someone has already begun processing your common function, then you're probably pretty well off with test_and_set_bit(). I think that the atomic_inc/dec api may work well also, but it just uses test_and_set_bit under the covers I believe.

Neil

--
/***************************************************
  *Neil Horman
 Click to see the full signature
Reply to
Neil Horman

Okay. Thanks.

The biggest problem I seem to be having is one of lack of documentation. This area of programming seems to be such a small segment, that there aren't enough people around to code and document, and this type of programming is extremely complex with many caveats.

While the code is very clean and understandable, it is very powerful. This style of kernel programming is way simpler and more intuitive since my operating systems classes in computer science that I took more than ten years ago.

There are so many neat instructions now that save registers, and do all the dirty stuff under the covers.

Reply to
Brian K. Michalk

I make a point to go an get the hardware programmers guides from intel/motorola/amd/etc for the architecutres I work with. They're free, and they usually help me understand alot of the low level kernel code that would otherwise seem quite cryptic. Neil

--
/***************************************************
  *Neil Horman
 Click to see the full signature
Reply to
Neil Horman

Thanks. You've been very helpful. I'm having a different problem now, where wait_event_interruptible() is not acting as expected.

I'll snip the code for brevity:

137 MODULE_AUTHOR ("Brian K. Michalk"); 138 139 DECLARE_WAIT_QUEUE_HEAD (tpmod_queue); ... 698 ssize_t tpmod_read (struct file *filp, char *buf, size_t count, loff_t *f_pos) { 699 unsigned char *kbuf; 700 char tbuf[100]; 701 int i; 702 703 printk(KERN_WARNING "sleeping %d %d\n", trace_head, trace_tail); 704 wait_event_interruptible(tpmod_queue, (trace_tail != trace_head)); 705 mb(); 706 printk(KERN_WARNING "read waking up\n"); 707 if (signal_pending (current)) return -ERESTARTSYS; ... 727 trace_head = (trace_head+1)%tracebufsize; ... 730 return (count); 731 }

For some reason, when I cat the device, it will emit bufsize lines of data, but when the buffer is empty(head==tail), it seems like the process never gets woken up by the interrupt subroutine.

Reply to
Brian K. Michalk

Who is doing the waking up? The condition you pass in to this routine is only checked when the task wakes up, and released from the call if its true. Some context still needs to call wake_up on the wait queue.

HTH Neil

--
/***************************************************
  *Neil Horman
 Click to see the full signature
Reply to
Neil Horman

Yup. That was it. I thought that the kernel automagically checked the condition from time to time. Thanks.

Reply to
Brian K. Michalk

I'm making some real headway here. The problem I'm having though, is that sometimes when I cat the device with a "cat /dev/tpmod", I get a "cat: /dev/tpmod: Bad address" error.

This can be easily repeated by pressing CTRL-Z, and then putting it back in the foreground.

I suspect it has to do with a buffer overflow, and the file position pointer not matching up with where valid data lives. I've scoured, and am unable to find reference to some test function that will tell me if the receiver is capable of reading more data.

Reply to
Brian K. Michalk

Thats really going to be your drivers responsibility. If you doing a cat of a device that maps to your driver, your going to have to maintain all the information needed to determine weather or not the read request is providing a valid offset and read length for your device. If you give us more details regarding your driver, I might be able to give you more detailed advice.

Neil

--
/***************************************************
  *Neil Horman
 Click to see the full signature
Reply to
Neil Horman

After many printk's, I narrowed it down to the sprintf statements. I don't recall what the magic number was, but over a certain size, the read() method would return an error. I simply made sure I couldn't transfer more than x amount of data.

I'm pretty sure I was overflowing the ssize_t return variable. I have not yet done an analysis to see what the limits of ssize_t are, or if it was the sprintf command failing.

Reply to
Brian K. Michalk

size_t's are unsigned ints so unless your the math you do to compute the amount of data you're sending back to user space is wrong, or you're transferring 4GB of data to user space, I don't think you're overflowing that value. :) I frequently get errors of this nature though, when I try to copy data to user space past the end of the buffer the user space app sent down to the driver. The count parameter of the read function should be telling you how many bytes _at_most_ to send back to user space (effectively this is the size of the buf parameter).

HTH Neil

--
/***************************************************
  *Neil Horman
 Click to see the full signature
Reply to
Neil Horman

ElectronDepot website is not affiliated with any of the manufacturers or service providers discussed here. All logos and trade names are the property of their respective owners.