[PATCH] drm/i915: Use rcu to defer freeing of irq_work

[PATCH] drm/i915: Use rcu to defer freeing of irq_work

[Chris Wilson]

It is illegal to perform an immediate free of the struct irq_work from
inside the irq_work callback (as irq_work_run_list modifies work->flags
after execution of the work->func()). As we use the irq_work to
coordinate the freeing of the callback from two different softirq paths,
we need to defer the kfree from inside our irq_work callback, for which
we can use kfree_rcu.

Fixes: 81c0ed21aa91 ("drm/i915/fence: Avoid del_timer_sync() from inside a timer")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_sw_fence.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index e8ca67a129d2..ac236b88c99c 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -367,6 +367,7 @@ struct i915_sw_dma_fence_cb {
 	struct dma_fence *dma;
 	struct timer_list timer;
 	struct irq_work work;
+	struct rcu_head rcu;
 };
 
 static void timer_i915_sw_fence_wake(struct timer_list *t)
@@ -406,7 +407,7 @@ static void irq_i915_sw_fence_work(struct irq_work *wrk)
 	del_timer_sync(&cb->timer);
 	dma_fence_put(cb->dma);
 
-	kfree(cb);
+	kfree_rcu(cb, rcu);
 }
 
 int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,
-- 
2.15.1

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

✓ Fi.CI.BAT: success for drm/i915: Use rcu to defer freeing of irq_work

[Patchwork]

== Series Details ==

Series: drm/i915: Use rcu to defer freeing of irq_work
URL   : https://patchwork.freedesktop.org/series/35277/
State : success

== Summary ==

Series 35277v1 drm/i915: Use rcu to defer freeing of irq_work
https://patchwork.freedesktop.org/api/1.0/series/35277/revisions/1/mbox/

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-kbl-r) fdo#104172 +1

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:441s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:440s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:385s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:511s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:280s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:506s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:506s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:488s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:478s
fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:269s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:537s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:372s
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:261s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:397s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:480s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:446s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:487s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:527s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:476s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:537s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:595s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:542s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:567s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:520s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:494s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:452s
fi-snb-2520m     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:549s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:415s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:594s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:630s
fi-glk-dsi       total:288  pass:257  dwarn:0   dfail:0   fail:1   skip:30  time:503s

8874c0f95698c533c0daf69c6c41834d837fef87 drm-tip: 2017y-12m-12d-21h-34m-33s UTC integration manifest
2ed8632b8afe drm/i915: Use rcu to defer freeing of irq_work

== Logs ==

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

Re: [PATCH] drm/i915: Use rcu to defer freeing of irq_work

[Tvrtko Ursulin]

On 13/12/2017 09:48, Chris Wilson wrote:
> It is illegal to perform an immediate free of the struct irq_work from
> inside the irq_work callback (as irq_work_run_list modifies work->flags
> after execution of the work->func()). As we use the irq_work to
> coordinate the freeing of the callback from two different softirq paths,
> we need to defer the kfree from inside our irq_work callback, for which
> we can use kfree_rcu.
> 
> Fixes: 81c0ed21aa91 ("drm/i915/fence: Avoid del_timer_sync() from inside a timer")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_sw_fence.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
> index e8ca67a129d2..ac236b88c99c 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> @@ -367,6 +367,7 @@ struct i915_sw_dma_fence_cb {
>   	struct dma_fence *dma;
>   	struct timer_list timer;
>   	struct irq_work work;
> +	struct rcu_head rcu;
>   };
>   
>   static void timer_i915_sw_fence_wake(struct timer_list *t)
> @@ -406,7 +407,7 @@ static void irq_i915_sw_fence_work(struct irq_work *wrk)
>   	del_timer_sync(&cb->timer);
>   	dma_fence_put(cb->dma);
>   
> -	kfree(cb);
> +	kfree_rcu(cb, rcu);
>   }
>   
>   int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

✓ Fi.CI.IGT: success for drm/i915: Use rcu to defer freeing of irq_work

[Patchwork]

== Series Details ==

Series: drm/i915: Use rcu to defer freeing of irq_work
URL   : https://patchwork.freedesktop.org/series/35277/
State : success

== Summary ==

Test gem_softpin:
        Subgroup noreloc-s4:
                fail       -> SKIP       (shard-snb) fdo#103375
Test gem_tiled_swapping:
        Subgroup non-threaded:
                incomplete -> PASS       (shard-snb) fdo#104009
                pass       -> INCOMPLETE (shard-hsw) fdo#104218
Test gem_eio:
        Subgroup in-flight:
                pass       -> DMESG-WARN (shard-snb) fdo#104058
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                pass       -> FAIL       (shard-snb) fdo#101623

fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#104009 https://bugs.freedesktop.org/show_bug.cgi?id=104009
fdo#104218 https://bugs.freedesktop.org/show_bug.cgi?id=104218
fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623

