All of lore.kernel.org
 help / color / mirror / Atom feed
* Setting the priority of an IRQ thread
@ 2009-06-15 22:44 Martin Shepherd
  2009-06-16  3:54 ` Suresh Kumar SHUKLA
  0 siblings, 1 reply; 27+ messages in thread
From: Martin Shepherd @ 2009-06-15 22:44 UTC (permalink / raw)
  To: linux-rt-users

Is there a way for a device driver to set the priority of the IRQ thread that
services interrupts from its hardware? I am writing a device driver for a
digital I/O board with general purpose interrupt capability, and I would like
the application to be able to tell the driver what priority it needs those
interrupts to be given.

Currently it appears that preemp_rt spawns all IRQ threads with priority 50, and
that the only way to change the priority of an IRQ is for an application program
to execute the chrt program, after the IRQ thread has been created. However this
requires that the driver pass back information to the calling program about
which IRQ it is using, so that the program can then call popen to first search
for the PID of the corresponding thread, and then execute chrt to change its
priority. This is possible but messy, and relies on the name of IRQ threads not
changing in the future.

So is there a way for driver code in the kernel to identify the thread of a
given IRQ, and then change its priority?

Thanks,

Martin



^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: Setting the priority of an IRQ thread
  2009-06-15 22:44 Setting the priority of an IRQ thread Martin Shepherd
@ 2009-06-16  3:54 ` Suresh Kumar SHUKLA
  2009-06-16  7:59   ` Martin Shepherd
  0 siblings, 1 reply; 27+ messages in thread
From: Suresh Kumar SHUKLA @ 2009-06-16  3:54 UTC (permalink / raw)
  To: 'Martin Shepherd', linux-rt-users

Hi Martin,

> Is there a way for a device driver to set the priority of the 
> IRQ thread that services interrupts from its hardware? I am 
> writing a device driver for a digital I/O board with general 
> purpose interrupt capability, and I would like the 
> application to be able to tell the driver what priority it 
> needs those interrupts to be given.
> 
> Currently it appears that preemp_rt spawns all IRQ threads 
> with priority 50, and that the only way to change the 
> priority of an IRQ is for an application program to execute 
> the chrt program, after the IRQ thread has been created. 
> However this requires that the driver pass back information 
> to the calling program about which IRQ it is using, so that 
> the program can then call popen to first search for the PID 
> of the corresponding thread, and then execute chrt to change 
> its priority. This is possible but messy, and relies on the 
> name of IRQ threads not changing in the future.
> 
> So is there a way for driver code in the kernel to identify 
> the thread of a given IRQ, and then change its priority?


I tried this exercise in a driver for 2.6.26. I succeeded in identifying the
thread for IRQ and its priority. I couldn't do the priority changes.

 #if 1
	struct sched_param param = { 0, };
	struct irq_desc *desc;

	// 2. Raise the priority of ISR thread
	param.sched_priority = 90;

	// get IRQ descriptor from IRQ, it contains pid
	desc = &irq_desc[IRQ_BASIC_TIMER_3_1];
	printk("Thread Priority = %u\n", desc->thread->prio);	
#endif

Along with this, some minor changes ('static' to 'extern' etc) for irq_desc
structure in corresponding files.

For changing the priority, you may try patches sent by Remy Bohmer
[linux@bohmer.net] on 28-May.

-Suresh


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-16  3:54 ` Suresh Kumar SHUKLA
@ 2009-06-16  7:59   ` Martin Shepherd
  2009-06-16 18:27     ` Uwe Kleine-König
  2009-06-17  8:27     ` Thomas Pfaff
  0 siblings, 2 replies; 27+ messages in thread
From: Martin Shepherd @ 2009-06-16  7:59 UTC (permalink / raw)
  To: linux-rt-users

Suresh Kumar SHUKLA <suresh.shukla <at> st.com> writes:
> 	// get IRQ descriptor from IRQ, it contains pid
> 	desc = &irq_desc[IRQ_BASIC_TIMER_3_1];

That's a very useful clue. I was thinking that I would have to walk the process
tree to find the thread by name. Following up on this clue, it appears as though
in kernel 2.6.29.4 I should be able to use desc=irq_to_desc(irq) to look up the
IRQ descriptor, then use pid=get_task_pid(desc->thread), to get the PID of the
IRQ thread, then use sys_sched_setscheduler(pid,...) to change its priority.
I'll try that out in the morning.

Thanks for the reply,

Martin



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-16  7:59   ` Martin Shepherd
@ 2009-06-16 18:27     ` Uwe Kleine-König
  2009-06-16 21:25       ` Martin Shepherd
                         ` (2 more replies)
  2009-06-17  8:27     ` Thomas Pfaff
  1 sibling, 3 replies; 27+ messages in thread
From: Uwe Kleine-König @ 2009-06-16 18:27 UTC (permalink / raw)
  To: Martin Shepherd; +Cc: linux-rt-users

On Tue, Jun 16, 2009 at 07:59:28AM +0000, Martin Shepherd wrote:
> Suresh Kumar SHUKLA <suresh.shukla <at> st.com> writes:
> > 	// get IRQ descriptor from IRQ, it contains pid
> > 	desc = &irq_desc[IRQ_BASIC_TIMER_3_1];
> 
> That's a very useful clue. I was thinking that I would have to walk the process
> tree to find the thread by name. Following up on this clue, it appears as though
> in kernel 2.6.29.4 I should be able to use desc=irq_to_desc(irq) to look up the
> IRQ descriptor, then use pid=get_task_pid(desc->thread), to get the PID of the
> IRQ thread, then use sys_sched_setscheduler(pid,...) to change its priority.
> I'll try that out in the morning.
I wouldn't recommend calling sys_sched_setscheduler from kernel space.
That's the userspace API and you need to pass a __user pointer as third
argument.

The right way is to do this change from userspace.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-16 18:27     ` Uwe Kleine-König
@ 2009-06-16 21:25       ` Martin Shepherd
  2009-06-16 21:54       ` Martin Shepherd
  2009-06-16 22:28       ` Thomas Gleixner
  2 siblings, 0 replies; 27+ messages in thread
From: Martin Shepherd @ 2009-06-16 21:25 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: linux-rt-users

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1885 bytes --]


On Tue, 16 Jun 2009, Uwe Kleine-König wrote:
>> IRQ thread, then use sys_sched_setscheduler(pid,...) to change its priority.
>> I'll try that out in the morning.
> I wouldn't recommend calling sys_sched_setscheduler from kernel space.
> That's the userspace API and you need to pass a __user pointer as third
> argument.

Good point, although my ioctl() will be passing the scheduling
parameters from user-space. So the need for a __user pointer for this
would not be a problem. Alternatively one could call task_setprio(),
instead of sys_sched_setscheduler(), and omit the then reduntant step
of figuring out the PID of the task.

> The right way is to do this change from userspace.

Which is what I am trying to do.  I want the application thread that
uses my driver to be able to set the priority of its interrupt to
equal the priority of the calling thread.  The problem with calling
sched_setscheduler directly from user-space, is figuring out the PID
of the IRQ thread. First of all, I have multiple boards of the same
type, such that looking in /proc/interrupts won't tell me which IRQ is
being used by a given board. So I would have to implement an ioctl()
anyway, just to query the IRQ from the device driver. Secondly, the
PID of the corresponding IRQ thread changes each time that
request_irq() is called, and then later freed. So searching for the
IRQ thread couldn't be done by a wrapper script around the program.

It seems much more friendly and efficient for my driver to provide
applications with an ioctl that tells it to set the priority of its
IRQ thread.

