Handle EAGAIN error for the mincore syscall used

base/trace_event/process_memory_dump.cc

Index: base/trace_event/process_memory_dump.cc
diff --git a/base/trace_event/process_memory_dump.cc b/base/trace_event/process_memory_dump.cc
index 0ec007e2ee46f5140bf83ee5580a7f5c1ee221f4..86026070941d1b1248c714d01500008f80ca0973 100644
--- a/base/trace_event/process_memory_dump.cc
+++ b/base/trace_event/process_memory_dump.cc
@@ -30,8 +30,10 @@ std::string GetSharedGlobalAllocatorDumpName(
 
 #if defined(COUNT_RESIDENT_BYTES_SUPPORTED)
 // static
-size_t ProcessMemoryDump::CountResidentBytes(void* start_address,
-                                             size_t mapped_size) {
+bool ProcessMemoryDump::CountResidentBytes(void* start_address,
+                                           size_t mapped_size,
+                                           size_t* resident_size) {
+  *resident_size = 0;
   const size_t page_size = GetPageSize();
   const uintptr_t start_pointer = reinterpret_cast<uintptr_t>(start_address);
   DCHECK_EQ(0u, start_pointer % page_size);
@@ -42,7 +44,6 @@ size_t ProcessMemoryDump::CountResidentBytes(void* start_address,
   // kPageChunkSize / page_size.
   const size_t kMaxChunkSize = 32 * 1024 * 1024;
   size_t offset = 0;
-  size_t total_resident_size = 0;
   while (offset < mapped_size) {
     void* chunk_start = reinterpret_cast<void*>(start_pointer + offset);
     const size_t chunk_size = std::min(mapped_size - offset, kMaxChunkSize);
@@ -52,21 +53,29 @@ size_t ProcessMemoryDump::CountResidentBytes(void* start_address,
 #if defined(OS_MACOSX) || defined(OS_IOS)
     std::vector<char> vec(page_count + 1);
     int res = mincore(chunk_start, chunk_size, vector_as_array(&vec));
-    DCHECK(!res);
+    if (res)
+      return false;
+
     for (size_t i = 0; i < page_count; i++)
       resident_page_count += vec[i] & MINCORE_INCORE ? 1 : 0;
 #else   // defined(OS_MACOSX) || defined(OS_IOS)
     std::vector<unsigned char> vec(page_count + 1);
-    int res = mincore(chunk_start, chunk_size, vector_as_array(&vec));
-    DCHECK(!res);
+    int error_counter = 0;
+    int res = 0;
+    do {

[reveman, 2015/10/14 12:31:28]

why do we allow retries here but not above?

[Primiano Tucci (use gerrit), 2015/10/14 12:41:33]

I guess because mac's manpage for mincore says it can only return EINVAL or
EFAULT

[ssid, 2015/10/14 12:44:29]

yes.

[reveman, 2015/10/14 13:03:53]

Got it. Maybe add a comment here or above to explain the difference.

[ssid, 2015/10/14 13:35:55]

Done.

+      res = mincore(chunk_start, chunk_size, vector_as_array(&vec));
+    } while (res == -1 && errno == EAGAIN && error_counter++ < 100);

[Primiano Tucci (use gerrit), 2015/10/14 11:27:08]

i'd cap this at 3. Typically either this is a temporary issue, or it will last
longer.

[ssid, 2015/10/14 12:44:29]

Added comment why it is 100.

[reveman, 2015/10/14 13:03:53]

is it worth retrying at all?

[ssid, 2015/10/14 13:35:55]

That is how eagain is handled in code. Like in case of EINTR.

[reveman, 2015/10/14 14:17:11]

Most of the EAGAIN code I see in the chromium code base doesn't retry. Debug
builds retry forever in the case of EINTR and release builds will ignore the
failure and pretend it worked fine after a number of attempts. Can we do the
same here? That would mean that this function doesn't need a bool return value
after all.

I think one of these two options should be used:

1. CountResidentBytes can fail and we don't bother retrying inside this function
when receiving EAGAIN. The caller can always retry if that makes sense. 
2. CountResidentBytes always succeeds and we retry inside this function when
receiving EAGAIN.

I prefer option 2) as that makes it easier for users of this function but I'm
not familiar with the reason EAGAIN would be returned to tell if this would be
OK.

+    if (res)
+      return false;
+
     for (size_t i = 0; i < page_count; i++)
       resident_page_count += vec[i];
 #endif  // defined(OS_MACOSX) || defined(OS_IOS)
 
-    total_resident_size += resident_page_count * page_size;
+    *resident_size += resident_page_count * page_size;
     offset += kMaxChunkSize;
   }
-  return total_resident_size;
+  return true;
 }
 #endif  // defined(COUNT_RESIDENT_BYTES_SUPPORTED)