All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/irq: Add irq as false detection
@ 2020-11-02 11:19 ` Tian Tao
  0 siblings, 0 replies; 16+ messages in thread
From: Tian Tao @ 2020-11-02 11:19 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
	dri-devel, linux-kernel

Add the detection of false for irq, so that the EINVAL is not
returned when dev->irq_enabled is false.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
---
 drivers/gpu/drm/drm_irq.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 09d6e9e..7537a3d 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -172,6 +172,9 @@ int drm_irq_uninstall(struct drm_device *dev)
 	bool irq_enabled;
 	int i;
 
+	if (!dev->irq_enabled || !dev)
+		return 0;
+
 	irq_enabled = dev->irq_enabled;
 	dev->irq_enabled = false;
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH] drm/irq: Add irq as false detection
@ 2020-11-02 11:19 ` Tian Tao
  0 siblings, 0 replies; 16+ messages in thread
From: Tian Tao @ 2020-11-02 11:19 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
	dri-devel, linux-kernel

Add the detection of false for irq, so that the EINVAL is not
returned when dev->irq_enabled is false.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
---
 drivers/gpu/drm/drm_irq.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 09d6e9e..7537a3d 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -172,6 +172,9 @@ int drm_irq_uninstall(struct drm_device *dev)
 	bool irq_enabled;
 	int i;
 
+	if (!dev->irq_enabled || !dev)
+		return 0;
+
 	irq_enabled = dev->irq_enabled;
 	dev->irq_enabled = false;
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* Re: [PATCH] drm/irq: Add irq as false detection
  2020-11-02 11:19 ` Tian Tao
@ 2020-11-02 12:09   ` Thomas Zimmermann
  -1 siblings, 0 replies; 16+ messages in thread
From: Thomas Zimmermann @ 2020-11-02 12:09 UTC (permalink / raw)
  To: Tian Tao, maarten.lankhorst, mripard, airlied, daniel, dri-devel,
	linux-kernel


[-- Attachment #1.1.1: Type: text/plain, Size: 1464 bytes --]

Hi

Am 02.11.20 um 12:19 schrieb Tian Tao:
> Add the detection of false for irq, so that the EINVAL is not
> returned when dev->irq_enabled is false.
> 
> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> ---
>  drivers/gpu/drm/drm_irq.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 09d6e9e..7537a3d 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -172,6 +172,9 @@ int drm_irq_uninstall(struct drm_device *dev)
>  	bool irq_enabled;
>  	int i;
>  
> +	if (!dev->irq_enabled || !dev)
> +		return 0;
> +

Dereferencing a pointer before NULL-checking it, is Yoda programming. :)
I'd just drop the test for !dev and assume that dev is always valid.

I suggest to change the int return value to void and return nothing.

Re-reading the actual implementation of this function, this location
might be too early. Further below there already is a test for
irq_enabled. Put a drm_WARN_ON around it and it should be fine. This way
the vblank handlers will be disabled and erroneous callers will see a
warning in the kernel's log.

Best regards
Thomas

>  	irq_enabled = dev->irq_enabled;
>  	dev->irq_enabled = false;
>  
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

