From e3518d41d88c2a3c36e7b806c1d04f4101ebfef5 Mon Sep 17 00:00:00 2001
From: Erick Reyes <erickreyes@google.com>
Date: Wed, 30 Jan 2019 12:02:26 -0800
Subject: [PATCH] meminfo: Fix dmabufinfo total ref accounting

The number of references to a buffer total_refs was not accounting for
a process holding multiple references to the same buffer.

Also, clarify AppendDmaBufInfo functionality in comments

Change-Id: I044a18acad1570492a925d51b9464d5b0e60ca35
Signed-off-by: Erick Reyes <erickreyes@google.com>
---
 libmeminfo/libdmabufinfo/dmabufinfo.cpp        |  2 +-
 .../include/dmabufinfo/dmabufinfo.h            | 18 +++++++++++-------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/libmeminfo/libdmabufinfo/dmabufinfo.cpp b/libmeminfo/libdmabufinfo/dmabufinfo.cpp
index b4ad6678a..0212cd23a 100644
--- a/libmeminfo/libdmabufinfo/dmabufinfo.cpp
+++ b/libmeminfo/libdmabufinfo/dmabufinfo.cpp
@@ -130,7 +130,7 @@ static bool ReadDmaBufFdRefs(pid_t pid, std::vector<DmaBuffer>* dmabufs) {
             if (buf->count() == 0)
                 buf->SetCount(count);
             buf->AddFdRef(pid);
-            return true;
+            continue;
         }
 
         DmaBuffer& db =
diff --git a/libmeminfo/libdmabufinfo/include/dmabufinfo/dmabufinfo.h b/libmeminfo/libdmabufinfo/include/dmabufinfo/dmabufinfo.h
index e3be320b4..a16c3fdaf 100644
--- a/libmeminfo/libdmabufinfo/include/dmabufinfo/dmabufinfo.h
+++ b/libmeminfo/libdmabufinfo/include/dmabufinfo/dmabufinfo.h
@@ -30,17 +30,21 @@ struct DmaBuffer {
   public:
     DmaBuffer(ino_t inode, uint64_t size, uint64_t count, const std::string& exporter,
               const std::string& name)
-        : inode_(inode), size_(size), count_(count), exporter_(exporter), name_(name) {}
+        : inode_(inode), size_(size), count_(count), exporter_(exporter), name_(name) {
+        total_refs_ = 0;
+    }
     ~DmaBuffer() = default;
 
     // Adds one file descriptor reference for the given pid
     void AddFdRef(pid_t pid) {
         AddRefToPidMap(pid, &fdrefs_);
+        total_refs_++;
     }
 
     // Adds one map reference for the given pid
     void AddMapRef(pid_t pid) {
         AddRefToPidMap(pid, &maprefs_);
+        total_refs_++;
     }
 
     // Getters for each property
@@ -48,7 +52,7 @@ struct DmaBuffer {
     const std::unordered_map<pid_t, int>& fdrefs() const { return fdrefs_; }
     const std::unordered_map<pid_t, int>& maprefs() const { return maprefs_; }
     ino_t inode() const { return inode_; }
-    uint64_t total_refs() const { return fdrefs_.size() + maprefs_.size(); }
+    uint64_t total_refs() const { return total_refs_; }
     uint64_t count() const { return count_; };
     const std::string& name() const { return name_; }
     const std::string& exporter() const { return exporter_; }
@@ -65,6 +69,7 @@ struct DmaBuffer {
     ino_t inode_;
     uint64_t size_;
     uint64_t count_;
+    uint64_t total_refs_;
     std::string exporter_;
     std::string name_;
     std::unordered_map<pid_t, int> fdrefs_;
@@ -81,7 +86,6 @@ struct DmaBuffer {
 // Read and return current dma buf objects from
 // DEBUGFS/dma_buf/bufinfo. The references to each dma buffer are not
 // populated here and will return an empty vector.
-//
 // Returns false if something went wrong with the function, true otherwise.
 bool ReadDmaBufInfo(std::vector<DmaBuffer>* dmabufs,
                     const std::string& path = "/sys/kernel/debug/dma_buf/bufinfo");
@@ -89,13 +93,13 @@ bool ReadDmaBufInfo(std::vector<DmaBuffer>* dmabufs,
 
 // Read and return dmabuf objects for a given process without the help
 // of DEBUGFS
-//
 // Returns false if something went wrong with the function, true otherwise.
 bool ReadDmaBufInfo(pid_t pid, std::vector<DmaBuffer>* dmabufs);
 
-// Append dmabuf objects for a given process without the help
-// of DEBUGFS to an existing vector
-//
+// Append new dmabuf objects from a given process to an existing vector.
+// When the vector contains an existing element with a matching inode,
+// the reference counts will be updated.
+// Does not depend on DEBUGFS.
 // Returns false if something went wrong with the function, true otherwise.
 bool AppendDmaBufInfo(pid_t pid, std::vector<DmaBuffer>* dmabufs);