Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] iio: trigger: sysfs: Disable irqs before calling
 iio_trigger_poll()
To:     Christian Eggers <ceggers@arri.de>
Cc:     Jonathan Cameron <jic23@kernel.org>, stable@vger.kernel.org,
        Hartmut Knaack <knaack.h@gmx.de>,
        Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
        linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20200727145714.4377-1-ceggers@arri.de>
 <4871626.01MspNxQH7@n95hx1g2>
 <a59d204e-aeb0-2649-5e6f-f07815713d1a@metafoo.de>
 <3847827.rc3nFVyU9p@n95hx1g2>
From:   Lars-Peter Clausen <lars@metafoo.de>
Message-ID: <f6fc5fbe-5c51-fe9a-670e-4b5201353356@metafoo.de>
Date:   Thu, 13 Aug 2020 09:23:24 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <3847827.rc3nFVyU9p@n95hx1g2>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 8/12/20 1:01 PM, Christian Eggers wrote:
> Hi Lars
>
> On Monday, 3 August 2020, 08:52:54 CEST, Lars-Peter Clausen wrote:
>> On 8/3/20 8:44 AM, Christian Eggers wrote:
>>> ...
>>> is my patch sufficient, or would you prefer a different solution?
>> The code in normal upstream is correct, there is no need to patch it
>> since iio_sysfs_trigger_work() always runs with IRQs disabled.
>>
>>>> Are you using a non-upstream kernel? Maybe a RT kernel?
>>> I use v5.4.<almost-latest>-rt
>> That explains it. Have a look at
>> 0200-irqwork-push-most-work-into-softirq-context.patch.
>>
>> The right fix for this issue is to add the following snippet to the RT
>> patchset.
>>
>> diff --git a/drivers/iio/trigger/iio-trig-sysfs.c
>> b/drivers/iio/trigger/iio-trig-sysfs.c
>> --- a/drivers/iio/trigger/iio-trig-sysfs.c
>> +++ b/drivers/iio/trigger/iio-trig-sysfs.c
>> @@ -161,6 +161,7 @@ static int iio_sysfs_trigger_probe(int id)
>>        iio_trigger_set_drvdata(t->trig, t);
>>
>>        init_irq_work(&t->work, iio_sysfs_trigger_work);
>> +    t->work.flags = IRQ_WORK_HARD_IRQ;
>>
>>        ret = iio_trigger_register(t->trig);
>>        if (ret)
> I can confirm that this works for iio-trig-sysfs on 5.4.54-rt32. Currently I
> do not use iio-trig-hrtimer, but if I remember correctly, the problem was also
> present there.

Similar story, I think. On mainline hrtimers run in hardirq mode by 
default, whereas in RT they run in softirq mode by default. So we 
haven't see the issue in mainline.

To fix this we need to explicitly specify that the IIO hrtimer always 
needs to run in hardirq mode by using the HRTIMER_MODE_REL_HARD flag. 
I'll send a patch.

> Do you want to apply your patch for mainline? In contrast to v5.4,
> IRQ_WORK_HARD_IRQ is already available there (moved to smp_types.h).
> Unfortunately I cannot test it on mainline for now, as my BSP stuff is not
> ported yet.

Sounds like a plan :)