shard-hsw        total:2681 pass:1518 dwarn:1   dfail:0   fail:10  skip:1151 time:9161s
shard-snb        total:2712 pass:1308 dwarn:2   dfail:0   fail:11  skip:1391 time:8113s
Blacklisted hosts:
shard-apl        total:2636 pass:1640 dwarn:1   dfail:0   fail:22  skip:973 time:13541s
shard-kbl        total:2656 pass:1768 dwarn:1   dfail:0   fail:22  skip:864 time:10804s

== Logs ==

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

Re: ✓ Fi.CI.IGT: success for drm/i915: Use rcu to defer freeing of irq_work

[Chris Wilson]

Quoting Patchwork (2017-12-13 11:50:03)
> == Series Details ==
> 
> Series: drm/i915: Use rcu to defer freeing of irq_work
> URL   : https://patchwork.freedesktop.org/series/35277/
> State : success
> 
> == Summary ==
> 
> Test gem_softpin:
>         Subgroup noreloc-s4:
>                 fail       -> SKIP       (shard-snb) fdo#103375
> Test gem_tiled_swapping:
>         Subgroup non-threaded:
>                 incomplete -> PASS       (shard-snb) fdo#104009
>                 pass       -> INCOMPLETE (shard-hsw) fdo#104218

I was hopeful this might explain a few incompletes, this one in
particular. Also hoping for a magic glk-dsi fix.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Use rcu to defer freeing of irq_work

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-12-13 10:54:55)
> 
> On 13/12/2017 09:48, Chris Wilson wrote:
> > It is illegal to perform an immediate free of the struct irq_work from
> > inside the irq_work callback (as irq_work_run_list modifies work->flags
> > after execution of the work->func()). As we use the irq_work to
> > coordinate the freeing of the callback from two different softirq paths,
> > we need to defer the kfree from inside our irq_work callback, for which
> > we can use kfree_rcu.
> > 
> > Fixes: 81c0ed21aa91 ("drm/i915/fence: Avoid del_timer_sync() from inside a timer")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_sw_fence.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
> > index e8ca67a129d2..ac236b88c99c 100644
> > --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> > +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> > @@ -367,6 +367,7 @@ struct i915_sw_dma_fence_cb {
> >       struct dma_fence *dma;
> >       struct timer_list timer;
> >       struct irq_work work;
> > +     struct rcu_head rcu;
> >   };
> >   
> >   static void timer_i915_sw_fence_wake(struct timer_list *t)
> > @@ -406,7 +407,7 @@ static void irq_i915_sw_fence_work(struct irq_work *wrk)
> >       del_timer_sync(&cb->timer);
> >       dma_fence_put(cb->dma);
> >   
> > -     kfree(cb);
> > +     kfree_rcu(cb, rcu);
> >   }
> >   
> >   int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,
> > 
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Ta, applied. Still hopeful this was more than a one-off on my poor
Braswell.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Use rcu to defer freeing of irq_work

[Joonas Lahtinen]

On Wed, 2017-12-13 at 09:48 +0000, Chris Wilson wrote:
> It is illegal to perform an immediate free of the struct irq_work from
> inside the irq_work callback (as irq_work_run_list modifies work->flags
> after execution of the work->func()). As we use the irq_work to
> coordinate the freeing of the callback from two different softirq paths,
> we need to defer the kfree from inside our irq_work callback, for which
> we can use kfree_rcu.
> 
> Fixes: 81c0ed21aa91 ("drm/i915/fence: Avoid del_timer_sync() from inside a timer")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

<SNIP>

> @@ -367,6 +367,7 @@ struct i915_sw_dma_fence_cb {
>  	struct dma_fence *dma;
>  	struct timer_list timer;
>  	struct irq_work work;
> +	struct rcu_head rcu;

To reduce the chances of hitting the 4095 limit, maybe throw this right
after the base? OTOH, it'll yell if it breaks...

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Use rcu to defer freeing of irq_work

[Chris Wilson]

Quoting Joonas Lahtinen (2017-12-13 12:34:10)
> On Wed, 2017-12-13 at 09:48 +0000, Chris Wilson wrote:
> > It is illegal to perform an immediate free of the struct irq_work from
> > inside the irq_work callback (as irq_work_run_list modifies work->flags
> > after execution of the work->func()). As we use the irq_work to
> > coordinate the freeing of the callback from two different softirq paths,
> > we need to defer the kfree from inside our irq_work callback, for which
> > we can use kfree_rcu.
> > 
> > Fixes: 81c0ed21aa91 ("drm/i915/fence: Avoid del_timer_sync() from inside a timer")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> <SNIP>
> 
> > @@ -367,6 +367,7 @@ struct i915_sw_dma_fence_cb {
> >       struct dma_fence *dma;
> >       struct timer_list timer;
> >       struct irq_work work;
> > +     struct rcu_head rcu;
> 
> To reduce the chances of hitting the 4095 limit, maybe throw this right
> after the base? OTOH, it'll yell if it breaks...

Yup, and for the moment, the accidental increment of workarounds is
providing some merriment.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx