Log when creating a second buffer mapping in a process

Bug: 8468756
Change-Id: Ia883f459ea9e2648ca4a0b5a6f09ded4f46f13b3

1 files changed, 40 insertions, 0 deletions

diff --git a/modules/gralloc/mapper.cpp b/modules/gralloc/mapper.cpp
index 8aadb4a..e57dba9 100644
--- a/modules/gralloc/mapper.cpp
+++ b/modules/gralloc/mapper.cpp
@@ -92,6 +92,46 @@ int gralloc_register_buffer(gralloc_module_t const* module,
     if (private_handle_t::validate(handle) < 0)
         return -EINVAL;
 
+    // *** WARNING WARNING WARNING ***
+    //
+    // If a buffer handle is passed from the process that allocated it to a
+    // different process, and then back to the allocator process, we will
+    // create a second mapping of the buffer. If the process reads and writes
+    // through both mappings, normal memory ordering guarantees may be
+    // violated, depending on the processor cache implementation*.
+    //
+    // If you are deriving a new gralloc implementation from this code, don't
+    // do this. A "real" gralloc should provide a single reference-counted
+    // mapping for each buffer in a process.
+    //
+    // In the current system, there is one case that needs a buffer to be
+    // registered in the same process that allocated it. The SurfaceFlinger
+    // process acts as the IGraphicBufferAlloc Binder provider, so all gralloc
+    // allocations happen in its process. After returning the buffer handle to
+    // the IGraphicBufferAlloc client, SurfaceFlinger free's its handle to the
+    // buffer (unmapping it from the SurfaceFlinger process). If
+    // SurfaceFlinger later acts as the producer end of the buffer queue the
+    // buffer belongs to, it will get a new handle to the buffer in response
+    // to IGraphicBufferProducer::requestBuffer(). Like any buffer handle
+    // received through Binder, the SurfaceFlinger process will register it.
+    // Since it already freed its original handle, it will only end up with
+    // one mapping to the buffer and there will be no problem.
+    //
+    // Currently SurfaceFlinger only acts as a buffer producer for a remote
+    // consumer when taking screenshots and when using virtual displays.
+    //
+    // Eventually, each application should be allowed to make its own gralloc
+    // allocations, solving the problem. Also, this ashmem-based gralloc
+    // should go away, replaced with a real ion-based gralloc.
+    //
+    // * Specifically, associative virtually-indexed caches are likely to have
+    //   problems. Most modern L1 caches fit that description.
+
+    private_handle_t* hnd = (private_handle_t*)handle;
+    ALOGD_IF(hnd->pid == getpid(),
+            "Registering a buffer in the process that created it. "
+            "This may cause memory ordering problems.");
+
     void *vaddr;
     return gralloc_map(module, handle, &vaddr);
 }