[PATCH v2 1/2] drm: drm_vblank_cleanup: WARN when refcount > 0

[PATCH v2 1/2] drm: drm_vblank_cleanup: WARN when refcount > 0

[PrasannaKumar Muralidharan]

Warn when refcount > 0 in drm_vblank_cleanup.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
No change in v2.

 drivers/gpu/drm/drm_vblank.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 70f2b95..3e61aeb 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -405,6 +405,8 @@ void drm_vblank_cleanup(struct drm_device *dev)
 	for (pipe = 0; pipe < dev->num_crtcs; pipe++) {
 		struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
 
+		WARN_ON(atomic_read(&vblank->refcount) > 0);
+
 		WARN_ON(READ_ONCE(vblank->enabled) &&
 			drm_core_check_feature(dev, DRIVER_MODESET));
 
-- 
2.10.0

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

[PATCH v2 2/2] Test case for drm_vblank_cleanup refcount validation patch

[PrasannaKumar Muralidharan]

In i915 driver unload drm_vblank_get is added to test whether
drm_vblank_cleanup refcount validation patch is working.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
Changes in v2:
Use drm_crtc_vblank_get instead of _put. In previous patch _put was wrongly
used.

 drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9f45cfe..4aee1c0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1373,6 +1373,13 @@ 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;
 
+	enum pipe pipe;
+	for_each_pipe(dev_priv, pipe) {
+		struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
+								  pipe);
+		drm_crtc_vblank_get(&crtc->base);
+	}
+
 	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

Re: [PATCH v2 2/2] Test case for drm_vblank_cleanup refcount validation patch

[PrasannaKumar Muralidharan]

Hi All,

On 24 October 2017 at 22:18, PrasannaKumar Muralidharan
<prasannatsmkumar@gmail.com> wrote:
> In i915 driver unload drm_vblank_get is added to test whether
> drm_vblank_cleanup refcount validation patch is working.
>
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
> Changes in v2:
> Use drm_crtc_vblank_get instead of _put. In previous patch _put was wrongly
> used.
>
>  drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9f45cfe..4aee1c0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1373,6 +1373,13 @@ 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;
>
> +       enum pipe pipe;
> +       for_each_pipe(dev_priv, pipe) {
> +               struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
> +                                                                 pipe);
> +               drm_crtc_vblank_get(&crtc->base);
> +       }
> +
>         i915_driver_unregister(dev_priv);
>
>         if (i915_gem_suspend(dev_priv))
> --
> 2.10.0
>

From https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6167/fi-ilk-650/igt@drv_module_reload@basic-reload.html
it can be seen that this patch triggers warning when vblank->refcount
> 0 in vblank cleanup. This tests patch 1 successfully.

I think patch 1 is good to go.

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

Re: [Intel-gfx] [PATCH v2 2/2] Test case for drm_vblank_cleanup refcount validation patch

[Daniel Vetter]

On Wed, Oct 25, 2017 at 08:44:45PM +0530, PrasannaKumar Muralidharan wrote:
> Hi All,
> 
> On 24 October 2017 at 22:18, PrasannaKumar Muralidharan
> <prasannatsmkumar@gmail.com> wrote:
> > In i915 driver unload drm_vblank_get is added to test whether
> > drm_vblank_cleanup refcount validation patch is working.
> >
> > Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> > ---
> > Changes in v2:
> > Use drm_crtc_vblank_get instead of _put. In previous patch _put was wrongly
> > used.
> >
> >  drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 9f45cfe..4aee1c0 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1373,6 +1373,13 @@ 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;
> >
> > +       enum pipe pipe;
> > +       for_each_pipe(dev_priv, pipe) {
> > +               struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
> > +                                                                 pipe);
> > +               drm_crtc_vblank_get(&crtc->base);
> > +       }
> > +
> >         i915_driver_unregister(dev_priv);
> >
> >         if (i915_gem_suspend(dev_priv))
> > --
> > 2.10.0
> >
> 
> From https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6167/fi-ilk-650/igt@drv_module_reload@basic-reload.html
> it can be seen that this patch triggers warning when vblank->refcount
> > 0 in vblank cleanup. This tests patch 1 successfully.
> 
> I think patch 1 is good to go.

Yeah it works, but we don't know whether it breaks anything yet. Can you
pls resubmit just the first patch? Abusing CI was just an idea to get it
fully tested, before we can merge it still needs to pass full CI. We just
had an issue 2 weeks ago where CI blew up because an untested patch landed
that broke every test :-/
-Daniel
-- 
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

Re: [PATCH v2 2/2] Test case for drm_vblank_cleanup refcount validation patch

[PrasannaKumar Muralidharan]

Hi Daniel,

On 30 October 2017 at 15:40, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Oct 25, 2017 at 08:44:45PM +0530, PrasannaKumar Muralidharan wrote:
>> Hi All,
>>
>> On 24 October 2017 at 22:18, PrasannaKumar Muralidharan
>> <prasannatsmkumar@gmail.com> wrote:
>> > In i915 driver unload drm_vblank_get is added to test whether
>> > drm_vblank_cleanup refcount validation patch is working.
>> >
>> > Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
>> > ---
>> > Changes in v2:
>> > Use drm_crtc_vblank_get instead of _put. In previous patch _put was wrongly
>> > used.
>> >
>> >  drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
>> >  1 file changed, 7 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> > index 9f45cfe..4aee1c0 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.c
>> > +++ b/drivers/gpu/drm/i915/i915_drv.c
>> > @@ -1373,6 +1373,13 @@ 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;
>> >
>> > +       enum pipe pipe;
>> > +       for_each_pipe(dev_priv, pipe) {
>> > +               struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
>> > +                                                                 pipe);
>> > +               drm_crtc_vblank_get(&crtc->base);
>> > +       }
>> > +
>> >         i915_driver_unregister(dev_priv);
>> >
>> >         if (i915_gem_suspend(dev_priv))
>> > --
>> > 2.10.0
>> >
>>
>> From https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6167/fi-ilk-650/igt@drv_module_reload@basic-reload.html
>> it can be seen that this patch triggers warning when vblank->refcount
>> > 0 in vblank cleanup. This tests patch 1 successfully.
>>
>> I think patch 1 is good to go.
>
> Yeah it works, but we don't know whether it breaks anything yet. Can you
> pls resubmit just the first patch? Abusing CI was just an idea to get it
> fully tested, before we can merge it still needs to pass full CI. We just
> had an issue 2 weeks ago where CI blew up because an untested patch landed
> that broke every test :-/
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

I have already sent that patch alone. Please look at
https://patchwork.freedesktop.org/patch/184866/.

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

Re: [PATCH v2 2/2] Test case for drm_vblank_cleanup refcount validation patch

[Daniel Vetter]

On Mon, Oct 30, 2017 at 06:01:12PM +0530, PrasannaKumar Muralidharan wrote:
> Hi Daniel,
> 
> On 30 October 2017 at 15:40, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Oct 25, 2017 at 08:44:45PM +0530, PrasannaKumar Muralidharan wrote:
> >> Hi All,
> >>
> >> On 24 October 2017 at 22:18, PrasannaKumar Muralidharan
> >> <prasannatsmkumar@gmail.com> wrote:
> >> > In i915 driver unload drm_vblank_get is added to test whether
> >> > drm_vblank_cleanup refcount validation patch is working.
> >> >
> >> > Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> >> > ---
> >> > Changes in v2:
> >> > Use drm_crtc_vblank_get instead of _put. In previous patch _put was wrongly
> >> > used.
> >> >
> >> >  drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
> >> >  1 file changed, 7 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> > index 9f45cfe..4aee1c0 100644
> >> > --- a/drivers/gpu/drm/i915/i915_drv.c
> >> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> > @@ -1373,6 +1373,13 @@ 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;
> >> >
> >> > +       enum pipe pipe;
> >> > +       for_each_pipe(dev_priv, pipe) {
> >> > +               struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
> >> > +                                                                 pipe);
> >> > +               drm_crtc_vblank_get(&crtc->base);
> >> > +       }
> >> > +
> >> >         i915_driver_unregister(dev_priv);
> >> >
> >> >         if (i915_gem_suspend(dev_priv))
> >> > --
> >> > 2.10.0
> >> >
> >>
> >> From https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6167/fi-ilk-650/igt@drv_module_reload@basic-reload.html
> >> it can be seen that this patch triggers warning when vblank->refcount
> >> > 0 in vblank cleanup. This tests patch 1 successfully.
> >>
> >> I think patch 1 is good to go.
> >
> > Yeah it works, but we don't know whether it breaks anything yet. Can you
> > pls resubmit just the first patch? Abusing CI was just an idea to get it
> > fully tested, before we can merge it still needs to pass full CI. We just
> > had an issue 2 weeks ago where CI blew up because an untested patch landed
> > that broke every test :-/
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> I have already sent that patch alone. Please look at
> https://patchwork.freedesktop.org/patch/184866/.