Note that none of this would be necessary if the IRQ thread of a
device automatically inherited the priority of the highest priority
realtime thread that has requested (and not yet free'd) its IRQ. Not
doing so, leads to a form of priority inversion.

Thanks,

Martin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-16 18:27     ` Uwe Kleine-König
  2009-06-16 21:25       ` Martin Shepherd
@ 2009-06-16 21:54       ` Martin Shepherd
  2009-06-17  5:21         ` Thomas Gleixner
  2009-06-16 22:28       ` Thomas Gleixner
  2 siblings, 1 reply; 27+ messages in thread
From: Martin Shepherd @ 2009-06-16 21:54 UTC (permalink / raw)
  To: linux-rt-users

Uwe Kleine-König <u.kleine-koenig <at> pengutronix.de> writes:
> > IRQ thread, then use sys_sched_setscheduler(pid,...) to change its priority.
> > I'll try that out in the morning.
> I wouldn't recommend calling sys_sched_setscheduler from kernel space.
> That's the userspace API and you need to pass a __user pointer as third
> argument.

Good point, although my ioctl() will be passing the scheduling
parameters from user-space. So the need for a __user pointer for this
would not be a problem. Alternatively one could call task_setprio(),
instead of sys_sched_setscheduler(), and omit the then reduntant step
of figuring out the PID of the task.
 
> The right way is to do this change from userspace.

Which is what I am trying to do.  I want the application thread that
uses my driver to be able to set the priority of its interrupt to
equal the priority of the calling thread.  The problem with calling
sched_setscheduler directly from user-space, is figuring out the PID
of the IRQ thread. First of all, I have multiple boards of the same
type, such that looking in /proc/interrupts won't tell me which IRQ is
being used by a given board. So I would have to implement an ioctl()
anyway, just to query the IRQ from the device driver. Secondly, the
PID of the corresponding IRQ thread changes each time that
request_irq() is called, and then later freed. So searching for the
IRQ thread couldn't be done by a wrapper script around the program.

It seems much more friendly and efficient for my driver to provide
applications with an ioctl that tells it to set the priority of its
IRQ thread.

Note that none of this would be necessary if the IRQ thread of a
device automatically inherited the priority of the highest priority
realtime thread that has requested (and not yet free'd) its IRQ. Not
doing so, leads to a form of priority inversion.

Thanks,

Martin




--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-16 18:27     ` Uwe Kleine-König
  2009-06-16 21:25       ` Martin Shepherd
  2009-06-16 21:54       ` Martin Shepherd
@ 2009-06-16 22:28       ` Thomas Gleixner
  2009-06-17  1:12         ` GeunSik Lim
                           ` (2 more replies)
  2 siblings, 3 replies; 27+ messages in thread
From: Thomas Gleixner @ 2009-06-16 22:28 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: Martin Shepherd, linux-rt-users

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1180 bytes --]

On Tue, 16 Jun 2009, Uwe Kleine-König wrote:
> On Tue, Jun 16, 2009 at 07:59:28AM +0000, Martin Shepherd wrote:
> > Suresh Kumar SHUKLA <suresh.shukla <at> st.com> writes:
> > > 	// get IRQ descriptor from IRQ, it contains pid
> > > 	desc = &irq_desc[IRQ_BASIC_TIMER_3_1];
> > 
> > That's a very useful clue. I was thinking that I would have to walk the process
> > tree to find the thread by name. Following up on this clue, it appears as though
> > in kernel 2.6.29.4 I should be able to use desc=irq_to_desc(irq) to look up the
> > IRQ descriptor, then use pid=get_task_pid(desc->thread), to get the PID of the
> > IRQ thread, then use sys_sched_setscheduler(pid,...) to change its priority.
> > I'll try that out in the morning.
> I wouldn't recommend calling sys_sched_setscheduler from kernel space.
> That's the userspace API and you need to pass a __user pointer as third
> argument.
> 
> The right way is to do this change from userspace.

What a nonsense. Care to look at the code ?

The irq/softirq threads call sys_sched_setscheduler() already to
change their priorities.

The right way to do this is to have an interface
set_irq_thread_prio(irq, prio)

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-16 22:28       ` Thomas Gleixner
@ 2009-06-17  1:12         ` GeunSik Lim
  2009-06-17  2:27           ` Martin Shepherd
  2009-06-17  3:27         ` Martin Shepherd
  2009-06-17  9:17         ` Thomas Gleixner
  2 siblings, 1 reply; 27+ messages in thread
From: GeunSik Lim @ 2009-06-17  1:12 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Uwe Kleine-König, Martin Shepherd, linux-rt-users

2009/6/17 Thomas Gleixner <tglx@linutronix.de>:
> On Tue, 16 Jun 2009, Uwe Kleine-König wrote:
>> On Tue, Jun 16, 2009 at 07:59:28AM +0000, Martin Shepherd wrote:
>> > Suresh Kumar SHUKLA <suresh.shukla <at> st.com> writes:
>> > >   // get IRQ descriptor from IRQ, it contains pid
>> > >   desc = &irq_desc[IRQ_BASIC_TIMER_3_1];
>> >
>> > That's a very useful clue. I was thinking that I would have to walk the process
>> > tree to find the thread by name. Following up on this clue, it appears as though
>> > in kernel 2.6.29.4 I should be able to use desc=irq_to_desc(irq) to look up the
>> > IRQ descriptor, then use pid=get_task_pid(desc->thread), to get the PID of the
>> > IRQ thread, then use sys_sched_setscheduler(pid,...) to change its priority.
>> > I'll try that out in the morning.
>> I wouldn't recommend calling sys_sched_setscheduler from kernel space.
>> That's the userspace API and you need to pass a __user pointer as third
>> argument.
>>
>> The right way is to do this change from userspace.
>
> What a nonsense. Care to look at the code ?
>
> The irq/softirq threads call sys_sched_setscheduler() already to
> change their priorities.
Yes, I agree with your explanation.
> The right way to do this is to have an interface
> set_irq_thread_prio(irq, prio)
You means that we have to utilize wrapper function using
sys_sched_setscheduler()
function for PID setting of IRQ thread. It's right?

Thks,
GeunSik Lim

> Thanks,
>
>        tglx



-- 
Regards,
GeunSik Lim ( Samsung Electronics )
Blog : http://blog.naver.com/invain/
e-Mail: geunsik.lim@samsung.com
           leemgs@gmail.com , leemgs1@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17  1:12         ` GeunSik Lim
@ 2009-06-17  2:27           ` Martin Shepherd
  2009-06-17  5:28             ` Thomas Gleixner
  0 siblings, 1 reply; 27+ messages in thread
From: Martin Shepherd @ 2009-06-17  2:27 UTC (permalink / raw)
  To: GeunSik Lim; +Cc: Thomas Gleixner, Uwe Kleine-König, linux-rt-users


On Wed, 17 Jun 2009, GeunSik Lim wrote:
>...
> You means that we have to utilize wrapper function using
> sys_sched_setscheduler() function for PID setting of IRQ thread. It's right?

I may be wrong, but from looking through the kernel code, it appears
as though sys_sched_setscheduler() is used in places where the target
thread could spontaneously dissapear, whereas its non-syscall variant,
sched_setscheduler(), is only used when it is clear that the
underlying thread can't dissapear, such as during the creation of a
new thread. I think that this must be because sys_sched_setscheduler()
takes a pid argument, which remains valid even if the underlying
thread dissapears, whereas sched_setscheduler() takes a raw
task_struct argument, which doesn't provide such guarantees.

I don't personally understand why the copy_from_user() call in the
do_sched_setscheduler() part of sys_sched_setscheduler() doesn't get
upset by being handed an argument from the kernel stack, instead of a
user pointer, but as Thomas says, sys_sched_setscheduler() is called
from a few places in the kernel, and apparently it works.

Note that my attempt to do this from my device-driver didn't work out,
because it turns out that irq_to_desc() isn't exported. So my driver
can't use this to find the IRQ thread. I'm currently having a quick go
at hacking the kernel to provide the set_irq_thread_prio(irq, prio)
function that Thomas suggested, but I haven't fiddled with the kernel
before. So it remains to be seen whether this works :-).

Martin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-16 22:28       ` Thomas Gleixner
  2009-06-17  1:12         ` GeunSik Lim
@ 2009-06-17  3:27         ` Martin Shepherd
  2009-06-17  5:38           ` Thomas Gleixner
  2009-06-17  9:17         ` Thomas Gleixner
  2 siblings, 1 reply; 27+ messages in thread
From: Martin Shepherd @ 2009-06-17  3:27 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Uwe Kleine-König, linux-rt-users


On Wed, 17 Jun 2009, Thomas Gleixner wrote:
> The irq/softirq threads call sys_sched_setscheduler() already to
> change their priorities.
>
> The right way to do this is to have an interface
> set_irq_thread_prio(irq, prio)

I just wrote such a function, and it appears to work. I couldn't get
it to work with sys_sched_setscheduler(), since this returned -EFAULT,
indicating that it didn't like the provenance of the sched_param
argument. I thus instead used the non-syscall variant,
sched_setscheduler(), protecting the target task structure from
deletion by bracketing the code with calls to
read_lock(&tasklist_lock), and read_unlock(&tasklist_lock). Hopefully
that is the right lock to take. The implemenation, which I added to
kernel/irq/manage.c, is as follows:

#ifdef CONFIG_PREEMPT_HARDIRQS

int set_irq_thread_prio(unsigned int irq, int prio)
{
         struct irq_desc *desc = irq_to_desc(irq);
         struct sched_param param = { 0, };
         int status;

         param.sched_priority = prio;

         if(desc) {
                 read_lock(&tasklist_lock);
                 if(desc->thread)
                         status = sched_setscheduler(desc->thread, SCHED_FIFO,
                                                     &param);
                 else
                         status = -ENODEV;
                 read_unlock(&tasklist_lock);
         } else {
                 status = -ENODEV;
         }
         return status;
}

EXPORT_SYMBOL(set_irq_thread_prio);

#endif

I also put the corresponding function prototype in
include/linux/irq.h. I called this from my device-driver, and ran my
test program with it a few times, and it worked fine.

Should I submit this as a patch as-is, or is there anything
problematic that I should fix first?

Thanks,

Martin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-16 21:54       ` Martin Shepherd
@ 2009-06-17  5:21         ` Thomas Gleixner
  2009-06-17  6:09           ` Martin Shepherd
  0 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2009-06-17  5:21 UTC (permalink / raw)
  To: Martin Shepherd; +Cc: linux-rt-users

On Tue, 16 Jun 2009, Martin Shepherd wrote:
> Which is what I am trying to do.  I want the application thread that
> uses my driver to be able to set the priority of its interrupt to
> equal the priority of the calling thread.  The problem with calling
> sched_setscheduler directly from user-space, is figuring out the PID
> of the IRQ thread. First of all, I have multiple boards of the same
> type, such that looking in /proc/interrupts won't tell me which IRQ is
> being used by a given board. So I would have to implement an ioctl()
> anyway, just to query the IRQ from the device driver. Secondly, the
> PID of the corresponding IRQ thread changes each time that
> request_irq() is called, and then later freed. So searching for the
> IRQ thread couldn't be done by a wrapper script around the program.
>
> It seems much more friendly and efficient for my driver to provide
> applications with an ioctl that tells it to set the priority of its
> IRQ thread.

Wrong. That information is already available in sysfs and there is no
reason to use an ioctl for that. ioctls are neither friendly nor
efficient.

The only missing information is the thread id, which needs to
exposed. This is completely independent of a single driver and needs
to be implemented in a generic way.

> Note that none of this would be necessary if the IRQ thread of a
> device automatically inherited the priority of the highest priority
> realtime thread that has requested (and not yet free'd) its IRQ. Not
> doing so, leads to a form of priority inversion.

Eek. That's even more wrong. Why should the priority of a user task
influence the priority of a device driver irq thread ? That's a
question of policy and policies are set from user space. The kernel
merily provides the interfaces.

Also your claim that it leads to a form of priority inversion is not
necessarily true. I can see what you mean, but there are cases where
the high prio thread just wants an async notification from a driver
that something has changed. It maybe or maybe not required that this
information comes in with high prio. Again that's a question of policy
and needs to be decided by the system designer.

Also why do you claim that the user task requests/frees the drivers
irq ? That depends on the driver, i.e. if it is implemented to request
the irq on open(), and can not be generalized.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17  2:27           ` Martin Shepherd
@ 2009-06-17  5:28             ` Thomas Gleixner
  0 siblings, 0 replies; 27+ messages in thread
From: Thomas Gleixner @ 2009-06-17  5:28 UTC (permalink / raw)
  To: Martin Shepherd; +Cc: GeunSik Lim, Uwe Kleine-König, linux-rt-users

On Tue, 16 Jun 2009, Martin Shepherd wrote:
> Note that my attempt to do this from my device-driver didn't work out,
> because it turns out that irq_to_desc() isn't exported. So my driver

For a good reaseon. The interrupt description structures are not to be
fiddled with by drivers.

Thanks,

	tglx


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17  3:27         ` Martin Shepherd
@ 2009-06-17  5:38           ` Thomas Gleixner
  0 siblings, 0 replies; 27+ messages in thread
From: Thomas Gleixner @ 2009-06-17  5:38 UTC (permalink / raw)
  To: Martin Shepherd; +Cc: Uwe Kleine-König, linux-rt-users

On Tue, 16 Jun 2009, Martin Shepherd wrote:
> #ifdef CONFIG_PREEMPT_HARDIRQS
> 
> int set_irq_thread_prio(unsigned int irq, int prio)
> {
>         struct irq_desc *desc = irq_to_desc(irq);
>         struct sched_param param = { 0, };
>         int status;
> 
>         param.sched_priority = prio;
> 
>         if(desc) {
>                 read_lock(&tasklist_lock);
>                 if(desc->thread)

That's racy. desc->thread can go away between the check and the
call. That needs locking of desc->lock, but that might have other
implications. Need to have a closer look at the semantics.

>                         status = sched_setscheduler(desc->thread, SCHED_FIFO,
>                                                     &param);
>                 else
>                         status = -ENODEV;
>                 read_unlock(&tasklist_lock);
>         } else {
>                 status = -ENODEV;
>         }
>         return status;
> }
> 
> EXPORT_SYMBOL(set_irq_thread_prio);

EXPORT_SYMBOL_GPL if any.

OTOH, I think that as I said in the other reply already that none of
this magic is really necessary. We need some sysfs based information
of the thread id in addition to the irq nr which is already
there. Then you can just use sched_setscheduler(2) or chrt(1) from user
space. You can do this already, the irq information in sysfs is
already there you just have to decode output of ps(1) to find the IRQ
thread.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17  5:21         ` Thomas Gleixner
@ 2009-06-17  6:09           ` Martin Shepherd
  2009-06-17  9:00             ` Thomas Gleixner
  0 siblings, 1 reply; 27+ messages in thread
From: Martin Shepherd @ 2009-06-17  6:09 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-rt-users


On Wed, 17 Jun 2009, Thomas Gleixner wrote:
>> It seems much more friendly and efficient for my driver to provide
>> applications with an ioctl that tells it to set the priority of its
>> IRQ thread.
>
> Wrong. That information is already available in sysfs and there is no
> reason to use an ioctl for that.

Could you point out where please? Searching sysfs I see the following
two files that contain the IRQ numbers of two of the boards that my
driver handles:

  /bus/pci/drivers/accpcidio/0000:03:02.0/irq
  /bus/pci/drivers/accpcidio/0000:03:03.0/irq

However, I don't see how a user application could figure out which of
these corresponded to the board that it opened using a particular
device file. Only the device driver knows which device file is
associated with a given PCI device ID, and that isn't recorded in
sysfs.

>> Note that none of this would be necessary if the IRQ thread of a
>> device automatically inherited the priority of the highest priority
>> realtime thread that has requested (and not yet free'd) its IRQ. Not
>> doing so, leads to a form of priority inversion.
>
> Eek. That's even more wrong. Why should the priority of a user task
> influence the priority of a device driver irq thread ? That's a
> question of policy and policies are set from user space. The kernel
> merily provides the interfaces.

Agreed. I didn't think this through properly.

> Also why do you claim that the user task requests/frees the drivers
> irq ? That depends on the driver, i.e. if it is implemented to request
> the irq on open(), and can not be generalized.

I was just saying that it is common for device drivers not to request
IRQs until a user application calls their open methods, and that in
such cases only the application and the driver know when is the right
time to search for the newly created IRQ thread. An external script
whose job it was to start the realtime program, and set the priorities
of associated IRQs, would have a hard time doing this reliably.

Martin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-16  7:59   ` Martin Shepherd
  2009-06-16 18:27     ` Uwe Kleine-König
@ 2009-06-17  8:27     ` Thomas Pfaff
  1 sibling, 0 replies; 27+ messages in thread
From: Thomas Pfaff @ 2009-06-17  8:27 UTC (permalink / raw)
  To: Martin Shepherd; +Cc: linux-rt-users



On Tue, 16 Jun 2009, Martin Shepherd wrote:

> Suresh Kumar SHUKLA <suresh.shukla <at> st.com> writes:
> > 	// get IRQ descriptor from IRQ, it contains pid
> > 	desc = &irq_desc[IRQ_BASIC_TIMER_3_1];
> 
> That's a very useful clue. I was thinking that I would have to walk the process
> tree to find the thread by name. Following up on this clue, it appears as though
> in kernel 2.6.29.4 I should be able to use desc=irq_to_desc(irq) to look up the
> IRQ descriptor, then use pid=get_task_pid(desc->thread), to get the PID of the
> IRQ thread, then use sys_sched_setscheduler(pid,...) to change its priority.
> I'll try that out in the morning.
> 

FYI, i use following code to set the interrupt thread prio on first interrupt 
and it works so far:

atomic_t first_irq;

static irqreturn_t irq_handler (int irq, void *devp)
{
#ifdef CONFIG_PREEMPT_HARDIRQS
	if (atomic_cmpxchg (&first_irq, 1, 0))
	{
		struct sched_param param = { .sched_priority = 55 };

		/* raise thread prio on first interrupt */
		sched_setscheduler(current, SCHED_FIFO, &param);
	}
#endif

	/* handle interrupt */

	return IRQ_RETVAL(IRQ_HANDLED);
}

static int init_irqhandler (void)
{
	request_irq (...);
	atomic_set (&dev->first_irq, 1);

	...
}

Thomas

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17  6:09           ` Martin Shepherd
@ 2009-06-17  9:00             ` Thomas Gleixner
  2009-06-18  1:08               ` Martin Shepherd
  0 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2009-06-17  9:00 UTC (permalink / raw)
  To: Martin Shepherd; +Cc: linux-rt-users

On Tue, 16 Jun 2009, Martin Shepherd wrote:
> On Wed, 17 Jun 2009, Thomas Gleixner wrote:
> > > It seems much more friendly and efficient for my driver to provide
> > > applications with an ioctl that tells it to set the priority of its
> > > IRQ thread.
> > 
> > Wrong. That information is already available in sysfs and there is no
> > reason to use an ioctl for that.
> 
> Could you point out where please? Searching sysfs I see the following
> two files that contain the IRQ numbers of two of the boards that my
> driver handles:
> 
>  /bus/pci/drivers/accpcidio/0000:03:02.0/irq
>  /bus/pci/drivers/accpcidio/0000:03:03.0/irq
> 
> However, I don't see how a user application could figure out which of
> these corresponded to the board that it opened using a particular
> device file. Only the device driver knows which device file is
> associated with a given PCI device ID, and that isn't recorded in
> sysfs.

Errm, and how does udev figure out which device node to create for
which device ?

/sys/devices/pci0000:00/0000:00:1d.7/usb1/dev

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-16 22:28       ` Thomas Gleixner
  2009-06-17  1:12         ` GeunSik Lim
  2009-06-17  3:27         ` Martin Shepherd
@ 2009-06-17  9:17         ` Thomas Gleixner
  2009-06-17 11:21           ` Remy Bohmer
  2 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2009-06-17  9:17 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: Martin Shepherd, linux-rt-users

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1467 bytes --]

On Wed, 17 Jun 2009, Thomas Gleixner wrote:
> On Tue, 16 Jun 2009, Uwe Kleine-König wrote:
> > On Tue, Jun 16, 2009 at 07:59:28AM +0000, Martin Shepherd wrote:
> > > Suresh Kumar SHUKLA <suresh.shukla <at> st.com> writes:
> > > > 	// get IRQ descriptor from IRQ, it contains pid
> > > > 	desc = &irq_desc[IRQ_BASIC_TIMER_3_1];
> > > 
> > > That's a very useful clue. I was thinking that I would have to walk the process
> > > tree to find the thread by name. Following up on this clue, it appears as though
> > > in kernel 2.6.29.4 I should be able to use desc=irq_to_desc(irq) to look up the
> > > IRQ descriptor, then use pid=get_task_pid(desc->thread), to get the PID of the
> > > IRQ thread, then use sys_sched_setscheduler(pid,...) to change its priority.
> > > I'll try that out in the morning.
> > I wouldn't recommend calling sys_sched_setscheduler from kernel space.
> > That's the userspace API and you need to pass a __user pointer as third
> > argument.
> > 
> > The right way is to do this change from userspace.
> 
> What a nonsense. Care to look at the code ?
> 
> The irq/softirq threads call sys_sched_setscheduler() already to
> change their priorities.

More awake, it should be changed for correctness sake. :)

> The right way to do this is to have an interface
> set_irq_thread_prio(irq, prio)

This still stands, if we need a way to change the prio from the
driver.

But the more I think about it should be done from user space.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17  9:17         ` Thomas Gleixner
@ 2009-06-17 11:21           ` Remy Bohmer
  2009-06-17 13:32             ` GeunSik Lim
                               ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Remy Bohmer @ 2009-06-17 11:21 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Uwe Kleine-König, Martin Shepherd, linux-rt-users

Hello Thomas,

>> > The right way is to do this change from userspace.
>> What a nonsense. Care to look at the code ?
>> The irq/softirq threads call sys_sched_setscheduler() already to
>> change their priorities.
> More awake, it should be changed for correctness sake. :)
>> The right way to do this is to have an interface
>> set_irq_thread_prio(irq, prio)
> This still stands, if we need a way to change the prio from the
> driver.
> But the more I think about it should be done from user space.

Some time ago we discussed on lkml this same subject.
I made several patches for it back then, but due to change of focus I
somewhat forgot these patches...
First post: http://www.nabble.com/-patch-0-3--add-kernel-cmdline-support-for-interrupt-thread-priorities-(repost:CC-to-LKML)-td14424269.html
I posted them recently again forward ported to 2.6.26:
http://www.spinics.net/lists/linux-rt-users/msg04237.html

I do not agree that it is a userspace issue, the rationale behind this
is that users care about functionality, not specific interrupt thread
ids.
Thread-ids can/will vary, and a lot of scripting is needed to map the
proper interrupt handler of the right device driver to the proper
kernel thread, and to set the priorities accordingly.
And what if there is not a userland at all? and init is the only
process in the system, or there is no shell installed?
Or some kernel developer change the name of the interrupt handler? All
different implementations in userspace has to follow as well... And
why is 50 the right default to use, and not 30 or 60?
For a realtime embedded device this could all be the case, and it is
not that strange.

This was for me the reasoning back then to provide patches that do it
from inside the kernel. The patches provide a means by configuring it
via the kernel commandline, or Kconfig.
The configuration is based on the name provided by the interrupt
handler name provided by request_irq(). If the irq-thread is created
it is looked up inside a map what the priority of the IRQ-thread needs
to be. The IRQ-thread is created with the required prio. It also
checks if the IRQ is shared, and it checks if there are conflicts as
well.

If there is interest I can rework this and make it mainline ready on
the short term for mainline 2.6.29-RT. (in that case comments are very
welcome on the 2.6.26 edition, see above)


Kind Regards,

Remy

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17 11:21           ` Remy Bohmer
@ 2009-06-17 13:32             ` GeunSik Lim
  2009-06-17 15:45               ` Thomas Gleixner
  2009-06-17 13:54             ` Leon Woestenberg
  2009-06-17 15:35             ` Thomas Gleixner
  2 siblings, 1 reply; 27+ messages in thread
From: GeunSik Lim @ 2009-06-17 13:32 UTC (permalink / raw)
  To: Remy Bohmer
  Cc: Thomas Gleixner, Uwe Kleine-König, Martin Shepherd, linux-rt-users

On Wed, Jun 17, 2009 at 8:21 PM, Remy Bohmer<linux@bohmer.net> wrote:
> Hello Thomas,
> Or some kernel developer change the name of the interrupt handler? All
> different implementations in userspace has to follow as well... And
> why is 50 the right default to use, and not 30 or 60?
> For a realtime embedded device this could all be the case, and it is
> not that strange.
Um..., I also don't know exact reason that seted irq priority with 50.
I think that  the value is irq priotiry  is promise in Realtime system.
In my case, When I have to change( or set) RT priority about RT Kernel thread or
Userspace RT applications in a realtime embedded device,
I  change( or set ) priority of  RT Process(or thread) after consider
that irq priority is 50.

-- 
Regards,
GeunSik Lim ( Samsung Electronics )
Blog : http://blog.naver.com/invain/
e-Mail: geunsik.lim@samsung.com
           leemgs@gmail.com , leemgs1@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17 11:21           ` Remy Bohmer
  2009-06-17 13:32             ` GeunSik Lim
@ 2009-06-17 13:54             ` Leon Woestenberg
  2009-06-17 15:35             ` Thomas Gleixner
  2 siblings, 0 replies; 27+ messages in thread
From: Leon Woestenberg @ 2009-06-17 13:54 UTC (permalink / raw)
  To: Remy Bohmer
  Cc: Thomas Gleixner, Uwe Kleine-König, Martin Shepherd, linux-rt-users

Hello,

On Wed, Jun 17, 2009 at 1:21 PM, Remy Bohmer<linux@bohmer.net> wrote:
> Thread-ids can/will vary, and a lot of scripting is needed to map the
> proper interrupt handler of the right device driver to the proper
> kernel thread, and to set the priorities accordingly.
> And what if there is not a userland at all? and init is the only
> process in the system, or there is no shell installed?
> Or some kernel developer change the name of the interrupt handler? All
> different implementations in userspace has to follow as well... And
> why is 50 the right default to use, and not 30 or 60?
> For a realtime embedded device this could all be the case, and it is
> not that strange.
>

To make sure the end solution covers all use cases, I re-submit my case:

>From my email: "[PATCH 2.6.24.7-rt14 v2] rt:
{queue,schedule}_work_prio() allowing work priorities other than
caller's priority".

"Adds schedule_work_prio() and queue_work_prio() so that work can be
scheduled with a different priority than the caller.

Currently under PREEMPT RT, queue_work() and therefore schedule_work() makes
the work inherit the caller's priority.

However, if a real-time kernel interrupt handler thread defers work it may want
the deferred work have lower priority so it does not contend for the CPU with
succeeding interrupt handling. In the non real-time kernel, this was guaranteed
because interrupts preempt workqueues.

The faulty case was a non-FIFO serial driver that deferred LED blinking to
a workqueue using schedule_work(). However, that work used GPIO bitbanged I2C,
which uses 50 usecs udelay()s. With the work inheriting the serial IRQ priority,
it easily missed the ~60 usec deadline of 115200 bps communications.

This patch solves the issue by introducing and using schedule_work_prio()."


Basically, what I did to fix our system, was run the workqueue with
lower priority (current - 1) than the interrupt handler.
So that when the interrupt handler started the work on the queue, that
work would not increase interrupt service latency.


What would be the solution forward for this case, for mainlined
drivers having the same problem?



Regards,
-- 
Leon

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17 11:21           ` Remy Bohmer
  2009-06-17 13:32             ` GeunSik Lim
  2009-06-17 13:54             ` Leon Woestenberg
@ 2009-06-17 15:35             ` Thomas Gleixner
  2009-06-17 20:14               ` Remy Bohmer
  2 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2009-06-17 15:35 UTC (permalink / raw)
  To: Remy Bohmer; +Cc: Uwe Kleine-König, Martin Shepherd, linux-rt-users

On Wed, 17 Jun 2009, Remy Bohmer wrote:
> I do not agree that it is a userspace issue, the rationale behind this
> is that users care about functionality, not specific interrupt thread
> ids.

It _is_ solely a user space issue. The kernel has absolutely no idea
what a system is used for and what the system designer decides is the
best priority balance. The kernel provides a default value which just
works for bootup.

We want drivers and the kernel to be as generic as possible to avoid
tons of extra special cases in the code which are no benefit at all.

> Thread-ids can/will vary, and a lot of scripting is needed to map the
> proper interrupt handler of the right device driver to the proper
> kernel thread, and to set the priorities accordingly.
> And what if there is not a userland at all? and init is the only
> process in the system, or there is no shell installed?

Can we talk about real world issues please ?

> Or some kernel developer change the name of the interrupt handler? All
> different implementations in userspace has to follow as well... And

As I said already, we need to expose the tid of the irq thread to
sysfs as we do with the irq number right now.

> why is 50 the right default to use, and not 30 or 60?

50 is a default startup value and the driver does not know in which
device it is built in. It does not care at all whether the device is a
radio a screwdriver or a blinkenlight.

> For a realtime embedded device this could all be the case, and it is
> not that strange.

There is no such thing "realtime embedded device". There is a device
which requires a real time kernel. Again the kernel does not care
about the overall system requirements. This can only be decided by the
system designer and the system designer has to do the same work for
the user space as well. It's all about policies and the kernel does
not implement policies except some safety ones.

The kernel provides selinux as well and the system designer/admin
decides which policies to use.

> If there is interest I can rework this and make it mainline ready on
> the short term for mainline 2.6.29-RT. (in that case comments are very
> welcome on the 2.6.26 edition, see above)

You can post it and I'm not going to take it for the above reasons.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17 13:32             ` GeunSik Lim
@ 2009-06-17 15:45               ` Thomas Gleixner
  0 siblings, 0 replies; 27+ messages in thread
From: Thomas Gleixner @ 2009-06-17 15:45 UTC (permalink / raw)
  To: GeunSik Lim
  Cc: Remy Bohmer, Uwe Kleine-König, Martin Shepherd, linux-rt-users

On Wed, 17 Jun 2009, GeunSik Lim wrote:
> On Wed, Jun 17, 2009 at 8:21 PM, Remy Bohmer<linux@bohmer.net> wrote:
> > Hello Thomas,
> > Or some kernel developer change the name of the interrupt handler? All
> > different implementations in userspace has to follow as well... And
> > why is 50 the right default to use, and not 30 or 60?
> > For a realtime embedded device this could all be the case, and it is
> > not that strange.

> Um..., I also don't know exact reason that seted irq priority with 50.
> I think that  the value is irq priotiry  is promise in Realtime system.

No, it depends on the system. If your super important device irq needs
to run above all others then you configure it, but it's not a kernel
problem. We have run unimportant irq threads on prio 1 to keep them
out of the way.

> In my case, When I have to change( or set) RT priority about RT
> Kernel thread or Userspace RT applications in a realtime embedded
> device, I change( or set ) priority of RT Process(or thread) after
> consider that irq priority is 50.
 
Again, 50 is a default value. There is no law which prohibits that the
irq priorities can be adjusted to the needs of the system.

The kernel treats by default all irq threads in the same way as it
resembles the mainline behaviour closely. But in contrary to the
mainline hardirq handlers you have now the possibility to change
that. If you are happy with the default then of course you can leave
it as is.

Also a vanilla non-rt kernel comes up with defaults and gives you
enough freedom to customize it to your needs. preempt-rt gives you
some more knobs to tweak and of course an easier way to shoot yourself
into the foot.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17 15:35             ` Thomas Gleixner
@ 2009-06-17 20:14               ` Remy Bohmer
  2009-06-17 23:32                 ` Thomas Gleixner
  2009-06-18 11:05                 ` Suresh Kumar SHUKLA
  0 siblings, 2 replies; 27+ messages in thread
From: Remy Bohmer @ 2009-06-17 20:14 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Uwe Kleine-König, Martin Shepherd, linux-rt-users

Hello Thomas,

2009/6/17 Thomas Gleixner <tglx@linutronix.de>:
> On Wed, 17 Jun 2009, Remy Bohmer wrote:
>> I do not agree that it is a userspace issue, the rationale behind this
>> is that users care about functionality, not specific interrupt thread
>> ids.
> It _is_ solely a user space issue. The kernel has absolutely no idea
> what a system is used for and what the system designer decides is the
> best priority balance. The kernel provides a default value which just
> works for bootup.

No, it is a system designer issue. The type of users of the kernel is
much wider than only the person who is sitting between the keyboard
and chair. The RT-kernel is also used in embedded devices which are
only configured for 1 single purpose, without a 'user' that knows how
to configure the system, or even cares/knows that there is linux
running behind the surface. That same person might not even have a
login shell to be able to configure anything.

In these cases the system designer wants to be able to configure those
priorities at build time, just like he is used to configure
high-resolution-timers, tickless kernel, board support options, or
whatever dedicated kernel config option.
This type of user do not want to configure it 'runtime' over and over
on every boot. (for example, to reduce boot times as well)
Configuring the kernel IRQ-thread priorities falls in the same
category as the other kernel config options, or kernel commandline
options (why does the average user care to select a certain
clocksource from the kernel commandline? It can be done after boot as
well...)

> The kernel has absolutely no idea what a system is used for

This sounds very weird, the kconfig options determine what the kernel
is used for. It decides if it is full featured, or tiny. What
architecture it runs on, and even if it is purposed for a realtime or
regular application.

So, we agree that determining the kernel IRQ-prios by the _user_, but
the definition of the user is much wider than only the person behind
keyboard and chair at a login shell, and the RT-kernel is used in many
different areas than X86 desktop only. So, even it is a _user_ issue,
it is wrong to conclude that is therefor also a _userspace_ issue.

> We want drivers and the kernel to be as generic as possible to avoid
> tons of extra special cases in the code which are no benefit at all.

I completely agree on that, but there is nothing wrong by integrating
a _generic_ mechanism to allow the user to customise their kernel for
their needs. (To be clear: I am _only_ talking about the patch series
to configure kernel threads from Kconfig of kernel cmd-line, not about
fixed relations between workqueues, softirqs and irq-threads, that is
up to the system designer to work out while tweaking the kernel)
It is as wrong to select a default of 50 instead of 30,40,whatever
(hypothetical) as to integrate a generic mechanism to choose the
default at compile time.

>> Thread-ids can/will vary, and a lot of scripting is needed to map the
>> proper interrupt handler of the right device driver to the proper
>> kernel thread, and to set the priorities accordingly.
>> And what if there is not a userland at all? and init is the only
>> process in the system, or there is no shell installed?
>
> Can we talk about real world issues please ?

Who said it was not a real world example? I have seen systems that do
just that. Just to squeeze out the last tiny bytes of flash space, to
be able to choose a 2 cent cheaper flash, and to minimize the memory
footprint. I talk about dedicated embedded devices here, not multi
gigabytes X86 systems. You might believe it is crazy to do this, but
it is real life...
One example is where the complete realtime application was implemented
inside the kernel as well, as a kernel driver. (This is extremely good
for RT-behaviour -> less/no cache flushes due to process context
switches).
While it is using kernel threads as well and the driver chooses to use
certain kernel thread priorities, the default of 50 could even result
in an unbootable system. In that case the user that defines the
kernel-configuration and selects the driver in Kconfig also needs to
configure the IRQ/SIRQ thread prios.

>> Or some kernel developer change the name of the interrupt handler? All
>> different implementations in userspace has to follow as well... And
>
> As I said already, we need to expose the tid of the irq thread to
> sysfs as we do with the irq number right now.

But the irq-thread-id and irq-number are _not_ relevant _at all_, it
is about device drivers and their interrupt handlers.
If I want to, for example, boost the interrupt handler
'uhci_hcd:usb6', because I have RT requirements to the device
connected to this USB-port, I want the irq-thread boosted that handles
this particular interrupt handler. The irq-thread that belongs to this
driver is hardware deployment specific and can differ on different
revisions of hardware as well. By making the tid public in sysfs,
there still has to be made a mapping between /proc/interrupts and the
sysfs entries.
The same is valid for soft-irqs, I might want to boost 'sirq-net', or
'sirq-timer', regardless what tid it might have.

By adding the tid to sysfs only, it still requires tools like a shell,
cat, grep, cut, ps, and chrt in the filesystem as well, but this cost
memory footprint, and that can make the difference in flash type to
choose in an embedded device, where every penny counts up.
Following your opinion here: you can skip the tid in sysfs as well,
because it can be looked up with 'ps' also...

The kernel can no longer keep its own pants up for stuff that is
implemented inside the kernel for the kernel its own needs. The
default options, only might give a bootable system, but no _realtime_
system. To get a _usable_ realtime kernel there should be no
dependencies to user space.
realtime systems always require lots of tweaking by system-designers
who configure the kernel to their application needs.

>> why is 50 the right default to use, and not 30 or 60?
>
> 50 is a default startup value and the driver does not know in which
> device it is built in. It does not care at all whether the device is a
> radio a screwdriver or a blinkenlight.

uuhh, but there is a selection option in kconfig and kernel cmd-line
for the tickless kernel and highres-timers as well, but why should it?
it does not need to know where it is built in, so it does not need to
be configurable...???
Or is there a different reason for making things configurable: that
the location where the kernel is built in _does_ make sense to make
something configurable?

> The kernel treats by default all irq threads in the same way as it resembles the mainline behaviour closely.

Not completely true: On mainline interrupt handlers have prios in
hardware as well, based on the hardware layout and the behaviour of
the interrupt controller one interrupt can preempt another interrupt
if it has a higher priority. The RT-kernel default maps everything to
50 and handles them equally, ignoring the priority mechanism in
hardware.

>> For a realtime embedded device this could all be the case, and it is
>> not that strange.
>
> There is no such thing "realtime embedded device". There is a device

I said 'a' realtime embedded device, in the sense of 'an example'

> which requires a real time kernel. Again the kernel does not care
> about the overall system requirements. This can only be decided by the
> system designer and the system designer has to do the same work for
> the user space as well. It's all about policies and the kernel does
> not implement policies except some safety ones.
> The kernel provides selinux as well and the system designer/admin
> decides which policies to use.

You seem to talk about X86 only here, and seem to forget those tiny
little devices for which SE-Linux is completely overkill.

>> If there is interest I can rework this and make it mainline ready on
>> the short term for mainline 2.6.29-RT. (in that case comments are very
>> welcome on the 2.6.26 edition, see above)
>
> You can post it and I'm not going to take it for the above reasons.

Unfortunately you seem to be missing the point here.

I certainly not state that that patchset is mainline-ready as it is
now, although others were quite positive about it, as you can see in
the older threads. I would prefer to see useful remarks to be able to
create a patchset that is useful for everybody, honouring the generic
purpose of the kernel, and that helps users of non-X86 systems as
well.
I notice a need for it lately, so I am willing to invest some good
time in it, but I am not going to waste it...

Kind regards,

Remy

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17 20:14               ` Remy Bohmer
@ 2009-06-17 23:32                 ` Thomas Gleixner
  2009-06-18 11:05                 ` Suresh Kumar SHUKLA
  1 sibling, 0 replies; 27+ messages in thread
From: Thomas Gleixner @ 2009-06-17 23:32 UTC (permalink / raw)
  To: Remy Bohmer; +Cc: Uwe Kleine-König, Martin Shepherd, linux-rt-users

Remy,

On Wed, 17 Jun 2009, Remy Bohmer wrote:
> 2009/6/17 Thomas Gleixner <tglx@linutronix.de>:
> > On Wed, 17 Jun 2009, Remy Bohmer wrote:
> >> I do not agree that it is a userspace issue, the rationale behind this
> >> is that users care about functionality, not specific interrupt thread
> >> ids.
> > It _is_ solely a user space issue. The kernel has absolutely no idea
> > what a system is used for and what the system designer decides is the
> > best priority balance. The kernel provides a default value which just
> > works for bootup.
> 
> No, it is a system designer issue. The type of users of the kernel is

Care to read, what I write ? 

>> It _is_ solely a user space issue.

I did not say it is a user issue. 

And get it: A system is more than a kernel. It has a user space as
well. And the system designer cares about both.

> much wider than only the person who is sitting between the keyboard
> and chair. The RT-kernel is also used in embedded devices which are
> only configured for 1 single purpose, without a 'user' that knows how
> to configure the system, or even cares/knows that there is linux
> running behind the surface. That same person might not even have a
> login shell to be able to configure anything.

I did not talk about a person.
 
> In these cases the system designer wants to be able to configure those
> priorities at build time, just like he is used to configure
> high-resolution-timers, tickless kernel, board support options, or
> whatever dedicated kernel config option.
> This type of user do not want to configure it 'runtime' over and over
> on every boot. (for example, to reduce boot times as well)

Groan. We talk about how many ms to tweak the damned irq threads ? And
it's not the irq threads alone. There are softirq threads, kernel
threads for other purposes. 

You want to tweak them all via Kconfig or kernel command line ? Really
a great plan.

> Configuring the kernel IRQ-thread priorities falls in the same
> category as the other kernel config options, or kernel commandline
> options (why does the average user care to select a certain
> clocksource from the kernel commandline? It can be done after boot as
> well...)

No, in same cases it can't.

There is a completely different reason. clocksource selection is not
for system designers sake. It is for debugability of (mostly x86)
systems so users have a fair chance to boot a stock kernel which
selects an disfunctional TSC, HPET or whatever broken piece of crap
happens to be in the users box.

> > The kernel has absolutely no idea what a system is used for
> 
> This sounds very weird, the kconfig options determine what the kernel
> is used for. It decides if it is full featured, or tiny. What
> architecture it runs on, and even if it is purposed for a realtime or
> regular application.

Right. It restricts or widens the set of features which are exposed to
user space. That restricts or widens the number of knobs user space
can tweak. Not more and not less.

> So, we agree that determining the kernel IRQ-prios by the _user_, but
> the definition of the user is much wider than only the person behind
> keyboard and chair at a login shell, and the RT-kernel is used in many
> different areas than X86 desktop only. So, even it is a _user_ issue,
> it is wrong to conclude that is therefor also a _userspace_ issue.

You seem to be completely confused. 

    _user_       != Joe User
    _user space_ != desktop GUI.

The minimal user space needs to run some kind of init and starts the
application. So this setup can either be part of init or of the
application itself.

> > We want drivers and the kernel to be as generic as possible to avoid
> > tons of extra special cases in the code which are no benefit at all.
> 
> I completely agree on that, but there is nothing wrong by integrating
> a _generic_ mechanism to allow the user to customise their kernel for
> their needs. (To be clear: I am _only_ talking about the patch series
> to configure kernel threads from Kconfig of kernel cmd-line, not about
> fixed relations between workqueues, softirqs and irq-threads, that is
> up to the system designer to work out while tweaking the kernel)

Wrong again. The system designer should not tweak the kernel by
hacking around in it and compiling system policies into the kernel.

> It is as wrong to select a default of 50 instead of 30,40,whatever
> (hypothetical) as to integrate a generic mechanism to choose the
> default at compile time.

There is a lot wrong with that approach. You'd move policy into the
kernel which does not belong there by definition.
 
> >> Thread-ids can/will vary, and a lot of scripting is needed to map the
> >> proper interrupt handler of the right device driver to the proper
> >> kernel thread, and to set the priorities accordingly.
> >> And what if there is not a userland at all? and init is the only
> >> process in the system, or there is no shell installed?
> >
> > Can we talk about real world issues please ?
> 
> Who said it was not a real world example? I have seen systems that do
> just that. Just to squeeze out the last tiny bytes of flash space, to
> be able to choose a 2 cent cheaper flash, and to minimize the memory
> footprint. I talk about dedicated embedded devices here, not multi
> gigabytes X86 systems. You might believe it is crazy to do this, but
> it is real life...

Dude. I'm working in the embedded space since more than 20 years and I
know what's going on there. In my lab are probably more non x86 based
real world embedded machines than x86 based gigabyte systems.

> One example is where the complete realtime application was implemented
> inside the kernel as well, as a kernel driver. (This is extremely good

Great design. This is so uninteresting as it can be. The only
interesting point might be the GPL violation which is probably
happening there.

Those kernels are hacked anyway beyond recognition so why does it
matter if there is another ugly and horrible patch which allows that
driver/application to tweak the irq handler priority?

The mainline development cares about sane use cases and not about the
weirdness of some crackpot design which happens to use a completely
crippeled and unfixable kernel. I have seen such things already and
when I need to look at a patch which is larger than the preempt-rt
patch and even larger than the delta of two mainline versions then I
do not care at all. 

I even refuse to touch such shit for money, except they raise the
offer by some orders of magnitude.

The embedded space is nuts. Google for "Embedded nightmare" if you
care.

> for RT-behaviour -> less/no cache flushes due to process context
> switches).

Start using some ARM CPU which does not need that :)

