All of lore.kernel.org
 help / color / mirror / Atom feed
* [rtc-linux] [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
@ 2016-02-18  8:06 ` Valentin Rothberg
  0 siblings, 0 replies; 14+ messages in thread
From: Valentin Rothberg @ 2016-02-18  8:06 UTC (permalink / raw)
  To: ldewangan
  Cc: kbuild-all, alexandre.belloni, javier, k.kozlowski, cw00.choi,
	a.zummo, linux-kernel, rtc-linux, kbuild test robot,
	Valentin Rothberg

From: kbuild test robot <fengguang.wu@intel.com>

 Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
 threaded IRQs without a primary handler need to be requested with
 IRQF_ONESHOT, otherwise the request will fail.

 So pass the IRQF_ONESHOT flag in this case.

Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci

CC: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
---
 drivers/rtc/rtc-max77686.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 5e924f3cde90..027ee7ebfff4 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -742,8 +742,8 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 		goto err_rtc;
 	}
 
-	ret = request_threaded_irq(info->virq, NULL, max77686_rtc_alarm_irq, 0,
-				   "rtc-alarm1", info);
+	ret = request_threaded_irq(info->virq, NULL, max77686_rtc_alarm_irq,
+				   IRQF_ONESHOT, "rtc-alarm1", info);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
 			info->virq, ret);
-- 
2.5.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
@ 2016-02-18  8:06 ` Valentin Rothberg
  0 siblings, 0 replies; 14+ messages in thread
From: Valentin Rothberg @ 2016-02-18  8:06 UTC (permalink / raw)
  To: ldewangan
  Cc: kbuild-all, alexandre.belloni, javier, k.kozlowski, cw00.choi,
	a.zummo, linux-kernel, rtc-linux, kbuild test robot,
	Valentin Rothberg

From: kbuild test robot <fengguang.wu@intel.com>

 Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
 threaded IRQs without a primary handler need to be requested with
 IRQF_ONESHOT, otherwise the request will fail.

 So pass the IRQF_ONESHOT flag in this case.

Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci

CC: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
---
 drivers/rtc/rtc-max77686.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 5e924f3cde90..027ee7ebfff4 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -742,8 +742,8 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 		goto err_rtc;
 	}
 
-	ret = request_threaded_irq(info->virq, NULL, max77686_rtc_alarm_irq, 0,
-				   "rtc-alarm1", info);
+	ret = request_threaded_irq(info->virq, NULL, max77686_rtc_alarm_irq,
+				   IRQF_ONESHOT, "rtc-alarm1", info);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
 			info->virq, ret);
-- 
2.5.0

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

* [rtc-linux] Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
  2016-02-18  8:06 ` Valentin Rothberg
@ 2016-02-18  8:13   ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-18  8:13 UTC (permalink / raw)
  To: Valentin Rothberg, ldewangan
  Cc: kbuild-all, alexandre.belloni, javier, cw00.choi, a.zummo,
	linux-kernel, rtc-linux, kbuild test robot

On 18.02.2016 17:06, Valentin Rothberg wrote:
> From: kbuild test robot <fengguang.wu@intel.com>
> 
>  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
>  threaded IRQs without a primary handler need to be requested with
>  IRQF_ONESHOT, otherwise the request will fail.
> 
>  So pass the IRQF_ONESHOT flag in this case.
> 
> Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
> 
> CC: Laxman Dewangan <ldewangan@nvidia.com>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
> ---
>  drivers/rtc/rtc-max77686.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Nack, because:
1. AFAIR this is a false positive.
2. Was it tested? Was it reproduced? Was the bug actually spotted or
just coccicheck pointed this and you assumed that "request will fail"?

Coccicheck is a great tool... but not necessarily for pointing run-time
bugs.

Best regards,
Krzysztof



-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

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

* Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
@ 2016-02-18  8:13   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-18  8:13 UTC (permalink / raw)
  To: Valentin Rothberg, ldewangan
  Cc: kbuild-all, alexandre.belloni, javier, cw00.choi, a.zummo,
	linux-kernel, rtc-linux, kbuild test robot

On 18.02.2016 17:06, Valentin Rothberg wrote:
> From: kbuild test robot <fengguang.wu@intel.com>
> 
>  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
>  threaded IRQs without a primary handler need to be requested with
>  IRQF_ONESHOT, otherwise the request will fail.
> 
>  So pass the IRQF_ONESHOT flag in this case.
> 
> Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
> 
> CC: Laxman Dewangan <ldewangan@nvidia.com>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
> ---
>  drivers/rtc/rtc-max77686.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Nack, because:
1. AFAIR this is a false positive.
2. Was it tested? Was it reproduced? Was the bug actually spotted or
just coccicheck pointed this and you assumed that "request will fail"?

Coccicheck is a great tool... but not necessarily for pointing run-time
bugs.

Best regards,
Krzysztof

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

* [rtc-linux] Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
  2016-02-18  8:13   ` Krzysztof Kozlowski