[-- Attachment #1.1.2: OpenPGP_0x680DC11D530B7A23.asc --]
[-- Type: application/pgp-keys, Size: 4259 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH] drm/irq: Add irq as false detection
@ 2020-11-02 12:09   ` Thomas Zimmermann
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Zimmermann @ 2020-11-02 12:09 UTC (permalink / raw)
  To: Tian Tao, maarten.lankhorst, mripard, airlied, daniel, dri-devel,
	linux-kernel


[-- Attachment #1.1.1.1: Type: text/plain, Size: 1464 bytes --]

Hi

Am 02.11.20 um 12:19 schrieb Tian Tao:
> Add the detection of false for irq, so that the EINVAL is not
> returned when dev->irq_enabled is false.
> 
> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> ---
>  drivers/gpu/drm/drm_irq.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 09d6e9e..7537a3d 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -172,6 +172,9 @@ int drm_irq_uninstall(struct drm_device *dev)
>  	bool irq_enabled;
>  	int i;
>  
> +	if (!dev->irq_enabled || !dev)
> +		return 0;
> +

Dereferencing a pointer before NULL-checking it, is Yoda programming. :)
I'd just drop the test for !dev and assume that dev is always valid.

I suggest to change the int return value to void and return nothing.

Re-reading the actual implementation of this function, this location
might be too early. Further below there already is a test for
irq_enabled. Put a drm_WARN_ON around it and it should be fine. This way
the vblank handlers will be disabled and erroneous callers will see a
warning in the kernel's log.

Best regards
Thomas

>  	irq_enabled = dev->irq_enabled;
>  	dev->irq_enabled = false;
>  
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

[-- Attachment #1.1.1.2: OpenPGP_0x680DC11D530B7A23.asc --]
[-- Type: application/pgp-keys, Size: 4259 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH] drm/irq: Add irq as false detection
  2020-11-02 12:09   ` Thomas Zimmermann
@ 2020-11-02 12:32     ` tiantao (H)
  -1 siblings, 0 replies; 16+ messages in thread
From: tiantao (H) @ 2020-11-02 12:32 UTC (permalink / raw)
  To: Thomas Zimmermann, Tian Tao, maarten.lankhorst, mripard, airlied,
	daniel, dri-devel, linux-kernel



在 2020/11/2 20:09, Thomas Zimmermann 写道:
> Hi
> 
> Am 02.11.20 um 12:19 schrieb Tian Tao:
>> Add the detection of false for irq, so that the EINVAL is not
>> returned when dev->irq_enabled is false.
>>
>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
>> ---
>>   drivers/gpu/drm/drm_irq.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> index 09d6e9e..7537a3d 100644
>> --- a/drivers/gpu/drm/drm_irq.c
>> +++ b/drivers/gpu/drm/drm_irq.c
>> @@ -172,6 +172,9 @@ int drm_irq_uninstall(struct drm_device *dev)
>>   	bool irq_enabled;
>>   	int i;
>>   
>> +	if (!dev->irq_enabled || !dev)
>> +		return 0;
>> +
> 
> Dereferencing a pointer before NULL-checking it, is Yoda programming. :)
> I'd just drop the test for !dev and assume that dev is always valid.
> 
> I suggest to change the int return value to void and return nothing.
> 
> Re-reading the actual implementation of this function, this location
> might be too early. Further below there already is a test for
> irq_enabled. Put a drm_WARN_ON around it and it should be fine. This way
> the vblank handlers will be disabled and erroneous callers will see a
> warning in the kernel's log.
> 
thank you，I will send a new patch to fix that.
> Best regards
> Thomas
> 
>>   	irq_enabled = dev->irq_enabled;
>>   	dev->irq_enabled = false;
>>   
>>
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH] drm/irq: Add irq as false detection
@ 2020-11-02 12:32     ` tiantao (H)
  0 siblings, 0 replies; 16+ messages in thread
From: tiantao (H) @ 2020-11-02 12:32 UTC (permalink / raw)
  To: Thomas Zimmermann, Tian Tao, maarten.lankhorst, mripard, airlied,
	daniel, dri-devel, linux-kernel



在 2020/11/2 20:09, Thomas Zimmermann 写道:
> Hi
> 
> Am 02.11.20 um 12:19 schrieb Tian Tao:
>> Add the detection of false for irq, so that the EINVAL is not
>> returned when dev->irq_enabled is false.
>>
>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
>> ---
>>   drivers/gpu/drm/drm_irq.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> index 09d6e9e..7537a3d 100644
>> --- a/drivers/gpu/drm/drm_irq.c
>> +++ b/drivers/gpu/drm/drm_irq.c
>> @@ -172,6 +172,9 @@ int drm_irq_uninstall(struct drm_device *dev)
>>   	bool irq_enabled;
>>   	int i;
>>   
>> +	if (!dev->irq_enabled || !dev)
>> +		return 0;
>> +
> 
> Dereferencing a pointer before NULL-checking it, is Yoda programming. :)
> I'd just drop the test for !dev and assume that dev is always valid.
> 
> I suggest to change the int return value to void and return nothing.
> 
> Re-reading the actual implementation of this function, this location
> might be too early. Further below there already is a test for
> irq_enabled. Put a drm_WARN_ON around it and it should be fine. This way
> the vblank handlers will be disabled and erroneous callers will see a
> warning in the kernel's log.
> 
thank you，I will send a new patch to fix that.
> Best regards
> Thomas
> 
>>   	irq_enabled = dev->irq_enabled;
>>   	dev->irq_enabled = false;
>>   
>>
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH] drm/irq: Add irq as false detection
  2020-11-02 12:09   ` Thomas Zimmermann
@ 2020-11-02 12:40     ` Thomas Zimmermann
  -1 siblings, 0 replies; 16+ messages in thread
From: Thomas Zimmermann @ 2020-11-02 12:40 UTC (permalink / raw)
  To: Tian Tao, maarten.lankhorst, mripard, airlied, daniel, dri-devel,
	linux-kernel


[-- Attachment #1.1.1: Type: text/plain, Size: 2267 bytes --]

Hi

Am 02.11.20 um 13:09 schrieb Thomas Zimmermann:
> Hi
> 
> Am 02.11.20 um 12:19 schrieb Tian Tao:
>> Add the detection of false for irq, so that the EINVAL is not
>> returned when dev->irq_enabled is false.
>>
>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
>> ---
>>  drivers/gpu/drm/drm_irq.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> index 09d6e9e..7537a3d 100644
>> --- a/drivers/gpu/drm/drm_irq.c
>> +++ b/drivers/gpu/drm/drm_irq.c
>> @@ -172,6 +172,9 @@ int drm_irq_uninstall(struct drm_device *dev)
>>  	bool irq_enabled;
>>  	int i;
>>  
>> +	if (!dev->irq_enabled || !dev)
>> +		return 0;
>> +
> 
> Dereferencing a pointer before NULL-checking it, is Yoda programming. :)
> I'd just drop the test for !dev and assume that dev is always valid.
> 
> I suggest to change the int return value to void and return nothing.
> 
> Re-reading the actual implementation of this function, this location
> might be too early. Further below there already is a test for
> irq_enabled. Put a drm_WARN_ON around it and it should be fine. This way
> the vblank handlers will be disabled and erroneous callers will see a
> warning in the kernel's log.

In addition to these changes, you could also add a managed
implementation of drm_irq_install(). The canonical name should be
devm_drm_irq_install(). The function would call drm_irq_install() and
register a cleanup handler via devm_add_action(). Example code is at [1].

KMS drivers, such as hibmc, would then not have to bother about
uninstalling the DRM irq.

Best regards
Thomas

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_drv.c#n664


> 
> Best regards
> Thomas
> 
>>  	irq_enabled = dev->irq_enabled;
>>  	dev->irq_enabled = false;
>>  
>>
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

[-- Attachment #1.1.2: OpenPGP_0x680DC11D530B7A23.asc --]
[-- Type: application/pgp-keys, Size: 4259 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH] drm/irq: Add irq as false detection
@ 2020-11-02 12:40     ` Thomas Zimmermann
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Zimmermann @ 2020-11-02 12:40 UTC (permalink / raw)
  To: Tian Tao, maarten.lankhorst, mripard, airlied, daniel, dri-devel,
	linux-kernel


[-- Attachment #1.1.1.1: Type: text/plain, Size: 2267 bytes --]

Hi

Am 02.11.20 um 13:09 schrieb Thomas Zimmermann:
> Hi
> 
> Am 02.11.20 um 12:19 schrieb Tian Tao:
>> Add the detection of false for irq, so that the EINVAL is not
>> returned when dev->irq_enabled is false.
>>
>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
>> ---
>>  drivers/gpu/drm/drm_irq.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> index 09d6e9e..7537a3d 100644
>> --- a/drivers/gpu/drm/drm_irq.c
>> +++ b/drivers/gpu/drm/drm_irq.c
>> @@ -172,6 +172,9 @@ int drm_irq_uninstall(struct drm_device *dev)
>>  	bool irq_enabled;
>>  	int i;
>>  
>> +	if (!dev->irq_enabled || !dev)
>> +		return 0;
>> +
> 
> Dereferencing a pointer before NULL-checking it, is Yoda programming. :)
> I'd just drop the test for !dev and assume that dev is always valid.
> 
> I suggest to change the int return value to void and return nothing.
> 
> Re-reading the actual implementation of this function, this location
> might be too early. Further below there already is a test for
> irq_enabled. Put a drm_WARN_ON around it and it should be fine. This way
> the vblank handlers will be disabled and erroneous callers will see a
> warning in the kernel's log.

In addition to these changes, you could also add a managed
implementation of drm_irq_install(). The canonical name should be
devm_drm_irq_install(). The function would call drm_irq_install() and
register a cleanup handler via devm_add_action(). Example code is at [1].

KMS drivers, such as hibmc, would then not have to bother about
uninstalling the DRM irq.

Best regards
Thomas

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_drv.c#n664


> 
> Best regards
> Thomas
> 
>>  	irq_enabled = dev->irq_enabled;
>>  	dev->irq_enabled = false;
>>  
>>
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

[-- Attachment #1.1.1.2: OpenPGP_0x680DC11D530B7A23.asc --]
[-- Type: application/pgp-keys, Size: 4259 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH] drm/irq: Add irq as false detection
  2020-11-02 12:40     ` Thomas Zimmermann
@ 2020-11-02 12:51       ` Daniel Vetter
  -1 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-11-02 12:51 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Tian Tao, Maarten Lankhorst, Maxime Ripard, Dave Airlie,
	dri-devel, Linux Kernel Mailing List

On Mon, Nov 2, 2020 at 1:40 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 02.11.20 um 13:09 schrieb Thomas Zimmermann:
> > Hi
> >
> > Am 02.11.20 um 12:19 schrieb Tian Tao:
> >> Add the detection of false for irq, so that the EINVAL is not
> >> returned when dev->irq_enabled is false.
> >>
> >> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> >> ---
> >>  drivers/gpu/drm/drm_irq.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> >> index 09d6e9e..7537a3d 100644
> >> --- a/drivers/gpu/drm/drm_irq.c
> >> +++ b/drivers/gpu/drm/drm_irq.c
> >> @@ -172,6 +172,9 @@ int drm_irq_uninstall(struct drm_device *dev)
> >>      bool irq_enabled;
> >>      int i;
> >>
> >> +    if (!dev->irq_enabled || !dev)
> >> +            return 0;
> >> +
> >
> > Dereferencing a pointer before NULL-checking it, is Yoda programming. :)
> > I'd just drop the test for !dev and assume that dev is always valid.
> >
> > I suggest to change the int return value to void and return nothing.
> >
> > Re-reading the actual implementation of this function, this location
> > might be too early. Further below there already is a test for
> > irq_enabled. Put a drm_WARN_ON around it and it should be fine. This way
> > the vblank handlers will be disabled and erroneous callers will see a
> > warning in the kernel's log.
>
> In addition to these changes, you could also add a managed
> implementation of drm_irq_install(). The canonical name should be
> devm_drm_irq_install(). The function would call drm_irq_install() and
> register a cleanup handler via devm_add_action(). Example code is at [1].
>
> KMS drivers, such as hibmc, would then not have to bother about
> uninstalling the DRM irq.

Yup, devm_ is the right fix here imo, not trying to make the uninstall
hook resilient against drivers which can't keep track of stuff. So if
that's all there is, imo this patch is a bad idea, since we have
proper tools to make driver unloading easier on driver author's
nowadays.
-Daniel

> Best regards
> Thomas
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_drv.c#n664
>
>
> >
> > Best regards
> > Thomas
> >
> >>      irq_enabled = dev->irq_enabled;
> >>      dev->irq_enabled = false;
> >>
> >>
> >
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH] drm/irq: Add irq as false detection
@ 2020-11-02 12:51       ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-11-02 12:51 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Dave Airlie, Linux Kernel Mailing List, dri-devel, Tian Tao

On Mon, Nov 2, 2020 at 1:40 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 02.11.20 um 13:09 schrieb Thomas Zimmermann:
> > Hi
> >
> > Am 02.11.20 um 12:19 schrieb Tian Tao:
> >> Add the detection of false for irq, so that the EINVAL is not
> >> returned when dev->irq_enabled is false.
> >>
> >> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> >> ---
> >>  drivers/gpu/drm/drm_irq.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> >> index 09d6e9e..7537a3d 100644
> >> --- a/drivers/gpu/drm/drm_irq.c
> >> +++ b/drivers/gpu/drm/drm_irq.c
> >> @@ -172,6 +172,9 @@ int drm_irq_uninstall(struct drm_device *dev)
> >>      bool irq_enabled;
> >>      int i;
> >>
> >> +    if (!dev->irq_enabled || !dev)
> >> +            return 0;
> >> +
> >
> > Dereferencing a pointer before NULL-checking it, is Yoda programming. :)
> > I'd just drop the test for !dev and assume that dev is always valid.
> >
> > I suggest to change the int return value to void and return nothing.
> >
> > Re-reading the actual implementation of this function, this location
> > might be too early. Further below there already is a test for
> > irq_enabled. Put a drm_WARN_ON around it and it should be fine. This way
> > the vblank handlers will be disabled and erroneous callers will see a
> > warning in the kernel's log.
>
> In addition to these changes, you could also add a managed
> implementation of drm_irq_install(). The canonical name should be
> devm_drm_irq_install(). The function would call drm_irq_install() and
> register a cleanup handler via devm_add_action(). Example code is at [1].
>
> KMS drivers, such as hibmc, would then not have to bother about
> uninstalling the DRM irq.

Yup, devm_ is the right fix here imo, not trying to make the uninstall
hook resilient against drivers which can't keep track of stuff. So if
that's all there is, imo this patch is a bad idea, since we have
proper tools to make driver unloading easier on driver author's
nowadays.
-Daniel

> Best regards
> Thomas
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_drv.c#n664
>
>
> >
> > Best regards
> > Thomas
> >
> >>      irq_enabled = dev->irq_enabled;
> >>      dev->irq_enabled = false;
> >>
> >>
> >
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread
* [kbuild] Re: [PATCH] drm/irq: Add irq as false detection
  2020-11-02 11:19 ` Tian Tao
  (?)
  (?)
@ 2020-11-02 18:30   ` Dan Carpenter
  -1 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2020-11-02 18:30 UTC (permalink / raw)
  To: kbuild, Tian Tao, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, dri-devel, linux-kernel
  Cc: lkp, Dan Carpenter, kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2421 bytes --]

Hi Tian,

url:    https://github.com/0day-ci/linux/commits/Tian-Tao/drm-irq-Add-irq-as-false-detection/20201102-192137 
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git  3cea11cd5e3b00d91caf0b4730194039b45c5891
config: x86_64-randconfig-m001-20201102 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/gpu/drm/drm_irq.c:175 drm_irq_uninstall() warn: variable dereferenced before check 'dev' (see line 175)

Old smatch warnings:
drivers/gpu/drm/drm_irq.c:149 drm_irq_install() warn: 'irq' not released on lines: 133.

vim +/dev +175 drivers/gpu/drm/drm_irq.c

84b1fd103dbbe01 drivers/char/drm/drm_irq.c Dave Airlie    2007-07-11  169  int drm_irq_uninstall(struct drm_device *dev)
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  170  {
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  171  	unsigned long irqflags;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  172  	bool irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  173  	int i;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  174  
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02 @175  	if (!dev->irq_enabled || !dev)
                                                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^
This is checking "dev" after dereferencing it.  Can "dev" even be NULL?

fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  176  		return 0;
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  177  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  178  	irq_enabled = dev->irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  179  	dev->irq_enabled = false;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  180  
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  181  	/*
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  182  	 * Wake up any waiters so they don't hang. This is just to paper over

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org 

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30701 bytes --]

[-- Attachment #3: Type: text/plain, Size: 149 bytes --]

_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org

^ permalink raw reply	[flat|nested] 16+ messages in thread
* [kbuild] Re: [PATCH] drm/irq: Add irq as false detection
@ 2020-11-02 18:30   ` Dan Carpenter
  0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2020-11-02 18:30 UTC (permalink / raw)
  To: kbuild, Tian Tao, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, dri-devel, linux-kernel
  Cc: kbuild-all, lkp, Dan Carpenter

[-- Attachment #1: Type: text/plain, Size: 2421 bytes --]

Hi Tian,

url:    https://github.com/0day-ci/linux/commits/Tian-Tao/drm-irq-Add-irq-as-false-detection/20201102-192137 
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git  3cea11cd5e3b00d91caf0b4730194039b45c5891
config: x86_64-randconfig-m001-20201102 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/gpu/drm/drm_irq.c:175 drm_irq_uninstall() warn: variable dereferenced before check 'dev' (see line 175)

Old smatch warnings:
drivers/gpu/drm/drm_irq.c:149 drm_irq_install() warn: 'irq' not released on lines: 133.

vim +/dev +175 drivers/gpu/drm/drm_irq.c

84b1fd103dbbe01 drivers/char/drm/drm_irq.c Dave Airlie    2007-07-11  169  int drm_irq_uninstall(struct drm_device *dev)
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  170  {
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  171  	unsigned long irqflags;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  172  	bool irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  173  	int i;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  174  
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02 @175  	if (!dev->irq_enabled || !dev)
                                                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^
This is checking "dev" after dereferencing it.  Can "dev" even be NULL?

fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  176  		return 0;
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  177  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  178  	irq_enabled = dev->irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  179  	dev->irq_enabled = false;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  180  
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  181  	/*
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  182  	 * Wake up any waiters so they don't hang. This is just to paper over

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org 

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30701 bytes --]

[-- Attachment #3: Type: text/plain, Size: 149 bytes --]

_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org

[-- Attachment #4: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH] drm/irq: Add irq as false detection
@ 2020-11-02 18:30   ` Dan Carpenter
  0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2020-11-02 18:30 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 2627 bytes --]

Hi Tian,

url:    https://github.com/0day-ci/linux/commits/Tian-Tao/drm-irq-Add-irq-as-false-detection/20201102-192137 
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git  3cea11cd5e3b00d91caf0b4730194039b45c5891
config: x86_64-randconfig-m001-20201102 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/gpu/drm/drm_irq.c:175 drm_irq_uninstall() warn: variable dereferenced before check 'dev' (see line 175)

Old smatch warnings:
drivers/gpu/drm/drm_irq.c:149 drm_irq_install() warn: 'irq' not released on lines: 133.

vim +/dev +175 drivers/gpu/drm/drm_irq.c

84b1fd103dbbe01 drivers/char/drm/drm_irq.c Dave Airlie    2007-07-11  169  int drm_irq_uninstall(struct drm_device *dev)
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  170  {
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  171  	unsigned long irqflags;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  172  	bool irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  173  	int i;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  174  
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02 @175  	if (!dev->irq_enabled || !dev)
                                                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^
This is checking "dev" after dereferencing it.  Can "dev" even be NULL?

fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  176  		return 0;
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  177  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  178  	irq_enabled = dev->irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  179  	dev->irq_enabled = false;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  180  
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  181  	/*
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  182  	 * Wake up any waiters so they don't hang. This is just to paper over

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org 

_______________________________________________
kbuild mailing list -- kbuild(a)lists.01.org
To unsubscribe send an email to kbuild-leave(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30701 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread
* [kbuild] Re: [PATCH] drm/irq: Add irq as false detection
@ 2020-11-02 18:30   ` Dan Carpenter
  0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2020-11-02 18:30 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2627 bytes --]

Hi Tian,

url:    https://github.com/0day-ci/linux/commits/Tian-Tao/drm-irq-Add-irq-as-false-detection/20201102-192137 
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git  3cea11cd5e3b00d91caf0b4730194039b45c5891
config: x86_64-randconfig-m001-20201102 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/gpu/drm/drm_irq.c:175 drm_irq_uninstall() warn: variable dereferenced before check 'dev' (see line 175)

Old smatch warnings:
drivers/gpu/drm/drm_irq.c:149 drm_irq_install() warn: 'irq' not released on lines: 133.

vim +/dev +175 drivers/gpu/drm/drm_irq.c

84b1fd103dbbe01 drivers/char/drm/drm_irq.c Dave Airlie    2007-07-11  169  int drm_irq_uninstall(struct drm_device *dev)
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  170  {
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  171  	unsigned long irqflags;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  172  	bool irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  173  	int i;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  174  
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02 @175  	if (!dev->irq_enabled || !dev)
                                                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^
This is checking "dev" after dereferencing it.  Can "dev" even be NULL?

fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  176  		return 0;
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  177  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  178  	irq_enabled = dev->irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  179  	dev->irq_enabled = false;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  180  
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  181  	/*
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  182  	 * Wake up any waiters so they don't hang. This is just to paper over

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org 

_______________________________________________
kbuild mailing list -- kbuild(a)lists.01.org
To unsubscribe send an email to kbuild-leave(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30701 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH] drm/irq: Add irq as false detection
@ 2020-11-02 20:06 kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2020-11-02 20:06 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 8955 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <1604315990-56787-1-git-send-email-tiantao6@hisilicon.com>
References: <1604315990-56787-1-git-send-email-tiantao6@hisilicon.com>
TO: Tian Tao <tiantao6@hisilicon.com>
TO: maarten.lankhorst(a)linux.intel.com
TO: mripard(a)kernel.org
TO: tzimmermann(a)suse.de
TO: airlied(a)linux.ie
TO: daniel(a)ffwll.ch
TO: dri-devel(a)lists.freedesktop.org
TO: linux-kernel(a)vger.kernel.org

Hi Tian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.10-rc2 next-20201102]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Tian-Tao/drm-irq-Add-irq-as-false-detection/20201102-192137
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3cea11cd5e3b00d91caf0b4730194039b45c5891
:::::: branch date: 9 hours ago
:::::: commit date: 9 hours ago
compiler: nds32le-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


cppcheck possible warnings: (new ones prefixed by >>, may not real problems)

>> drivers/gpu/drm/drm_irq.c:175:7: warning: Either the condition '!dev' is redundant or there is possible null pointer dereference: dev. [nullPointerRedundantCheck]
    if (!dev->irq_enabled || !dev)
         ^
   drivers/gpu/drm/drm_irq.c:175:27: note: Assuming that condition '!dev' is not redundant
    if (!dev->irq_enabled || !dev)
                             ^
   drivers/gpu/drm/drm_irq.c:175:7: note: Null pointer dereference
    if (!dev->irq_enabled || !dev)
         ^

vim +175 drivers/gpu/drm/drm_irq.c

^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  152  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  153  /**
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  154   * drm_irq_uninstall - uninstall the IRQ handler
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  155   * @dev: DRM device
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  156   *
16584b204573ece drivers/gpu/drm/drm_irq.c  Daniel Vetter  2017-05-31  157   * Calls the driver's &drm_driver.irq_uninstall function and unregisters the IRQ
16584b204573ece drivers/gpu/drm/drm_irq.c  Daniel Vetter  2017-05-31  158   * handler.  This should only be called by drivers which used drm_irq_install()
16584b204573ece drivers/gpu/drm/drm_irq.c  Daniel Vetter  2017-05-31  159   * to set up their interrupt handler. Other drivers must only reset
ef40cbf9998528e drivers/gpu/drm/drm_irq.c  Daniel Vetter  2017-01-25  160   * &drm_device.irq_enabled to false.
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  161   *
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  162   * Note that for kernel modesetting drivers it is a bug if this function fails.
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  163   * The sanity checks are only to catch buggy user modesetting drivers which call
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  164   * the same function through an ioctl.
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  165   *
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  166   * Returns:
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  167   * Zero on success or a negative error code on failure.
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  168   */
84b1fd103dbbe01 drivers/char/drm/drm_irq.c Dave Airlie    2007-07-11  169  int drm_irq_uninstall(struct drm_device *dev)
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  170  {
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  171  	unsigned long irqflags;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  172  	bool irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  173  	int i;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  174  
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02 @175  	if (!dev->irq_enabled || !dev)
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  176  		return 0;
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  177  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  178  	irq_enabled = dev->irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  179  	dev->irq_enabled = false;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  180  
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  181  	/*
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  182  	 * Wake up any waiters so they don't hang. This is just to paper over
c78d1ca2d2e3e72 drivers/gpu/drm/drm_irq.c  Frank Binns    2016-06-24  183  	 * issues for UMS drivers which aren't in full control of their
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  184  	 * vblank/irq handling. KMS drivers must ensure that vblanks are all
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  185  	 * disabled when uninstalling the irq handler.
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  186  	 */
6015002ece38dac drivers/gpu/drm/drm_irq.c  Daniel Vetter  2020-05-27  187  	if (drm_dev_has_vblank(dev)) {
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  188  		spin_lock_irqsave(&dev->vbl_lock, irqflags);
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  189  		for (i = 0; i < dev->num_crtcs; i++) {
8a51d5bef07f1c8 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2014-08-06  190  			struct drm_vblank_crtc *vblank = &dev->vblank[i];
8a51d5bef07f1c8 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2014-08-06  191  
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  192  			if (!vblank->enabled)
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  193  				continue;
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  194  
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  195  			WARN_ON(drm_core_check_feature(dev, DRIVER_MODESET));
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  196  
3ed4351a83ca05d drivers/gpu/drm/drm_irq.c  Daniel Vetter  2017-05-31  197  			drm_vblank_disable_and_save(dev, i);
8a51d5bef07f1c8 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2014-08-06  198  			wake_up(&vblank->queue);
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  199  		}
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  200  		spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
bde4889aaa2d59c drivers/gpu/drm/drm_irq.c  Ben Skeggs     2011-07-04  201  	}
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  202  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  203  	if (!irq_enabled)
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  204  		return -EINVAL;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  205  
7c1a38e391745cc drivers/gpu/drm/drm_irq.c  Daniel Vetter  2013-12-16  206  	DRM_DEBUG("irq=%d\n", dev->irq);
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  207  
fa5386459f06dc3 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2016-08-03  208  	if (drm_core_check_feature(dev, DRIVER_LEGACY))
28d520433b63757 drivers/gpu/drm/drm_irq.c  Dave Airlie    2009-09-21  209  		vga_client_register(dev->pdev, NULL, NULL, NULL);
28d520433b63757 drivers/gpu/drm/drm_irq.c  Dave Airlie    2009-09-21  210  
5037f8acf448dd0 drivers/gpu/drm/drm_irq.c  Joonyoung Shim 2011-08-04  211  	if (dev->driver->irq_uninstall)
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  212  		dev->driver->irq_uninstall(dev);
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  213  
7c1a38e391745cc drivers/gpu/drm/drm_irq.c  Daniel Vetter  2013-12-16  214  	free_irq(dev->irq, dev);
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  215  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  216  	return 0;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  217  }
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  218  EXPORT_SYMBOL(drm_irq_uninstall);
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  219  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH] drm/irq: Add irq as false detection
@ 2020-11-02 18:05 kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2020-11-02 18:05 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 8756 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <1604315990-56787-1-git-send-email-tiantao6@hisilicon.com>
References: <1604315990-56787-1-git-send-email-tiantao6@hisilicon.com>
TO: Tian Tao <tiantao6@hisilicon.com>
TO: maarten.lankhorst(a)linux.intel.com
TO: mripard(a)kernel.org
TO: tzimmermann(a)suse.de
TO: airlied(a)linux.ie
TO: daniel(a)ffwll.ch
TO: dri-devel(a)lists.freedesktop.org
TO: linux-kernel(a)vger.kernel.org

Hi Tian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.10-rc2 next-20201102]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Tian-Tao/drm-irq-Add-irq-as-false-detection/20201102-192137
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3cea11cd5e3b00d91caf0b4730194039b45c5891
:::::: branch date: 7 hours ago
:::::: commit date: 7 hours ago
config: x86_64-randconfig-m001-20201102 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/gpu/drm/drm_irq.c:175 drm_irq_uninstall() warn: variable dereferenced before check 'dev' (see line 175)

Old smatch warnings:
drivers/gpu/drm/drm_irq.c:149 drm_irq_install() warn: 'irq' not released on lines: 133.

vim +/dev +175 drivers/gpu/drm/drm_irq.c

^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  152  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  153  /**
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  154   * drm_irq_uninstall - uninstall the IRQ handler
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  155   * @dev: DRM device
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  156   *
16584b204573ece drivers/gpu/drm/drm_irq.c  Daniel Vetter  2017-05-31  157   * Calls the driver's &drm_driver.irq_uninstall function and unregisters the IRQ
16584b204573ece drivers/gpu/drm/drm_irq.c  Daniel Vetter  2017-05-31  158   * handler.  This should only be called by drivers which used drm_irq_install()
16584b204573ece drivers/gpu/drm/drm_irq.c  Daniel Vetter  2017-05-31  159   * to set up their interrupt handler. Other drivers must only reset
ef40cbf9998528e drivers/gpu/drm/drm_irq.c  Daniel Vetter  2017-01-25  160   * &drm_device.irq_enabled to false.
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  161   *
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  162   * Note that for kernel modesetting drivers it is a bug if this function fails.
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  163   * The sanity checks are only to catch buggy user modesetting drivers which call
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  164   * the same function through an ioctl.
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  165   *
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  166   * Returns:
f5752b38970990f drivers/gpu/drm/drm_irq.c  Daniel Vetter  2014-05-08  167   * Zero on success or a negative error code on failure.
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  168   */
84b1fd103dbbe01 drivers/char/drm/drm_irq.c Dave Airlie    2007-07-11  169  int drm_irq_uninstall(struct drm_device *dev)
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  170  {
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  171  	unsigned long irqflags;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  172  	bool irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  173  	int i;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  174  
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02 @175  	if (!dev->irq_enabled || !dev)
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  176  		return 0;
fa889e8baa5377b drivers/gpu/drm/drm_irq.c  Tian Tao       2020-11-02  177  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  178  	irq_enabled = dev->irq_enabled;
4423843cde65232 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2013-10-04  179  	dev->irq_enabled = false;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  180  
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  181  	/*
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  182  	 * Wake up any waiters so they don't hang. This is just to paper over
c78d1ca2d2e3e72 drivers/gpu/drm/drm_irq.c  Frank Binns    2016-06-24  183  	 * issues for UMS drivers which aren't in full control of their
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  184  	 * vblank/irq handling. KMS drivers must ensure that vblanks are all
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  185  	 * disabled when uninstalling the irq handler.
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  186  	 */
6015002ece38dac drivers/gpu/drm/drm_irq.c  Daniel Vetter  2020-05-27  187  	if (drm_dev_has_vblank(dev)) {
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  188  		spin_lock_irqsave(&dev->vbl_lock, irqflags);
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  189  		for (i = 0; i < dev->num_crtcs; i++) {
8a51d5bef07f1c8 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2014-08-06  190  			struct drm_vblank_crtc *vblank = &dev->vblank[i];
8a51d5bef07f1c8 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2014-08-06  191  
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  192  			if (!vblank->enabled)
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  193  				continue;
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  194  
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  195  			WARN_ON(drm_core_check_feature(dev, DRIVER_MODESET));
3bff93d64cf59f0 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2015-02-22  196  
3ed4351a83ca05d drivers/gpu/drm/drm_irq.c  Daniel Vetter  2017-05-31  197  			drm_vblank_disable_and_save(dev, i);
8a51d5bef07f1c8 drivers/gpu/drm/drm_irq.c  Ville Syrjälä  2014-08-06  198  			wake_up(&vblank->queue);
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  199  		}
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  200  		spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
bde4889aaa2d59c drivers/gpu/drm/drm_irq.c  Ben Skeggs     2011-07-04  201  	}
dc1336ff4fe08ae drivers/gpu/drm/drm_irq.c  Jesse Barnes   2009-01-06  202  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  203  	if (!irq_enabled)
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  204  		return -EINVAL;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  205  
7c1a38e391745cc drivers/gpu/drm/drm_irq.c  Daniel Vetter  2013-12-16  206  	DRM_DEBUG("irq=%d\n", dev->irq);
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  207  
fa5386459f06dc3 drivers/gpu/drm/drm_irq.c  Daniel Vetter  2016-08-03  208  	if (drm_core_check_feature(dev, DRIVER_LEGACY))
28d520433b63757 drivers/gpu/drm/drm_irq.c  Dave Airlie    2009-09-21  209  		vga_client_register(dev->pdev, NULL, NULL, NULL);
28d520433b63757 drivers/gpu/drm/drm_irq.c  Dave Airlie    2009-09-21  210  
5037f8acf448dd0 drivers/gpu/drm/drm_irq.c  Joonyoung Shim 2011-08-04  211  	if (dev->driver->irq_uninstall)
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  212  		dev->driver->irq_uninstall(dev);
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  213  
7c1a38e391745cc drivers/gpu/drm/drm_irq.c  Daniel Vetter  2013-12-16  214  	free_irq(dev->irq, dev);
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  215  
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  216  	return 0;
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  217  }
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  218  EXPORT_SYMBOL(drm_irq_uninstall);
^1da177e4c3f415 drivers/char/drm/drm_irq.c Linus Torvalds 2005-04-16  219  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30701 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-11-02 20:06 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-02 11:19 [PATCH] drm/irq: Add irq as false detection Tian Tao
2020-11-02 11:19 ` Tian Tao
2020-11-02 12:09 ` Thomas Zimmermann
2020-11-02 12:09   ` Thomas Zimmermann
2020-11-02 12:32   ` tiantao (H)
2020-11-02 12:32     ` tiantao (H)
2020-11-02 12:40   ` Thomas Zimmermann
2020-11-02 12:40     ` Thomas Zimmermann
2020-11-02 12:51     ` Daniel Vetter
2020-11-02 12:51       ` Daniel Vetter
2020-11-02 18:30 ` [kbuild] " Dan Carpenter
2020-11-02 18:30   ` Dan Carpenter
2020-11-02 18:30   ` Dan Carpenter
2020-11-02 18:30   ` [kbuild] " Dan Carpenter
2020-11-02 18:05 kernel test robot
2020-11-02 20:06 kernel test robot

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.