> While it is using kernel threads as well and the driver chooses to use
> certain kernel thread priorities, the default of 50 could even result
> in an unbootable system. In that case the user that defines the
> kernel-configuration and selects the driver in Kconfig also needs to
> configure the IRQ/SIRQ thread prios.

Stop hand waving. Provide a real use case where the kernel does not
reach user space due to the default setting. And please do not provide
one of the kind you described above as I couldn't care less.
 
> >> Or some kernel developer change the name of the interrupt handler? All
> >> different implementations in userspace has to follow as well... And
> >
> > As I said already, we need to expose the tid of the irq thread to
> > sysfs as we do with the irq number right now.
> 
> But the irq-thread-id and irq-number are _not_ relevant _at all_, it
> is about device drivers and their interrupt handlers.
> If I want to, for example, boost the interrupt handler
> 'uhci_hcd:usb6', because I have RT requirements to the device
> connected to this USB-port, I want the irq-thread boosted that handles
> this particular interrupt handler. The irq-thread that belongs to this
> driver is hardware deployment specific and can differ on different
> revisions of hardware as well. By making the tid public in sysfs,
> there still has to be made a mapping between /proc/interrupts and the
> sysfs entries.

Again. You are confused. If you want to tweak uhci_hcd:usb6 then you
look at the very sysfs entry of uhci_hcd:usb6, get the tid and call
sched_setscheduler(2) or chrt(1) and be done.

