* [PATCH] drm/i915: Clean the request structure on alloc @ 2014-12-05 17:54 Mika Kuoppala 2014-12-05 17:54 ` John Harrison 2014-12-05 21:41 ` shuang.he 0 siblings, 2 replies; 6+ messages in thread From: Mika Kuoppala @ 2014-12-05 17:54 UTC (permalink / raw) To: intel-gfx Otherwise we might end up referencing uninitialized fields. This is apparent when we try to cleanup the preallocated request on ring reset, before any request has been submitted to the ring. The request->ctx is foobar and we end up freeing the foobarness. References: https://bugs.freedesktop.org/show_bug.cgi?id=86959 References: https://bugs.freedesktop.org/show_bug.cgi?id=86962 References: https://bugs.freedesktop.org/show_bug.cgi?id=86992 Cc: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 79b4ca5..2c6c6f8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2030,7 +2030,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring) if (ring->outstanding_lazy_request) return 0; - request = kmalloc(sizeof(*request), GFP_KERNEL); + request = kzalloc(sizeof(*request), GFP_KERNEL); if (request == NULL) return -ENOMEM; -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Clean the request structure on alloc 2014-12-05 17:54 [PATCH] drm/i915: Clean the request structure on alloc Mika Kuoppala @ 2014-12-05 17:54 ` John Harrison 2014-12-05 18:02 ` John Harrison 2014-12-05 18:09 ` Mika Kuoppala 2014-12-05 21:41 ` shuang.he 1 sibling, 2 replies; 6+ messages in thread From: John Harrison @ 2014-12-05 17:54 UTC (permalink / raw) To: Mika Kuoppala, intel-gfx This is already part of the seqno/request patch series and has been right from the start. See email 'drm/i915: Zero fill the request structure'. On 05/12/2014 17:54, Mika Kuoppala wrote: > Otherwise we might end up referencing uninitialized fields. > This is apparent when we try to cleanup the preallocated request > on ring reset, before any request has been submitted to the ring. > The request->ctx is foobar and we end up freeing the foobarness. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=86959 > References: https://bugs.freedesktop.org/show_bug.cgi?id=86962 > References: https://bugs.freedesktop.org/show_bug.cgi?id=86992 > Cc: John Harrison <John.C.Harrison@Intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 79b4ca5..2c6c6f8 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -2030,7 +2030,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring) > if (ring->outstanding_lazy_request) > return 0; > > - request = kmalloc(sizeof(*request), GFP_KERNEL); > + request = kzalloc(sizeof(*request), GFP_KERNEL); > if (request == NULL) > return -ENOMEM; > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Clean the request structure on alloc 2014-12-05 17:54 ` John Harrison @ 2014-12-05 18:02 ` John Harrison 2014-12-05 20:44 ` Daniel Vetter 2014-12-05 18:09 ` Mika Kuoppala 1 sibling, 1 reply; 6+ messages in thread From: John Harrison @ 2014-12-05 18:02 UTC (permalink / raw) To: Mika Kuoppala, intel-gfx But yes, the reasoning why it crashes without the zero fill is correct. Dodgy context pointers that used to be ignored now get processed. Doing the zero fill keeps it all sane. On 05/12/2014 17:54, John Harrison wrote: > This is already part of the seqno/request patch series and has been > right from the start. See email 'drm/i915: Zero fill the request > structure'. > > On 05/12/2014 17:54, Mika Kuoppala wrote: >> Otherwise we might end up referencing uninitialized fields. >> This is apparent when we try to cleanup the preallocated request >> on ring reset, before any request has been submitted to the ring. >> The request->ctx is foobar and we end up freeing the foobarness. >> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=86959 >> References: https://bugs.freedesktop.org/show_bug.cgi?id=86962 >> References: https://bugs.freedesktop.org/show_bug.cgi?id=86992 >> Cc: John Harrison <John.C.Harrison@Intel.com> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> >> --- >> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c >> b/drivers/gpu/drm/i915/intel_ringbuffer.c >> index 79b4ca5..2c6c6f8 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c >> @@ -2030,7 +2030,7 @@ intel_ring_alloc_request(struct intel_engine_cs >> *ring) >> if (ring->outstanding_lazy_request) >> return 0; >> - request = kmalloc(sizeof(*request), GFP_KERNEL); >> + request = kzalloc(sizeof(*request), GFP_KERNEL); >> if (request == NULL) >> return -ENOMEM; > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Clean the request structure on alloc 2014-12-05 18:02 ` John Harrison @ 2014-12-05 20:44 ` Daniel Vetter 0 siblings, 0 replies; 6+ messages in thread From: Daniel Vetter @ 2014-12-05 20:44 UTC (permalink / raw) To: John Harrison; +Cc: intel-gfx On Fri, Dec 05, 2014 at 06:02:35PM +0000, John Harrison wrote: > But yes, the reasoning why it crashes without the zero fill is correct. > Dodgy context pointers that used to be ignored now get processed. Doing the > zero fill keeps it all sane. > > On 05/12/2014 17:54, John Harrison wrote: > >This is already part of the seqno/request patch series and has been right > >from the start. See email 'drm/i915: Zero fill the request structure'. > > > >On 05/12/2014 17:54, Mika Kuoppala wrote: > >>Otherwise we might end up referencing uninitialized fields. > >>This is apparent when we try to cleanup the preallocated request > >>on ring reset, before any request has been submitted to the ring. > >>The request->ctx is foobar and we end up freeing the foobarness. > >> > >>References: https://bugs.freedesktop.org/show_bug.cgi?id=86959 > >>References: https://bugs.freedesktop.org/show_bug.cgi?id=86962 > >>References: https://bugs.freedesktop.org/show_bug.cgi?id=86992 > >>Cc: John Harrison <John.C.Harrison@Intel.com> > >>Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> I've added this explanation plus the note about which commit introduced the regression to John's patch and merged that one. Thanks for tracking this down. -Daniel > >>--- > >> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > >>b/drivers/gpu/drm/i915/intel_ringbuffer.c > >>index 79b4ca5..2c6c6f8 100644 > >>--- a/drivers/gpu/drm/i915/intel_ringbuffer.c > >>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > >>@@ -2030,7 +2030,7 @@ intel_ring_alloc_request(struct intel_engine_cs > >>*ring) > >> if (ring->outstanding_lazy_request) > >> return 0; > >> - request = kmalloc(sizeof(*request), GFP_KERNEL); > >>+ request = kzalloc(sizeof(*request), GFP_KERNEL); > >> if (request == NULL) > >> return -ENOMEM; > > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Clean the request structure on alloc 2014-12-05 17:54 ` John Harrison 2014-12-05 18:02 ` John Harrison @ 2014-12-05 18:09 ` Mika Kuoppala 1 sibling, 0 replies; 6+ messages in thread From: Mika Kuoppala @ 2014-12-05 18:09 UTC (permalink / raw) To: John Harrison, intel-gfx John Harrison <John.C.Harrison@Intel.com> writes: > This is already part of the seqno/request patch series and has been > right from the start. See email 'drm/i915: Zero fill the request structure'. Indeed. Sorry for missing this one. My patch can be ignored. But do you agree on failure path? -Mika > On 05/12/2014 17:54, Mika Kuoppala wrote: >> Otherwise we might end up referencing uninitialized fields. >> This is apparent when we try to cleanup the preallocated request >> on ring reset, before any request has been submitted to the ring. >> The request->ctx is foobar and we end up freeing the foobarness. >> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=86959 >> References: https://bugs.freedesktop.org/show_bug.cgi?id=86962 >> References: https://bugs.freedesktop.org/show_bug.cgi?id=86992 >> Cc: John Harrison <John.C.Harrison@Intel.com> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> >> --- >> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c >> index 79b4ca5..2c6c6f8 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c >> @@ -2030,7 +2030,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring) >> if (ring->outstanding_lazy_request) >> return 0; >> >> - request = kmalloc(sizeof(*request), GFP_KERNEL); >> + request = kzalloc(sizeof(*request), GFP_KERNEL); >> if (request == NULL) >> return -ENOMEM; >> _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Clean the request structure on alloc 2014-12-05 17:54 [PATCH] drm/i915: Clean the request structure on alloc Mika Kuoppala 2014-12-05 17:54 ` John Harrison @ 2014-12-05 21:41 ` shuang.he 1 sibling, 0 replies; 6+ messages in thread From: shuang.he @ 2014-12-05 21:41 UTC (permalink / raw) To: shuang.he, intel-gfx, mika.kuoppala Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) -------------------------------------Summary------------------------------------- Platform Delta drm-intel-nightly Series Applied PNV 364/364 364/364 ILK -4 366/366 362/366 SNB -1 450/450 449/450 IVB +17 481/498 498/498 BYT 289/289 289/289 HSW -1 564/564 563/564 BDW 417/417 417/417 -------------------------------------Detailed------------------------------------- Platform Test drm-intel-nightly Series Applied ILK igt_kms_flip_rcs-wf_vblank-vs-dpms-interruptible DMESG_WARN(1, M26)PASS(4, M37M26) DMESG_WARN(1, M26) *ILK igt_kms_flip_flip-vs-panning PASS(2, M37M26) DMESG_WARN(1, M26) ILK igt_kms_flip_plain-flip-ts-check-interruptible DMESG_WARN(1, M26)PASS(1, M37) DMESG_WARN(1, M26) *ILK igt_kms_flip_rcs-flip-vs-modeset PASS(3, M37M26) DMESG_WARN(1, M26) SNB igt_kms_force_connector NRUN(4, M35M22)PASS(1, M35) NRUN(1, M35) IVB igt_kms_3d DMESG_WARN(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-onscreen NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-random NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-sliding NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-offscreen NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-onscreen NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-sliding NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-offscreen NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-onscreen NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-random NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-sliding NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-size-change NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_fence_pin_leak NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_rotation_crc_primary-rotation NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_rotation_crc_sprite-rotation NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) HSW igt_kms_force_connector NRUN(4, M40M19M20)PASS(1, M40) NRUN(1, M40) Note: You need to pay more attention to line start with '*' _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-12-05 21:41 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-12-05 17:54 [PATCH] drm/i915: Clean the request structure on alloc Mika Kuoppala 2014-12-05 17:54 ` John Harrison 2014-12-05 18:02 ` John Harrison 2014-12-05 20:44 ` Daniel Vetter 2014-12-05 18:09 ` Mika Kuoppala 2014-12-05 21:41 ` shuang.he
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox