Re: [Question] Hooks for scheduler tracing (CFS)
Hello Arjan, Thanks for this suggestion. But I looked at Systemtap before, and as I remember, it is very flexible, but it only traces function calls. (or am I missing something?) I also need this scheduler-tracer, besides for fully fledged X86 systems, also for ARM based boards also with a very minimal userland, and limited flash. So, it has to be lightweight also. I can only use cross-compilers to build the tools... (something Systemtap is not good at as I remember...) So, I am missing something here, or it is unfortunately not what I am looking for... Kind Regards, Remy 2007/7/16, Arjan van de Ven [EMAIL PROTECTED]: On Mon, 2007-07-16 at 21:46 +0200, Remy Bohmer wrote: So I was wondering if anybody knows some tool/kernel mechanism which can do this? If not, I will build a kernel extension for it myself (new extension to 'latency_trace' ?) systemtap has been able to do such things for me in the past... -- if you want to mail me at work (you don't), use arjan (at) linux.intel.com Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to control IRQ thread priority from inside the driver
Hello Darren, I disagree. The point of real-time is to provide user-space with more control of what get's run, when, instead of something else - including system interrupts, etc. For instance, if networking should take a higher priority than their application, but disk i/o should take a lower one, the user needs to be able to control that. chrt is the correct approach for -rt in my opinion. I believe this depends on the environment where the RT-kernel is used. Therefor this might be true for Realtime Desktop/Server applications, but, for RT-Embedded systems it is possible (likely) that the entire kernel is pre-configured for 1 single purpose, and that some developer wants to set the priorities for that purpose only. In that case there is usually no user that cares anything about what is running on the system, as long as his device works, so that user will never come to the idea to use 'chrt'. In these case the default priorities at 50 are useless and the developer has to have a way to configure the priorities. So I understand the question of Juergen. To suit my needs, I did this by just patching the kernel thread priorities at the places where they are started. (And I am still happy with my solution. Note that because it is very application specific, I never expect a patch like this to get to the mainline) Kind Regards, Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to control IRQ thread priority from inside the driver
Hello Steven, Actually, the handler thread runs _all_ register interrupt handlers. So there's no way to make one a higher priority than the other. This is a tough problem. If you have two shared handlers on the same irq line, and think you can make one a higher priority than the other, you can't. If the lower one is running (at the lower priority), and it gets preempted by an RT task with a priority that is between the lower priority handler and the higher priority handler, the higher priority handler may never run. The interrupt line for the IRQ is masked while the handlers are running. So by preempting the lower priority handler you are also preempting the higher priority IRQ from coming in. Hmm, I think you did not get my point... What you mention is basic knowledge... ;-)) What I meant was that from Embedded point of view, some developer probably don't care about what the irq-line-number is of some device. But, such a developer probably wants to prioritize the interrupt handler of his device. Dependant on how the hardware layout looks like it can be interrupt # X, Y or Z. If the kernel can match the device name to a certain IRQ #, the implementation is probably much more flexible than by just specifying the IRQ # at the kernel commandline. In the case where the IRQ handler is shared with some other device, than there are 2 options: the developer does not care, or should choose different hardware. Arnaldo mentioned to use the irq-name for this, and to me this sounds good. possible to use 1 target image usable on multiple boards with 1 single kernel and 1 single config. You can still have one kernel and one config. A command line is separate. My mistake: I meant with config the grub, u-boot or ... config, not the kernel config five if you count my writing of the chapter in the O'Reilly book Building Embedded Linux Systems about RT. But these are some of the Interesting... When will this book be finished? I am always curious about the point of view of others on RT... (I am working with Ingo's RT-patch for several years now, started with the early preemption patches on 2.4 kernels) Kind Regards, Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to control IRQ thread priority from inside the driver
Hello Steven, Perhaps adding in a new command line interface (We may need to extend the command line size limit). Where one could do irq_priority_map=10-51,21-40,... Where the first number is the irq number followed by the irq priority. We can also do something similar to softirqs. Would something like that satisfy your needs? Yeah, it would at least suit my needs :-) But, the irq-numbers to be defined is hardware specific, and even board specific on X86. Maybe someone wants to give a certain interrupt handler a different priority, but not a certain IRQ-number to keep it possible to use 1 target image usable on multiple boards with 1 single kernel and 1 single config. How to define that on a kernel commandline is still a question to me... When I get time, I can write up a patch to do something like this. I could write a proposal on a solution for this, and if we all agree for one solution, I could make the patch for this. (Maybe I have more time free than you ;-) ) Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to control IRQ thread priority from inside the driver
Hello Darren, Juergen and Ingo,
All I need is one IRQ at priority 51, all other can still run at 50. And I
don't want to search at system's runtime for the right PID of driver's
thread to set its priority via chrt. There is no user at this system, its
an embedded one.
It seems reasonable then, for such a specialized case, to consider Remy's
reply to my previous mail:
To suit my needs, I did this by just patching the kernel thread
priorities at the places where they are started.
Attached I have put my patch to change the priorities of the
soft-irq's and the IRQ threads.
Maybe Juergen can use it so suit his needs. (Sorry for attaching
instead of inlining, but I have not find a way to inline a patch
without corrupting it while using Gmail from within Firefox)
But, I think that the mechanism to change the soft-irq priorities can
be integrated in the mainline RT-patch, I have modified the
softirq_info struct with the soft-irq thread names such that it also
contains the thread priority per soft-irq. For customisation it is
easier to adapt this struct, than to apply the entire patch over and
over. Until now the code seems to change that frequently that I have
to reimplement this patch over and over...
Maybe Ingo can look at it, and find a way to implement it more
generic? (In the softirq_info struct we could default set all prios
back to 50, and than from functional point of view there will be no
change for other RT-patch users, but makes Embedded-users life easier.
While looking at the IRQ-threads: Notice that setting all IRQ-threads
priorities to 50 with SCHED_FIFO is a bit strange to begin with,
because on a non-RT kernel all IRQ's have different priorities and a
higher prio irq can preempt a lower prio one. (on X86) The current
implementation will not preempt another irq-thread due to the same
prios and fifo scheduling.
The implementation I have in this patch follows the original behaviour
more closely, but changing that also could lead to some discussion...
Kind Regards,
Remy
This patch changes the soft-irq priority for all kernel daemon threads
like Interrupt threads and soft-irqs to match them with our
applications.
Signed-off-by: Remy Bohmer [EMAIL PROTECTED]
---
kernel/irq/manage.c | 12 +--
kernel/softirq.c| 53 +---
2 files changed, 44 insertions(+), 21 deletions(-)
Index: linux-2.6.22/kernel/irq/manage.c
===
--- linux-2.6.22.orig/kernel/irq/manage.c 2007-07-24 16:40:15.0 +0200
+++ linux-2.6.22/kernel/irq/manage.c 2007-07-24 16:46:52.0 +0200
@@ -757,10 +757,13 @@ static void do_hardirq(struct irq_desc *
wake_up(desc-wait_for_handler);
}
+#define base_irq_prio 45
+
static int do_irqd(void * __desc)
{
struct sched_param param = { 0, };
struct irq_desc *desc = __desc;
+ int irq = desc - irq_desc;
#ifdef CONFIG_SMP
cpumask_t cpus_allowed, mask;
@@ -776,12 +779,17 @@ static int do_irqd(void * __desc)
current-flags |= PF_NOFREEZE | PF_HARDIRQ;
/*
- * Set irq thread priority to SCHED_FIFO/50:
+ * Use a custom irq thread priority mechanism:
*/
- param.sched_priority = MAX_USER_RT_PRIO/2;
+ param.sched_priority = base_irq_prio - irq;
+ if (param.sched_priority 1)
+ param.sched_priority = 1;
sys_sched_setscheduler(current-pid, SCHED_FIFO, param);
+ printk(KERN_INFO [IRQ-%d] started with prio:%i\n,
+ irq, param.sched_priority);
+
while (!kthread_should_stop()) {
local_irq_disable_nort();
do {
Index: linux-2.6.22/kernel/softirq.c
===
--- linux-2.6.22.orig/kernel/softirq.c 2007-07-24 16:40:15.0 +0200
+++ linux-2.6.22/kernel/softirq.c 2007-07-24 16:49:05.0 +0200
@@ -764,10 +764,30 @@ EXPORT_SYMBOL(tasklet_unlock_wait);
#endif
+static struct
+{
+ const char *name;
+ const int prio;
+} softirq_info [] =
+{
+ [HI_SOFTIRQ] = {high,33},
+ [SCHED_SOFTIRQ] = {sched, 73},
+ [TIMER_SOFTIRQ] = {timer, 72},
+ [NET_TX_SOFTIRQ] = {net-tx, 20},
+ [NET_RX_SOFTIRQ] = {net-rx, 20},
+ [BLOCK_SOFTIRQ] = {block, 32},
+ [TASKLET_SOFTIRQ] = {tasklet, 31},
+#ifdef CONFIG_HIGH_RES_TIMERS
+ [HRTIMER_SOFTIRQ] = {hrtimer, 72},
+#endif
+ [RCU_SOFTIRQ] = {rcu, 20}
+};
+
static int ksoftirqd(void * __data)
{
- struct sched_param param = { .sched_priority = MAX_USER_RT_PRIO/2 };
struct softirqdata *data = __data;
+ struct sched_param param = {
+ .sched_priority = softirq_info[data-nr].prio };
u32 softirq_mask = (1 data-nr);
struct softirq_action *h;
int cpu = data-cpu;
@@ -776,6 +796,16 @@ static int ksoftirqd(void * __data)
init_waitqueue_head(data-wait);
#endif
+ if ( (param.sched_priority 1) ||
+ (param.sched_priority MAX_RT_PRIO) ) {
+ BUG();
+ param.sched_priority = 1;
+ }
+
+ printk(KERN_INFO softirq-%s/%lu started up, pid:%d, RT prio: %d.\n,
+ softirq_info[data-nr].name, data
Re: PI mutex support on ARM
Hello Manfred, At http://www.spinics.net/lists/arm-kernel/msg42349.html you can find a patch that will probably work. Due to the lack of proper atomic instructions on the ARM-v4 core this patch is likely to be the best possible solution. Kind Regards, Remy 2007/8/20, Manfred Gruber [EMAIL PROTECTED]: hi ! Is about the PI Mutex issue on ARM some work going on? Or does someone have an information for me how to implement this for a armv4 correct ? thanks fred - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PI mutex support on ARM
Hello Manfred, My question if I run the pi_stress, my watchdog triggers, if watchdog is disabled i get this: Is this the correct behaviour ? Or does it show a problem ? Depends on the priority of your watchdog-feeder thread. If it is not running in RT mode, with a priority higher than used in the pi-stress test, then yes it is normal that the watchdog kicks in. pi-stress takes 100% CPU-load, and leaves no room for non-RT processes. The output you get seems normal to me... Remy 2007/9/4, Manfred Gruber [EMAIL PROTECTED]: hi ! I have now run 2.6.23-rc4-rt1 on my arm920t with the pi_stress test programm from http://people.redhat.com/williams/tests My question if I run the pi_stress, my watchdog triggers, if watchdog is disabled i get this: Admin threads running on processor: 0 Test threads running on processor: 0 Starting PI Stress Test Number of thread groups: 10 Number of inversions per group: infinite Test threads using scheduler policy: SCHED_FIFO Admin thread priority: 4 10 groups of 3 threads will be created High thread priority: 3 Med thread priority: 2 Low thread priority:1 Press Control-C to stop test Current Inversions: 15551 Keyboard Interrupt! Stopping test Total inversion performed: 15570 Test Duration: 0 days, 0 hours, 0 minutes, 14 seconds Is this the correct behaviour ? Or does it show a problem ? thanks, regards fred Am Tuesday 21 August 2007 schrieben Sie: Hi Remy ! thanks a lot, i will test it on a ARM-v4 with 2.6.23-rc2-rt2 and 2.6.21.5-rt20. I have a application which makes a lot of pthread_mutex_lock/unlock between rt and non-rt tasks with PI. So i will look what happens. thanks fred Am Monday 20 August 2007 schrieben Sie: Hello Manfred, At http://www.spinics.net/lists/arm-kernel/msg42349.html you can find a patch that will probably work. Due to the lack of proper atomic instructions on the ARM-v4 core this patch is likely to be the best possible solution. Kind Regards, Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PI mutex support on ARM
Hello Manfred, The only difference between you patch and now 2.6.23-rc4-rt1 is that now a raw_ spinlock is used. So i think when using your patch on 2.6.21.5-rt20, because a customer has this kernel running, i have there also to use a raw_spinlock ? The same patch applies to older kernels. Besides, I use this patch for a while now on 2.6.21 and 2.6.22, but with an interrupt lock instead of a spinlock. A raw_spinlock is better because it is safe in SMP also, so Ingo has the proper implementation in his patch, which you should follow. (Probably this is technically the best possible solution, given the limited amount of atomic-instructions on ARM-v4/v5) So, A normal (non-raw) spinlock should NOT be used here, because that does would be converted to a rt-mutex, which is preemptible. If this code is preemptible, it could be preempted by another user space thread which could, in theory, modify the same piece of memory, and thus causing a race condition. Notice that there have been raised questions about the interrupt lock while accessing user space memory in this code, it is considered as not-safe. AFAIK the only situation where it is not-safe, is the case where the memory is not available in RAM, but e.g. somewhere in a swap space. If the memory has to be paged in, probably interrupts are needed. In my system things like this will never happen... Maybe Ingo, Thomas or someone else can acknowledge (or trash) my opinion here? Kind Regards, Remy Bohmer - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PI mutex support on ARM
Hello Steven, Thanks for your explanation, here are my answers: So, A normal (non-raw) spinlock should NOT be used here, because that does would be converted to a rt-mutex, which is preemptible. If this code is preemptible, it could be preempted by another user space thread which could, in theory, modify the same piece of memory, and thus causing a race condition. Are you talking about this code? Yes! + spin_lock(futex_atomic_lock); + + __asm__ __volatile__( @futex_atomic_cmpxchg_inatomic \n + 1: ldrt%0, [%3]\n + teq %0, %1 \n + 2: streqt %2, [%3]\n + 3: \n + .section __ex_table, \a\ \n + .align 3\n + .long 1b, 4f, 2b, 4f \n + .previous \n + .section .fixup,\ax\ \n + 4: mov %0, %4 \n + b 3b \n + .previous + : =r (val) + : r (oldval), r (newval), r (uaddr), Ir (-EFAULT) + : cc); + + spin_unlock(futex_atomic_lock); I don't quite remember ARM (been a few years since working with it), but this looks like the standard load store-conditional, or is it simply a read/test/write? (I don't have a ARM book handy at the moment). it is a read/test/write, or in other words a cmpxchg(). But I would ask, what would touch that memory without first taking the futex_atomic_lock? So I don't understand your issue of being preempted. AFAIK, it is user space memory, allocated in the context of a user space process. If this memory location is modified outside this 'protected' section, like for example in some codepath in glibc also (e.g. futex slowpath implementation), then this implementation could be dangerous with a normal spinlock/rt-mutex. Notice that there have been raised questions about the interrupt lock while accessing user space memory in this code, it is considered as not-safe. AFAIK the only situation where it is not-safe, is the case where the memory is not available in RAM, but e.g. somewhere in a swap space. If the memory has to be paged in, probably interrupts are needed. In my system things like this will never happen... That's nice, but other systems might have that happen ;-) And that is exactly the reason, why I have questions with this implementation... Kind Regards, Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FAQ for realtime support in Linux Kernel
Hello Jaswinder, I have some troubles with the question: 'What effects by realtime support and how much and samples to test it and who maintains these samples?' I doubt anyone can fill in this paragraph properly. It is too vague, and the figures you ask are different for every architecture and processor type/speed that is supported. What do you think? Kind Regards, Remy Bohmer 2007/10/1, Jaswinder Singh [EMAIL PROTECTED]: Hello all, I tried to fill remaining entries in : http://rt.wiki.kernel.org/index.php/Frequently_Asked_Questions Please it out, is these OK. Thank you, Jaswinder Singh. On 9/30/07, Jaswinder Singh [EMAIL PROTECTED] wrote: Thank you Theodore Ts and Remy Bohmer for updating : http://rt.wiki.kernel.org/index.php/Frequently_Asked_Questions And giving us such a valuable information. Thank you, Jaswinder. On 9/29/07, Jaswinder Singh [EMAIL PROTECTED] wrote: Hello Ted, Thanks for your suggestions, I tried to fixed it. Now waiting from other guys to complete this FAQ as soon as possible ;-) http://rt.wiki.kernel.org/index.php/Frequently_Asked_Questions Thank you, Jaswinder Singh. On 9/29/07, Theodore Tso [EMAIL PROTECTED] wrote: On Sat, Sep 29, 2007 at 08:46:20PM +0530, Jaswinder Singh wrote: http://rt.wiki.kernel.org/index.php/FAQ_for_realtime_support_in_Linux_Kernel Thank you for putting it together and contributing it to the wiki! I renamed the page to make the page title be a bit more succint and less redundant (everything in rt.wiki.kernel.org is relating to the Realtime Linux :-), and I've added a link to the FAQ to the front page of the wiki. As a suggestion, there's probably no need to have the revisions table, since anyone who cares about the revision can just click on the revisions tab on the wiki. (BTW, I normally like to use the preview button during the edit cycle, instead of constantly using the save button and then looking at the new page; you might want to make sure that Show preview before first edit and Show preview before edit box are checked in your user preferences.) Note by the way that some of the answers to the questions are located elsewhere in the Wiki; which doesn't make them be bad questions, but for example, it would probably be a good idea for some of the questions to not try to give a partial answer, but to direct them to areas such as the RT PREEMPT HOWTO page (for example). Regards, - Ted - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FAQ for realtime support in Linux Kernel
Hello Theodore, At some point it would be good to add some commentary about the resulting throughput degradation (which is why I kept the question open-ended about performance, instead of latency). Is there still a throughput degradation noticeable on some platforms? I know for sure it was there in the past, but I thought these were all fixed by now. AFAIK: The latest kernels have fixed the netwerk performance degradation bug, and on Dual-Core we even notice a performance improvement compared to non-RT. Kind Regards, Remy Bohmer 2007/10/2, Theodore Tso [EMAIL PROTECTED]: On Mon, Oct 01, 2007 at 10:18:15PM +0200, Remy Bohmer wrote: I doubt anyone can fill in this paragraph properly. It is too vague, and the figures you ask are different for every architecture and processor type/speed that is supported. I've already replaced that question with one asking about what kind of performance one could expect, and answered it with the standard it depends answer. I moved it to the general Getting Started section. At some point it would be good to add some commentary about the resulting throughput degradation (which is why I kept the question open-ended about performance, instead of latency). - Ted - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[RT] seqlocks: use of PICK_FUNCTION breaks kernel compile when CONFIG_GENERIC_TIME is NOT set
Hello Daniel and Ingo,
I use 2.6.23-rt1 and the patch of Daniel below which seems to be in
there breaks the compilation of the RT-kernel when CONFIG_GENERIC_TIME
is NOT set.
It breaks in the do_gettimeofday(struct timeval *tv) code in the
architecture specific code
where there is a call to: seq = read_seqbegin_irqsave(xtime_lock, flags);
The PICK_FUNCTION implementation does not return anything, so the
compile breaks here.
I am figuring out how to solve this problem nicely, but I have not
found a nice solution yet. Attached I have put my hack to make it
compile on ARM again, but maybe one of you can do a better proposal to
fix this.
Here is a list of files that will probably all fail to build: (I only
tested ARM)
linux-2.6.23/arch/arm/kernel/time.c:254
linux-2.6.23/arch/xtensa/kernel/time.c:132
linux-2.6.23/arch/sparc/kernel/time.c:453
linux-2.6.23/arch/sparc/kernel/pcic.c:776
linux-2.6.23/arch/blackfin/kernel/time.c:269
linux-2.6.23/arch/m68knommu/kernel/time.c:130
linux-2.6.23/arch/sh64/kernel/time.c:193
linux-2.6.23/arch/alpha/kernel/time.c:408
linux-2.6.23/arch/sh/kernel/time.c:65
linux-2.6.23/arch/m68k/kernel/time.c:104
linux-2.6.23/arch/ppc/kernel/time.c:207
linux-2.6.23/arch/s390/kernel/time.c:197
Kind Regards,
Remy Bohmer
2007/8/28, Daniel Walker [EMAIL PROTECTED]:
Replace the old PICK_OP style macros with PICK_FUNCTION. Although,
seqlocks has some alien code, which I also replaced as can be seen
from the line count below.
Signed-off-by: Daniel Walker [EMAIL PROTECTED]
---
include/linux/pickop.h |4
include/linux/seqlock.h | 235
+++-
2 files changed, 135 insertions(+), 104 deletions(-)
Index: linux-2.6.22/include/linux/pickop.h
===
--- linux-2.6.22.orig/include/linux/pickop.h
+++ linux-2.6.22/include/linux/pickop.h
@@ -1,10 +1,6 @@
#ifndef _LINUX_PICKOP_H
#define _LINUX_PICKOP_H
-#undef TYPE_EQUAL
-#define TYPE_EQUAL(var, type) \
- __builtin_types_compatible_p(typeof(var), type *)
-
#undef PICK_TYPE_EQUAL
#define PICK_TYPE_EQUAL(var, type) \
__builtin_types_compatible_p(typeof(var), type)
Index: linux-2.6.22/include/linux/seqlock.h
===
--- linux-2.6.22.orig/include/linux/seqlock.h
+++ linux-2.6.22/include/linux/seqlock.h
@@ -90,6 +90,12 @@ static inline void __write_seqlock(seqlo
smp_wmb();
}
+static __always_inline unsigned long __write_seqlock_irqsave(seqlock_t *sl)
+{
+ __write_seqlock(sl);
+ return 0;
+}
+
static inline void __write_sequnlock(seqlock_t *sl)
{
smp_wmb();
@@ -97,6 +103,8 @@ static inline void __write_sequnlock(seq
spin_unlock(sl-lock);
}
+#define __write_sequnlock_irqrestore(sl, flags)__write_sequnlock(sl)
+
static inline int __write_tryseqlock(seqlock_t *sl)
{
int ret = spin_trylock(sl-lock);
@@ -149,6 +157,28 @@ static __always_inline void __write_seql
smp_wmb();
}
+static __always_inline unsigned long
+__write_seqlock_irqsave_raw(raw_seqlock_t *sl)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+ __write_seqlock_raw(sl);
+ return flags;
+}
+
+static __always_inline void __write_seqlock_irq_raw(raw_seqlock_t *sl)
+{
+ local_irq_disable();
+ __write_seqlock_raw(sl);
+}
+
+static __always_inline void __write_seqlock_bh_raw(raw_seqlock_t *sl)
+{
+ local_bh_disable();
+ __write_seqlock_raw(sl);
+}
+
static __always_inline void __write_sequnlock_raw(raw_seqlock_t *sl)
{
smp_wmb();
@@ -156,6 +186,27 @@ static __always_inline void __write_sequ
spin_unlock(sl-lock);
}
+static __always_inline void
+__write_sequnlock_irqrestore_raw(raw_seqlock_t *sl, unsigned long flags)
+{
+ __write_sequnlock_raw(sl);
+ local_irq_restore(flags);
+ preempt_check_resched();
+}
+
+static __always_inline void __write_sequnlock_irq_raw(raw_seqlock_t *sl)
+{
+ __write_sequnlock_raw(sl);
+ local_irq_enable();
+ preempt_check_resched();
+}
+
+static __always_inline void __write_sequnlock_bh_raw(raw_seqlock_t *sl)
+{
+ __write_sequnlock_raw(sl);
+ local_bh_enable();
+}
+
static __always_inline int __write_tryseqlock_raw(raw_seqlock_t *sl)
{
int ret = spin_trylock(sl-lock);
@@ -182,60 +233,93 @@ static __always_inline int __read_seqret
extern int __bad_seqlock_type(void);
-#define PICK_SEQOP(op, lock) \
+/*
+ * PICK_SEQ_OP() is a small redirector to allow less typing of the lock
+ * types raw_seqlock_t, seqlock_t, at the front of the PICK_FUNCTION
+ * macro.
+ */
+#define PICK_SEQ_OP(...) \
+ PICK_FUNCTION(raw_seqlock_t *, seqlock_t *, ##__VA_ARGS__)
+#define PICK_SEQ_OP_RET(...) \
+ PICK_FUNCTION_RET(raw_seqlock_t
Re: [BUG on PREEMPT_RT, 2.6.23.1-rt5] in rt-mutex code and signals
Hello Steven, It should print out warnings, do you have CONFIG_DEBUG_RT_MUTEXES set? Nope, not yet... I will do that on Monday also. (On ARM I have as less as debug options enabled per default, because it eats too much CPU-power) * PI should only take place if it is for 100% sure that the 'struct semaphore' is used as a mutex. And this is only the case when it is initialised with init_MUTEX(). Well, we can't determine that with code ;-) Remember, there are still drivers out in the world that use semaphores as mutexes. So the PI on semaphores is really more of a compatibility issue. Very strange, although there is now a real mutex in non-RT as in RT, all semaphores are still converted to some badly-implemented-recursive-but-not-recursive-callable-mutexes, because the assumption is made that semaphores are intentionally always used in the wrong way?!! So, the code that is written nicely must be adapted to prevent adaption of bad code? I would expect a more logical solution like the introduction of raw_ types for the exceptions, just like what is done with spinlocks and so on. So, this is a regression if exactly the same driver is used in both non-preempt-rt patched kernel and preempt-rt patched kernels. Not really. There are things that the preempt-rt kernels require. One, is that things that need to keep semaphores instead of using them as mutexes, they should be converted to compat_semaphores. Perhaps now that we have mutexes, we can remove the PI on semaphores, and out-of-tree drivers will need to make sure they don't use semaphores as mutexes anymore. Up to this semaphore implementation the standard was always that a driver written for a non-RT kernel should compile and work properly without any adaption on a RT-kernel, until there are some specific realtime requirements. When it comes to semaphores this is thus completely not true. And this is where the whole thing confused me completely. Yeah, that code is more of a hack to convert counting semaphores into mutexes. Enough said... But semaphores still need to have owners, and they should not block on themselves. That may be where the bug is. Or there should be a real recursive mutex implementation. But the need for recursive mutexes is usually a sign for bad locking, and should therefor be avoided. OK, I wont be able to work on this this weekend, but I'll try to get to it on Monday. A better example to show the bug you are looking for is simply create a mutex and create a thread that grabs that mutex and goes to sleep. Have your driver read grab that mutex with mutex_lock_interruptible. And if the signal code is broken with this, then you definitely got a point that the inerruptible code is broken. I will work on an example like this on Monday. This will keep the semantics clean and not obfuscate it with the semaphore code. I agree. I'll write up that example on Monday if you don't have the time. I will make time ;-) Note, that the unloading of the module should wake up the thread that grabbed the mutex so it can release it. Unloading of a module is not possible as long as there is still a handle open to it. So, it should be safe without waking up the thread. And returning from the read call and still have a mutex locked is not nice either. Thanks for the explanation. I will keep you informed on Monday. Have a nice weekend ! Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG on PREEMPT_RT, 2.6.23.1-rt5] in rt-mutex code and signals
Hello Daniel, Thanks for looking into it also. Steven already made clear to me that the 'struct semaphore' type on the RT-kernel should not be used as a counting-semaphore, but as some sort of legacy-mutex... (The confusion that this will cause is clear by now...) I still do not understand the problems I had with the interruptible-waits on a real rt-mutex, but I have to figure that out again on Monday. Maybe one confusion let to another... (Note, A completion will not work for me, because they are not designed for reuse across several threads. The read/write runs in user context and as such it can be called by different threads, which would require a init of a completion before waiting on it, but that would be racy, I could miss the awake by the init) So I converted your code to use a compat_semaphore, and no oops happens.. Which makes sense because compat_semaphores are designed to work the way your using them. Actually, IMO, compat_semaphores behave like semaphores should behave, and thus the same as they behave on a non-RT kernel, and at the locations where the semaphores are now misused as mutexes on RT, we should replace them by differently-named-mutex-type-semaphores, or better: real-RT-mutexes.. IMO this wrong usage of semaphores is solved by modifying the code that actually made proper use of the semaphores, and I think that if the naming matches the mainline kernel, we only need to patch the files that really need to be patched during the integration in mainline of the RT-patch. Kind Regards, Remy Bohmer - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG on PREEMPT_RT, 2.6.23.1-rt5] in rt-mutex code and signals
Sure, you want to split the list? split the list with you? Feel free to take any of those :-) dev-sem is nontrivial and probably not possible right now - and some of the others might be problematic too. But there might be fixable ones in the list. This shouldnt become like the BKL conversion - never truly finished. Hey Ingo and Daniel, Leave some of the fun open for me :-) I just looked at the list and I found a few that seem doable. Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG on PREEMPT_RT, 2.6.23.1-rt5] in rt-mutex code and signals
Hello Daniel, Can we coordinate who is doing what somehow, to prevent double work? Remy 2007/11/19, Daniel Walker [EMAIL PROTECTED]: On Mon, 2007-11-19 at 02:24 -0500, Jon Masters wrote: On Sat, 2007-11-17 at 09:55 -0800, Daniel Walker wrote: Sure, you want to split the list? I'm happy to grab a few of these too. Let me know if either of you or Ingo is working on the whole lot and about to dump it on us ;-) I was planning to just try a few, so I think your safe.. Daniel - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH PREEMPT_RT]: On AT91 ARM: GPIO Interrupt handling can/will stall forever
Hello,
I use 2.6.23.1-rt5 on the Atmel AT91 series.
Interrupt threading on Preempt-RT and ARM works fine, except for
(edge-triggered) GPIO interrupts. There is a problem when a new
interrupt arives while the interrupt thread is handling the previous
interrupt. If this occurs the interrupt handling stalls forever.
This is caused by a unbalanced interrupt mask/unmask problem in the kernel.
The attached patch fixes this. More information about this problem is
documented inside the patch itself.
This patch is meant for Preempt-RT only.
Kind Regards,
Remy Bohmer
On ARM there is a problem where the interrupt handler stalls when they are
coming faster than the kernel can handle.
The problem occurs when the routine handle_simple_irq() masks the interrupt
when an IRQ-thread is handling the interrupt at the same time. (IRQ_INPROGRESS
is set). The interrupt thread, however does **never** a
desc-chip-unmask(), so the interrupt becomes disabled forever.
IRQ_DISABLED is usually not set for this interrupt
This is in kernel/irq/chip.c, where the irq is masked when a IRQ-thread is
running:
--
void fastcall
handle_simple_irq(unsigned int irq, struct irq_desc *desc)
{
if (unlikely(!action || (desc-status (IRQ_INPROGRESS |
IRQ_DISABLED {
(!!)- if (desc-chip-mask)
(!!)- desc-chip-mask(irq);
desc-status = ~(IRQ_REPLAY | IRQ_WAITING);
desc-status |= IRQ_PENDING;
goto out_unlock;
}
}
--
Masking the interrupt seems valid, because the interrupt handler thread is
still running, so it can handle the new pending interrupt. But, it has to be
umasked somewhere. The logical place is to do this in kernel/irq/manage.c,
because this situation is also handled for the thread_level_irq() and
thread_fasteoi_irq(), but not for thread_simple_irq(). This patch adds this
for these kind of interrupts also.
Signed-off-by: Remy Bohmer [EMAIL PROTECTED]
---
kernel/irq/manage.c |3 +++
1 file changed, 3 insertions(+)
Index: linux-2.6.23/kernel/irq/manage.c
===
--- linux-2.6.23.orig/kernel/irq/manage.c 2007-11-26 13:46:58.0 +0100
+++ linux-2.6.23/kernel/irq/manage.c 2007-11-26 13:48:30.0 +0100
@@ -646,6 +646,9 @@ static void thread_simple_irq(irq_desc_t
note_interrupt(irq, desc, action_ret);
}
desc-status = ~IRQ_INPROGRESS;
+
+ if (!(desc-status IRQ_DISABLED) desc-chip-unmask)
+ desc-chip-unmask(irq);
}
/*
Re: [PATCH PREEMPT_RT]: On AT91 ARM: GPIO Interrupt handling can/will stall forever
Attached the same patch, but it also cleans the manage.c code a bit,
because the IRQ types 'simple IRQ', 'level-IRQ' and 'FastEOI' were
handled differently while they should be handled the same.
Kind Regards,
Remy
2007/11/26, Remy Bohmer [EMAIL PROTECTED]:
Hello,
I use 2.6.23.1-rt5 on the Atmel AT91 series.
Interrupt threading on Preempt-RT and ARM works fine, except for
(edge-triggered) GPIO interrupts. There is a problem when a new
interrupt arives while the interrupt thread is handling the previous
interrupt. If this occurs the interrupt handling stalls forever.
This is caused by a unbalanced interrupt mask/unmask problem in the kernel.
The attached patch fixes this. More information about this problem is
documented inside the patch itself.
This patch is meant for Preempt-RT only.
Kind Regards,
Remy Bohmer
On ARM there is a problem where the interrupt handler stalls when they are
coming faster than the kernel can handle.
The problem occurs when the routine handle_simple_irq() masks the interrupt
when an IRQ-thread is handling the interrupt at the same time. (IRQ_INPROGRESS
is set). The interrupt thread, however does **never** a
desc-chip-unmask(), so the interrupt becomes disabled forever.
IRQ_DISABLED is usually not set for this interrupt
This is in kernel/irq/chip.c, where the irq is masked when a IRQ-thread is
running:
--
void fastcall
handle_simple_irq(unsigned int irq, struct irq_desc *desc)
{
if (unlikely(!action || (desc-status (IRQ_INPROGRESS |
IRQ_DISABLED {
(!!)- if (desc-chip-mask)
(!!)- desc-chip-mask(irq);
desc-status = ~(IRQ_REPLAY | IRQ_WAITING);
desc-status |= IRQ_PENDING;
goto out_unlock;
}
}
--
Masking the interrupt seems valid, because the interrupt handler thread is
still running, so it can handle the new pending interrupt. But, it has to be
umasked somewhere. The logical place is to do this in kernel/irq/manage.c,
because this situation is also handled for the thread_level_irq() and
thread_fasteoi_irq(), but not for thread_simple_irq(). This patch adds this
for these kind of interrupts also.
Signed-off-by: Remy Bohmer [EMAIL PROTECTED]
---
kernel/irq/manage.c | 29 +++--
1 file changed, 3 insertions(+), 26 deletions(-)
Index: linux-2.6.23/kernel/irq/manage.c
===
--- linux-2.6.23.orig/kernel/irq/manage.c 2007-11-26 13:46:58.0 +0100
+++ linux-2.6.23/kernel/irq/manage.c 2007-11-26 14:43:32.0 +0100
@@ -646,28 +646,7 @@ static void thread_simple_irq(irq_desc_t
note_interrupt(irq, desc, action_ret);
}
desc-status = ~IRQ_INPROGRESS;
-}
-
-/*
- * threaded level type irq handler
- */
-static void thread_level_irq(irq_desc_t *desc)
-{
- unsigned int irq = desc - irq_desc;
-
- thread_simple_irq(desc);
- if (!(desc-status IRQ_DISABLED) desc-chip-unmask)
- desc-chip-unmask(irq);
-}
-
-/*
- * threaded fasteoi type irq handler
- */
-static void thread_fasteoi_irq(irq_desc_t *desc)
-{
- unsigned int irq = desc - irq_desc;
- thread_simple_irq(desc);
if (!(desc-status IRQ_DISABLED) desc-chip-unmask)
desc-chip-unmask(irq);
}
@@ -747,12 +726,10 @@ static void do_hardirq(struct irq_desc *
if (!(desc-status IRQ_INPROGRESS))
goto out;
- if (desc-handle_irq == handle_simple_irq)
+ if ((desc-handle_irq == handle_simple_irq) ||
+ (desc-handle_irq == handle_level_irq) ||
+ (desc-handle_irq == handle_fasteoi_irq))
thread_simple_irq(desc);
- else if (desc-handle_irq == handle_level_irq)
- thread_level_irq(desc);
- else if (desc-handle_irq == handle_fasteoi_irq)
- thread_fasteoi_irq(desc);
else if (desc-handle_irq == handle_edge_irq)
thread_edge_irq(desc);
else
Re: [PATCH PREEMPT_RT]: On AT91 ARM: GPIO Interrupt handling can/will stall forever
Hello Kevin, Just copied your mail to the list, maybe your solution is also worth looking at. Remy I had a similar issue when using the chained GPIO interrupts on OMAP under PREEMPT_RT. I believe the chained handler itself is supposed to be doing the ack/unmask instead of the simple_handler. However, currently there is no way for the chained handler to know when the threaded handler has acutally run. So the way I fixed this was to have the simple handler call the chip-end hook so that the chained handler could do the ack/unmask after the threaded handler has actually run. I've been using this against the -rt patch since 2.6.21, and submitted and RFC a while back, but got no comments. This patch is against 2.6.24-rc2-rt1. Signed-off-by: Kevin Hilman [EMAIL PROTECTED] diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index b35d209..2f9e09e 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -661,6 +661,8 @@ static void thread_simple_irq(irq_desc_t *desc) note_interrupt(irq, desc, action_ret); } desc-status = ~IRQ_INPROGRESS; + if (desc-chip-end) + desc-chip-end(irq); } /* - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH PREEMPT_RT]: On AT91 ARM: GPIO Interrupt handling can/will stall forever
Hello Russell,
If 'no' then it's the right handler and the mask/unmask methods associated
with the interrupt will be no-ops.
I completely understand what you keep on saying, but that would imply
that the following piece of code in chip.c is completely bogus anyway!
(snip from mainline 2.6.23)
handle_simple_irq(unsigned int irq, struct irq_desc *desc)
...
if (unlikely(!action || (desc-status IRQ_DISABLED))) {
if (desc-chip-mask)
desc-chip-mask(irq);
...
}
Why trying to mask the interrupt if that is illegal use of the interrupt type?
This piece of code assumes that there CAN be a handler for masking
interrupts for the simple_irq type. This is in contradiction what you
are trying to explain.
If what you say is (completely) true the code is better to be:
if (desc-chip-mask)
BUG();
IMO, interrupts can still be simple_irq types if the interrupt source
does **not have to** be disabled to be able to handle them. These
(GPIO) interrupts signal an event, that is already gone when the
interrupt handler starts, they are **never** really pending, waiting
for some device driver to handle the event. The device does not really
care if the interrupt is really handled, it just generates a new
interrupt on the next event.
And yes, it is possible that these interrupts can be seperately
masked/unmasked, but it is not necessary, and therefor others chose
apparantly for the simple_irq() types.
So, I do not think that the argument of **can** masked/unmasked is
valid, but more the **need** to be masked/unmasked is valid.
I think we should not argue if this is correct use, or misuse.
Fact is now that there is a bug in RT, that this type of 'misuse' does
not work on RT and breaks the interrupt handling. There are even more
patches floating around to fix this.
According to what Daniel also mentions, there are already several
architectures that use this type of interrupt with a masking/unmasking
implementation, apparantly all without the **need** to masking them.
So, the code must prevent this type of 'misuse' (and thus not
supporting it as it is doing now) or fully support it on RT, just like
on mainline.
So, in short: IMO, only RT is broken, not mainline - thus fix RT,
otherwise we will have regression compared to mainline.
Kind Regards,
Remy
2007/11/29, Russell King - ARM Linux [EMAIL PROTECTED]:
On Thu, Nov 29, 2007 at 11:14:30AM +0100, Remy Bohmer wrote:
I do not think Russell is right here with assuming that the wrong
interrupt handler type is used. Looking at the behaviour of the
mainline kernel (non-RT), the implementation is quite different: On
mainline the handle_simple_irq() in chip.c is not re-entrant, the
masking is **only** done in case of errors, and therefor never
unmasked again, of course.
The issue comes down to a very simple question:
Are you using handle_simple_irq() with an interrupt which can be masked?
If the answer to that question is 'yes' then you're using the wrong handler.
If 'no' then it's the right handler and the mask/unmask methods associated
with the interrupt will be no-ops.
Therefore, if you have mask/unmask methods for a simple IRQ which actually
do something, clearly the first answer is the one which applies. You're
using the wrong handler. Use the level or edge handlers instead.
-
To unsubscribe from this list: send the line unsubscribe linux-rt-users in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH PREEMPT_RT]: On AT91 ARM: GPIO Interrupt handling can/will stall forever
Hello Russell, While I realise that, I'm telling you that the _problem_ is being caused by the wrong handler being used. Okay, I believe you... (someone told me once, Russell is right, and if you do not believe him, he is still right ;-) SA1100 and PXA have exactly the same setup. They use the correct handler. Why is AT91 special? This remark is what convinced me :-)) I changed the interrupt handler from the simple_irq to the edge_irq, and it works...!! (I added a noop routine for that .ack part, because there is no ack) I believe I was too focussed on the masking bug in the RT kernel on the simple_irq() that I did not see that for the AT91 series the edge type interrupt handler also works... (even better...) What I thought was 1 single bug in the RT-kernel turned out to be a number of things together that aren't correct, even for mainline. So, to come to a conclusion: The masking bug in RT is still there in the simple_irq path, and masking has to be removed from the simple_irq code. Also for mainline. AT91 can live without simple_irq. I think we are in sync again... I will post a patch for the AT91 later on, after some more testing. Kind Regards, Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH PREEMPT_RT]: On AT91 ARM: GPIO Interrupt handling can/will stall forever
Hello All, I tested some more with the edge_triggered interrupt handler on AT91, and I had already a long time a problem with the AT91SAM9261-EK kit, that the DM9000 Ethernet controller did not work _at all_ on RT. I just tried if the edge triggered interrupt handler works on that board also. And now that board also works properly on RT. (my first patch did not solve this problem either, so apparantly that one still misses interrupts.) This proves again that the simple_irq usage on AT91 is just crap... Russell, Thanks again for the latest (and greatest) hint. This saves me a lot of debug time on that board. Kind Regards, Remy 2007/11/29, Remy Bohmer [EMAIL PROTECTED]: Hello Russell, While I realise that, I'm telling you that the _problem_ is being caused by the wrong handler being used. Okay, I believe you... (someone told me once, Russell is right, and if you do not believe him, he is still right ;-) SA1100 and PXA have exactly the same setup. They use the correct handler. Why is AT91 special? This remark is what convinced me :-)) I changed the interrupt handler from the simple_irq to the edge_irq, and it works...!! (I added a noop routine for that .ack part, because there is no ack) I believe I was too focussed on the masking bug in the RT kernel on the simple_irq() that I did not see that for the AT91 series the edge type interrupt handler also works... (even better...) What I thought was 1 single bug in the RT-kernel turned out to be a number of things together that aren't correct, even for mainline. So, to come to a conclusion: The masking bug in RT is still there in the simple_irq path, and masking has to be removed from the simple_irq code. Also for mainline. AT91 can live without simple_irq. I think we are in sync again... I will post a patch for the AT91 later on, after some more testing. Kind Regards, Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]: Atmel Serial Console interrupt handler splitup
In that case, shall I create a new patchset for this compared to mainline? Then we can merge the DMA patch in the interrupt handler later on. (it is really difficult to do) Oops, Typo, I meant: it is NOT really difficult... Remy 2007/12/7, Remy Bohmer [EMAIL PROTECTED]: Hello Andrew, The serial driver is also used for Atmel's AVR32 architecture. There was some bug in the DMA patch that causes it to fail there - Haavard was going to look into in when he had time. Once that is resolved, the DMA patch will be submitted to mainline. In that case, shall I create a new patchset for this compared to mainline? Then we can merge the DMA patch in the interrupt handler later on. (it is really difficult to do) Kind Regards, Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RT] Revert Softdisable for simple irqs.
Hello Steven, In commit 76d2160147f43f982dfe881404cfde9fd0a9da21 lazy irq disabling was implemented, and the simple irq handler had a masking set to it. Remy Bohmer discovered that some devices in the ARM architecture would trigger the mask, but never unmask it. His patch to do the unmasking was questioned by Russell King about masking simple irqs to begin with. Looking further, it was discovered that the problems Remy was seeing was due to improper use of the simple handler by devices, and he later submitted patches to fix those. But the issue that was uncovered was that the simple handler should never mask. This patch reverts the masking in the simple handler. Also: Acked-by: Remy Bohmer [EMAIL PROTECTED] Thanks for the effort also, I still had it on my todo list, but that is needed anymore... Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RT] Revert Softdisable for simple irqs.
No problem. Could you also ACK the one I sent for mainline. I will test it first tomorrow morning. Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]: Atmel Serial Console interrupt handler splitup
Hello Steven, So, I have here 3 patches: * atmel_serial_cleanup - This patch adapts the driver to the coding rules, splits the interrupt handler into 3 routines (cuts the routine in more readable pieces) , but there is no functional change involved. * atmel_serial_irq_splitup - This patch splits up the interrupt handler. * atmel_serial_irqf_nodelay - This patch is additionally required to get it properly working on Preempt-RT. (This patch should thus go into the RT-patch, AFTER integration of the other 2 patches into mainline) BUT: I based the patch on the 2.6.23.1 + Andrews patch collection at http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz I could pull all the patches into RT (although I would like Thomas to take a look first and give an OK). Instead of pulling the complete patchset, we can also pull the minimal set of patches to make Preempt-RT to work properly on AT91; Preempt-RT depends on: 1) ClockSource ClockEvents for AT91RM9200. [David Brownell, and others] 2) Support configurable HZ. [David Brownell] 3) The Serial port interrupt handler splitup patchset. 4) And of course, the use-edge-irq-instead-of-simple-irq-interrupt-handler patch for AT91, I posted last week. Maybe, Andrew can provide the patches 1 and 2 separate from his patchset, so you can add them to RT for 2.6.23 only? patch 1 seems to be in 2.6.24-rc4 already. And then apply your patches on top. I'm assuming that this only affects the ARM architecture and the AT91 device? The complete patchset from Andrew also effects other Atmel cores, and I only focussed on AT91sam/at91rm. Andrew mentioned that the DMA patch for Serial in his set did not work on AVR32, so, we probably do not want to have that change until it is really stable. Tomorrow, I can easily make my set separate from that DMA part, so that the patches 1-4 can be mainlined without other dependencies, including the irqf_nodelay-patch which can be put in the RT-patchset. I can post this mainline-able patch (3) tomorrow. I'm leaving this out for -rt13 and for the next cut of 2.6.24-rc-rt. But if Thomas is OK with pulling in the external patch queue, I'll do it for -rt14. OK. It is best if the patch queue in question makes it into mainline. Agree. Kind Regards, Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.23.9-rt13
Hello Steven,
If I compile -rt13 I get some compile warnings on ARM (AT91):
1) This one did not exist in rt1:
In file included from kernel/sched.c:911:
kernel/sched_rt.c: In function 'dec_rt_tasks':
kernel/sched_rt.c:88: warning: unused variable 'highest_prio'
2) This one is there already for a much longer time:
CC kernel/rcupreempt.o
kernel/rcupreempt.c:1001: warning: 'per_cpu__rcu_dyntick_snapshot'
defined but not used
Both warnings are fixed by the attached patch, but warning 2 needs some review.
This var is defined twice in this file, 1 in the NO_HZ ifdef, and 1
which seems to be not used.
Kind Regards,
Remy
2007/12/13, Steven Rostedt [EMAIL PROTECTED]:
We are pleased to announce the 2.6.23.9-rt13 tree, which can be
downloaded from the location:
http://www.kernel.org/pub/linux/kernel/projects/rt/
Changes since 2.6.23.9-rt12
- Backported the new RT Balancing code from sched-devel
New changes by Steven Rostedt, Gregory Haskins,
Ingo Molnar, and Dmitry Adamushko
- 2 dimension CPU Prio RT balancing search (Gregory Haskins)
- ARM compile fix (Kevin Hilman)
- Disable HPET legacy replacement for kdump (OGAWA Hirofumi)
- disable HPET on shutdown (OGAWA Hirofumi)
- fix for futex_wait signal stack corruption (Steven Rostedt)
- Handle IRQ_PENDING for simple irq thread (Steven Rostedt)
- latency tracer updates (Daniel Walker)
- Remove warning in local_bh_enable (Kevin Hilman)
- use real time pcp locking for page draining during cpu (Andi Kleen)
- Revert lazy disable irq from simple irq handler (Steven Rostedt)
- AT91 switch to edge from simple irq (Remy Bohmer)
to build a 2.6.23.9-rt13 tree, the following patches should be applied:
http://www.kernel.org/pub/linux/kernel/v2.6/linux-2.6.23.9.tar.bz2
http://www.kernel.org/pub/linux/kernel/projects/rt/patch-2.6.23.9-rt13.bz2
And like always, my RT version of Matt Mackall's ketchup will get this
for you nicely:
http://people.redhat.com/srostedt/rt/tools/ketchup-0.9.8-rt2
The broken out patches are also available.
-- Steve
-
To unsubscribe from this list: send the line unsubscribe linux-rt-users in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Fix a compile warning on -rt13, and UP (ARM AT91rm9200)
Signed-off-by: Remy Bohmer [EMAIL PROTECTED]
---
kernel/rcupreempt.c |2 --
kernel/sched_rt.c |3 ++-
2 files changed, 2 insertions(+), 3 deletions(-)
Index: linux-2.6.23/kernel/sched_rt.c
===
--- linux-2.6.23.orig/kernel/sched_rt.c 2007-12-13 10:59:54.0 +0100
+++ linux-2.6.23/kernel/sched_rt.c 2007-12-13 11:06:41.0 +0100
@@ -85,8 +85,9 @@ static inline void inc_rt_tasks(struct t
static inline void dec_rt_tasks(struct task_struct *p, struct rq *rq)
{
+#ifdef CONFIG_SMP
int highest_prio = rq-rt.highest_prio;
-
+#endif
WARN_ON(!rt_task(p));
WARN_ON(!rq-rt.rt_nr_running);
rq-rt.rt_nr_running--;
Index: linux-2.6.23/kernel/rcupreempt.c
===
--- linux-2.6.23.orig/kernel/rcupreempt.c 2007-12-13 10:59:54.0 +0100
+++ linux-2.6.23/kernel/rcupreempt.c 2007-12-13 11:38:28.0 +0100
@@ -998,8 +998,6 @@ void __init rcu_init_rt(void)
rcu_preempt_boost_init();
}
-static DEFINE_PER_CPU(long, rcu_dyntick_snapshot);
-
/*
* Deprecated, use synchronize_rcu() or synchronize_sched() instead.
*/
Re: [PATCH]: Atmel Serial Console interrupt handler splitup
Hello Steven and Andrew, Attached I have put a patchset that is independant from the patchset at: http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz These patches should go to mainline: * atmel_serial_cleanup_no_at91.patch * atmel_serial_irq_splitup_no_at91.patch And this one into the RT-patch: * atmel_serial_irqf_nodelay.patch I hope this set is easier to integrate. Kind Regards, Remy 2007/12/12, Remy Bohmer [EMAIL PROTECTED]: Hello Steven, So, I have here 3 patches: * atmel_serial_cleanup - This patch adapts the driver to the coding rules, splits the interrupt handler into 3 routines (cuts the routine in more readable pieces) , but there is no functional change involved. * atmel_serial_irq_splitup - This patch splits up the interrupt handler. * atmel_serial_irqf_nodelay - This patch is additionally required to get it properly working on Preempt-RT. (This patch should thus go into the RT-patch, AFTER integration of the other 2 patches into mainline) BUT: I based the patch on the 2.6.23.1 + Andrews patch collection at http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz I could pull all the patches into RT (although I would like Thomas to take a look first and give an OK). Instead of pulling the complete patchset, we can also pull the minimal set of patches to make Preempt-RT to work properly on AT91; Preempt-RT depends on: 1) ClockSource ClockEvents for AT91RM9200. [David Brownell, and others] 2) Support configurable HZ. [David Brownell] 3) The Serial port interrupt handler splitup patchset. 4) And of course, the use-edge-irq-instead-of-simple-irq-interrupt-handler patch for AT91, I posted last week. Maybe, Andrew can provide the patches 1 and 2 separate from his patchset, so you can add them to RT for 2.6.23 only? patch 1 seems to be in 2.6.24-rc4 already. And then apply your patches on top. I'm assuming that this only affects the ARM architecture and the AT91 device? The complete patchset from Andrew also effects other Atmel cores, and I only focussed on AT91sam/at91rm. Andrew mentioned that the DMA patch for Serial in his set did not work on AVR32, so, we probably do not want to have that change until it is really stable. Tomorrow, I can easily make my set separate from that DMA part, so that the patches 1-4 can be mainlined without other dependencies, including the irqf_nodelay-patch which can be put in the RT-patchset. I can post this mainline-able patch (3) tomorrow. I'm leaving this out for -rt13 and for the next cut of 2.6.24-rc-rt. But if Thomas is OK with pulling in the external patch queue, I'll do it for -rt14. OK. It is best if the patch queue in question makes it into mainline. Agree. Kind Regards, Remy This patch cleans up the atmel_serial driver to conform the coding rules. It contains no functional change. Signed-off-by: Remy Bohmer [EMAIL PROTECTED] --- drivers/serial/atmel_serial.c | 348 +++--- 1 file changed, 196 insertions(+), 152 deletions(-) Index: linux-2.6.23/drivers/serial/atmel_serial.c === --- linux-2.6.23.orig/drivers/serial/atmel_serial.c 2007-12-13 17:30:46.0 +0100 +++ linux-2.6.23/drivers/serial/atmel_serial.c 2007-12-13 17:30:48.0 +0100 @@ -74,47 +74,51 @@ #define ATMEL_ISR_PASS_LIMIT 256 -#define UART_PUT_CR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_CR) -#define UART_GET_MR(port) __raw_readl((port)-membase + ATMEL_US_MR) -#define UART_PUT_MR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_MR) -#define UART_PUT_IER(port,v) __raw_writel(v, (port)-membase + ATMEL_US_IER) -#define UART_PUT_IDR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_IDR) -#define UART_GET_IMR(port) __raw_readl((port)-membase + ATMEL_US_IMR) -#define UART_GET_CSR(port) __raw_readl((port)-membase + ATMEL_US_CSR) -#define UART_GET_CHAR(port) __raw_readl((port)-membase + ATMEL_US_RHR) -#define UART_PUT_CHAR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_THR) -#define UART_GET_BRGR(port) __raw_readl((port)-membase + ATMEL_US_BRGR) -#define UART_PUT_BRGR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_BRGR) -#define UART_PUT_RTOR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_RTOR) +#define lread(port) __raw_readl(port) +#define lwrite(v, port) __raw_writel(v, port) -// #define UART_GET_CR(port) __raw_readl((port)-membase + ATMEL_US_CR) // is write-only +#define UART_PUT_CR(port, v) lwrite(v, (port)-membase + ATMEL_US_CR) +#define UART_PUT_MR(port, v) lwrite(v, (port)-membase + ATMEL_US_MR) +#define UART_PUT_IER(port, v) lwrite(v, (port)-membase + ATMEL_US_IER) +#define UART_PUT_IDR(port, v) lwrite(v, (port)-membase + ATMEL_US_IDR) +#define UART_PUT_CHAR(port, v) lwrite(v, (port)-membase + ATMEL_US_THR) +#define UART_PUT_BRGR(port, v) lwrite(v, (port)-membase + ATMEL_US_BRGR) +#define UART_PUT_RTOR(port, v) lwrite(v, (port)-membase + ATMEL_US_RTOR
Re: [PATCH]: Atmel Serial Console interrupt handler splitup
Hello Andrew, * atmel_serial_cleanup_no_at91.patch I don't know what coding rules you're following... :) Funny... I intended to follow these rules: Documentation/CodingStyle I know I should not do it, but I cannot resist pointing you these rules (for the fun, and because you ask me to... :-))) -- Chapter 6: Functions Functions should be short and sweet, and do just one thing. They should fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, as we all know), and do one thing and do that well. -- For now it is not absolutely necessary, but the routine become quite long when the DMA patch comes in. AND: -- Chapter 15: The inline disease A reasonable rule of thumb is to not put inline at functions that have more than 3 lines of code in them. + Often people argue that adding inline to functions that are static and used only once is always a win since there is no space tradeoff. While this is technically correct, ***gcc is capable of inlining these automatically without help***, and the maintenance issue of removing the inline when a second user appears outweighs the potential value of the hint that tells gcc to do something it would have done anyway. -- The code you've moved out of the interrupt handler should probably now be marked as inline. Because I know it is common practice in the kernel, I attached 3 new patches to inline these :-)) To begin with, I made the cleanup patch separate, because otherwise it would make the split-up quite unclear to follow, and scripts/checkpatch.pl generated a huge list of violations on this file. BTW: I am currently generating a re-diff for the DMA patch so that it can be applied on top of this set to make this patch queue easier to handle. Regards, Remy This patch cleans up the atmel_serial driver to conform the coding rules. It contains no functional change. Signed-off-by: Remy Bohmer [EMAIL PROTECTED] --- drivers/serial/atmel_serial.c | 351 +++--- 1 file changed, 199 insertions(+), 152 deletions(-) Index: linux-2.6.23.1-rt5/drivers/serial/atmel_serial.c === --- linux-2.6.23.1-rt5.orig/drivers/serial/atmel_serial.c 2007-10-09 22:31:38.0 +0200 +++ linux-2.6.23.1-rt5/drivers/serial/atmel_serial.c 2007-12-13 20:59:16.0 +0100 @@ -74,47 +74,51 @@ #define ATMEL_ISR_PASS_LIMIT 256 -#define UART_PUT_CR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_CR) -#define UART_GET_MR(port) __raw_readl((port)-membase + ATMEL_US_MR) -#define UART_PUT_MR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_MR) -#define UART_PUT_IER(port,v) __raw_writel(v, (port)-membase + ATMEL_US_IER) -#define UART_PUT_IDR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_IDR) -#define UART_GET_IMR(port) __raw_readl((port)-membase + ATMEL_US_IMR) -#define UART_GET_CSR(port) __raw_readl((port)-membase + ATMEL_US_CSR) -#define UART_GET_CHAR(port) __raw_readl((port)-membase + ATMEL_US_RHR) -#define UART_PUT_CHAR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_THR) -#define UART_GET_BRGR(port) __raw_readl((port)-membase + ATMEL_US_BRGR) -#define UART_PUT_BRGR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_BRGR) -#define UART_PUT_RTOR(port,v) __raw_writel(v, (port)-membase + ATMEL_US_RTOR) +#define lread(port) __raw_readl(port) +#define lwrite(v, port) __raw_writel(v, port) -// #define UART_GET_CR(port) __raw_readl((port)-membase + ATMEL_US_CR) // is write-only +#define UART_PUT_CR(port, v) lwrite(v, (port)-membase + ATMEL_US_CR) +#define UART_PUT_MR(port, v) lwrite(v, (port)-membase + ATMEL_US_MR) +#define UART_PUT_IER(port, v) lwrite(v, (port)-membase + ATMEL_US_IER) +#define UART_PUT_IDR(port, v) lwrite(v, (port)-membase + ATMEL_US_IDR) +#define UART_PUT_CHAR(port, v) lwrite(v, (port)-membase + ATMEL_US_THR) +#define UART_PUT_BRGR(port, v) lwrite(v, (port)-membase + ATMEL_US_BRGR) +#define UART_PUT_RTOR(port, v) lwrite(v, (port)-membase + ATMEL_US_RTOR) + +#define UART_GET_MR(port) lread((port)-membase + ATMEL_US_MR) +#define UART_GET_IMR(port) lread((port)-membase + ATMEL_US_IMR) +#define UART_GET_CSR(port) lread((port)-membase + ATMEL_US_CSR) +#define UART_GET_CHAR(port) lread((port)-membase + ATMEL_US_RHR) +#define UART_GET_BRGR(port) lread((port)-membase + ATMEL_US_BRGR) + +/* is write-only */ +/* #define UART_GET_CR(port)lread((port)-membase + ATMEL_US_CR) */ /* PDC registers */ -#define UART_PUT_PTCR(port,v) __raw_writel(v, (port)-membase + ATMEL_PDC_PTCR) -#define UART_GET_PTSR(port) __raw_readl((port)-membase + ATMEL_PDC_PTSR) +#define UART_PUT_PTCR(port, v) lwrite(v, (port)-membase + ATMEL_PDC_PTCR) +#define UART_PUT_RPR(port, v) lwrite(v, (port)-membase + ATMEL_PDC_RPR) +#define UART_PUT_RCR(port, v) lwrite(v, (port)-membase + ATMEL_PDC_RCR) +#define UART_PUT_RNPR(port, v) lwrite(v, (port)-membase
Re: [PATCH]: Atmel Serial Console interrupt handler splitup
Because I know it is common practice in the kernel, I attached 3 new
patches to inline these :-))
Grmbl, 1 wrong file attached. Here is the correct one.
Regards,
Remy
This patch splits up the interrupt handler of the serial port
into a interrupt top-half and some tasklets.
The goal is to get the interrupt top-half as short as possible to
minimize latencies on interrupts. But the old code also does some
calls in the interrupt handler that are not allowed on preempt-RT
in IRQF_NODELAY context. This handler is executed in this context
because of the interrupt sharing with the timer interrupt.
The timer interrupt on Preempt-RT runs in IRQF_NODELAY context.
2 tasklets are used:
* one for handling the error statuses
* one for pushing the incoming characters into the tty-layer.
The Transmit path was IRQF_NODELAY safe by itself, and is not adapted.
The read path for DMA(PDC) is also not adapted, because that code
does not run in IRQF_NODELAY context due to irq-sharing. The DBGU
which is shared with the timer-irq does not work with DMA, so
therefor this is no problem.
Reading the complete receive queue is still done in the top-half
because we never want to miss any incoming character.
This patch demands the following patches to be installed first:
* atmel_serial_cleanup.patch
Signed-off-by: Remy Bohmer [EMAIL PROTECTED]
---
drivers/serial/atmel_serial.c | 150 +-
1 file changed, 133 insertions(+), 17 deletions(-)
Index: linux-2.6.23.1-rt5/drivers/serial/atmel_serial.c
===
--- linux-2.6.23.1-rt5.orig/drivers/serial/atmel_serial.c 2007-12-13 21:33:08.0 +0100
+++ linux-2.6.23.1-rt5/drivers/serial/atmel_serial.c 2007-12-13 21:33:10.0 +0100
@@ -111,6 +111,22 @@
static int (*atmel_open_hook) (struct uart_port *);
static void (*atmel_close_hook) (struct uart_port *);
+struct atmel_uart_char {
+ unsigned int status;
+ unsigned int overrun;
+ unsigned int ch;
+ unsigned int flg;
+};
+
+#define ATMEL_SERIAL_RINGSIZE 1024
+
+struct atmel_uart_ring {
+ unsigned int head;
+ unsigned int tail;
+ unsigned int count;
+ struct atmel_uart_char data[ATMEL_SERIAL_RINGSIZE];
+};
+
/*
* We wrap our port structure around the generic uart_port.
*/
@@ -119,6 +135,13 @@ struct atmel_uart_port {
struct clk *clk; /* uart clock */
unsigned short suspended; /* is port suspended? */
int break_active; /* break being received */
+
+ struct tasklet_struct rx_task;
+ struct tasklet_struct status_task;
+ unsigned int irq_pending;
+ unsigned int irq_status;
+
+ struct atmel_uart_ring rx_ring;
};
static struct atmel_uart_port atmel_ports[ATMEL_MAX_UART];
@@ -249,12 +272,41 @@ static void atmel_break_ctl(struct uart_
}
/*
+ * Stores the incoming character in the ring buffer
+ */
+static void
+atmel_buffer_rx_char(struct uart_port *port, unsigned int status,
+ unsigned int overrun, unsigned int ch,
+ unsigned int flg)
+{
+ struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+ struct atmel_uart_ring *ring = atmel_port-rx_ring;
+ struct atmel_uart_char *c;
+
+ spin_lock(port-lock);
+
+ if (ring-count == ATMEL_SERIAL_RINGSIZE)
+ goto out; /* Buffer overflow, ignore char */
+
+ c = ring-data[ring-head];
+ c-status = status;
+ c-overrun = overrun;
+ c-ch = ch;
+ c-flg = flg;
+
+ ring-head++;
+ ring-head %= ATMEL_SERIAL_RINGSIZE;
+ ring-count++;
+out:
+ spin_unlock(port-lock);
+}
+
+/*
* Characters received (called from interrupt handler)
*/
static void atmel_rx_chars(struct uart_port *port)
{
struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
- struct tty_struct *tty = port-info-tty;
unsigned int status, ch, flg;
status = UART_GET_CSR(port);
@@ -315,13 +367,13 @@ static void atmel_rx_chars(struct uart_p
if (uart_handle_sysrq_char(port, ch))
goto ignore_char;
- uart_insert_char(port, status, ATMEL_US_OVRE, ch, flg);
+ atmel_buffer_rx_char(port, status, ATMEL_US_OVRE, ch, flg);
ignore_char:
status = UART_GET_CSR(port);
}
- tty_flip_buffer_push(tty);
+ tasklet_schedule(atmel_port-rx_task);
}
/*
@@ -381,7 +433,7 @@ atmel_handle_receive(struct uart_port *p
}
/*
- * transmit interrupt handler.
+ * transmit interrupt handler. (Transmit is IRQF_NODELAY safe)
*/
static inline void
atmel_handle_transmit(struct uart_port *port, unsigned int pending)
@@ -398,19 +450,16 @@ static inline void
atmel_handle_status(struct uart_port *port, unsigned int pending,
unsigned int status)
{
- /* TODO: All reads to CSR will clear these interrupts! */
- if (pending ATMEL_US_RIIC)
- port-icount.rng++;
- if (pending ATMEL_US_DSRIC)
- port-icount.dsr++;
- if (pending ATMEL_US_DCDIC)
- uart_handle_dcd_change(port, !(status ATMEL_US_DCD));
- if (pending ATMEL_US_CTSIC)
- uart_handle_cts_change(port, !(status ATMEL_US_CTS));
- if (pending
- (ATMEL_US_RIIC
Re: [PATCH]: Atmel Serial Console interrupt handler splitup
Hello Haavard, I'll give it a shot, but first I have some comments on your other patches. Good news someone is working on this bug again. Also good news you already found a bug in there. Btw, it would be nice if patches that affect more or less architecture-independent drivers were posted to linux-kernel (added to Cc.) Not really architecture independant, I believe, because thos are drivers for peripherals _inside_ Atmel Cores ;-) Also, it would be easier to review if you posted just one patch per e-mail. I'm going to cut paste a bit from your attachments. I know, but I have some troubles to get 'quilt mail' to work from behind a proxy server, and attaching to a mail, at least it does not corrupt the contents of the patches. 4) For RT only: atmel_serial_irqf_nodelay.patch can be applied anywhere after 1 and 2 I'll ignore this for now. OK. +#define lread(port) __raw_readl(port) +#define lwrite(v, port) __raw_writel(v, port) Why is this necessary, and what does 'l' stand for? There is a huge list of macros below these definitions. By doing it this way, the macros still fit on 80 characters wide, while without them, I had split up the macros over several lines, which does not make it more readable. That's all. 'l' refers at the last letter of __raw_readl, and writel. Nothing special. - struct uart_portuart; /* uart */ - struct clk *clk; /* uart clock */ - unsigned short suspended; /* is port suspended? */ - int break_active; /* break being received */ + struct uart_portuart; /* uart */ + struct clk *clk; /* uart clock */ + unsigned short suspended; /* is port suspended? */ + int break_active; /* break being received */ Looks like you're adding one or more spaces before each TAB here. Why is that an improvement? I used scripts/Lindent to reformat the file, and then I removed the quirks Lindent put in the file. Apparantly I missed that one. These conflict with David Brownell's atmel_serial build warnings begone patch which was merged into mainline a few weeks ago. Hmm, I seem to have missed that one. Why is it not there in a big-AT91-patch from Andrew? /* + * receive interrupt handler. + */ +static inline void +atmel_handle_receive(struct uart_port *port, unsigned int pending) Please drop inline here. The compiler will do it automatically if it has only one caller, and if it at some point gets several callers, we might not want to inline it after all. Funny, This was the first thing that Andrew started complaining about. He suggested to put an inline there which I had not. I already mentioned that this was against the CodingStyle, but I also mentioned that I did not wanted to start a fight about this :-) So, to prevent a discussion, I added the inline... @@ -422,7 +454,9 @@ static int atmel_startup(struct uart_por /* * Allocate the IRQ */ - retval = request_irq(port-irq, atmel_interrupt, IRQF_SHARED, atmel_serial, port); + retval = + request_irq(port-irq, atmel_interrupt, IRQF_SHARED, + atmel_serial, port); I think request_irq() belongs on the same line as retval =. I blame scripts/Lindent ;-) Please use TABs, not spaces. Might as well remove those comments...they don't seem all that useful. Go ahead... I did not remove any comment, even if they appear useless to me. I am not the maintainer of this driver, and just wanted to improve it, so that it was able of running on Preempt-RT. Before being able to submit the change that really mattered to me, I had to make the driver pass the scripts/checkpatch.pl check, otherwise the patch-that-matters would be completely unreadable. + while (!(UART_GET_CSR(port) ATMEL_US_TXEMPTY)) + barrier(); Should probably use cpu_relax(), but it's probably out of scope for a codingstyle cleanup patch (and I don't think it matters on AVR32 or ARM.) Agree. /* - * First, save IMR and then disable interrupts + * First, save IMR and then disable interrupts */ imr = UART_GET_IMR(port); /* get interrupt mask */ UART_PUT_IDR(port, ATMEL_US_RXRDY | ATMEL_US_TXRDY); @@ -790,30 +828,32 @@ static void atmel_console_write(struct c uart_console_write(port, s, count, atmel_console_putchar); /* - * Finally, wait for transmitter to become empty - * and restore IMR + * Finally, wait for transmitter to become empty + * and restore IMR */ Looks like you're replacing TABs with spaces. Why? -// TODO: CR is a write-only register -// unsigned int cr; +/* TODO: CR is a write-only register +// unsigned int cr; // -// cr = UART_GET_CR(port)
[Question]: Regarding time stability, NTP, clock adjustments
Hi Steven and/or Thomas, I have a question regarding the stability of the time-of-day on PREEMPT_RT and NTP and such. I hope one of you can answer them. In a hard real time user space application, running on a PREEMPT_RT kernel (currently 2.6.20-rt8), we are using the clock_gettime() API from time.h to get the time for the CLOCK_REALTIME clock. We use this to perform all time based operations including timer timeouts and timed waits on semaphores and mutexes. We use CLOCK_REALTIME and not CLOCK_MONOTONIC since semaphores only support timed waits for CLOCK_REALTIME. Other time related APIs have similar constraints. Our requirement is that 'the' time returned by clock_gettime() is stable (e.g does not slow down or accelerate) and never changes disjunctly (e.g never jumps to the future or past). If this cannot be guaranteed, the real time behaviour of our application cannot be guaranteed. We achieve time stability by not enabling NTP on the system. Looking at kernel source code we found that NTP time manipulation is deeply embedded so we disable this. However, things are less clear when we add time zones and daylight savings time to the equation. We have observed that dynamic changes in the time zone (e.g changing /etc/localtime) result is a different time returned by the `date` command, but not by clock_gettime(). This is excellent, but is this a fluke or design? What factors influence the (kernel) time as returned by clock_gettime() for CLOCK_REALTIME in terms of rate of passage of time (NTP), and disjunct (time zone and DST) time changes? Next, what is allowed and disallowed related to time in order to meet the stable clock requirment for a real time application? We want to make this clear and then publish something on the rt kernel wiki since the existing man pages do not mention potential interactions and design decisions related to this. This information is very relevant to running real time applications on a real time kernel. If you have any other comments related to this then these are more than welcome. Thanks in advance for your time, Kind Regards, Remy - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 0/3] add kernel-cmdline support for interrupt threads priorities
Hello Steven, Back in August we all had a discussion about the default priorities of IRQ-threads and softirqs, and the lack of a mechanism to configure them from within the kernel. (see http://www.mail-archive.com/linux-rt-users@vger.kernel.org/msg01022.html) Here is a patchset that implements such a nice feature from the kernel cmdline, similar to what we all agreed on, back then. The set exist of 3 parts: 1) add a generic routine to the kernel commandline-parse-library to parse the format of the map as we agreed on. 2) Add a __setup(irq_pmap=) routine to kernel/irq/manage.c 3) Add a __setup(sirq_pmap=) routine to kernel/softirq.c I hope you can find the time to review this set. Kind Regards, Remy Bohmer -- - To unsubscribe from this list: send the line unsubscribe linux-rt-users in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 3/3] Enable setting of IRQ-thread priorities from kernel cmdline.
The RT-patch originally creates all its softirq-threads at
priority 50. Of course, this is not a usable default for many
realtime systems and therefor these priorities has to be tuneable
for each RT-system. But, currently there is no way within the
kernel to adjust this to the needs of the user. Some scripts
are floating around to do this from userspace, but for several
applications the priorities should already be set properly by
the kernel itself before userland is started.
This patch changes this by adding a kernel cmd-line option that
can handle a map of priorities.
Remarks:
* Priorities are only set at creation time of the softirq.
* Priorities has to be set per-cpu.
* If userland overrules, it is NOT restored by this code.
* if no new kernel cmdline options are given, the kernel works
as before, and all softirqs start at 50.
* No wildcards are supported on the cmdline.
See kernel/Documentation/kernel-parameters.txt for usage info.
Signed-off-by: Remy Bohmer [EMAIL PROTECTED]
---
Documentation/kernel-parameters.txt | 11 +++
kernel/softirq.c| 105 ++--
2 files changed, 99 insertions(+), 17 deletions(-)
Index: linux-2.6.24-rc5-rt1/Documentation/kernel-parameters.txt
===
--- linux-2.6.24-rc5-rt1.orig/Documentation/kernel-parameters.txt
2007-12-18 22:06:11.0 +0100
+++ linux-2.6.24-rc5-rt1/Documentation/kernel-parameters.txt2007-12-18
22:07:19.0 +0100
@@ -806,6 +806,17 @@ and is between 256 and 4096 characters.
If this cmdline argument is ommitted, every thread
runs at prio 50.
+ sirq_pmap= [IRQ-threading] List of priorities each softirq
+ thread must have.
+ Format: sirq_pmap=block/0:90,sched/0:75,50
+ The priorities have to be specified per-cpu.
+ The first field without ':', is the default prio.
+ The names have to match the softirq_names[] table in
+ kernel/softirq.c, (thus without 'softirq-' prefix) to
+ keep the cmd-line short.
+ If this cmdline argument is ommitted, every softirq
+ runs at prio 50.
+
ports= [IP_VS_FTP] IPVS ftp helper module
Default is 21.
Up to 8 (IP_VS_APP_MAX_PORTS) ports
Index: linux-2.6.24-rc5-rt1/kernel/softirq.c
===
--- linux-2.6.24-rc5-rt1.orig/kernel/softirq.c 2007-12-18 22:06:11.0
+0100
+++ linux-2.6.24-rc5-rt1/kernel/softirq.c 2007-12-18 22:08:54.0
+0100
@@ -66,6 +66,10 @@ struct softirqdata {
static DEFINE_PER_CPU(struct softirqdata [MAX_SOFTIRQ], ksoftirqd);
+static char *cmdline;
+static int default_prio = MAX_USER_RT_PRIO/2;
+
+
#ifdef CONFIG_PREEMPT_SOFTIRQS
/*
* Preempting the softirq causes cases that would not be a
@@ -770,10 +774,28 @@ EXPORT_SYMBOL(tasklet_unlock_wait);
#endif
+static const char *softirq_names [] =
+{
+ [HI_SOFTIRQ] = high,
+ [SCHED_SOFTIRQ] = sched,
+ [TIMER_SOFTIRQ] = timer,
+ [NET_TX_SOFTIRQ] = net-tx,
+ [NET_RX_SOFTIRQ] = net-rx,
+ [BLOCK_SOFTIRQ] = block,
+ [TASKLET_SOFTIRQ]= tasklet,
+#ifdef CONFIG_HIGH_RES_TIMERS
+ [HRTIMER_SOFTIRQ]= hrtimer,
+#endif
+ [RCU_SOFTIRQ]= rcu,
+};
+
+static int get_softirq_prio(const char *name);
+
static int ksoftirqd(void * __data)
{
- struct sched_param param = { .sched_priority = MAX_USER_RT_PRIO/2 };
+ struct sched_param param = { 0, };
struct softirqdata *data = __data;
+ char buf[50];
u32 softirq_mask = (1 data-nr);
struct softirq_action *h;
int cpu = data-cpu;
@@ -781,8 +803,12 @@ static int ksoftirqd(void * __data)
#ifdef CONFIG_PREEMPT_SOFTIRQS
init_waitqueue_head(data-wait);
#endif
-
+ /* Lookup the priority of this softirq, and set the prio accordingly */
+ snprintf(buf, sizeof(buf), %s/%lu,
+softirq_names[data-nr], data-cpu);
+ param.sched_priority = get_softirq_prio(buf);
sys_sched_setscheduler(current-pid, SCHED_FIFO, param);
+
current-flags |= PF_SOFTIRQ;
set_current_state(TASK_INTERRUPTIBLE);
@@ -911,21 +937,6 @@ void takeover_tasklets(unsigned int cpu)
}
#endif /* CONFIG_HOTPLUG_CPU */
-static const char *softirq_names [] =
-{
- [HI_SOFTIRQ] = high,
- [SCHED_SOFTIRQ] = sched,
- [TIMER_SOFTIRQ] = timer,
- [NET_TX_SOFTIRQ] = net-tx,
- [NET_RX_SOFTIRQ] = net-rx,
- [BLOCK_SOFTIRQ] = block,
- [TASKLET_SOFTIRQ]= tasklet,
-#ifdef CONFIG_HIGH_RES_TIMERS
- [HRTIMER_SOFTIRQ]= hrtimer,
-#endif
- [RCU_SOFTIRQ]= rcu,
-};
-
static int __cpuinit cpu_callback(struct notifier_block
[patch 3/3] Enable setting of IRQ-thread priorities from kernel cmdline. (repost:CC to LKML)
The RT-patch originally creates all its softirq-threads at
priority 50. Of course, this is not a usable default for many
realtime systems and therefor these priorities has to be tuneable
for each RT-system. But, currently there is no way within the
kernel to adjust this to the needs of the user. Some scripts
are floating around to do this from userspace, but for several
applications the priorities should already be set properly by
the kernel itself before userland is started.
This patch changes this by adding a kernel cmd-line option that
can handle a map of priorities.
Remarks:
* Priorities are only set at creation time of the softirq.
* Priorities has to be set per-cpu.
* If userland overrules, it is NOT restored by this code.
* if no new kernel cmdline options are given, the kernel works
as before, and all softirqs start at 50.
* No wildcards are supported on the cmdline.
See kernel/Documentation/kernel-parameters.txt for usage info.
Signed-off-by: Remy Bohmer [EMAIL PROTECTED]
---
Documentation/kernel-parameters.txt | 11 +++
kernel/softirq.c| 105 ++--
2 files changed, 99 insertions(+), 17 deletions(-)
Index: linux-2.6.24-rc5-rt1/Documentation/kernel-parameters.txt
===
--- linux-2.6.24-rc5-rt1.orig/Documentation/kernel-parameters.txt
2007-12-18 22:06:11.0 +0100
+++ linux-2.6.24-rc5-rt1/Documentation/kernel-parameters.txt2007-12-18
22:07:19.0 +0100
@@ -806,6 +806,17 @@ and is between 256 and 4096 characters.
If this cmdline argument is ommitted, every thread
runs at prio 50.
+ sirq_pmap= [IRQ-threading] List of priorities each softirq
+ thread must have.
+ Format: sirq_pmap=block/0:90,sched/0:75,50
+ The priorities have to be specified per-cpu.
+ The first field without ':', is the default prio.
+ The names have to match the softirq_names[] table in
+ kernel/softirq.c, (thus without 'softirq-' prefix) to
+ keep the cmd-line short.
+ If this cmdline argument is ommitted, every softirq
+ runs at prio 50.
+
ports= [IP_VS_FTP] IPVS ftp helper module
Default is 21.
Up to 8 (IP_VS_APP_MAX_PORTS) ports
Index: linux-2.6.24-rc5-rt1/kernel/softirq.c
===
--- linux-2.6.24-rc5-rt1.orig/kernel/softirq.c 2007-12-18 22:06:11.0
+0100
+++ linux-2.6.24-rc5-rt1/kernel/softirq.c 2007-12-18 22:08:54.0
+0100
@@ -66,6 +66,10 @@ struct softirqdata {
static DEFINE_PER_CPU(struct softirqdata [MAX_SOFTIRQ], ksoftirqd);
+static char *cmdline;
+static int default_prio = MAX_USER_RT_PRIO/2;
+
+
#ifdef CONFIG_PREEMPT_SOFTIRQS
/*
* Preempting the softirq causes cases that would not be a
@@ -770,10 +774,28 @@ EXPORT_SYMBOL(tasklet_unlock_wait);
#endif
+static const char *softirq_names [] =
+{
+ [HI_SOFTIRQ] = high,
+ [SCHED_SOFTIRQ] = sched,
+ [TIMER_SOFTIRQ] = timer,
+ [NET_TX_SOFTIRQ] = net-tx,
+ [NET_RX_SOFTIRQ] = net-rx,
+ [BLOCK_SOFTIRQ] = block,
+ [TASKLET_SOFTIRQ]= tasklet,
+#ifdef CONFIG_HIGH_RES_TIMERS
+ [HRTIMER_SOFTIRQ]= hrtimer,
+#endif
+ [RCU_SOFTIRQ]= rcu,
+};
+
+static int get_softirq_prio(const char *name);
+
static int ksoftirqd(void * __data)
{
- struct sched_param param = { .sched_priority = MAX_USER_RT_PRIO/2 };
+ struct sched_param param = { 0, };
struct softirqdata *data = __data;
+ char buf[50];
u32 softirq_mask = (1 data-nr);
struct softirq_action *h;
int cpu = data-cpu;
@@ -781,8 +803,12 @@ static int ksoftirqd(void * __data)
#ifdef CONFIG_PREEMPT_SOFTIRQS
init_waitqueue_head(data-wait);
#endif
-
+ /* Lookup the priority of this softirq, and set the prio accordingly */
+ snprintf(buf, sizeof(buf), %s/%lu,
+softirq_names[data-nr], data-cpu);
+ param.sched_priority = get_softirq_prio(buf);
sys_sched_setscheduler(current-pid, SCHED_FIFO, param);
+
current-flags |= PF_SOFTIRQ;
set_current_state(TASK_INTERRUPTIBLE);
@@ -911,21 +937,6 @@ void takeover_tasklets(unsigned int cpu)
}
#endif /* CONFIG_HOTPLUG_CPU */
-static const char *softirq_names [] =
-{
- [HI_SOFTIRQ] = high,
- [SCHED_SOFTIRQ] = sched,
- [TIMER_SOFTIRQ] = timer,
- [NET_TX_SOFTIRQ] = net-tx,
- [NET_RX_SOFTIRQ] = net-rx,
- [BLOCK_SOFTIRQ] = block,
- [TASKLET_SOFTIRQ]= tasklet,
-#ifdef CONFIG_HIGH_RES_TIMERS
- [HRTIMER_SOFTIRQ]= hrtimer,
-#endif
- [RCU_SOFTIRQ]= rcu,
-};
-
static int __cpuinit cpu_callback(struct notifier_block
Re: [patch 1/3] Add generic routine for parsing map-like options on kernel cmd-line (repost:CC to LKML)
Hello Randy,
Sorry for the language errors, English is not my Native language, so I
make these stupid errors...
+ * get_map_option - Parse integer from an option map
The @param lines (below) need to go here, immediately following the
function short description (the line above). No intervening blank
lines.
OK, I will adapt that.
+int get_map_option(const char *str, const char *key, int *pint)
+{
+ char buf[COMMAND_LINE_SIZE];
COMMAND_LINE_SIZE varies from 256 to 2048, depending on $ARCH.
That's a bit too much to declare on a function's local stack --
unless you are very certain of the call tree to this point and
that the total stack size is safe. Can you just kmalloc() this
buf?
I know it is big on a 4k stack, and I also think it is not very nice...
But kmalloc() panics the kernel if I do it as soon as in the __setup() code.
The problem is that the original string may not be modified by the
cmdline-parser, and I do not know the length of the command up front,
(except that it cannot be longer than this define). Allocating a
global buffer is not safe and needs locking. So actually only what
left for me was the stack... or rewrite the used C-library-routines
completely myself, for this purpose only, which is also not nice.
I hope someone could point to me to another possibilty, that I did not
think of yet.
Kind Regards,
Remy
+ char *p, *substr;
+ int found = 0;
+
+ /* We must copy the string to the stack, because strsep()
+changes it.*/
+ strncpy(buf, str, COMMAND_LINE_SIZE);
+ buf[COMMAND_LINE_SIZE-1] = '\0';
+
+ p = buf;
+ substr = strsep(p, ,);
+ while ((!found) (substr != NULL)) {
+ if (strlen(substr) != 0) {
+ if (key == NULL) {
+ /* Check for the absence of any ':' */
+ if (strchr(substr, ':') == NULL) {
+ sscanf(substr, %d, pint);
+ found = 1;
+ }
+ } else {
+ /* check if the first part of the key matches
*/
+ if (!strncmp(substr, key, strlen(key))) {
+ substr += strlen(key);
+ /* Now the next char must be a ':',
+if not, search for the next match
*/
+ if (*substr == ':') {
+ substr++;
+ sscanf(substr, %d, pint);
+ found = 1;
+ }
+ }
+ }
+ }
+ substr = strsep(p, ,);
+ }
+ return found;
+}
---
~Randy
-
To unsubscribe from this list: send the line unsubscribe linux-rt-users in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
