* uio drivers with IRQF_NO_THREAD on preempt-rt kernel @ 2018-05-08 15:59 Matthias Fuchs 2018-05-09 17:56 ` Julia Cartwright 0 siblings, 1 reply; 6+ messages in thread From: Matthias Fuchs @ 2018-05-08 15:59 UTC (permalink / raw) To: linux-rt-users Hi folks, I am running stable kernel v4.4.110 with preempt-rt patch rt125 on a AM335x non-SMP system. There is one thread with hard realtime requirements running on this system. This thread is scheduled by a hardware interrupt (either AM335x PRUSS or external FPGA). Latencies from interrupt into process are as expected. Interrupt thread prio has been bumped to 90. But I want/need even shorter latencies. So I tried to use IRQF_NO_THREAD in my uio driver to get rid of the scheduling detour through over the interrupt thread. The interrupt handling should be quiet fast - most handling is done in userspace. Here comes the problem. The uio framework uses wake_up_interruptible() in the isr which does not work from hard interrupt handlers. I tried to modify uio.c to use wake_up_process() with a limitation to support a single process having opened the device. This works fine in most cases :-( Userspace uses select() on the device with timeouts. Sometimes my process is woken by interrupt event and sometimes by timeout - fine. But it looks like I am missing some interrupts. I expect some issue with uio poll/waitqueue handling. Any idea how to fix this? It would be fine to stay with poll in the uio drivers. I already thought about a special hrtimer handling in my situation. But I hope about something more pretty. BTW, latencies are shorter with this approach! Matthias diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index bcc1fc0..7eb8257 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -394,8 +394,12 @@ void uio_event_notify(struct uio_info *info) struct uio_device *idev = info->uio_dev; atomic_inc(&idev->event); - wake_up_interruptible(&idev->wait); - kill_fasync(&idev->async_queue, SIGIO, POLL_IN); + if (idev->rt_consumer) { + wake_up_process(idev->rt_consumer); + } else { + wake_up_interruptible(&idev->wait); + kill_fasync(&idev->async_queue, SIGIO, POLL_IN); + } } EXPORT_SYMBOL_GPL(uio_event_notify); @@ -439,6 +443,17 @@ static int uio_open(struct inode *inode, struct file *filep) goto out; } + if (idev->rt_consumer) { + ret = -EBUSY; + goto err_alloc_listener; + } + +#ifdef CONFIG_PREEMPT_RT_FULL + if (idev->info->irq_flags & IRQF_NO_THREAD) { + idev->rt_consumer = current; + } +#endif + listener = kmalloc(sizeof(*listener), GFP_KERNEL); if (!listener) { ret = -ENOMEM; @@ -480,6 +495,7 @@ static int uio_release(struct inode *inode, struct file *filep) struct uio_listener *listener = filep->private_data; struct uio_device *idev = listener->dev; + idev->rt_consumer = NULL; if (idev->info->release) ret = idev->info->release(idev->info, inode); diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h index 32c0e83..cf314da 100644 --- a/include/linux/uio_driver.h +++ b/include/linux/uio_driver.h @@ -73,6 +73,7 @@ struct uio_device { struct uio_info *info; struct kobject *map_dir; struct kobject *portio_dir; + struct task_struct *rt_consumer; }; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: uio drivers with IRQF_NO_THREAD on preempt-rt kernel 2018-05-08 15:59 uio drivers with IRQF_NO_THREAD on preempt-rt kernel Matthias Fuchs @ 2018-05-09 17:56 ` Julia Cartwright 2018-05-15 14:02 ` Sebastian Andrzej Siewior 0 siblings, 1 reply; 6+ messages in thread From: Julia Cartwright @ 2018-05-09 17:56 UTC (permalink / raw) To: Matthias Fuchs; +Cc: linux-rt-users On Tue, May 08, 2018 at 05:59:27PM +0200, Matthias Fuchs wrote: > Hi folks, Hello Matthias- > I am running stable kernel v4.4.110 with preempt-rt patch rt125 on a AM335x non-SMP system. > There is one thread with hard realtime requirements running on this system. This thread is scheduled > by a hardware interrupt (either AM335x PRUSS or external FPGA). > > Latencies from interrupt into process are as expected. Interrupt thread prio has been > bumped to 90. But I want/need even shorter latencies. > > So I tried to use IRQF_NO_THREAD in my uio driver to get rid of the scheduling detour through over the interrupt thread. The interrupt handling should be quiet fast - most handling is done in userspace. > > Here comes the problem. The uio framework uses wake_up_interruptible() in the isr which does > not work from hard interrupt handlers. I tried to modify uio.c to use wake_up_process() with a limitation > to support a single process having opened the device. I didn't look at your code in detail, but you might consider looking at the simple waitqueue implementation. See include/linux/swait.h in the kernel tree. In -rt, completions have been reworked to use them, if you want to look at an example. swake_up_*() can be used in hardirq context. Good luck, Julia ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: uio drivers with IRQF_NO_THREAD on preempt-rt kernel 2018-05-09 17:56 ` Julia Cartwright @ 2018-05-15 14:02 ` Sebastian Andrzej Siewior 2018-05-28 20:26 ` Matthias Fuchs 0 siblings, 1 reply; 6+ messages in thread From: Sebastian Andrzej Siewior @ 2018-05-15 14:02 UTC (permalink / raw) To: Julia Cartwright; +Cc: Matthias Fuchs, linux-rt-users On 2018-05-09 12:56:38 [-0500], Julia Cartwright wrote: > On Tue, May 08, 2018 at 05:59:27PM +0200, Matthias Fuchs wrote: > > Hi folks, > > Hello Matthias- > > > I am running stable kernel v4.4.110 with preempt-rt patch rt125 on a AM335x non-SMP system. > > There is one thread with hard realtime requirements running on this system. This thread is scheduled > > by a hardware interrupt (either AM335x PRUSS or external FPGA). > > > > Latencies from interrupt into process are as expected. Interrupt thread prio has been > > bumped to 90. But I want/need even shorter latencies. > > > > So I tried to use IRQF_NO_THREAD in my uio driver to get rid of the scheduling detour through over the interrupt thread. The interrupt handling should be quiet fast - most handling is done in userspace. > > > > Here comes the problem. The uio framework uses wake_up_interruptible() in the isr which does > > not work from hard interrupt handlers. I tried to modify uio.c to use wake_up_process() with a limitation > > to support a single process having opened the device. > > I didn't look at your code in detail, but you might consider looking at > the simple waitqueue implementation. See include/linux/swait.h in the > kernel tree. In -rt, completions have been reworked to use them, if you > want to look at an example. swake_up_*() can be used in hardirq context. This can be done but the "normal" waitqueue has to remain. If a process blocks on read() then you can wake it up via swait() from hardirq context. You need to keep the waitqueue for a possible poll() user. > Good luck, > > Julia Sebastian ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: uio drivers with IRQF_NO_THREAD on preempt-rt kernel 2018-05-15 14:02 ` Sebastian Andrzej Siewior @ 2018-05-28 20:26 ` Matthias Fuchs 2018-05-29 16:51 ` Sebastian Andrzej Siewior 0 siblings, 1 reply; 6+ messages in thread From: Matthias Fuchs @ 2018-05-28 20:26 UTC (permalink / raw) To: Sebastian Andrzej Siewior, Julia Cartwright; +Cc: linux-rt-users Hi, I updated my modified uio.c code using simple wake queues. See below. Blocking read on the uio device is fine. But select() with timeout behaves a little strange. I am still digging to find out what happens, but it seems that even I should never run into a timeout in my test application, the event_count of two consecutive select()/read() pairs is not advanced by one. So is my implementation correct? Does using the normal waitqueue in this manner satisfy uio_poll(). So in my case irq_flags has IRQF_NO_THREAD always set. This means idev->wait never gets a wake_up_interruptible(). diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index bcc1fc0..779dcaf 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -25,6 +25,7 @@ #include <linux/kobject.h> #include <linux/cdev.h> #include <linux/uio_driver.h> +#include <linux/swait.h> #define UIO_MAX_DEVICES (1U << MINORBITS) @@ -394,8 +395,12 @@ void uio_event_notify(struct uio_info *info) struct uio_device *idev = info->uio_dev; atomic_inc(&idev->event); - wake_up_interruptible(&idev->wait); - kill_fasync(&idev->async_queue, SIGIO, POLL_IN); + if (idev->info->irq_flags & IRQF_NO_THREAD) { + swake_up_locked(&idev->swait); + } else { + wake_up_interruptible(&idev->wait); + kill_fasync(&idev->async_queue, SIGIO, POLL_IN); + } } EXPORT_SYMBOL_GPL(uio_event_notify); @@ -508,6 +513,7 @@ static ssize_t uio_read(struct file *filep, char __user *buf, struct uio_listener *listener = filep->private_data; struct uio_device *idev = listener->dev; DECLARE_WAITQUEUE(wait, current); + DECLARE_SWAITQUEUE(swait); ssize_t retval; s32 event_count; @@ -520,11 +526,10 @@ static ssize_t uio_read(struct file *filep, char __user *buf, add_wait_queue(&idev->wait, &wait); do { - set_current_state(TASK_INTERRUPTIBLE); + prepare_to_swait(&idev->swait, &swait, TASK_INTERRUPTIBLE); event_count = atomic_read(&idev->event); if (event_count != listener->event_count) { - __set_current_state(TASK_RUNNING); if (copy_to_user(buf, &event_count, count)) retval = -EFAULT; else { @@ -546,7 +551,7 @@ static ssize_t uio_read(struct file *filep, char __user *buf, schedule(); } while (1); - __set_current_state(TASK_RUNNING); + finish_swait(&idev->swait, &swait); remove_wait_queue(&idev->wait, &wait); return retval; @@ -814,6 +819,7 @@ int __uio_register_device(struct module *owner, idev->owner = owner; idev->info = info; init_waitqueue_head(&idev->wait); + init_swait_queue_head(&idev->swait); atomic_set(&idev->event, 0); ret = uio_get_minor(idev); Cheers, Matthias On 15.05.2018 16:02, Sebastian Andrzej Siewior wrote: > On 2018-05-09 12:56:38 [-0500], Julia Cartwright wrote: >> On Tue, May 08, 2018 at 05:59:27PM +0200, Matthias Fuchs wrote: >>> Hi folks, >> >> Hello Matthias- >> >>> I am running stable kernel v4.4.110 with preempt-rt patch rt125 on a AM335x non-SMP system. >>> There is one thread with hard realtime requirements running on this system. This thread is scheduled >>> by a hardware interrupt (either AM335x PRUSS or external FPGA). >>> >>> Latencies from interrupt into process are as expected. Interrupt thread prio has been >>> bumped to 90. But I want/need even shorter latencies. >>> >>> So I tried to use IRQF_NO_THREAD in my uio driver to get rid of the scheduling detour through over the interrupt thread. The interrupt handling should be quiet fast - most handling is done in userspace. >>> >>> Here comes the problem. The uio framework uses wake_up_interruptible() in the isr which does >>> not work from hard interrupt handlers. I tried to modify uio.c to use wake_up_process() with a limitation >>> to support a single process having opened the device. >> >> I didn't look at your code in detail, but you might consider looking at >> the simple waitqueue implementation. See include/linux/swait.h in the >> kernel tree. In -rt, completions have been reworked to use them, if you >> want to look at an example. swake_up_*() can be used in hardirq context. > > This can be done but the "normal" waitqueue has to remain. If a process > blocks on read() then you can wake it up via swait() from hardirq > context. You need to keep the waitqueue for a possible poll() user. > >> Good luck, >> >> Julia > > Sebastian > ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: uio drivers with IRQF_NO_THREAD on preempt-rt kernel 2018-05-28 20:26 ` Matthias Fuchs @ 2018-05-29 16:51 ` Sebastian Andrzej Siewior 2018-05-30 17:50 ` Matthias Fuchs 0 siblings, 1 reply; 6+ messages in thread From: Sebastian Andrzej Siewior @ 2018-05-29 16:51 UTC (permalink / raw) To: Matthias Fuchs; +Cc: Julia Cartwright, linux-rt-users On 2018-05-28 22:26:55 [+0200], Matthias Fuchs wrote: > Hi, > > I updated my modified uio.c code using simple wake queues. See below. > Blocking read on the uio device is fine. But select() with timeout > behaves a little strange. I am still digging to find out what happens, > but it seems that even I should never run into a timeout in my test application, > the event_count of two consecutive select()/read() pairs is not advanced by one. > > So is my implementation correct? Does using the normal waitqueue in this > manner satisfy uio_poll(). So in my case irq_flags has IRQF_NO_THREAD always set. This means > idev->wait never gets a wake_up_interruptible(). > > diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c > index bcc1fc0..779dcaf 100644 > --- a/drivers/uio/uio.c > +++ b/drivers/uio/uio.c > @@ -25,6 +25,7 @@ > #include <linux/kobject.h> > #include <linux/cdev.h> > #include <linux/uio_driver.h> > +#include <linux/swait.h> > > #define UIO_MAX_DEVICES (1U << MINORBITS) > > @@ -394,8 +395,12 @@ void uio_event_notify(struct uio_info *info) > struct uio_device *idev = info->uio_dev; > > atomic_inc(&idev->event); > - wake_up_interruptible(&idev->wait); > - kill_fasync(&idev->async_queue, SIGIO, POLL_IN); > + if (idev->info->irq_flags & IRQF_NO_THREAD) { > + swake_up_locked(&idev->swait); you want swake_up(). > + } else { > + wake_up_interruptible(&idev->wait); > + kill_fasync(&idev->async_queue, SIGIO, POLL_IN); also you need do this if you have someone is in poll(). You could the upper part in the primary handler this in the threaded handler. > + } > } > EXPORT_SYMBOL_GPL(uio_event_notify); > > @@ -508,6 +513,7 @@ static ssize_t uio_read(struct file *filep, char __user *buf, > struct uio_listener *listener = filep->private_data; > struct uio_device *idev = listener->dev; > DECLARE_WAITQUEUE(wait, current); > + DECLARE_SWAITQUEUE(swait); > ssize_t retval; > s32 event_count; > > @@ -520,11 +526,10 @@ static ssize_t uio_read(struct file *filep, char __user *buf, > add_wait_queue(&idev->wait, &wait); > > do { > - set_current_state(TASK_INTERRUPTIBLE); > + prepare_to_swait(&idev->swait, &swait, TASK_INTERRUPTIBLE); > > event_count = atomic_read(&idev->event); > if (event_count != listener->event_count) { > - __set_current_state(TASK_RUNNING); > if (copy_to_user(buf, &event_count, count)) > retval = -EFAULT; > else { > @@ -546,7 +551,7 @@ static ssize_t uio_read(struct file *filep, char __user *buf, > schedule(); > } while (1); > > - __set_current_state(TASK_RUNNING); > + finish_swait(&idev->swait, &swait); > remove_wait_queue(&idev->wait, &wait); and ->wait isn't used in ->read() anymore, right? Just in ->poll(). If so it could go. > return retval; > @@ -814,6 +819,7 @@ int __uio_register_device(struct module *owner, > idev->owner = owner; > idev->info = info; > init_waitqueue_head(&idev->wait); > + init_swait_queue_head(&idev->swait); > atomic_set(&idev->event, 0); > > ret = uio_get_minor(idev); > > > Cheers, > Matthias Sebastian ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: uio drivers with IRQF_NO_THREAD on preempt-rt kernel 2018-05-29 16:51 ` Sebastian Andrzej Siewior @ 2018-05-30 17:50 ` Matthias Fuchs 0 siblings, 0 replies; 6+ messages in thread From: Matthias Fuchs @ 2018-05-30 17:50 UTC (permalink / raw) To: Sebastian Andrzej Siewior; +Cc: Julia Cartwright, linux-rt-users Hi Sebastian, On 29.05.2018 18:51, Sebastian Andrzej Siewior wrote: > On 2018-05-28 22:26:55 [+0200], Matthias Fuchs wrote: >> Hi, >> >> I updated my modified uio.c code using simple wake queues. See below. >> Blocking read on the uio device is fine. But select() with timeout >> behaves a little strange. I am still digging to find out what happens, >> but it seems that even I should never run into a timeout in my test application, >> the event_count of two consecutive select()/read() pairs is not advanced by one. >> >> So is my implementation correct? Does using the normal waitqueue in this >> manner satisfy uio_poll(). So in my case irq_flags has IRQF_NO_THREAD always set. This means >> idev->wait never gets a wake_up_interruptible(). >> >> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c >> index bcc1fc0..779dcaf 100644 >> --- a/drivers/uio/uio.c >> +++ b/drivers/uio/uio.c >> @@ -25,6 +25,7 @@ >> #include <linux/kobject.h> >> #include <linux/cdev.h> >> #include <linux/uio_driver.h> >> +#include <linux/swait.h> >> >> #define UIO_MAX_DEVICES (1U << MINORBITS) >> >> @@ -394,8 +395,12 @@ void uio_event_notify(struct uio_info *info) >> struct uio_device *idev = info->uio_dev; >> >> atomic_inc(&idev->event); >> - wake_up_interruptible(&idev->wait); >> - kill_fasync(&idev->async_queue, SIGIO, POLL_IN); >> + if (idev->info->irq_flags & IRQF_NO_THREAD) { >> + swake_up_locked(&idev->swait); > > you want swake_up(). > >> + } else { >> + wake_up_interruptible(&idev->wait); >> + kill_fasync(&idev->async_queue, SIGIO, POLL_IN); > > also you need do this if you have someone is in poll(). You could the > upper part in the primary handler this in the threaded handler. uio.c only has a single handler that calls uio_event_notify(). That handler is typically threaded and in my case not (uio driver passes IRQF_NO_THREAD). So what threaded handler do you mean? Do you mean from uio_read()? Does this work? My itention is to use select() (aka poll) on the uio driver from my RT application and have no irq thread(). I do not want to implement a separate timeout mechanism on read(). I've seen this requirement in other places before. Matthias > >> + } >> } >> EXPORT_SYMBOL_GPL(uio_event_notify); >> >> @@ -508,6 +513,7 @@ static ssize_t uio_read(struct file *filep, char __user *buf, >> struct uio_listener *listener = filep->private_data; >> struct uio_device *idev = listener->dev; >> DECLARE_WAITQUEUE(wait, current); >> + DECLARE_SWAITQUEUE(swait); >> ssize_t retval; >> s32 event_count; >> >> @@ -520,11 +526,10 @@ static ssize_t uio_read(struct file *filep, char __user *buf, >> add_wait_queue(&idev->wait, &wait); >> >> do { >> - set_current_state(TASK_INTERRUPTIBLE); >> + prepare_to_swait(&idev->swait, &swait, TASK_INTERRUPTIBLE); >> >> event_count = atomic_read(&idev->event); >> if (event_count != listener->event_count) { >> - __set_current_state(TASK_RUNNING); >> if (copy_to_user(buf, &event_count, count)) >> retval = -EFAULT; >> else { >> @@ -546,7 +551,7 @@ static ssize_t uio_read(struct file *filep, char __user *buf, >> schedule(); >> } while (1); >> >> - __set_current_state(TASK_RUNNING); >> + finish_swait(&idev->swait, &swait); >> remove_wait_queue(&idev->wait, &wait); > > and ->wait isn't used in ->read() anymore, right? Just in ->poll(). If > so it could go. > >> return retval; >> @@ -814,6 +819,7 @@ int __uio_register_device(struct module *owner, >> idev->owner = owner; >> idev->info = info; >> init_waitqueue_head(&idev->wait); >> + init_swait_queue_head(&idev->swait); >> atomic_set(&idev->event, 0); >> >> ret = uio_get_minor(idev); >> >> >> Cheers, >> Matthias > > Sebastian > Matthias ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-30 17:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-08 15:59 uio drivers with IRQF_NO_THREAD on preempt-rt kernel Matthias Fuchs 2018-05-09 17:56 ` Julia Cartwright 2018-05-15 14:02 ` Sebastian Andrzej Siewior 2018-05-28 20:26 ` Matthias Fuchs 2018-05-29 16:51 ` Sebastian Andrzej Siewior 2018-05-30 17:50 ` Matthias Fuchs
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).