All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] drm: drm_vblank_cleanup: WARN when refcount is more than 0
@ 2017-10-12 17:00 PrasannaKumar Muralidharan
  2017-10-12 17:07 ` Chris Wilson
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: PrasannaKumar Muralidharan @ 2017-10-12 17:00 UTC (permalink / raw)
  To: daniel.vetter, intel-gfx; +Cc: PrasannaKumar Muralidharan

Warn when refcount > 0 in drm_vblank_cleanup.

In i915 driver unload drm_vblank_get is added to test whether this patch
is working. This will be removed once the patch gets tested.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
 drivers/gpu/drm/drm_vblank.c    | 2 ++
 drivers/gpu/drm/i915/i915_drv.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 70f2b95..2a6630a 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -402,6 +402,8 @@ void drm_vblank_cleanup(struct drm_device *dev)
 	if (dev->num_crtcs == 0)
 		return;
 
+	WARN_ON(atomic_read(&vblank->refcount) > 0);
+
 	for (pipe = 0; pipe < dev->num_crtcs; pipe++) {
 		struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9f45cfe..c7a93a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1373,6 +1373,7 @@ void i915_driver_unload(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct pci_dev *pdev = dev_priv->drm.pdev;
 
+	drm_vblank_get(dev_priv, 0);
 	i915_driver_unregister(dev_priv);
 
 	if (i915_gem_suspend(dev_priv))
-- 
2.10.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC] drm: drm_vblank_cleanup: WARN when refcount is more than 0
  2017-10-12 17:00 [RFC] drm: drm_vblank_cleanup: WARN when refcount is more than 0 PrasannaKumar Muralidharan
@ 2017-10-12 17:07 ` Chris Wilson
  2017-10-12 17:10   ` PrasannaKumar Muralidharan
  2017-10-12 17:21 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2017-10-12 17:22 ` [RFC] " Ville Syrjälä
  2 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2017-10-12 17:07 UTC (permalink / raw)
  To: daniel.vetter, intel-gfx; +Cc: PrasannaKumar Muralidharan

Quoting PrasannaKumar Muralidharan (2017-10-12 18:00:06)
> Warn when refcount > 0 in drm_vblank_cleanup.
> 
> In i915 driver unload drm_vblank_get is added to test whether this patch
> is working. This will be removed once the patch gets tested.

You want to send it as two patches. The first to add the debug
infrastructure, then the second to trigger it. We obviously don't want
to commit a patch that itself warns about the bug it introduces! ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC] drm: drm_vblank_cleanup: WARN when refcount is more than 0
  2017-10-12 17:07 ` Chris Wilson
@ 2017-10-12 17:10   ` PrasannaKumar Muralidharan
  0 siblings, 0 replies; 6+ messages in thread
From: PrasannaKumar Muralidharan @ 2017-10-12 17:10 UTC (permalink / raw)
  To: Chris Wilson; +Cc: daniel.vetter, intel-gfx

Hi Chris,

On 12 October 2017 at 22:37, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting PrasannaKumar Muralidharan (2017-10-12 18:00:06)
>> Warn when refcount > 0 in drm_vblank_cleanup.
>>
>> In i915 driver unload drm_vblank_get is added to test whether this patch
>> is working. This will be removed once the patch gets tested.
>
> You want to send it as two patches. The first to add the debug
> infrastructure, then the second to trigger it. We obviously don't want
> to commit a patch that itself warns about the bug it introduces! ;)
> -Chris

True. That's why I marked it as RFC. I hope this will trigger Intel
graphics driver CI so I can wait for the outcome and send a single
patch based on the result. Will that work?

Thanks,
PrasannaKumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.BAT: failure for drm: drm_vblank_cleanup: WARN when refcount is more than 0
  2017-10-12 17:00 [RFC] drm: drm_vblank_cleanup: WARN when refcount is more than 0 PrasannaKumar Muralidharan
  2017-10-12 17:07 ` Chris Wilson
@ 2017-10-12 17:21 ` Patchwork
  2017-10-12 17:22 ` [RFC] " Ville Syrjälä
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-10-12 17:21 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan; +Cc: intel-gfx

== Series Details ==

Series: drm: drm_vblank_cleanup: WARN when refcount is more than 0
URL   : https://patchwork.freedesktop.org/series/31838/
State : failure

== Summary ==

  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/bounds.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CHK     scripts/mod/devicetable-offsets.h
  CHK     include/generated/compile.h
  CHK     kernel/config_data.h
  CC      drivers/gpu/drm/drm_vblank.o
In file included from ./arch/x86/include/asm/bug.h:81:0,
                 from ./include/linux/bug.h:4,
                 from ./include/linux/thread_info.h:11,
                 from ./arch/x86/include/asm/preempt.h:6,
                 from ./include/linux/preempt.h:80,
                 from ./include/linux/spinlock.h:50,
                 from ./include/linux/seqlock.h:35,
                 from ./include/drm/drm_vblank.h:27,
                 from drivers/gpu/drm/drm_vblank.c:27:
drivers/gpu/drm/drm_vblank.c: In function ‘drm_vblank_cleanup’:
drivers/gpu/drm/drm_vblank.c:405:23: error: ‘vblank’ undeclared (first use in this function)
  WARN_ON(atomic_read(&vblank->refcount) > 0);
                       ^
./include/asm-generic/bug.h:107:25: note: in definition of macro ‘WARN_ON’
  int __ret_warn_on = !!(condition);    \
                         ^~~~~~~~~
drivers/gpu/drm/drm_vblank.c:405:23: note: each undeclared identifier is reported only once for each function it appears in
  WARN_ON(atomic_read(&vblank->refcount) > 0);
                       ^
./include/asm-generic/bug.h:107:25: note: in definition of macro ‘WARN_ON’
  int __ret_warn_on = !!(condition);    \
                         ^~~~~~~~~
scripts/Makefile.build:313: recipe for target 'drivers/gpu/drm/drm_vblank.o' failed
make[3]: *** [drivers/gpu/drm/drm_vblank.o] Error 1
scripts/Makefile.build:572: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:572: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1019: recipe for target 'drivers' failed
make: *** [drivers] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC] drm: drm_vblank_cleanup: WARN when refcount is more than 0
  2017-10-12 17:00 [RFC] drm: drm_vblank_cleanup: WARN when refcount is more than 0 PrasannaKumar Muralidharan
  2017-10-12 17:07 ` Chris Wilson
  2017-10-12 17:21 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2017-10-12 17:22 ` Ville Syrjälä
  2017-10-12 17:32   ` PrasannaKumar Muralidharan
  2 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2017-10-12 17:22 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan; +Cc: daniel.vetter, intel-gfx

On Thu, Oct 12, 2017 at 10:30:06PM +0530, PrasannaKumar Muralidharan wrote:
> Warn when refcount > 0 in drm_vblank_cleanup.
> 
> In i915 driver unload drm_vblank_get is added to test whether this patch
> is working. This will be removed once the patch gets tested.
> 
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
>  drivers/gpu/drm/drm_vblank.c    | 2 ++
>  drivers/gpu/drm/i915/i915_drv.c | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 70f2b95..2a6630a 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -402,6 +402,8 @@ void drm_vblank_cleanup(struct drm_device *dev)
>  	if (dev->num_crtcs == 0)
>  		return;
>  
> +	WARN_ON(atomic_read(&vblank->refcount) > 0);
> +

That won't even compile. Always at least compile + sparse check +
checkpatch your stuff, otherwise you're just going to be wasting
other people's or the CI systems's time.

>  	for (pipe = 0; pipe < dev->num_crtcs; pipe++) {
>  		struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9f45cfe..c7a93a9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1373,6 +1373,7 @@ void i915_driver_unload(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct pci_dev *pdev = dev_priv->drm.pdev;
>  
> +	drm_vblank_get(dev_priv, 0);
>  	i915_driver_unregister(dev_priv);
>  
>  	if (i915_gem_suspend(dev_priv))
> -- 
> 2.10.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [RFC] drm: drm_vblank_cleanup: WARN when refcount is more than 0
  2017-10-12 17:22 ` [RFC] " Ville Syrjälä
@ 2017-10-12 17:32   ` PrasannaKumar Muralidharan
  0 siblings, 0 replies; 6+ messages in thread
From: PrasannaKumar Muralidharan @ 2017-10-12 17:32 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: daniel.vetter, intel-gfx

Hi Ville,

On 12 October 2017 at 22:52, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Oct 12, 2017 at 10:30:06PM +0530, PrasannaKumar Muralidharan wrote:
>> Warn when refcount > 0 in drm_vblank_cleanup.
>>
>> In i915 driver unload drm_vblank_get is added to test whether this patch
>> is working. This will be removed once the patch gets tested.
>>
>> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
>> ---
>>  drivers/gpu/drm/drm_vblank.c    | 2 ++
>>  drivers/gpu/drm/i915/i915_drv.c | 1 +
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>> index 70f2b95..2a6630a 100644
>> --- a/drivers/gpu/drm/drm_vblank.c
>> +++ b/drivers/gpu/drm/drm_vblank.c
>> @@ -402,6 +402,8 @@ void drm_vblank_cleanup(struct drm_device *dev)
>>       if (dev->num_crtcs == 0)
>>               return;
>>
>> +     WARN_ON(atomic_read(&vblank->refcount) > 0);
>> +
>
> That won't even compile. Always at least compile + sparse check +
> checkpatch your stuff, otherwise you're just going to be wasting
> other people's or the CI systems's time.

CI just told me this. I will keep this in mind even for simple patches.

>>       for (pipe = 0; pipe < dev->num_crtcs; pipe++) {
>>               struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 9f45cfe..c7a93a9 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1373,6 +1373,7 @@ void i915_driver_unload(struct drm_device *dev)
>>       struct drm_i915_private *dev_priv = to_i915(dev);
>>       struct pci_dev *pdev = dev_priv->drm.pdev;
>>
>> +     drm_vblank_get(dev_priv, 0);
>>       i915_driver_unregister(dev_priv);
>>
>>       if (i915_gem_suspend(dev_priv))
>> --
>> 2.10.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2017-10-12 17:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-12 17:00 [RFC] drm: drm_vblank_cleanup: WARN when refcount is more than 0 PrasannaKumar Muralidharan
2017-10-12 17:07 ` Chris Wilson
2017-10-12 17:10   ` PrasannaKumar Muralidharan
2017-10-12 17:21 ` ✗ Fi.CI.BAT: failure for " Patchwork
2017-10-12 17:22 ` [RFC] " Ville Syrjälä
2017-10-12 17:32   ` PrasannaKumar Muralidharan

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.