Stéphane would you mind triaging this?

Haixia, do you have cycles to look at this one?

Please note that this is again something that breaks DisplayLink on Chrome OS completely, and must find its way to R53 branch before stable - same as the fix for the original bug (626807). Thanks!

dnicoara@, does the import work correctly of if gbm_bo_destroy is not called?

Yes.

For clarity, gbm_bo_import() returns a handle in both cases. The only difference is that if gbm_bo_destroy() is called the kernel complains with the above errors.

Ok, we should get to the bottom of this and fix the real issue but if we need a short term fix then https://codereview.chromium.org/2171713004 is an option.

dnicoara@, can you check if the patch above works in case we need that as a short term solution?

Can confirm that the patch in comment #7 fixes the issue.

hshi@, do you think this can be fixed properly soon or should we land this workaround for now?

Re:#9,#10 thanks I don't see a quick proper fix for this so yes we do need to land the workaround for now.

and re #8, I too, can confirm that it fixes the issue.

reveman@ do you want to land this and merge to 53 or would you want me to do it?

reveman@: your patch (2171713004) seems reasonable and appears to be doing the right thing.

Why is this considered a workaround and not a permanent fix?

This fix requires us to re-create buffers if the GPU process crashes. We do this already for scanout buffers in chrome but there's no easy way to accomplish this for external clients (e.g. arc++) so future multi-display support and overlays is not going to work.

I can take care of landing and merging this workaround to M53.

+piman

hshi@, regarding your comment in the CL, why do you think the kernel behavior is working as intended?

Userspace is exporting the buffer handle to an FD. At that point there should be 2 valid handles for the buffer. This seems to be partially true for UDL/EVDI as importing the buffer works but the UDL/EVDI drivers are losing track of some information as a result of gbo_bo destruction (which result in failed mappings in the driver).

@16: when you import a dmabuf fd into a driver (driver A), this calls into another driver (driver B) behind the scenes.

How driver B implements dmabuf will decide what can happen at import time. If driver B implements it by grabbing an extra reference on the gem object, then yes you can do further imports/exports (that's the case for i915 for example). But there is no such guarantee for dmabuf in general, and I don't think we can enforce it everywhere...

About ARC++ and GPU process restarts: the buffer should still exist on the other side, since it is allocated there, so you should be able to re-import it on GPU process restart. So that shouldn't be a problem at all?

@17:

For ARC++, can we really rely on the buffer still existing on the other side? What if the process that allocated the buffer crashed or was killed by the low-memory killer?

For general wayland clients, it would complicated things quite a bit if we had to pay special attention to buffers after the connection was closed. For example, we might have to avoid window close animations, etc.

FYI, we are re-importing ARC++ buffers properly now as a result of a GPU process crashes.  crbug.com/597932  fixed that.

Hm, there is something which I don't understand here. If the process on the other side is gone, why do you need to import its buffer?

How do we avoid a race condition between import and us being notified that the process has died if the FD passed to us is only valid for import as long as the allocating process is alive? and how can we reliable run fade out animations of apps that have been closed?

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

commit 1ff18bae4738ee3d62c2310d9e03211054be0582
Author: reveman <reveman@chromium.org>
Date: Thu Jul 21 22:43:00 2016

gpu: Keep original GMB alive to workaround problem with importing of UDL/EVDI buffers.

Destroying the original buffer invalidates kernel state such that
importing from the FD doesn't work as expected. By keeping the
original buffer alive we avoid this issue for scanout buffers used
by the browser process.

BUG= 628334 
CQ_INCLUDE_TRYBOTS=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/2171713004
Cr-Commit-Position: refs/heads/master@{#406973}

[modify] https://crrev.com/1ff18bae4738ee3d62c2310d9e03211054be0582/gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.cc
[modify] https://crrev.com/1ff18bae4738ee3d62c2310d9e03211054be0582/gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.h

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

commit 4621081d1ffdc8a9048430f7c3d2e60cc0e309cd
Author: David Reveman <reveman@chromium.org>
Date: Fri Jul 22 18:05:12 2016

gpu: Keep original GMB alive to workaround problem with importing of UDL/EVDI buffers.

Destroying the original buffer invalidates kernel state such that
importing from the FD doesn't work as expected. By keeping the
original buffer alive we avoid this issue for scanout buffers used
by the browser process.

BUG= 628334 
CQ_INCLUDE_TRYBOTS=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/2171713004
Cr-Commit-Position: refs/heads/master@{#406973}
(cherry picked from commit 1ff18bae4738ee3d62c2310d9e03211054be0582)

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

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

[modify] https://crrev.com/4621081d1ffdc8a9048430f7c3d2e60cc0e309cd/gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.cc
[modify] https://crrev.com/4621081d1ffdc8a9048430f7c3d2e60cc0e309cd/gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.h

[Automated comment] There appears to be on-going work (i.e. bugroid changes), needs manual review.

FYI, 4621081d1ffdc8a9048430f7c3d2e60cc0e309cd was merged as part of 626807 and is a workaround for this issue. Keeping this issue open for now as I think we still need a more appropriate fix but removing RB label and it's no longer blocking 626807.

@2O: so you are concerned about the case where the process dies and the gpu process dies at the same time?

reveman@: would you like to take over from here? It sounds like you are much more familiar with the context than I am.

Approving merge to M53 cros. reveman@ Please merge this in asap.

Already merged.