Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2732223004:
DiscardableHandle Implementation (Closed)
DescriptionDiscardableHandle Implementation
This change adds a DiscardableHandle class, which acts as the primary
synchronization mechanism for the GPU Discardable Memory system.
This is the first in a sequence of CLs which implement the GPU
Discardable Memory system. This class is currently only created/used by
unit test code.
BUG=706456
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Review-Url: https://codereview.chromium.org/2732223004
Cr-Commit-Position: refs/heads/master@{#460610}
Committed: https://chromium.googlesource.com/chromium/src/+/6298c71c94585f0eb01c9fc1b420d04e935b5239
Patch Set 1 #Patch Set 2 : tweaks #Patch Set 3 : IsDeleted #Patch Set 4 : different init modes #Patch Set 5 : rebase #Patch Set 6 : byte offset #
Total comments: 12
Patch Set 7 : feedback #
Total comments: 7
Patch Set 8 : remove dcheck #Patch Set 9 : rebase #Patch Set 10 : remove unused var #
Dependent Patchsets: Messages
Total messages: 26 (13 generated)
Description was changed from ========== DiscardableHandle Implementation This change adds a DiscardableHandle class, which acts as the primary synchronization mechanism for the GPU Discardable Memory system. This is the first in a sequence of CLs which implement the GPU Discardable Memory system. This class is currently on created/used by unit test code. ========== to ========== DiscardableHandle Implementation This change adds a DiscardableHandle class, which acts as the primary synchronization mechanism for the GPU Discardable Memory system. This is the first in a sequence of CLs which implement the GPU Discardable Memory system. This class is currently on created/used by unit test code. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Description was changed from ========== DiscardableHandle Implementation This change adds a DiscardableHandle class, which acts as the primary synchronization mechanism for the GPU Discardable Memory system. This is the first in a sequence of CLs which implement the GPU Discardable Memory system. This class is currently on created/used by unit test code. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== DiscardableHandle Implementation This change adds a DiscardableHandle class, which acts as the primary synchronization mechanism for the GPU Discardable Memory system. This is the first in a sequence of CLs which implement the GPU Discardable Memory system. This class is currently only created/used by unit test code. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
ericrk@chromium.org changed reviewers: + piman@chromium.org
This is the first in a sequence of CLs to implement Discardable GPU memory. You can follow the dependent CL links to see how this ends up getting used, however these follow-ups aren't necessarily ready for review (I'm still polishing and adding tests to dependent CLs).
https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... File gpu/command_buffer/common/discardable_handle.cc (right): https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:36: DCHECK(base::subtle::NoBarrier_Load(AsAtomic()) >= kHandleLockedStart); Will this be called by the service? If so, a malicious renderer can break this DCHECK, making it invalid. Do we need other defenses? (Either way, the DCHECK is racy, since the value can change between here and the next line. An option is to take the return value and DCHECK/validate that). https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:38: base::subtle::Barrier_AtomicIncrement(AsAtomic(), -1); Here and other places where we use barriers (and/or acquire/release semantics): I'm not exactly sure what reordering they are protecting against, because there is otherwise no shared state that I can think of that would matter relative to it, or it's already well-ordered. For client writes to be visible by the service, there is already a barrier when we Flush (or OrderingBarrier) command buffers, from the locks we take. For service writes to be visible to the client, the only state that I think could matter is the gpu::SharedState to communicate the advance of the get pointer (which we need to be consistent with the unlock state), but that one already has barriers (both acquire and release, actually) when writing. I guess queries too, that happen after the Unlock, but they also use Release_Store so we'd be good there. Otherwise, IPC replies (WaitForGetOffset/Token) include stronger barriers as well. I just want to make sure I fully understand the logic, maybe I missed something. https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:49: while (true) { Will this be called on the service side? If so, can a malicious renderer live-lock this loop? (from reading the doc, it looks like it's only called from the client side, but double-checking) https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:69: return kHandleDeleted == base::subtle::Acquire_Load(AsAtomic()); Which side is planning to call this? From the client side, it's racy in the sense that if it returns false, it tells you nothing (because the service side may have deleted the texture right after you loaded the value) From the service side, it is looking at an untrusted value, making it possibly dangerous to rely on?
Patchset #7 (id:120001) has been deleted
Patchset #7 (id:130006) has been deleted
Patchset #7 (id:150001) has been deleted
Thanks for the feedback! Refactored into client/service classes, which should make the usage a lot clearer. It seemed like overkill to create three files for this (common/client/service), so put it all in one. Let me know if you'd prefer three. https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... File gpu/command_buffer/common/discardable_handle.cc (right): https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:36: DCHECK(base::subtle::NoBarrier_Load(AsAtomic()) >= kHandleLockedStart); On 2017/03/20 23:51:44, piman wrote: > Will this be called by the service? If so, a malicious renderer can break this > DCHECK, making it invalid. Do we need other defenses? > > (Either way, the DCHECK is racy, since the value can change between here and the > next line. An option is to take the return value and DCHECK/validate that). True, I wasn't so concerned with the DCHECK, as it's more of just a sanity check - if it ever fires, we have a problem. I agree that it can miss cases. I've moved this to use the return value from AtomicIncrement. I assume it's not a security concern that the client can trigger a DCHECK (as DCHECKs will be compiled out)? https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:38: base::subtle::Barrier_AtomicIncrement(AsAtomic(), -1); On 2017/03/20 23:51:44, piman wrote: > Here and other places where we use barriers (and/or acquire/release semantics): > I'm not exactly sure what reordering they are protecting against, because there > is otherwise no shared state that I can think of that would matter relative to > it, or it's already well-ordered. > > For client writes to be visible by the service, there is already a barrier when > we Flush (or OrderingBarrier) command buffers, from the locks we take. > > For service writes to be visible to the client, the only state that I think > could matter is the gpu::SharedState to communicate the advance of the get > pointer (which we need to be consistent with the unlock state), but that one > already has barriers (both acquire and release, actually) when writing. I guess > queries too, that happen after the Unlock, but they also use Release_Store so > we'd be good there. Otherwise, IPC replies (WaitForGetOffset/Token) include > stronger barriers as well. > > > I just want to make sure I fully understand the logic, maybe I missed something. I may have been over-thinking things. I was sort of thinking about this construct abstractly (without the command buffer) when I added these Acquire/Release semantics - trying to make it work for an arbitrarily threaded program. Changed to NoBarrier and added comments. https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:49: while (true) { On 2017/03/20 23:51:44, piman wrote: > Will this be called on the service side? If so, can a malicious renderer > live-lock this loop? > (from reading the doc, it looks like it's only called from the client side, but > double-checking) This should be called client-side only. I'll update comments. https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:69: return kHandleDeleted == base::subtle::Acquire_Load(AsAtomic()); On 2017/03/20 23:51:44, piman wrote: > Which side is planning to call this? > From the client side, it's racy in the sense that if it returns false, it tells > you nothing (because the service side may have deleted the texture right after > you loaded the value) > From the service side, it is looking at an untrusted value, making it possibly > dangerous to rely on? We only call this from the client side, to check if it's safe to re-use a buffer. When the client tells the service to delete a texture, we wait for IsDeleted to be true before re-using that texture's discardable handle memory. (We don't want to re-use the handle until after the service actually deletes it). We don't really care about the case where it returns false (it's fine to narrowly miss re-using it). We just care that it eventually returns true. I've changed the name to CanBeReUsed to make this clearer. Shouldn't be an ordering issue here, as after we delete a texture on the GPU thread and re-use it on the client, the next request to re-use the data on the GPU thread will have come over the command buffer.
https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... File gpu/command_buffer/common/discardable_handle.cc (right): https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:36: DCHECK(base::subtle::NoBarrier_Load(AsAtomic()) >= kHandleLockedStart); On 2017/03/27 22:58:13, ericrk wrote: > On 2017/03/20 23:51:44, piman wrote: > > Will this be called by the service? If so, a malicious renderer can break this > > DCHECK, making it invalid. Do we need other defenses? > > > > (Either way, the DCHECK is racy, since the value can change between here and > the > > next line. An option is to take the return value and DCHECK/validate that). > > True, I wasn't so concerned with the DCHECK, as it's more of just a sanity check > - if it ever fires, we have a problem. I agree that it can miss cases. I've > moved this to use the return value from AtomicIncrement. > > I assume it's not a security concern that the client can trigger a DCHECK (as > DCHECKs will be compiled out)? Generally, I prefer avoiding service-side DCHECKs, because they may give the wrong impression. In this case, it says "The atomic can't be < kHandleLockedStart". A reader might think "aha, so I'm safe to assume this is true" and add code based on that assumption (e.g. look up in a table for a text description of the value for debug purpose), therefore introducing a security issue, because the assumption is not true in the case of a malicious client. Even further, the DCHECK actively prevents one from writing a test that ensures we behave correctly in the malicious client case (since it would trigger the DCHECK). Also, my question is more: if this condition is not true, what could happen? E.g. could we end up in a situation where we double-delete a texture? Or leak it? Do we need to keep a "deleted" flag on the service side (that's not exposed to the client)? https://codereview.chromium.org/2732223004/diff/170001/gpu/command_buffer/com... File gpu/command_buffer/common/discardable_handle.cc (right): https://codereview.chromium.org/2732223004/diff/170001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:94: DCHECK(base::subtle::NoBarrier_Load(AsAtomic()) >= kHandleLockedStart); ditto on this DCHECK, we need to make sure we behave sanely even if it fails. https://codereview.chromium.org/2732223004/diff/170001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:112: DCHECK(value >= kHandleLockedStart); ditto https://codereview.chromium.org/2732223004/diff/170001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:120: AsAtomic(), kHandleUnlocked, kHandleDeleted); nit: indent looks wrong
https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... File gpu/command_buffer/common/discardable_handle.cc (right): https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:36: DCHECK(base::subtle::NoBarrier_Load(AsAtomic()) >= kHandleLockedStart); On 2017/03/28 00:43:00, piman wrote: > On 2017/03/27 22:58:13, ericrk wrote: > > On 2017/03/20 23:51:44, piman wrote: > > > Will this be called by the service? If so, a malicious renderer can break > this > > > DCHECK, making it invalid. Do we need other defenses? > > > > > > (Either way, the DCHECK is racy, since the value can change between here and > > the > > > next line. An option is to take the return value and DCHECK/validate that). > > > > True, I wasn't so concerned with the DCHECK, as it's more of just a sanity > check > > - if it ever fires, we have a problem. I agree that it can miss cases. I've > > moved this to use the return value from AtomicIncrement. > > > > I assume it's not a security concern that the client can trigger a DCHECK (as > > DCHECKs will be compiled out)? > > Generally, I prefer avoiding service-side DCHECKs, because they may give the > wrong impression. In this case, it says "The atomic can't be < > kHandleLockedStart". A reader might think "aha, so I'm safe to assume this is > true" and add code based on that assumption (e.g. look up in a table for a text > description of the value for debug purpose), therefore introducing a security > issue, because the assumption is not true in the case of a malicious client. > Even further, the DCHECK actively prevents one from writing a test that ensures > we behave correctly in the malicious client case (since it would trigger the > DCHECK). > > Also, my question is more: if this condition is not true, what could happen? > E.g. could we end up in a situation where we double-delete a texture? Or leak > it? Do we need to keep a "deleted" flag on the service side (that's not exposed > to the client)? Fair enough, I'll remove the DCHECK. It was more there as an "FYI we have a code bug" for me while developing, rather than as any sort of protection. The worst case scenario if this is false is: a) Texture becomes un-deletable by discardable system, increasing memory pressure. (Although it will still be deleted if the client (renderer) is destroyed or the client calls glDeleteTexture). This doesn't seem much worse than the existing problem where a client (renderer) can allocate huge amounts of textures. b) Texture is deleted while "in use" by the client (note that if this is shared with other clients, the server ID won't be deleted, just the client ID). This can already happen if a compromised client issues glDeleteTexture commands for a texture while it is still using it. If we wanted to be super thorough, we could detect these cases and put the object in an "invalid state", which would basically take it out of the discardable pool, minimizing its impact on other things in the pool - not sure this is necessary as a compromised client can already hammer the GPU process pretty hard? https://codereview.chromium.org/2732223004/diff/170001/gpu/command_buffer/com... File gpu/command_buffer/common/discardable_handle.cc (right): https://codereview.chromium.org/2732223004/diff/170001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:94: DCHECK(base::subtle::NoBarrier_Load(AsAtomic()) >= kHandleLockedStart); On 2017/03/28 00:43:00, piman wrote: > ditto on this DCHECK, we need to make sure we behave sanely even if it fails. same problems/mitigations as listed on the previous comment. https://codereview.chromium.org/2732223004/diff/170001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:112: DCHECK(value >= kHandleLockedStart); On 2017/03/28 00:43:00, piman wrote: > ditto same problems/mitigations as listed on the previous comment. https://codereview.chromium.org/2732223004/diff/170001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:120: AsAtomic(), kHandleUnlocked, kHandleDeleted); On 2017/03/28 00:43:00, piman wrote: > nit: indent looks wrong oddly enough, this is what git cl format produces - I messed it up and re-formatted to make sure it was working :/ I guess it's indenting 4 past the start of the function call on the previous line.
https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... File gpu/command_buffer/common/discardable_handle.cc (right): https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:36: DCHECK(base::subtle::NoBarrier_Load(AsAtomic()) >= kHandleLockedStart); On 2017/03/28 01:12:10, ericrk wrote: > On 2017/03/28 00:43:00, piman wrote: > > On 2017/03/27 22:58:13, ericrk wrote: > > > On 2017/03/20 23:51:44, piman wrote: > > > > Will this be called by the service? If so, a malicious renderer can break > > this > > > > DCHECK, making it invalid. Do we need other defenses? > > > > > > > > (Either way, the DCHECK is racy, since the value can change between here > and > > > the > > > > next line. An option is to take the return value and DCHECK/validate > that). > > > > > > True, I wasn't so concerned with the DCHECK, as it's more of just a sanity > > check > > > - if it ever fires, we have a problem. I agree that it can miss cases. I've > > > moved this to use the return value from AtomicIncrement. > > > > > > I assume it's not a security concern that the client can trigger a DCHECK > (as > > > DCHECKs will be compiled out)? > > > > Generally, I prefer avoiding service-side DCHECKs, because they may give the > > wrong impression. In this case, it says "The atomic can't be < > > kHandleLockedStart". A reader might think "aha, so I'm safe to assume this is > > true" and add code based on that assumption (e.g. look up in a table for a > text > > description of the value for debug purpose), therefore introducing a security > > issue, because the assumption is not true in the case of a malicious client. > > Even further, the DCHECK actively prevents one from writing a test that > ensures > > we behave correctly in the malicious client case (since it would trigger the > > DCHECK). > > > > Also, my question is more: if this condition is not true, what could happen? > > E.g. could we end up in a situation where we double-delete a texture? Or leak > > it? Do we need to keep a "deleted" flag on the service side (that's not > exposed > > to the client)? > > Fair enough, I'll remove the DCHECK. It was more there as an "FYI we have a code > bug" for me while developing, rather than as any sort of protection. > > The worst case scenario if this is false is: > a) Texture becomes un-deletable by discardable system, increasing memory > pressure. (Although it will still be deleted if the client (renderer) is > destroyed or the client calls glDeleteTexture). This doesn't seem much worse > than the existing problem where a client (renderer) can allocate huge amounts of > textures. > b) Texture is deleted while "in use" by the client (note that if this is shared > with other clients, the server ID won't be deleted, just the client ID). This > can already happen if a compromised client issues glDeleteTexture commands for a > texture while it is still using it. > > If we wanted to be super thorough, we could detect these cases and put the > object in an "invalid state", which would basically take it out of the > discardable pool, minimizing its impact on other things in the pool - not sure > this is necessary as a compromised client can already hammer the GPU process > pretty hard? hmm, plus, a malicious client could already just do the valid behavior of allocating and locking a large number of textures to put pressure on the pool, so not sure that the "invalid state" defense really buys us much.
LGTM, thanks for the explanations. Just see comment below. https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... File gpu/command_buffer/common/discardable_handle.cc (right): https://codereview.chromium.org/2732223004/diff/100001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:36: DCHECK(base::subtle::NoBarrier_Load(AsAtomic()) >= kHandleLockedStart); On 2017/03/28 01:19:44, ericrk wrote: > On 2017/03/28 01:12:10, ericrk wrote: > > On 2017/03/28 00:43:00, piman wrote: > > > On 2017/03/27 22:58:13, ericrk wrote: > > > > On 2017/03/20 23:51:44, piman wrote: > > > > > Will this be called by the service? If so, a malicious renderer can > break > > > this > > > > > DCHECK, making it invalid. Do we need other defenses? > > > > > > > > > > (Either way, the DCHECK is racy, since the value can change between here > > and > > > > the > > > > > next line. An option is to take the return value and DCHECK/validate > > that). > > > > > > > > True, I wasn't so concerned with the DCHECK, as it's more of just a sanity > > > check > > > > - if it ever fires, we have a problem. I agree that it can miss cases. > I've > > > > moved this to use the return value from AtomicIncrement. > > > > > > > > I assume it's not a security concern that the client can trigger a DCHECK > > (as > > > > DCHECKs will be compiled out)? > > > > > > Generally, I prefer avoiding service-side DCHECKs, because they may give the > > > wrong impression. In this case, it says "The atomic can't be < > > > kHandleLockedStart". A reader might think "aha, so I'm safe to assume this > is > > > true" and add code based on that assumption (e.g. look up in a table for a > > text > > > description of the value for debug purpose), therefore introducing a > security > > > issue, because the assumption is not true in the case of a malicious client. > > > Even further, the DCHECK actively prevents one from writing a test that > > ensures > > > we behave correctly in the malicious client case (since it would trigger the > > > DCHECK). > > > > > > Also, my question is more: if this condition is not true, what could happen? > > > E.g. could we end up in a situation where we double-delete a texture? Or > leak > > > it? Do we need to keep a "deleted" flag on the service side (that's not > > exposed > > > to the client)? > > > > Fair enough, I'll remove the DCHECK. It was more there as an "FYI we have a > code > > bug" for me while developing, rather than as any sort of protection. > > > > The worst case scenario if this is false is: > > a) Texture becomes un-deletable by discardable system, increasing memory > > pressure. (Although it will still be deleted if the client (renderer) is > > destroyed or the client calls glDeleteTexture). This doesn't seem much worse > > than the existing problem where a client (renderer) can allocate huge amounts > of > > textures. > > b) Texture is deleted while "in use" by the client (note that if this is > shared > > with other clients, the server ID won't be deleted, just the client ID). This > > can already happen if a compromised client issues glDeleteTexture commands for > a > > texture while it is still using it. > > > > If we wanted to be super thorough, we could detect these cases and put the > > object in an "invalid state", which would basically take it out of the > > discardable pool, minimizing its impact on other things in the pool - not sure > > this is necessary as a compromised client can already hammer the GPU process > > pretty hard? > > hmm, plus, a malicious client could already just do the valid behavior of > allocating and locking a large number of textures to put pressure on the pool, > so not sure that the "invalid state" defense really buys us much. Right, if the worse that could happen is that the (malicious) client can do things that it already can do (leak textures, access deleted textures), I'm perfectly fine. It's just a bit hard to reason about with just that CL - I assume we'll have to look closer at further CLs that use this. Just note that accessing a deleted texture has well-defined behavior today (DeleteTextures unbinds the texture if it's currently bound, BindTexture either recreates it if bind_generates_resources is true, or generates GL_INVALID_OPERATION otherwise), and we'll need to either be consistent with that or define our own semantics (e.g. unbinding a texture asynchronously as a side effect of eviction could be surprising; OTOH if you liked it you should have put a lock on it - either way, let's define it). https://codereview.chromium.org/2732223004/diff/170001/gpu/command_buffer/com... File gpu/command_buffer/common/discardable_handle.cc (right): https://codereview.chromium.org/2732223004/diff/170001/gpu/command_buffer/com... gpu/command_buffer/common/discardable_handle.cc:120: AsAtomic(), kHandleUnlocked, kHandleDeleted); On 2017/03/28 01:12:10, ericrk wrote: > On 2017/03/28 00:43:00, piman wrote: > > nit: indent looks wrong > > oddly enough, this is what git cl format produces - I messed it up and > re-formatted to make sure it was working :/ > > I guess it's indenting 4 past the start of the function call on the previous > line. Seems like a clang-format bug, but let's not fight the tool.
Great! Please add a tracking bug for this patch set.
Description was changed from ========== DiscardableHandle Implementation This change adds a DiscardableHandle class, which acts as the primary synchronization mechanism for the GPU Discardable Memory system. This is the first in a sequence of CLs which implement the GPU Discardable Memory system. This class is currently only created/used by unit test code. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== DiscardableHandle Implementation This change adds a DiscardableHandle class, which acts as the primary synchronization mechanism for the GPU Discardable Memory system. This is the first in a sequence of CLs which implement the GPU Discardable Memory system. This class is currently only created/used by unit test code. BUG=706456 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by ericrk@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by ericrk@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org Link to the patchset: https://codereview.chromium.org/2732223004/#ps230001 (title: "remove unused var")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 230001, "attempt_start_ts": 1490828104928630,
"parent_rev": "bd690b0bbec9e12f8554673c5501a36e4ec2d7b8", "commit_rev":
"6298c71c94585f0eb01c9fc1b420d04e935b5239"}
Message was sent while issue was closed.
Description was changed from ========== DiscardableHandle Implementation This change adds a DiscardableHandle class, which acts as the primary synchronization mechanism for the GPU Discardable Memory system. This is the first in a sequence of CLs which implement the GPU Discardable Memory system. This class is currently only created/used by unit test code. BUG=706456 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== DiscardableHandle Implementation This change adds a DiscardableHandle class, which acts as the primary synchronization mechanism for the GPU Discardable Memory system. This is the first in a sequence of CLs which implement the GPU Discardable Memory system. This class is currently only created/used by unit test code. BUG=706456 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2732223004 Cr-Commit-Position: refs/heads/master@{#460610} Committed: https://chromium.googlesource.com/chromium/src/+/6298c71c94585f0eb01c9fc1b420... ==========
Message was sent while issue was closed.
Committed patchset #10 (id:230001) as https://chromium.googlesource.com/chromium/src/+/6298c71c94585f0eb01c9fc1b420... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