@ 2016-02-18  8:46     ` Valentin Rothberg
  -1 siblings, 0 replies; 14+ messages in thread
From: Valentin Rothberg @ 2016-02-18  8:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski, ldewangan
  Cc: kbuild-all, alexandre.belloni, javier, cw00.choi, a.zummo,
	linux-kernel, rtc-linux, kbuild test robot



Hi Krzysztof,

On 2/18/16 9:13 AM, Krzysztof Kozlowski wrote:
> On 18.02.2016 17:06, Valentin Rothberg wrote:
>> From: kbuild test robot <fengguang.wu@intel.com>
>>
>>  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
>>  threaded IRQs without a primary handler need to be requested with
>>  IRQF_ONESHOT, otherwise the request will fail.
>>
>>  So pass the IRQF_ONESHOT flag in this case.
>>
>> Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
>>
>> CC: Laxman Dewangan <ldewangan@nvidia.com>
>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>> Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
>> ---
>>  drivers/rtc/rtc-max77686.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
> 
> Nack, because:
> 1. AFAIR this is a false positive.

Looking at kernel/irq/manage.c +1250 such requests will be rejected
unconditionally when the primary handler is NULL, except when the chip
is marked to be oneshot safe.

Is there another semantic that I am not aware of?  In case the script
produces false positives, I will change it immediately.

> 2. Was it tested? Was it reproduced? Was the bug actually spotted or
> just coccicheck pointed this and you assumed that "request will fail"?
> 
> Coccicheck is a great tool... but not necessarily for pointing run-time
> bugs.

I did not test it.  To me the issue rather seems seems like something
where Coccinelle is really good at, static analysis.

Kind regards,
 Valentin

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

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

* Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
@ 2016-02-18  8:46     ` Valentin Rothberg
  0 siblings, 0 replies; 14+ messages in thread
From: Valentin Rothberg @ 2016-02-18  8:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski, ldewangan
  Cc: kbuild-all, alexandre.belloni, javier, cw00.choi, a.zummo,
	linux-kernel, rtc-linux, kbuild test robot



Hi Krzysztof,

On 2/18/16 9:13 AM, Krzysztof Kozlowski wrote:
> On 18.02.2016 17:06, Valentin Rothberg wrote:
>> From: kbuild test robot <fengguang.wu@intel.com>
>>
>>  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
>>  threaded IRQs without a primary handler need to be requested with
>>  IRQF_ONESHOT, otherwise the request will fail.
>>
>>  So pass the IRQF_ONESHOT flag in this case.
>>
>> Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
>>
>> CC: Laxman Dewangan <ldewangan@nvidia.com>
>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>> Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
>> ---
>>  drivers/rtc/rtc-max77686.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
> 
> Nack, because:
> 1. AFAIR this is a false positive.

Looking at kernel/irq/manage.c +1250 such requests will be rejected
unconditionally when the primary handler is NULL, except when the chip
is marked to be oneshot safe.

Is there another semantic that I am not aware of?  In case the script
produces false positives, I will change it immediately.

> 2. Was it tested? Was it reproduced? Was the bug actually spotted or
> just coccicheck pointed this and you assumed that "request will fail"?
> 
> Coccicheck is a great tool... but not necessarily for pointing run-time
> bugs.

I did not test it.  To me the issue rather seems seems like something
where Coccinelle is really good at, static analysis.

Kind regards,
 Valentin

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

* [rtc-linux] Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
  2016-02-18  8:46     ` Valentin Rothberg
@ 2016-02-18  8:50       ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-18  8:50 UTC (permalink / raw)
  To: Valentin Rothberg, ldewangan
  Cc: kbuild-all, alexandre.belloni, javier, cw00.choi, a.zummo,
	linux-kernel, rtc-linux, kbuild test robot

On 18.02.2016 17:46, Valentin Rothberg wrote:
> 
> 
> Hi Krzysztof,
> 
> On 2/18/16 9:13 AM, Krzysztof Kozlowski wrote:
>> On 18.02.2016 17:06, Valentin Rothberg wrote:
>>> From: kbuild test robot <fengguang.wu@intel.com>
>>>
>>>  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
>>>  threaded IRQs without a primary handler need to be requested with
>>>  IRQF_ONESHOT, otherwise the request will fail.
>>>
>>>  So pass the IRQF_ONESHOT flag in this case.
>>>
>>> Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
>>>
>>> CC: Laxman Dewangan <ldewangan@nvidia.com>
>>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>>> Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
>>> ---
>>>  drivers/rtc/rtc-max77686.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>
>> Nack, because:
>> 1. AFAIR this is a false positive.
> 
> Looking at kernel/irq/manage.c +1250 such requests will be rejected
> unconditionally when the primary handler is NULL, except when the chip
> is marked to be oneshot safe.
> 
> Is there another semantic that I am not aware of?  In case the script
> produces false positives, I will change it immediately.

The handler is "irq_nested_primary_handler".

>> 2. Was it tested? Was it reproduced? Was the bug actually spotted or
>> just coccicheck pointed this and you assumed that "request will fail"?
>>
>> Coccicheck is a great tool... but not necessarily for pointing run-time
>> bugs.
> 
> I did not test it.  To me the issue rather seems seems like something
> where Coccinelle is really good at, static analysis.

Yet, this is somehow subtle (device inter-dependencies) so it falls out
of static into runtime (I mean runtime analysis is needed).

Best regards,
Krzysztof

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

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

* Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
@ 2016-02-18  8:50       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-18  8:50 UTC (permalink / raw)
  To: Valentin Rothberg, ldewangan
  Cc: kbuild-all, alexandre.belloni, javier, cw00.choi, a.zummo,
	linux-kernel, rtc-linux, kbuild test robot

On 18.02.2016 17:46, Valentin Rothberg wrote:
> 
> 
> Hi Krzysztof,
> 
> On 2/18/16 9:13 AM, Krzysztof Kozlowski wrote:
>> On 18.02.2016 17:06, Valentin Rothberg wrote:
>>> From: kbuild test robot <fengguang.wu@intel.com>
>>>
>>>  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
>>>  threaded IRQs without a primary handler need to be requested with
>>>  IRQF_ONESHOT, otherwise the request will fail.
>>>
>>>  So pass the IRQF_ONESHOT flag in this case.
>>>
>>> Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
>>>
>>> CC: Laxman Dewangan <ldewangan@nvidia.com>
>>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>>> Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
>>> ---
>>>  drivers/rtc/rtc-max77686.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>
>> Nack, because:
>> 1. AFAIR this is a false positive.
> 
> Looking at kernel/irq/manage.c +1250 such requests will be rejected
> unconditionally when the primary handler is NULL, except when the chip
> is marked to be oneshot safe.
> 
> Is there another semantic that I am not aware of?  In case the script
> produces false positives, I will change it immediately.

The handler is "irq_nested_primary_handler".

>> 2. Was it tested? Was it reproduced? Was the bug actually spotted or
>> just coccicheck pointed this and you assumed that "request will fail"?
>>
>> Coccicheck is a great tool... but not necessarily for pointing run-time
>> bugs.
> 
> I did not test it.  To me the issue rather seems seems like something
> where Coccinelle is really good at, static analysis.

Yet, this is somehow subtle (device inter-dependencies) so it falls out
of static into runtime (I mean runtime analysis is needed).

Best regards,
Krzysztof

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

* [rtc-linux] Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
  2016-02-18  8:13   ` Krzysztof Kozlowski
@ 2016-02-18  8:51     ` Alexandre Belloni
  -1 siblings, 0 replies; 14+ messages in thread
From: Alexandre Belloni @ 2016-02-18  8:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Julia Lawall
  Cc: Valentin Rothberg, ldewangan, kbuild-all, javier, cw00.choi,
	a.zummo, linux-kernel, rtc-linux, kbuild test robot

On 18/02/2016 at 17:13:18 +0900, Krzysztof Kozlowski wrote :
> On 18.02.2016 17:06, Valentin Rothberg wrote:
> > From: kbuild test robot <fengguang.wu@intel.com>
> > 
> >  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
> >  threaded IRQs without a primary handler need to be requested with
> >  IRQF_ONESHOT, otherwise the request will fail.
> > 
> >  So pass the IRQF_ONESHOT flag in this case.
> > 
> > Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
> > 
> > CC: Laxman Dewangan <ldewangan@nvidia.com>
> > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> > Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
> > ---
> >  drivers/rtc/rtc-max77686.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> 
> Nack, because:
> 1. AFAIR this is a false positive.
> 2. Was it tested? Was it reproduced? Was the bug actually spotted or
> just coccicheck pointed this and you assumed that "request will fail"?
> 
> Coccicheck is a great tool... but not necessarily for pointing run-time
> bugs.
> 

Definitively a false positive.

Julia, I've been receiving quite a lot of those, is it possible to add a
note that this generates false positives to try to stop people from
blindly sending patches? I would have expected Valentin to know that
though.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

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

* Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
@ 2016-02-18  8:51     ` Alexandre Belloni
  0 siblings, 0 replies; 14+ messages in thread
From: Alexandre Belloni @ 2016-02-18  8:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Julia Lawall
  Cc: Valentin Rothberg, ldewangan, kbuild-all, javier, cw00.choi,
	a.zummo, linux-kernel, rtc-linux, kbuild test robot

On 18/02/2016 at 17:13:18 +0900, Krzysztof Kozlowski wrote :
> On 18.02.2016 17:06, Valentin Rothberg wrote:
> > From: kbuild test robot <fengguang.wu@intel.com>
> > 
> >  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
> >  threaded IRQs without a primary handler need to be requested with
> >  IRQF_ONESHOT, otherwise the request will fail.
> > 
> >  So pass the IRQF_ONESHOT flag in this case.
> > 
> > Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
> > 
> > CC: Laxman Dewangan <ldewangan@nvidia.com>
> > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> > Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
> > ---
> >  drivers/rtc/rtc-max77686.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> 
> Nack, because:
> 1. AFAIR this is a false positive.
> 2. Was it tested? Was it reproduced? Was the bug actually spotted or
> just coccicheck pointed this and you assumed that "request will fail"?
> 
> Coccicheck is a great tool... but not necessarily for pointing run-time
> bugs.
> 

Definitively a false positive.

Julia, I've been receiving quite a lot of those, is it possible to add a
note that this generates false positives to try to stop people from
blindly sending patches? I would have expected Valentin to know that
though.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

* [rtc-linux] Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
  2016-02-18  8:50       ` Krzysztof Kozlowski
@ 2016-02-18  9:03         ` Valentin Rothberg
  -1 siblings, 0 replies; 14+ messages in thread
From: Valentin Rothberg @ 2016-02-18  9:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski, ldewangan
  Cc: kbuild-all, alexandre.belloni, javier, cw00.choi, a.zummo,
	linux-kernel, rtc-linux, kbuild test robot

Hi Krzysztof,

On 2/18/16 9:50 AM, Krzysztof Kozlowski wrote:
> On 18.02.2016 17:46, Valentin Rothberg wrote:
>>
>>
>> Hi Krzysztof,
>>
>> On 2/18/16 9:13 AM, Krzysztof Kozlowski wrote:
>>> On 18.02.2016 17:06, Valentin Rothberg wrote:
>>>> From: kbuild test robot <fengguang.wu@intel.com>
>>>>
>>>>  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
>>>>  threaded IRQs without a primary handler need to be requested with
>>>>  IRQF_ONESHOT, otherwise the request will fail.
>>>>
>>>>  So pass the IRQF_ONESHOT flag in this case.
>>>>
>>>> Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
>>>>
>>>> CC: Laxman Dewangan <ldewangan@nvidia.com>
>>>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>>>> Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
>>>> ---
>>>>  drivers/rtc/rtc-max77686.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>
>>> Nack, because:
>>> 1. AFAIR this is a false positive.
>>
>> Looking at kernel/irq/manage.c +1250 such requests will be rejected
>> unconditionally when the primary handler is NULL, except when the chip
>> is marked to be oneshot safe.
>>
>> Is there another semantic that I am not aware of?  In case the script
>> produces false positives, I will change it immediately.
> 
> The handler is "irq_nested_primary_handler".
> 
>>> 2. Was it tested? Was it reproduced? Was the bug actually spotted or
>>> just coccicheck pointed this and you assumed that "request will fail"?
>>>
>>> Coccicheck is a great tool... but not necessarily for pointing run-time
>>> bugs.
>>
>> I did not test it.  To me the issue rather seems seems like something
>> where Coccinelle is really good at, static analysis.
> 
> Yet, this is somehow subtle (device inter-dependencies) so it falls out
> of static into runtime (I mean runtime analysis is needed).

