New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 597932 link

Starred by 5 users

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Jun 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Native GpuMemoryBuffers become invalid after GPU process crash

Project Member Reported by reve...@chromium.org, Mar 25 2016

Issue description

Browser or renderer instances of GMBs cannot become invalid when the GPU process crashes for reliable usage.

Successful allocation of a native GMB must result in a GMB that can be used with any GPU process instance.
 
Labels: OS-Chrome
Ozone GMBs become invalid after GPU process crash as they are being tracked in the GBMFactory. Instead the GMBFactory needs to return a reference to the GMB in the GMBHandle that can be used to create a GLImage from the GMB with any GPU process instance.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a54ebb6919a4b484f5d58f92c6953a62e8f8a6b3

commit a54ebb6919a4b484f5d58f92c6953a62e8f8a6b3
Author: reveman <reveman@chromium.org>
Date: Wed Mar 30 16:07:04 2016

ozone: Avoid using gbm_bo_import until import and destroy can be synchronized.

The client needs to be able to synchronize destruction and import
of buffers before gbm_bo_import can be used as otherwise importing
a buffer a second time might return the same GEM handle.

BUG= 597932 

Review URL: https://codereview.chromium.org/1834643002

Cr-Commit-Position: refs/heads/master@{#383988}

[modify] https://crrev.com/a54ebb6919a4b484f5d58f92c6953a62e8f8a6b3/ui/ozone/platform/drm/gpu/gbm_buffer.cc
[modify] https://crrev.com/a54ebb6919a4b484f5d58f92c6953a62e8f8a6b3/ui/ozone/platform/drm/gpu/gbm_buffer.h

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a54ebb6919a4b484f5d58f92c6953a62e8f8a6b3

commit a54ebb6919a4b484f5d58f92c6953a62e8f8a6b3
Author: reveman <reveman@chromium.org>
Date: Wed Mar 30 16:07:04 2016

ozone: Avoid using gbm_bo_import until import and destroy can be synchronized.

The client needs to be able to synchronize destruction and import
of buffers before gbm_bo_import can be used as otherwise importing
a buffer a second time might return the same GEM handle.

BUG= 597932 

Review URL: https://codereview.chromium.org/1834643002

Cr-Commit-Position: refs/heads/master@{#383988}

[modify] https://crrev.com/a54ebb6919a4b484f5d58f92c6953a62e8f8a6b3/ui/ozone/platform/drm/gpu/gbm_buffer.cc
[modify] https://crrev.com/a54ebb6919a4b484f5d58f92c6953a62e8f8a6b3/ui/ozone/platform/drm/gpu/gbm_buffer.h

Cc: posciak@chromium.org

Comment 5 by tfiga@chromium.org, May 13 2016

Cc: tfiga@chromium.org
Hmm, I was discussing this bug with Pawel today and then realized that it's a bit strange that anything could happening to a GpuMemoryBuffer in one process when another process dies. If I'm looking at the code properly, in our case, GpuMemoryBufferHandle (https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/gpu_memory_buffer.h&l=40) contains a NativePixmapHandle, which in turn references a buffer using a file descriptor. It seems a bit strange for a file descriptor in one process to become invalid if another process is killed.
The problem is that we're not using file descriptors and what becomes invalid is the ability to refer to a buffer in the new instance of the GPU process. When this code was initially written a few years ago we didn't need to use FDs and reference the buffers that way. All we needed was to allocate the buffers and be able to refer to them using unique ids and we'd always recreate them when the GPU process crashes. That's no longer the case and this bug will be resolved by always passing buffer ownership using fds as then things just work. I have a patch to fix this somewhere. It just need a bit more work before it can land.

Comment 7 by tfiga@chromium.org, May 13 2016

Aha, so I guess I misread some of the code, which misled me to believe that we are already using FDs. Using FDs for passing ownership sounds good to me.
Cc: ccameron@chromium.org
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57111c9ffef56e96560373618aa02756b5194b36

commit 57111c9ffef56e96560373618aa02756b5194b36
Author: reveman <reveman@chromium.org>
Date: Mon Jun 20 18:16:49 2016

gpu: Export/import buffers created by Ozone GMB factory.

This allows GMBs created by one instance of the GPU process to
be used with another instance. It also allows the client to
synchronize creation of one buffer with destruction of another
buffer as they are now created when imported for CHROMIUM_image.

Note: GMB factory creation of buffer from handle can be
eliminated as a follow up to this.

BUG= 597932 , b/29239573
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/1837703002
Cr-Commit-Position: refs/heads/master@{#400730}

[modify] https://crrev.com/57111c9ffef56e96560373618aa02756b5194b36/gpu/ipc/client/gpu_channel_host.cc
[modify] https://crrev.com/57111c9ffef56e96560373618aa02756b5194b36/gpu/ipc/client/gpu_memory_buffer_impl_ozone_native_pixmap.cc
[modify] https://crrev.com/57111c9ffef56e96560373618aa02756b5194b36/gpu/ipc/client/gpu_memory_buffer_impl_ozone_native_pixmap.h
[modify] https://crrev.com/57111c9ffef56e96560373618aa02756b5194b36/gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.cc
[modify] https://crrev.com/57111c9ffef56e96560373618aa02756b5194b36/gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.h
[modify] https://crrev.com/57111c9ffef56e96560373618aa02756b5194b36/ui/ozone/platform/drm/gpu/gbm_buffer.cc

Status: Fixed (was: Assigned)
So this doesn't use the CreateGpuMemoryBufferFromClientId interface? If so, I think I can take that out, since we didn't end up using it on Mac.
Yes, we're not using that. We're just passing ownership using FDs now and that simplifies synchronization enormously. One major benefit is that sync points are not required when creating/destroying GLImages. I should be easy to get the same performance improvement for IOSurface GMBs as we already use mach_port_ts to pass ownership for there.
Labels: VerifyIn-53
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f

commit 15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f
Author: reveman <reveman@chromium.org>
Date: Tue Jul 19 18:21:12 2016

gpu: Take surface handle into account when importing buffers.

This makes us use the appropriate DRM device when importing
native GMBs on Ozone instead of always using the primary device.

BUG= 626807 , 597932 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2148983002
Cr-Commit-Position: refs/heads/master@{#406322}

[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/cc/test/test_image_factory.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/cc/test/test_image_factory.h
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/command_buffer/service/image_factory.h
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/command_buffer/service/in_process_command_buffer.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/command_buffer/tests/texture_image_factory.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/command_buffer/tests/texture_image_factory.h
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/ipc/service/gpu_channel.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/ipc/service/gpu_channel.h
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/ipc/service/gpu_command_buffer_stub.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.h
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.h
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/ipc/service/gpu_memory_buffer_factory_surface_texture.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/gpu/ipc/service/gpu_memory_buffer_factory_surface_texture.h
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/media/gpu/vaapi_drm_picture.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/ui/gl/gl_image_ozone_native_pixmap_drm_unittest.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/ui/ozone/platform/drm/gpu/drm_device_manager.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/ui/ozone/platform/drm/gpu/drm_thread.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/ui/ozone/platform/drm/gpu/drm_thread.h
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/ui/ozone/platform/drm/gpu/drm_thread_proxy.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/ui/ozone/platform/drm/gpu/drm_thread_proxy.h
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/ui/ozone/platform/drm/gpu/gbm_surface_factory.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/ui/ozone/platform/drm/gpu/gbm_surface_factory.h
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/ui/ozone/public/surface_factory_ozone.cc
[modify] https://crrev.com/15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f/ui/ozone/public/surface_factory_ozone.h

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 22 2016

Labels: merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/50fc23ee168e28cba338c9b3b9081d17e416834c

commit 50fc23ee168e28cba338c9b3b9081d17e416834c
Author: David Reveman <reveman@chromium.org>
Date: Fri Jul 22 17:56:39 2016

gpu: Take surface handle into account when importing buffers.

This makes us use the appropriate DRM device when importing
native GMBs on Ozone instead of always using the primary device.

BUG= 626807 , 597932 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2148983002
Cr-Commit-Position: refs/heads/master@{#406322}
(cherry picked from commit 15e3d9a4516ad22cc56c1ff162f32f3d03e8de9f)

Review URL: https://codereview.chromium.org/2178563002 .

Cr-Commit-Position: refs/branch-heads/2785@{#305}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/cc/test/test_image_factory.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/cc/test/test_image_factory.h
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/command_buffer/service/image_factory.h
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/command_buffer/service/in_process_command_buffer.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/command_buffer/tests/texture_image_factory.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/command_buffer/tests/texture_image_factory.h
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/ipc/service/gpu_channel.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/ipc/service/gpu_channel.h
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/ipc/service/gpu_command_buffer_stub.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.h
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.h
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/ipc/service/gpu_memory_buffer_factory_surface_texture.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/gpu/ipc/service/gpu_memory_buffer_factory_surface_texture.h
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/media/gpu/vaapi_drm_picture.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/ui/gl/gl_image_ozone_native_pixmap_drm_unittest.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/ui/ozone/platform/drm/gpu/drm_device_manager.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/ui/ozone/platform/drm/gpu/drm_thread.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/ui/ozone/platform/drm/gpu/drm_thread.h
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/ui/ozone/platform/drm/gpu/drm_thread_proxy.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/ui/ozone/platform/drm/gpu/drm_thread_proxy.h
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/ui/ozone/platform/drm/gpu/gbm_surface_factory.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/ui/ozone/platform/drm/gpu/gbm_surface_factory.h
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/ui/ozone/public/surface_factory_ozone.cc
[modify] https://crrev.com/50fc23ee168e28cba338c9b3b9081d17e416834c/ui/ozone/public/surface_factory_ozone.h

Labels: VerifyIn-54
Labels: VerifyIn-55

Comment 18 by dchan@google.com, Nov 19 2016

Labels: VerifyIn-56

Comment 19 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 20 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 21 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 22 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 24 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment