1348320 - Crash in igd10umd32.dll | CContext::ID3D11DeviceContext1_UpdateSubresource_<T>

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect, critical)

Description

[Julian Seward [:jseward]]

This bug was filed from the Socorro interface and is 
report bp-ce21c9d1-82c5-4ded-98f0-709a92170317.
=============================================================

This is ranked #12 for the Aurora crashes of 20170316004004.

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

David, did we ever figure out what this was?  It's also showing up as an increase in overall Intel crashes in bug 1359416

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Should take a look at https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1300699&attachment=8837062 just in case

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

bug 1359416 might be device reset related, though it's not clear why or whether the device is stale or newly-bogus. (Or if we're doing something that induces device resets).

The crashes here look like they might be device reset related too though it's hard to tell. Some of the reports have spam about previous DXVA crashes.

Comment 4

[Bas Schouten (:bas.schouten)]

Attachment: Bug 1348320: Use UpdateSubResource on crashy intel device/OS version combinations.

Review commit: https://reviewboard.mozilla.org/r/135078/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/135078/

Comment 5

[Bas Schouten (:bas.schouten)]

I'm guessing this will fix the crash. It may seem like a bad idea to grab a lock that can block the compositor. But realistically this is an upload which the compositor would otherwise just end up doing during the transaction anyway if we wouldn't allow for content side uploads. So you might be changing timings a little bit but wall-clock you're unlikely to take more time in the compositor thread. Also when working correctly the drivers likely internally do locking to prevent two threads uploading through the context at the same time anyway. Let me know if you disagree, Jeff.

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Bas it looks like this crash only ever happens in the UI process, and never in the GPU process. (Maybe this code doesn't run when video decoding is OOP?). It also exclusively happens on Windows 6.2.9200 and lower - I think that's Windows 8. Could we restrict the lock further to only activate under those two conditions?

Comment 7

[Bas Schouten (:bas.schouten)]

Comment on attachment 8863284 [details]
Bug 1348320: Use UpdateSubResource on crashy intel device/OS version combinations.

Hrm, good point David. This is different contention than I thought. This patch will likely not fix the problem. But I might have an idea for something else that will.

Comment 8

[Bas Schouten (:bas.schouten)]

Comment on attachment 8863284 [details]
Bug 1348320: Use UpdateSubResource on crashy intel device/OS version combinations.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135078/diff/1-2/

Comment 9

[Bas Schouten (:bas.schouten)]

Comment on attachment 8863284 [details]
Bug 1348320: Use UpdateSubResource on crashy intel device/OS version combinations.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135078/diff/2-3/

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8863284 [details]
Bug 1348320: Use UpdateSubResource on crashy intel device/OS version combinations.

https://reviewboard.mozilla.org/r/135078/#review138888

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c427691fb309
Use UpdateSubResource on crashy intel device/OS version combinations. r=jrmuizel

Comment 12

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/c427691fb309

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 14

[Bas Schouten (:bas.schouten)]

We have already flipped a pref on Beta that should prevent this crash from occurring at the expense of some performance. Has this patch dropped our actual crash rate?

Comment 15

[Ryan VanderMeulen [:RyanVM]]

I believe philipp answered your question in bug 1359416.

Comment 16

[Nathan Froyd [:froydnj]]

(In reply to Bas Schouten (:bas.schouten) from comment #14)
> We have already flipped a pref on Beta that should prevent this crash from
> occurring at the expense of some performance. Has this patch dropped our
> actual crash rate?

WDYT about philipp's answer in bug 1359416 comment 27?

Comment 17

[Bas Schouten (:bas.schouten)]

(In reply to Nathan Froyd [:froydnj] from comment #16)
> (In reply to Bas Schouten (:bas.schouten) from comment #14)
> > We have already flipped a pref on Beta that should prevent this crash from
> > occurring at the expense of some performance. Has this patch dropped our
> > actual crash rate?
> 
> WDYT about philipp's answer in bug 1359416 comment 27?

I say we just let it ride the trains. The video perf improvements this caused will still make it into 57, no need to add risk into the 55 cycle.