| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a78b1bde-cd1b-0deb-9470-47a980787d2b@codeaurora.org>
Date: Wed, 27 Mar 2019 18:52:53 +0530
From: Mukesh Ojha <mojha@...eaurora.org>
To: Fabrice Gasnier <fabrice.gasnier@...com>, jic23@...nel.org
Cc: lars@...afoo.de, alexandre.torgue@...com,
linux-iio@...r.kernel.org, pmeerw@...erw.net,
linux-kernel@...r.kernel.org, mcoquelin.stm32@...il.com,
knaack.h@....de, linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] iio: adc: stm32: fix sleep inside atomic section when
using DMA
On 3/27/2019 6:21 PM, Mukesh Ojha wrote:
>
> On 3/26/2019 6:14 PM, Fabrice Gasnier wrote:
>> Enabling CONFIG_DEBUG_ATOMIC_SLEEP=y triggers this BUG message:
>> BUG: sleeping function called from invalid context at
>> kernel/irq/chip.c...
>>
>> Call stack is as follows:
>> - __might_sleep
>> - handle_nested_irq <-- Expects threaded irq
>> - iio_trigger_poll_chained
>> - stm32_adc_dma_buffer_done
>> - vchan_complete
>> - tasklet_action_common
>> - tasklet_action
>> - __do_softirq <-- DMA completion raises a tasklet
>> - irq_exit
>> - __handle_domain_irq <-- DMA IRQ
>> - gic_handle_irq
>>
>> IIO expects threaded interrupt context when calling:
>> - iio_trigger_poll_chained()
>> Or it expects interrupt context when calling:
>> - iio_trigger_poll()
>>
>> This patch triggers an IRQ upon stm32_adc_dma_buffer_done() DMA callback
>> call, so the IIO trigger poll API gets called from IRQ context.
>>
>> Fixes: 2763ea0585c9 ("iio: adc: stm32: add optional dma support")
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@...com>
> Reviewed-by: Mukesh Ojha <mojha@...eaurora.org>
>
> -Mukesh
>
>
>> ---
>> drivers/iio/adc/Kconfig | 1 +
>> drivers/iio/adc/stm32-adc.c | 32 ++++++++++++++++++++++++++++++--
>> 2 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 76db6e5..059407a 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -775,6 +775,7 @@ config STM32_ADC_CORE
>> select MFD_STM32_TIMERS
>> select IIO_STM32_TIMER_TRIGGER
>> select IIO_TRIGGERED_BUFFER
>> + select IRQ_WORK
>> help
>> Select this option to enable the core driver for
>> STMicroelectronics
>> STM32 analog-to-digital converter (ADC).
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> index 205e169..1aa3189 100644
>> --- a/drivers/iio/adc/stm32-adc.c
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -20,6 +20,7 @@
>> #include <linux/interrupt.h>
>> #include <linux/io.h>
>> #include <linux/iopoll.h>
>> +#include <linux/irq_work.h>
>> #include <linux/module.h>
>> #include <linux/platform_device.h>
>> #include <linux/pm_runtime.h>
>> @@ -297,6 +298,7 @@ struct stm32_adc_cfg {
>> * @smpr_val: sampling time settings (e.g. smpr1 / smpr2)
>> * @cal: optional calibration data on some devices
>> * @chan_name: channel name array
>> + * @work: irq work used to call trigger poll routine
>> */
>> struct stm32_adc {
>> struct stm32_adc_common *common;
>> @@ -320,6 +322,7 @@ struct stm32_adc {
>> u32 smpr_val[2];
>> struct stm32_adc_calib cal;
>> char chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
>> + struct irq_work work;
>> };
>> struct stm32_adc_diff_channel {
>> @@ -1473,11 +1476,32 @@ static unsigned int
>> stm32_adc_dma_residue(struct stm32_adc *adc)
>> return 0;
>> }
>> +static void stm32_adc_dma_irq_work(struct irq_work *work)
>> +{
>> + struct stm32_adc *adc = container_of(work, struct stm32_adc, work);
>> + struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>> +
>> + /*
>> + * iio_trigger_poll calls generic_handle_irq(). So, it requires
>> hard
>> + * irq context, and cannot be called directly from dma callback,
>> + * dma cb has to schedule this work instead.
>> + */
>> + iio_trigger_poll(indio_dev->trig);
>> +}
>> +
>> static void stm32_adc_dma_buffer_done(void *data)
>> {
>> struct iio_dev *indio_dev = data;
>> + struct stm32_adc *adc = iio_priv(indio_dev);
>> - iio_trigger_poll_chained(indio_dev->trig);
>> + /*
>> + * Invoques iio_trigger_poll() from hard irq context: We can't
s/Invoques/invoke
overlooked this last time.
Take mine Review tag if you make above change.
Reviewed-by: Mukesh Ojha <mojha@...eaurora.org>
-Mukesh
>> + * call iio_trigger_poll() nor iio_trigger_poll_chained()
>> + * directly from DMA callback (under tasklet e.g. softirq).
>> + * They require respectively HW IRQ and threaded IRQ context
>> + * as it might sleep.
>> + */
>> + irq_work_queue(&adc->work);
>> }
>> static int stm32_adc_dma_start(struct iio_dev *indio_dev)
>> @@ -1585,8 +1609,10 @@ static void
>> __stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
>> if (!adc->dma_chan)
>> stm32_adc_conv_irq_disable(adc);
>> - if (adc->dma_chan)
>> + if (adc->dma_chan) {
>> dmaengine_terminate_all(adc->dma_chan);
>> + irq_work_sync(&adc->work);
>> + }
>> if (stm32_adc_set_trig(indio_dev, NULL))
>> dev_err(&indio_dev->dev, "Can't clear trigger\n");
>> @@ -1872,6 +1898,8 @@ static int stm32_adc_dma_request(struct iio_dev
>> *indio_dev)
>> if (ret)
>> goto err_free;
>> + init_irq_work(&adc->work, stm32_adc_dma_irq_work);
>> +
>> return 0;
>> err_free:
Powered by blists - more mailing lists