Seems to fail in CI:

https://patchwork.freedesktop.org/series/32648/

I guess you need to test this on a local box somewhere to figure out
what's wrong.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] Test case for drm_vblank_cleanup refcount validation patch

[PrasannaKumar Muralidharan]

Hi Daniel,

On 31 October 2017 at 15:51, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, Oct 30, 2017 at 06:01:12PM +0530, PrasannaKumar Muralidharan wrote:
> > Hi Daniel,
> >
> > On 30 October 2017 at 15:40, Daniel Vetter <daniel@ffwll.ch> wrote:
> > > On Wed, Oct 25, 2017 at 08:44:45PM +0530, PrasannaKumar Muralidharan wrote:
> > >> Hi All,
> > >>
> > >> On 24 October 2017 at 22:18, PrasannaKumar Muralidharan
> > >> <prasannatsmkumar@gmail.com> wrote:
> > >> > In i915 driver unload drm_vblank_get is added to test whether
> > >> > drm_vblank_cleanup refcount validation patch is working.
> > >> >
> > >> > Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> > >> > ---
> > >> > Changes in v2:
> > >> > Use drm_crtc_vblank_get instead of _put. In previous patch _put was wrongly
> > >> > used.
> > >> >
> > >> >  drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
> > >> >  1 file changed, 7 insertions(+)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > >> > index 9f45cfe..4aee1c0 100644
> > >> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > >> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > >> > @@ -1373,6 +1373,13 @@ 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;
> > >> >
> > >> > +       enum pipe pipe;
> > >> > +       for_each_pipe(dev_priv, pipe) {
> > >> > +               struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
> > >> > +                                                                 pipe);
> > >> > +               drm_crtc_vblank_get(&crtc->base);
> > >> > +       }
> > >> > +
> > >> >         i915_driver_unregister(dev_priv);
> > >> >
> > >> >         if (i915_gem_suspend(dev_priv))
> > >> > --
> > >> > 2.10.0
> > >> >
> > >>
> > >> From https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6167/fi-ilk-650/igt@drv_module_reload@basic-reload.html
> > >> it can be seen that this patch triggers warning when vblank->refcount
> > >> > 0 in vblank cleanup. This tests patch 1 successfully.
> > >>
> > >> I think patch 1 is good to go.
> > >
> > > Yeah it works, but we don't know whether it breaks anything yet. Can you
> > > pls resubmit just the first patch? Abusing CI was just an idea to get it
> > > fully tested, before we can merge it still needs to pass full CI. We just
> > > had an issue 2 weeks ago where CI blew up because an untested patch landed
> > > that broke every test :-/
> > > -Daniel
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> >
> > I have already sent that patch alone. Please look at
> > https://patchwork.freedesktop.org/patch/184866/.
>
> Seems to fail in CI:
>
> https://patchwork.freedesktop.org/series/32648/
>
> I guess you need to test this on a local box somewhere to figure out
> what's wrong.

My patch is supposed to catch problem with drivers. It warns when
vblank refcount is non-zero in drm_vblank_cleanup call. From CI log it
can be seen that warning being triggered. I feel that my patch is
exposing existing issue.

If I misinterpreted something please let me know.

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

Re: [PATCH v2 2/2] Test case for drm_vblank_cleanup refcount validation patch

[Daniel Vetter]

On Tue, Oct 31, 2017 at 08:37:21PM +0530, PrasannaKumar Muralidharan wrote:
> Hi Daniel,
> 
> On 31 October 2017 at 15:51, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Mon, Oct 30, 2017 at 06:01:12PM +0530, PrasannaKumar Muralidharan wrote:
> > > Hi Daniel,
> > >
> > > On 30 October 2017 at 15:40, Daniel Vetter <daniel@ffwll.ch> wrote:
> > > > On Wed, Oct 25, 2017 at 08:44:45PM +0530, PrasannaKumar Muralidharan wrote:
> > > >> Hi All,
> > > >>
> > > >> On 24 October 2017 at 22:18, PrasannaKumar Muralidharan
> > > >> <prasannatsmkumar@gmail.com> wrote:
> > > >> > In i915 driver unload drm_vblank_get is added to test whether
> > > >> > drm_vblank_cleanup refcount validation patch is working.
> > > >> >
> > > >> > Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> > > >> > ---
> > > >> > Changes in v2:
> > > >> > Use drm_crtc_vblank_get instead of _put. In previous patch _put was wrongly
> > > >> > used.
> > > >> >
> > > >> >  drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
> > > >> >  1 file changed, 7 insertions(+)
> > > >> >
> > > >> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > >> > index 9f45cfe..4aee1c0 100644
> > > >> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > >> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > >> > @@ -1373,6 +1373,13 @@ 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;
> > > >> >
> > > >> > +       enum pipe pipe;
> > > >> > +       for_each_pipe(dev_priv, pipe) {
> > > >> > +               struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
> > > >> > +                                                                 pipe);
> > > >> > +               drm_crtc_vblank_get(&crtc->base);
> > > >> > +       }
> > > >> > +
> > > >> >         i915_driver_unregister(dev_priv);
> > > >> >
> > > >> >         if (i915_gem_suspend(dev_priv))
> > > >> > --
> > > >> > 2.10.0
> > > >> >
> > > >>
> > > >> From https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6167/fi-ilk-650/igt@drv_module_reload@basic-reload.html
> > > >> it can be seen that this patch triggers warning when vblank->refcount
> > > >> > 0 in vblank cleanup. This tests patch 1 successfully.
> > > >>
> > > >> I think patch 1 is good to go.
> > > >
> > > > Yeah it works, but we don't know whether it breaks anything yet. Can you
> > > > pls resubmit just the first patch? Abusing CI was just an idea to get it
> > > > fully tested, before we can merge it still needs to pass full CI. We just
> > > > had an issue 2 weeks ago where CI blew up because an untested patch landed
> > > > that broke every test :-/
> > > > -Daniel
> > > > --
> > > > Daniel Vetter
> > > > Software Engineer, Intel Corporation
> > > > http://blog.ffwll.ch
> > >
> > > I have already sent that patch alone. Please look at
> > > https://patchwork.freedesktop.org/patch/184866/.
> >
> > Seems to fail in CI:
> >
> > https://patchwork.freedesktop.org/series/32648/
> >
> > I guess you need to test this on a local box somewhere to figure out
> > what's wrong.
> 
> My patch is supposed to catch problem with drivers. It warns when
> vblank refcount is non-zero in drm_vblank_cleanup call. From CI log it
> can be seen that warning being triggered. I feel that my patch is
> exposing existing issue.
> 
> If I misinterpreted something please let me know.

This is probably what's happening, but I can't merge a patch that breaks
CI. So we need to track down that issue before merging.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 2/2] Test case for drm_vblank_cleanup refcount validation patch

[PrasannaKumar Muralidharan]

Hi Daniel,

On 31 October 2017 at 21:57, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Oct 31, 2017 at 08:37:21PM +0530, PrasannaKumar Muralidharan wrote:
>> My patch is supposed to catch problem with drivers. It warns when
>> vblank refcount is non-zero in drm_vblank_cleanup call. From CI log it
>> can be seen that warning being triggered. I feel that my patch is
>> exposing existing issue.
>>
>> If I misinterpreted something please let me know.
>
> This is probably what's happening, but I can't merge a patch that breaks
> CI. So we need to track down that issue before merging.

I understand. Being new to DRM subsystem I don't know if I can
contribute in finding the issue. I would be able to help if I could
get some guidance.

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

Re: [Intel-gfx] [PATCH v2 2/2] Test case for drm_vblank_cleanup refcount validation patch

[Daniel Vetter]

On Wed, Nov 01, 2017 at 09:48:28AM +0530, PrasannaKumar Muralidharan wrote:
> Hi Daniel,
> 
> On 31 October 2017 at 21:57, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Oct 31, 2017 at 08:37:21PM +0530, PrasannaKumar Muralidharan wrote:
> >> My patch is supposed to catch problem with drivers. It warns when
> >> vblank refcount is non-zero in drm_vblank_cleanup call. From CI log it
> >> can be seen that warning being triggered. I feel that my patch is
> >> exposing existing issue.
> >>
> >> If I misinterpreted something please let me know.
> >
> > This is probably what's happening, but I can't merge a patch that breaks
> > CI. So we need to track down that issue before merging.
> 
> I understand. Being new to DRM subsystem I don't know if I can
> contribute in finding the issue. I would be able to help if I could
> get some guidance.

If you have an intel laptop anywhere at hand that you could use to
reproduce the issue, that would be a good start.

Then go through the setup for the kms/drm validation suite:

https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#testing-and-validation

That should allow you to locally reproduce the issue (it seems to affect
many machines, so I'd assume it doesn't matter which one you have).