Thanks for your answer.  I wasn't aware of this at all.

Best regards,
 Valentin

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

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

* Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
@ 2016-02-18  9:03         ` Valentin Rothberg
  0 siblings, 0 replies; 14+ messages in thread
From: Valentin Rothberg @ 2016-02-18  9:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski, ldewangan
  Cc: kbuild-all, alexandre.belloni, javier, cw00.choi, a.zummo,
	linux-kernel, rtc-linux, kbuild test robot

Hi Krzysztof,

On 2/18/16 9:50 AM, Krzysztof Kozlowski wrote:
> On 18.02.2016 17:46, Valentin Rothberg wrote:
>>
>>
>> Hi Krzysztof,
>>
>> On 2/18/16 9:13 AM, Krzysztof Kozlowski wrote:
>>> On 18.02.2016 17:06, Valentin Rothberg wrote:
>>>> From: kbuild test robot <fengguang.wu@intel.com>
>>>>
>>>>  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
>>>>  threaded IRQs without a primary handler need to be requested with
>>>>  IRQF_ONESHOT, otherwise the request will fail.
>>>>
>>>>  So pass the IRQF_ONESHOT flag in this case.
>>>>
>>>> Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
>>>>
>>>> CC: Laxman Dewangan <ldewangan@nvidia.com>
>>>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>>>> Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
>>>> ---
>>>>  drivers/rtc/rtc-max77686.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>
>>> Nack, because:
>>> 1. AFAIR this is a false positive.
>>
>> Looking at kernel/irq/manage.c +1250 such requests will be rejected
>> unconditionally when the primary handler is NULL, except when the chip
>> is marked to be oneshot safe.
>>
>> Is there another semantic that I am not aware of?  In case the script
>> produces false positives, I will change it immediately.
> 
> The handler is "irq_nested_primary_handler".
> 
>>> 2. Was it tested? Was it reproduced? Was the bug actually spotted or
>>> just coccicheck pointed this and you assumed that "request will fail"?
>>>
>>> Coccicheck is a great tool... but not necessarily for pointing run-time
>>> bugs.
>>
>> I did not test it.  To me the issue rather seems seems like something
>> where Coccinelle is really good at, static analysis.
> 
> Yet, this is somehow subtle (device inter-dependencies) so it falls out
> of static into runtime (I mean runtime analysis is needed).

Thanks for your answer.  I wasn't aware of this at all.

Best regards,
 Valentin

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

* [rtc-linux] Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
  2016-02-18  8:51     ` Alexandre Belloni
@ 2016-02-18  9:23       ` Valentin Rothberg
  -1 siblings, 0 replies; 14+ messages in thread
From: Valentin Rothberg @ 2016-02-18  9:23 UTC (permalink / raw)
  To: Alexandre Belloni, Krzysztof Kozlowski, Julia Lawall
  Cc: ldewangan, kbuild-all, javier, cw00.choi, a.zummo, linux-kernel,
	rtc-linux, kbuild test robot

Hi Alexandre,

On 2/18/16 9:51 AM, Alexandre Belloni wrote:
> On 18/02/2016 at 17:13:18 +0900, Krzysztof Kozlowski wrote :
>> On 18.02.2016 17:06, Valentin Rothberg wrote:
>>> From: kbuild test robot <fengguang.wu@intel.com>
>>>
>>>  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
>>>  threaded IRQs without a primary handler need to be requested with
>>>  IRQF_ONESHOT, otherwise the request will fail.
>>>
>>>  So pass the IRQF_ONESHOT flag in this case.
>>>
>>> Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
>>>
>>> CC: Laxman Dewangan <ldewangan@nvidia.com>
>>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>>> Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
>>> ---
>>>  drivers/rtc/rtc-max77686.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>
>> Nack, because:
>> 1. AFAIR this is a false positive.
>> 2. Was it tested? Was it reproduced? Was the bug actually spotted or
>> just coccicheck pointed this and you assumed that "request will fail"?
>>
>> Coccicheck is a great tool... but not necessarily for pointing run-time
>> bugs.
>>
> 
> Definitively a false positive.
> 
> Julia, I've been receiving quite a lot of those, is it possible to add a
> note that this generates false positives to try to stop people from
> blindly sending patches? I would have expected Valentin to know that
> though.

I don't have the device-specific knowledge for this issue.  It really
looked like a true positive to me, so I am sorry for the noise.

A warning about false positives seems promising.

Kind regards,
 Valentin

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

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

* Re: [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings
@ 2016-02-18  9:23       ` Valentin Rothberg
  0 siblings, 0 replies; 14+ messages in thread
From: Valentin Rothberg @ 2016-02-18  9:23 UTC (permalink / raw)
  To: Alexandre Belloni, Krzysztof Kozlowski, Julia Lawall
  Cc: ldewangan, kbuild-all, javier, cw00.choi, a.zummo, linux-kernel,
	rtc-linux, kbuild test robot

Hi Alexandre,

On 2/18/16 9:51 AM, Alexandre Belloni wrote:
> On 18/02/2016 at 17:13:18 +0900, Krzysztof Kozlowski wrote :
>> On 18.02.2016 17:06, Valentin Rothberg wrote:
>>> From: kbuild test robot <fengguang.wu@intel.com>
>>>
>>>  Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests")
>>>  threaded IRQs without a primary handler need to be requested with
>>>  IRQF_ONESHOT, otherwise the request will fail.
>>>
>>>  So pass the IRQF_ONESHOT flag in this case.
>>>
>>> Generated by: scripts/coccinelle/misc/irqf_oneshot.cocci
>>>
>>> CC: Laxman Dewangan <ldewangan@nvidia.com>
>>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>>> Signed-off-by: Valentin Rothberg <valentin.rothberg@posteo.net>
>>> ---
>>>  drivers/rtc/rtc-max77686.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>
>> Nack, because:
>> 1. AFAIR this is a false positive.
>> 2. Was it tested? Was it reproduced? Was the bug actually spotted or
>> just coccicheck pointed this and you assumed that "request will fail"?
>>
>> Coccicheck is a great tool... but not necessarily for pointing run-time
>> bugs.
>>
> 
> Definitively a false positive.
> 
> Julia, I've been receiving quite a lot of those, is it possible to add a
> note that this generates false positives to try to stop people from
> blindly sending patches? I would have expected Valentin to know that
> though.

I don't have the device-specific knowledge for this issue.  It really
looked like a true positive to me, so I am sorry for the noise.

A warning about false positives seems promising.

Kind regards,
 Valentin

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

end of thread, other threads:[~2016-02-18  9:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-18  8:06 [rtc-linux] [PATCH] rtc: max77686: fix irqf_oneshot.cocci warnings Valentin Rothberg
2016-02-18  8:06 ` Valentin Rothberg
2016-02-18  8:13 ` [rtc-linux] " Krzysztof Kozlowski
2016-02-18  8:13   ` Krzysztof Kozlowski
2016-02-18  8:46   ` [rtc-linux] " Valentin Rothberg
2016-02-18  8:46     ` Valentin Rothberg
2016-02-18  8:50     ` [rtc-linux] " Krzysztof Kozlowski
2016-02-18  8:50       ` Krzysztof Kozlowski
2016-02-18  9:03       ` [rtc-linux] " Valentin Rothberg
2016-02-18  9:03         ` Valentin Rothberg
2016-02-18  8:51   ` [rtc-linux] " Alexandre Belloni
2016-02-18  8:51     ` Alexandre Belloni
2016-02-18  9:23     ` [rtc-linux] " Valentin Rothberg
2016-02-18  9:23       ` Valentin Rothberg

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.