> The same is valid for soft-irqs, I might want to boost 'sirq-net', or
> 'sirq-timer', regardless what tid it might have.

Where is the point? You can get the tid already today. There might be
some more intuitive interface, but that can be fixed and does not move
any policy decision into the kernel proper.

> By adding the tid to sysfs only, it still requires tools like a shell,
> cat, grep, cut, ps, and chrt in the filesystem as well, but this cost
> memory footprint, and that can make the difference in flash type to
> choose in an embedded device, where every penny counts up.

Yeah. I'm wiping my tears away.

Get some reality. Small FLASH parts which are suitable for Linux have
started years ago to be more expensive than larger ones.

> The kernel can no longer keep its own pants up for stuff that is
> implemented inside the kernel for the kernel its own needs. The
> default options, only might give a bootable system, but no _realtime_
> system. To get a _usable_ realtime kernel there should be no
> dependencies to user space.
> realtime systems always require lots of tweaking by system-designers
> who configure the kernel to their application needs.

Again, that's the wrong way. This is not your old lovely home brewn
bare metal OS where you stick everything in just because you can. It's
about having a sane default system which can be tuned from user space.

> >> why is 50 the right default to use, and not 30 or 60?
> >
> > 50 is a default startup value and the driver does not know in which
> > device it is built in. It does not care at all whether the device is a
> > radio a screwdriver or a blinkenlight.
> 
> uuhh, but there is a selection option in kconfig and kernel cmd-line
> for the tickless kernel and highres-timers as well, but why should it?
> it does not need to know where it is built in, so it does not need to
> be configurable...???

Both help to save your precious $pennies and the command line options
are for debugability (again mostly due to x86 hardware wreckage),
nothing else.

> > The kernel treats by default all irq threads in the same way as it
> > resembles the mainline behaviour closely.
>
> Not completely true: On mainline interrupt handlers have prios in
> hardware as well, based on the hardware layout and the behaviour of
> the interrupt controller one interrupt can preempt another interrupt
> if it has a higher priority. The RT-kernel default maps everything to
> 50 and handles them equally, ignoring the priority mechanism in
> hardware.

I said closely _not_ entirely. And I'm well aware of that. If you care
to look I even wrote a paper about that almost a decade ago.
 
> >> For a realtime embedded device this could all be the case, and it is
> >> not that strange.
> >
> > There is no such thing "realtime embedded device". There is a device
> 
> I said 'a' realtime embedded device, in the sense of 'an example'
> 
> > which requires a real time kernel. Again the kernel does not care
> > about the overall system requirements. This can only be decided by the
> > system designer and the system designer has to do the same work for
> > the user space as well. It's all about policies and the kernel does
> > not implement policies except some safety ones.
> > The kernel provides selinux as well and the system designer/admin
> > decides which policies to use.
> 
> You seem to talk about X86 only here, and seem to forget those tiny
> little devices for which SE-Linux is completely overkill.

I used selinux as an example to illustrate the difference of mechanism
and policy. We need this clear separation otherwise we end up with
tons of requests like yours which are simply not maintainable for the
mainline kernel. From a sane system design perspective there is no
place for policy in the kernel.

Can you please stop to see the world with your narrow minded embedded
world view ?

I'm doing and have been doing embedded work a lot and know very well
that there is a world outside of x86.

But embedded is not that different from x86. It's all the same kernel
and the same philosophy. 

Just you and some others of the embedded species insist that there is
a different philosophy. It's not. It's just different in the embedded
mindset which does not care about anything.

If you can not cope with the UNIX/Linux way, then use whatever OS fits
your philosophy best, really.

> >> If there is interest I can rework this and make it mainline ready on
> >> the short term for mainline 2.6.29-RT. (in that case comments are very
> >> welcome on the 2.6.26 edition, see above)
> >
> > You can post it and I'm not going to take it for the above reasons.
> 
> Unfortunately you seem to be missing the point here.

No, I'm not.
 
> I certainly not state that that patchset is mainline-ready as it is
> now, although others were quite positive about it, as you can see in
> the older threads. I would prefer to see useful remarks to be able to
> create a patchset that is useful for everybody, honouring the generic
> purpose of the kernel, and that helps users of non-X86 systems as
> well.
> I notice a need for it lately, so I am willing to invest some good
> time in it, but I am not going to waste it...

I don't care about others being positive about it. I'm refusing to put
policy into the kernel for two reasons:

       - there is no place for policy in the kernel by definition
       - policy in the kernel is not maintainable

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
  2009-06-17  9:00             ` Thomas Gleixner
@ 2009-06-18  1:08               ` Martin Shepherd
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Shepherd @ 2009-06-18  1:08 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-rt-users

On Wed, 17 Jun 2009, Thomas Gleixner wrote:
> Errm, and how does udev figure out which device node to create for
> which device ?
>
> /sys/devices/pci0000:00/0000:00:1d.7/usb1/dev

Thanks for the hint. I wasn't aware of the need to call
device_create() etc in my driver, and had been creating device files
by hand.  Having now remedied this omission, I have taken the
opportunity to add an irq attribute file under each of my device
entries in sysfs. Next I would like to add an attribute that indicates
the corresponding thread PID. To do that, I need a kernel function
that can be called to determine the IRQ thread ID.

I have just written and tested the following function for doing this:

#ifdef CONFIG_PREEMPT_HARDIRQS

pid_t get_irq_thread_pid(unsigned int irq, struct pid_namespace *ns)
{
         struct irq_desc *desc = irq_to_desc(irq);
         pid_t pid = 0;

         if(desc) {
                 unsigned long flags;
                 spin_lock_irqsave(&desc->lock, flags);
                 if(desc->thread)
                         pid = task_pid_nr_ns(desc->thread, ns);
                 spin_unlock_irqrestore(&desc->lock, flags);
         }
         return pid;
}

EXPORT_SYMBOL(get_irq_thread_pid);

#endif

This would most frequently be called like:

   pid = get_irq_thread_pid(irq, current->nsproxy->pid_ns);
   if(pid) {
     ...
   }

Would this function be acceptible for inclusion in the preempt-rt
kernel, or do you have something else in mind?

Martin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: Setting the priority of an IRQ thread
  2009-06-17 20:14               ` Remy Bohmer
  2009-06-17 23:32                 ` Thomas Gleixner
@ 2009-06-18 11:05                 ` Suresh Kumar SHUKLA
  1 sibling, 0 replies; 27+ messages in thread
From: Suresh Kumar SHUKLA @ 2009-06-18 11:05 UTC (permalink / raw)
  To: linux-rt-users

> > It _is_ solely a user space issue. The kernel has 

I don't agree. Isn't this a policy of its own.

Mechanism for changing priority should be available within driver code too.
My desired functionality fits perfectly fine as driver/LKM, why force me to
create a user mode application.


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Setting the priority of an IRQ thread
       [not found] <4A37E0CE020000D90003727B@sinclair.provo.novell.com>
@ 2009-06-16 22:13 ` Peter Morreale
  0 siblings, 0 replies; 27+ messages in thread
From: Peter Morreale @ 2009-06-16 22:13 UTC (permalink / raw)
  To: mcs, u.kleine-koenig; +Cc: linux-rt-users

Sorry for the top post. On BB. Network is down. :-(

Why not have the driver ioctl return the pid of the IRQ thread?  That way the driver stays 'clean' and the app is free to manipulate the prio as it sees fit.  

Just a thought...

-PWM 
-----Original Message-----
From: Martin Shepherd <mcs@astro.caltech.edu>
To:  <u.kleine-koenig@pengutronix.de>
Cc:  <linux-rt-users@vger.kernel.org>

Sent: 6/16/2009 3:25:18 PM
Subject: Re: Setting the priority of an IRQ thread


On Tue, 16 Jun 2009, Uwe Kleine-König wrote:
>> IRQ thread, then use sys_sched_setscheduler(pid,...) to change its priority.
>> I'll try that out in the morning.
> I wouldn't recommend calling sys_sched_setscheduler from kernel space.
> That's the userspace API and you need to pass a __user pointer as third
> argument.

Good point, although my ioctl() will be passing the scheduling
parameters from user-space. So the need for a __user pointer for this
would not be a problem. Alternatively one could call task_setprio(),
instead of sys_sched_setscheduler(), and omit the then reduntant step
of figuring out the PID of the task.

> The right way is to do this change from userspace.

Which is what I am trying to do.  I want the application thread that
uses my driver to be able to set the priority of its interrupt to
equal the priority of the calling thread.  The problem with calling
sched_setscheduler directly from user-space, is figuring out the PID
of the IRQ thread. First of all, I have multiple boards of the same
type, such that looking in /proc/interrupts won't tell me which IRQ is
being used by a given board. So I would have to implement an ioctl()
anyway, just to query the IRQ from the device driver. Secondly, the
PID of the corresponding IRQ thread changes each time that
request_irq() is called, and then later freed. So searching for the
IRQ thread couldn't be done by a wrapper script around the program.

It seems much more friendly and efficient for my driver to provide
applications with an ioctl that tells it to set the priority of its
IRQ thread.

Note that none of this would be necessary if the IRQ thread of a
device automatically inherited the priority of the highest priority
realtime thread that has requested (and not yet free'd) its IRQ. Not
doing so, leads to a form of priority inversion.

Thanks,

Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2009-06-18 11:05 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-15 22:44 Setting the priority of an IRQ thread Martin Shepherd
2009-06-16  3:54 ` Suresh Kumar SHUKLA
2009-06-16  7:59   ` Martin Shepherd
2009-06-16 18:27     ` Uwe Kleine-König
2009-06-16 21:25       ` Martin Shepherd
2009-06-16 21:54       ` Martin Shepherd
2009-06-17  5:21         ` Thomas Gleixner
2009-06-17  6:09           ` Martin Shepherd
2009-06-17  9:00             ` Thomas Gleixner
2009-06-18  1:08               ` Martin Shepherd
2009-06-16 22:28       ` Thomas Gleixner
2009-06-17  1:12         ` GeunSik Lim
2009-06-17  2:27           ` Martin Shepherd
2009-06-17  5:28             ` Thomas Gleixner
2009-06-17  3:27         ` Martin Shepherd
2009-06-17  5:38           ` Thomas Gleixner
2009-06-17  9:17         ` Thomas Gleixner
2009-06-17 11:21           ` Remy Bohmer
2009-06-17 13:32             ` GeunSik Lim
2009-06-17 15:45               ` Thomas Gleixner
2009-06-17 13:54             ` Leon Woestenberg
2009-06-17 15:35             ` Thomas Gleixner
2009-06-17 20:14               ` Remy Bohmer
2009-06-17 23:32                 ` Thomas Gleixner
2009-06-18 11:05                 ` Suresh Kumar SHUKLA
2009-06-17  8:27     ` Thomas Pfaff
     [not found] <4A37E0CE020000D90003727B@sinclair.provo.novell.com>
2009-06-16 22:13 ` Peter Morreale

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.