Once you can reproduce it should be doable to figure out where we leak
this reference (but usually it's a bit of hard work).

Btw I discussed your patch a bit on irc, a first step would be to also
print the refcount when it's leaked, not just warn about it. Knowing how
many references are leaked is sometimes a good hint about what's going on.
-Daniel
-- 
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

Re: [PATCH v2 2/2] Test case for drm_vblank_cleanup refcount validation patch

[PrasannaKumar Muralidharan]

Hi Daniel,

On 1 November 2017 at 14:23, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Nov 01, 2017 at 09:48:28AM +0530, PrasannaKumar Muralidharan wrote:
>> Hi Daniel,
>>
>> On 31 October 2017 at 21:57, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Tue, Oct 31, 2017 at 08:37:21PM +0530, PrasannaKumar Muralidharan wrote:
>> >> My patch is supposed to catch problem with drivers. It warns when
>> >> vblank refcount is non-zero in drm_vblank_cleanup call. From CI log it
>> >> can be seen that warning being triggered. I feel that my patch is
>> >> exposing existing issue.
>> >>
>> >> If I misinterpreted something please let me know.
>> >
>> > This is probably what's happening, but I can't merge a patch that breaks
>> > CI. So we need to track down that issue before merging.
>>
>> I understand. Being new to DRM subsystem I don't know if I can
>> contribute in finding the issue. I would be able to help if I could
>> get some guidance.
>
> If you have an intel laptop anywhere at hand that you could use to
> reproduce the issue, that would be a good start.

I do have one but it is my primary machine.

> Then go through the setup for the kms/drm validation suite:
>
> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#testing-and-validation
>
> That should allow you to locally reproduce the issue (it seems to affect
> many machines, so I'd assume it doesn't matter which one you have).

If this setup is going to affect my normal workflow setup I would
prefer not to use it.

> Once you can reproduce it should be doable to figure out where we leak
> this reference (but usually it's a bit of hard work).

Anyway I will give it a try and see how far I can go.

> Btw I discussed your patch a bit on irc, a first step would be to also
> print the refcount when it's leaked, not just warn about it. Knowing how
> many references are leaked is sometimes a good hint about what's going on.

I will send a v4.

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

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

✗ Fi.CI.BAT: warning for series starting with [v2,1/2] drm: drm_vblank_cleanup: WARN when refcount > 0

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm: drm_vblank_cleanup: WARN when refcount > 0
URL   : https://patchwork.freedesktop.org/series/32561/
State : warning

== Summary ==

Series 32561v1 series starting with [v2,1/2] drm: drm_vblank_cleanup: WARN when refcount > 0
https://patchwork.freedesktop.org/api/1.0/series/32561/revisions/1/mbox/

Test gem_exec_reloc:
        Subgroup basic-gtt-active:
                dmesg-warn -> PASS       (fi-gdg-551) fdo#102582 +5
        Subgroup basic-write-cpu-active:
                skip       -> PASS       (fi-gdg-551)
        Subgroup basic-write-gtt-active:
                skip       -> PASS       (fi-gdg-551) fdo#102582
        Subgroup basic-softpin:
                skip       -> PASS       (fi-gdg-551)
Test gem_linear_blits:
        Subgroup basic:
                skip       -> PASS       (fi-gdg-551)
Test gem_render_linear_blits:
        Subgroup basic:
                skip       -> PASS       (fi-gdg-551)
Test gem_render_tiled_blits:
        Subgroup basic:
                skip       -> PASS       (fi-gdg-551)
Test gem_sync:
        Subgroup basic-all:
                skip       -> PASS       (fi-gdg-551)
        Subgroup basic-each:
                skip       -> PASS       (fi-gdg-551)
        Subgroup basic-many-each:
                skip       -> PASS       (fi-gdg-551)
        Subgroup basic-store-each:
                skip       -> PASS       (fi-gdg-551)
Test gem_tiled_blits:
        Subgroup basic:
                skip       -> PASS       (fi-gdg-551)
Test gem_tiled_fence_blits:
        Subgroup basic:
                skip       -> PASS       (fi-gdg-551)
Test gem_wait:
        Subgroup basic-busy-all:
                skip       -> PASS       (fi-gdg-551)
        Subgroup basic-wait-all:
                skip       -> PASS       (fi-gdg-551)
        Subgroup basic-await-all:
                skip       -> PASS       (fi-gdg-551)
Test kms_busy:
        Subgroup basic-flip-a:
                skip       -> PASS       (fi-gdg-551) fdo#102654 +1
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                skip       -> PASS       (fi-gdg-551) fdo#102618
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> DMESG-WARN (fi-blb-e6850)
                pass       -> DMESG-WARN (fi-bwr-2160)
                pass       -> DMESG-WARN (fi-elk-e7500)
                pass       -> DMESG-WARN (fi-ilk-650)
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
                pass       -> DMESG-WARN (fi-ivb-3520m)
                pass       -> DMESG-WARN (fi-ivb-3770)
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-hsw-4770)
                pass       -> DMESG-WARN (fi-hsw-4770r)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-bdw-gvtdvm)
                pass       -> DMESG-WARN (fi-bsw-n3050)
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-skl-6700k)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
                pass       -> DMESG-WARN (fi-bxt-dsi)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-kbl-7560u)
                pass       -> DMESG-WARN (fi-kbl-7567u)
                pass       -> DMESG-WARN (fi-kbl-r)
                pass       -> DMESG-WARN (fi-glk-1)
                pass       -> DMESG-WARN (fi-glk-dsi)
                pass       -> DMESG-WARN (fi-cnl-y)
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-gdg-551) fdo#102707 +1
                pass       -> DMESG-WARN (fi-blb-e6850)
                pass       -> DMESG-WARN (fi-bwr-2160)
                pass       -> DMESG-WARN (fi-elk-e7500)
                pass       -> DMESG-WARN (fi-ilk-650)
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
                pass       -> DMESG-WARN (fi-ivb-3520m)
                pass       -> DMESG-WARN (fi-ivb-3770)
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-hsw-4770)
                pass       -> DMESG-WARN (fi-hsw-4770r)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-bdw-gvtdvm)
                pass       -> DMESG-WARN (fi-bsw-n3050)
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-skl-6700k)
WARNING: Long output truncated

bcee836068d98bd2aaa5d64124c5994acce6a6c4 drm-tip: 2017y-10m-24d-15h-00m-14s UTC integration manifest
02a83286f4de Test case for drm_vblank_cleanup refcount validation patch
195020f9c227 drm: drm_vblank_cleanup: WARN when refcount > 0

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6167/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx