2015-07-20 23:15:12

by Xander Huff

[permalink] [raw]
Subject: [PATCH v3] iio: adc: xilinx-xadc: Push interrupts into threaded context

The driver currently registers a pair of irq handlers using
request_threaded_irq(), however the synchronization mechanism between the
hardirq and the threadedirq handler is a regular spinlock.

Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
and is thus not able to be acquired from a hardirq handler. This patch gets
rid of the hardirq handler and pushes all interrupt handling into the
threaded context.

To validate that this change has no impact on RT performance, here are
cyclictest values with no processes running:

$ sudo cyclictest -S -m -p 98
# /dev/cpu_dma_latency set to 0us
policy: fifo: loadavg: 0.05 0.04 0.05 1/237 828
T: 0 ( 1861) P:98 I:1000 C:56925046 Min: 9 Act: 12 Avg: 12 Max: 71
T: 1 ( 1862) P:98 I:1500 C:37950030 Min: 9 Act: 12 Avg: 13 Max: 74

Then, all xadc raw handles were accessed in a continuous loop via
/sys/bus/iio/devices/iio:device0:

$ sudo cyclictest -S -m -p 98
# /dev/cpu_dma_latency set to 0us
policy: fifo: loadavg: 7.81 7.64 7.541 2/247 5751
T: 0 ( 1568) P:98 I:1000 C:23845515 Min: 11 Act: 22 Avg: 21 Max: 71
T: 1 ( 1569) P:98 I:1500 C:15897239 Min: 11 Act: 21 Avg: 22 Max: 68

Signed-off-by: Xander Huff <[email protected]>
---
drivers/iio/adc/xilinx-xadc-core.c | 37 ++++++++-----------------------------
1 file changed, 8 insertions(+), 29 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index ce93bd8..e16afdb 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -267,40 +267,15 @@ static void xadc_zynq_unmask_worker(struct work_struct *work)
xadc_zynq_update_intmsk(xadc, 0, 0);

spin_unlock_irq(&xadc->lock);
-
- /* if still pending some alarm re-trigger the timer */
- if (xadc->zynq_masked_alarm) {
- schedule_delayed_work(&xadc->zynq_unmask_work,
- msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
- }
}

static irqreturn_t xadc_zynq_threaded_interrupt_handler(int irq, void *devid)
{
struct iio_dev *indio_dev = devid;
struct xadc *xadc = iio_priv(indio_dev);
- unsigned int alarm;
-
- spin_lock_irq(&xadc->lock);
- alarm = xadc->zynq_alarm;
- xadc->zynq_alarm = 0;
- spin_unlock_irq(&xadc->lock);
-
- xadc_handle_events(indio_dev, xadc_zynq_transform_alarm(alarm));
-
- /* unmask the required interrupts in timer. */
- schedule_delayed_work(&xadc->zynq_unmask_work,
- msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
-
- return IRQ_HANDLED;
-}
-
-static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
-{
- struct iio_dev *indio_dev = devid;
- struct xadc *xadc = iio_priv(indio_dev);
irqreturn_t ret = IRQ_HANDLED;
uint32_t status;
+ unsigned int alarm;

xadc_read_reg(xadc, XADC_ZYNQ_REG_INTSTS, &status);

@@ -312,7 +287,6 @@ static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
spin_lock(&xadc->lock);

xadc_write_reg(xadc, XADC_ZYNQ_REG_INTSTS, status);
-
if (status & XADC_ZYNQ_INT_DFIFO_GTH) {
xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH,
XADC_ZYNQ_INT_DFIFO_GTH);
@@ -330,8 +304,14 @@ static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
xadc_zynq_update_intmsk(xadc, 0, 0);
ret = IRQ_WAKE_THREAD;
}
+
+ alarm = xadc->zynq_alarm;
+ xadc->zynq_alarm = 0;
+
spin_unlock(&xadc->lock);

+ xadc_handle_events(indio_dev, xadc_zynq_transform_alarm(alarm));
+
return ret;
}

@@ -436,7 +416,6 @@ static const struct xadc_ops xadc_zynq_ops = {
.write = xadc_zynq_write_adc_reg,
.setup = xadc_zynq_setup,
.get_dclk_rate = xadc_zynq_get_dclk_rate,
- .interrupt_handler = xadc_zynq_interrupt_handler,
.threaded_interrupt_handler = xadc_zynq_threaded_interrupt_handler,
.update_alarm = xadc_zynq_update_alarm,
};
@@ -1225,7 +1204,7 @@ static int xadc_probe(struct platform_device *pdev)
if (ret)
goto err_free_samplerate_trigger;

- ret = request_threaded_irq(irq, xadc->ops->interrupt_handler,
+ ret = request_threaded_irq(irq, NULL,
xadc->ops->threaded_interrupt_handler,
0, dev_name(&pdev->dev), indio_dev);
if (ret)
--
1.9.1


2015-07-24 12:39:03

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH v3] iio: adc: xilinx-xadc: Push interrupts into threaded context

Hi,

Sorry, but I don't think this patch has been sufficiently tested against a
mainline kernel. The driver wont even probe the way it is right now.

On 07/21/2015 01:14 AM, Xander Huff wrote:
> The driver currently registers a pair of irq handlers using
> request_threaded_irq(), however the synchronization mechanism between the
> hardirq and the threadedirq handler is a regular spinlock.

If everything runs in threaded context we don't really need the spinlock
anymore and can use the mutex throughout.

>
> Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
> and is thus not able to be acquired from a hardirq handler. This patch gets
> rid of the hardirq handler and pushes all interrupt handling into the
> threaded context.

We actually might as well run everything in the hardirq handler (which will
be threaded in PREEMPT_RT). The reason why we have the threaded handler is
because xadc_handle_event() used to sleep, but it doesn't do this anymore.

> Signed-off-by: Xander Huff <[email protected]>
> ---
> drivers/iio/adc/xilinx-xadc-core.c | 37 ++++++++-----------------------------
> 1 file changed, 8 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
> index ce93bd8..e16afdb 100644
> --- a/drivers/iio/adc/xilinx-xadc-core.c
> +++ b/drivers/iio/adc/xilinx-xadc-core.c
> @@ -267,40 +267,15 @@ static void xadc_zynq_unmask_worker(struct work_struct *work)
> xadc_zynq_update_intmsk(xadc, 0, 0);
>
> spin_unlock_irq(&xadc->lock);
> -
> - /* if still pending some alarm re-trigger the timer */
> - if (xadc->zynq_masked_alarm) {
> - schedule_delayed_work(&xadc->zynq_unmask_work,
> - msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
> - }
> }
>
> static irqreturn_t xadc_zynq_threaded_interrupt_handler(int irq, void *devid)
> {
> struct iio_dev *indio_dev = devid;
> struct xadc *xadc = iio_priv(indio_dev);
> - unsigned int alarm;
> -
> - spin_lock_irq(&xadc->lock);
> - alarm = xadc->zynq_alarm;
> - xadc->zynq_alarm = 0;
> - spin_unlock_irq(&xadc->lock);
> -
> - xadc_handle_events(indio_dev, xadc_zynq_transform_alarm(alarm));
> -
> - /* unmask the required interrupts in timer. */
> - schedule_delayed_work(&xadc->zynq_unmask_work,
> - msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
> -

With nobody scheduling the unmask worker interrupts will stay disabled
indefinitely after they fired once, that's not very useful.

> - return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
> -{
> - struct iio_dev *indio_dev = devid;
> - struct xadc *xadc = iio_priv(indio_dev);
> irqreturn_t ret = IRQ_HANDLED;
> uint32_t status;
> + unsigned int alarm;
>
> xadc_read_reg(xadc, XADC_ZYNQ_REG_INTSTS, &status);
>
> @@ -312,7 +287,6 @@ static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
> spin_lock(&xadc->lock);
>
> xadc_write_reg(xadc, XADC_ZYNQ_REG_INTSTS, status);
> -
> if (status & XADC_ZYNQ_INT_DFIFO_GTH) {
> xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH,
> XADC_ZYNQ_INT_DFIFO_GTH);
> @@ -330,8 +304,14 @@ static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
> xadc_zynq_update_intmsk(xadc, 0, 0);
> ret = IRQ_WAKE_THREAD;
> }
> +
> + alarm = xadc->zynq_alarm;
> + xadc->zynq_alarm = 0;

With all running in the same handler we don't need those anymore.

> +
> spin_unlock(&xadc->lock);
>
> + xadc_handle_events(indio_dev, xadc_zynq_transform_alarm(alarm));
> +
> return ret;
> }
>
> @@ -436,7 +416,6 @@ static const struct xadc_ops xadc_zynq_ops = {
> .write = xadc_zynq_write_adc_reg,
> .setup = xadc_zynq_setup,
> .get_dclk_rate = xadc_zynq_get_dclk_rate,
> - .interrupt_handler = xadc_zynq_interrupt_handler,

The corresponding field should be removed from the xadc_ops struct and then
you'll also notice that interrupts now don't work anymore for the AXI
interface version.

> .threaded_interrupt_handler = xadc_zynq_threaded_interrupt_handler,
> .update_alarm = xadc_zynq_update_alarm,
> };
> @@ -1225,7 +1204,7 @@ static int xadc_probe(struct platform_device *pdev)
> if (ret)
> goto err_free_samplerate_trigger;
>
> - ret = request_threaded_irq(irq, xadc->ops->interrupt_handler,
> + ret = request_threaded_irq(irq, NULL,
> xadc->ops->threaded_interrupt_handler,
> 0, dev_name(&pdev->dev), indio_dev);
> if (ret)
>

2015-08-03 20:19:29

by Xander Huff

[permalink] [raw]
Subject: Re: [PATCH v3] iio: adc: xilinx-xadc: Push interrupts into threaded context

On 7/24/2015 7:38 AM, Lars-Peter Clausen wrote:
> Hi,
>
> Sorry, but I don't think this patch has been sufficiently tested against a
> mainline kernel. The driver wont even probe the way it is right now.
>
Thanks for your responses. I'm not sure why it doesn't probe for you since I was
able to do a fair amount of tests with it on our device.

I'll get to work on your suggestions and hopefully having a v4 out sometime next
week.

--
Xander Huff
Staff Software Engineer
National Instruments

2015-08-04 05:34:56

by Shubhrajyoti Datta

[permalink] [raw]
Subject: Re: [PATCH v3] iio: adc: xilinx-xadc: Push interrupts into threaded context

On Fri, Jul 24, 2015 at 6:08 PM, Lars-Peter Clausen <[email protected]> wrote:
> Hi,
>
> Sorry, but I don't think this patch has been sufficiently tested against a
> mainline kernel. The driver wont even probe the way it is right now.
>
> On 07/21/2015 01:14 AM, Xander Huff wrote:
>>
>> The driver currently registers a pair of irq handlers using
>> request_threaded_irq(), however the synchronization mechanism between the
>> hardirq and the threadedirq handler is a regular spinlock.
>
>
> If everything runs in threaded context we don't really need the spinlock
> anymore and can use the mutex throughout.

that should be better from the performance point of view.

>
>>
>> Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
>> and is thus not able to be acquired from a hardirq handler. This patch
>> gets
>> rid of the hardirq handler and pushes all interrupt handling into the
>> threaded context.
>
>
> We actually might as well run everything in the hardirq handler (which will
> be threaded in PREEMPT_RT). The reason why we have the threaded handler is
> because xadc_handle_event() used to sleep, but it doesn't do this anymore.

The point is why have the hard irq. If we use hardirq then not mutex
can be used and spinlock will
be busy.

is there something i may be missing?
>
>

2015-08-04 08:01:42

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH v3] iio: adc: xilinx-xadc: Push interrupts into threaded context

On 08/03/2015 10:18 PM, Xander Huff wrote:
> On 7/24/2015 7:38 AM, Lars-Peter Clausen wrote:
>> Hi,
>>
>> Sorry, but I don't think this patch has been sufficiently tested against a
>> mainline kernel. The driver wont even probe the way it is right now.
>>
> Thanks for your responses. I'm not sure why it doesn't probe for you since I
> was able to do a fair amount of tests with it on our device.

genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 157
xadc: probe of f8007100.xadc failed with error -22

You can' request a threaded IRQ without having IRQF_ONESHOT set, since this
will effectively result in a IRQ storm.

>
> I'll get to work on your suggestions and hopefully having a v4 out sometime
> next week.
>

2015-08-04 08:05:17

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH v3] iio: adc: xilinx-xadc: Push interrupts into threaded context

On 08/04/2015 07:34 AM, Shubhrajyoti Datta wrote:
> On Fri, Jul 24, 2015 at 6:08 PM, Lars-Peter Clausen <[email protected]> wrote:
>> Hi,
>>
>> Sorry, but I don't think this patch has been sufficiently tested against a
>> mainline kernel. The driver wont even probe the way it is right now.
>>
>> On 07/21/2015 01:14 AM, Xander Huff wrote:
>>>
>>> The driver currently registers a pair of irq handlers using
>>> request_threaded_irq(), however the synchronization mechanism between the
>>> hardirq and the threadedirq handler is a regular spinlock.
>>
>>
>> If everything runs in threaded context we don't really need the spinlock
>> anymore and can use the mutex throughout.
>
> that should be better from the performance point of view.
>
>>
>>>
>>> Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
>>> and is thus not able to be acquired from a hardirq handler. This patch
>>> gets
>>> rid of the hardirq handler and pushes all interrupt handling into the
>>> threaded context.
>>
>>
>> We actually might as well run everything in the hardirq handler (which will
>> be threaded in PREEMPT_RT). The reason why we have the threaded handler is
>> because xadc_handle_event() used to sleep, but it doesn't do this anymore.
>
> The point is why have the hard irq. If we use hardirq then not mutex
> can be used and spinlock will
> be busy.

Well there is no need to use a threaded IRQ. The interrupt handler is quite
small and doesn't take too much time and doesn't have any delays or sleeps
in it either.

2015-08-07 03:55:51

by Shubhrajyoti Datta

[permalink] [raw]
Subject: Re: [PATCH v3] iio: adc: xilinx-xadc: Push interrupts into threaded context

On Tue, Aug 4, 2015 at 1:35 PM, Lars-Peter Clausen <[email protected]> wrote:
>
> Well there is no need to use a threaded IRQ. The interrupt handler is quite
> small and doesn't take too much time and doesn't have any delays or sleeps
> in it either.
>
>
Ok thanks for the explanation.

2015-08-11 23:01:55

by Xander Huff

[permalink] [raw]
Subject: [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq context

The driver currently registers a pair of irq handlers using
request_threaded_irq(), however the synchronization mechanism between the
hardirq and the threadedirq handler is a regular spinlock.

Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
and is thus not able to be acquired from a hardirq handler. This patch gets
rid of the threaded handler and pushes all interrupt handling into the
hardirq context, and uses request_irq().

To validate that this change has no impact on RT performance, here are
cyclictest values with no processes running:

$ sudo cyclictest -S -m -p 98
# /dev/cpu_dma_latency set to 0us
policy: fifo: loadavg: 0.00 0.01 0.05 1/174 2539
T: 0 ( 1405) P:98 I:1000 C:167010520 Min: 9 Act: 12 Avg: 12 Max: 75
T: 1 ( 1862) P:98 I:1500 C:111340339 Min: 9 Act: 12 Avg: 12 Max: 73

Then, all xadc raw handles were accessed in a continuous loop via
/sys/bus/iio/devices/iio:device0:

$ sudo cyclictest -S -m -p 98
# /dev/cpu_dma_latency set to 0us
policy: fifo: loadavg: 7.84 7.70 7.63 3/182 4260
T: 0 ( 2559) P:98 I:1000 C:241557018 Min: 11 Act: 18 Avg: 21 Max: 74
T: 1 ( 2560) P:98 I:1500 C:161038006 Min: 10 Act: 21 Avg: 20 Max: 73

Signed-off-by: Xander Huff <[email protected]>
---
drivers/iio/adc/xilinx-xadc-core.c | 37 ++++++++++---------------------------
drivers/iio/adc/xilinx-xadc.h | 2 --
2 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index ce93bd8..0370624 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -273,33 +273,13 @@ static void xadc_zynq_unmask_worker(struct work_struct *work)
schedule_delayed_work(&xadc->zynq_unmask_work,
msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
}
-}
-
-static irqreturn_t xadc_zynq_threaded_interrupt_handler(int irq, void *devid)
-{
- struct iio_dev *indio_dev = devid;
- struct xadc *xadc = iio_priv(indio_dev);
- unsigned int alarm;
-
- spin_lock_irq(&xadc->lock);
- alarm = xadc->zynq_alarm;
- xadc->zynq_alarm = 0;
- spin_unlock_irq(&xadc->lock);
-
- xadc_handle_events(indio_dev, xadc_zynq_transform_alarm(alarm));

- /* unmask the required interrupts in timer. */
- schedule_delayed_work(&xadc->zynq_unmask_work,
- msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
-
- return IRQ_HANDLED;
}

static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
{
struct iio_dev *indio_dev = devid;
struct xadc *xadc = iio_priv(indio_dev);
- irqreturn_t ret = IRQ_HANDLED;
uint32_t status;

xadc_read_reg(xadc, XADC_ZYNQ_REG_INTSTS, &status);
@@ -321,18 +301,23 @@ static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)

status &= XADC_ZYNQ_INT_ALARM_MASK;
if (status) {
- xadc->zynq_alarm |= status;
xadc->zynq_masked_alarm |= status;
/*
* mask the current event interrupt,
* unmask it when the interrupt is no more active.
*/
xadc_zynq_update_intmsk(xadc, 0, 0);
- ret = IRQ_WAKE_THREAD;
+
+ xadc_handle_events(indio_dev,
+ xadc_zynq_transform_alarm(status));
+
+ /* unmask the required interrupts in timer. */
+ schedule_delayed_work(&xadc->zynq_unmask_work,
+ msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
}
spin_unlock(&xadc->lock);

- return ret;
+ return IRQ_HANDLED;
}

#define XADC_ZYNQ_TCK_RATE_MAX 50000000
@@ -437,7 +422,6 @@ static const struct xadc_ops xadc_zynq_ops = {
.setup = xadc_zynq_setup,
.get_dclk_rate = xadc_zynq_get_dclk_rate,
.interrupt_handler = xadc_zynq_interrupt_handler,
- .threaded_interrupt_handler = xadc_zynq_threaded_interrupt_handler,
.update_alarm = xadc_zynq_update_alarm,
};

@@ -1225,9 +1209,8 @@ static int xadc_probe(struct platform_device *pdev)
if (ret)
goto err_free_samplerate_trigger;

- ret = request_threaded_irq(irq, xadc->ops->interrupt_handler,
- xadc->ops->threaded_interrupt_handler,
- 0, dev_name(&pdev->dev), indio_dev);
+ ret = request_irq(irq, xadc->ops->interrupt_handler, 0,
+ dev_name(&pdev->dev), indio_dev);
if (ret)
goto err_clk_disable_unprepare;

diff --git a/drivers/iio/adc/xilinx-xadc.h b/drivers/iio/adc/xilinx-xadc.h
index 54adc50..f6f0819 100644
--- a/drivers/iio/adc/xilinx-xadc.h
+++ b/drivers/iio/adc/xilinx-xadc.h
@@ -60,7 +60,6 @@ struct xadc {

enum xadc_external_mux_mode external_mux_mode;

- unsigned int zynq_alarm;
unsigned int zynq_masked_alarm;
unsigned int zynq_intmask;
struct delayed_work zynq_unmask_work;
@@ -79,7 +78,6 @@ struct xadc_ops {
void (*update_alarm)(struct xadc *, unsigned int);
unsigned long (*get_dclk_rate)(struct xadc *);
irqreturn_t (*interrupt_handler)(int, void *);
- irqreturn_t (*threaded_interrupt_handler)(int, void *);

unsigned int flags;
};
--
1.9.1

2015-08-12 15:17:43

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq context

On 08/12/2015 01:00 AM, Xander Huff wrote:
> The driver currently registers a pair of irq handlers using
> request_threaded_irq(), however the synchronization mechanism between the
> hardirq and the threadedirq handler is a regular spinlock.
>
> Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
> and is thus not able to be acquired from a hardirq handler. This patch gets
> rid of the threaded handler and pushes all interrupt handling into the
> hardirq context, and uses request_irq().
>
> To validate that this change has no impact on RT performance, here are
> cyclictest values with no processes running:
>
> $ sudo cyclictest -S -m -p 98
> # /dev/cpu_dma_latency set to 0us
> policy: fifo: loadavg: 0.00 0.01 0.05 1/174 2539
> T: 0 ( 1405) P:98 I:1000 C:167010520 Min: 9 Act: 12 Avg: 12 Max: 75
> T: 1 ( 1862) P:98 I:1500 C:111340339 Min: 9 Act: 12 Avg: 12 Max: 73
>
> Then, all xadc raw handles were accessed in a continuous loop via
> /sys/bus/iio/devices/iio:device0:
>
> $ sudo cyclictest -S -m -p 98
> # /dev/cpu_dma_latency set to 0us
> policy: fifo: loadavg: 7.84 7.70 7.63 3/182 4260
> T: 0 ( 2559) P:98 I:1000 C:241557018 Min: 11 Act: 18 Avg: 21 Max: 74
> T: 1 ( 2560) P:98 I:1500 C:161038006 Min: 10 Act: 21 Avg: 20 Max: 73
>
> Signed-off-by: Xander Huff <[email protected]>

Looks good, thanks.

Acked-by: Lars-Peter Clausen <[email protected]>

Subject: Re: [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq context

On 08/12/2015 05:17 PM, Lars-Peter Clausen wrote:
> On 08/12/2015 01:00 AM, Xander Huff wrote:
>> Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
>> and is thus not able to be acquired from a hardirq handler. This patch gets
>> rid of the threaded handler and pushes all interrupt handling into the
>> hardirq context, and uses request_irq().
>>
>> To validate that this change has no impact on RT performance, here are
>> cyclictest values with no processes running:
>
> Looks good, thanks.
>
> Acked-by: Lars-Peter Clausen <[email protected]>

Yes, I'm fine with the rework, too.

Sebastian

2015-08-15 19:55:28

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq context

On 12/08/15 17:33, Sebastian Andrzej Siewior wrote:
> On 08/12/2015 05:17 PM, Lars-Peter Clausen wrote:
>> On 08/12/2015 01:00 AM, Xander Huff wrote:
>>> Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
>>> and is thus not able to be acquired from a hardirq handler. This patch gets
>>> rid of the threaded handler and pushes all interrupt handling into the
>>> hardirq context, and uses request_irq().
>>>
>>> To validate that this change has no impact on RT performance, here are
>>> cyclictest values with no processes running:
>>
>> Looks good, thanks.
>>
>> Acked-by: Lars-Peter Clausen <[email protected]>
>
> Yes, I'm fine with the rework, too.
Formal Acked-by would be good but I'll take this on the basis of Lars' one
and that the code clearly should work from a read through.

Applied to the togreg branch of iio.git. Thanks for following through
the various twists of this one. Will initially be pushed out as testing.
Note this has almost certainly (unless Linus announces a delay) missed the
upcoming merge window so will be lined up for the 4.4 one.

Jonathan
>
> Sebastian
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>