[compiler-rt] e202ff4 - [profile] Implement a non-mmap path when reading profile files from a non-local filesystem (#131177)
Wael Yehia via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 20 10:17:13 PDT 2025
Author: Wael Yehia
Date: 2025-03-20T17:34:34Z
New Revision: e202ff45df206859b6e3c2142a735348a0d449bb
URL: https://github.com/llvm/llvm-project/commit/e202ff45df206859b6e3c2142a735348a0d449bb
DIFF: https://github.com/llvm/llvm-project/commit/e202ff45df206859b6e3c2142a735348a0d449bb.diff
LOG: [profile] Implement a non-mmap path when reading profile files from a non-local filesystem (#131177)
On AIX, when accessing mmap'ed memory associated to a file on NFS, a
SIGBUS might be raised at random.
The problem is still in open state with the OS team.
This PR teaches the profile runtime, under certain conditions, to avoid
the mmap when reading the profile file during online merging.
This PR has no effect on any platform other than AIX because I'm not
aware of this problem on other platforms.
Other platforms can easily opt-in to this functionality in the future.
The logic in function `is_local_filesystem` was copied from
[llvm/lib/Support/Unix/Path.inc](https://github.com/llvm/llvm-project/blob/f388ca3d9d9a58e3d189458b590ba68dfd9e5a2d/llvm/lib/Support/Unix/Path.inc#L515)
(https://reviews.llvm.org/D58801), because it seems that the
compiler-rt/profile cannot reuse code from llvm except through
`InstrProfData.inc`.
Thanks to @hubert-reinterpretcast for substantial feedback downstream.
---------
Co-authored-by: Wael Yehia <wyehia at ca.ibm.com>
Co-authored-by: Hubert Tong <hubert.reinterpretcast at gmail.com>
Added:
compiler-rt/test/profile/instrprof-no-mmap-during-merging.c
Modified:
compiler-rt/lib/profile/InstrProfilingFile.c
compiler-rt/lib/profile/InstrProfilingUtil.c
compiler-rt/lib/profile/InstrProfilingUtil.h
compiler-rt/test/profile/Posix/instrprof-fork.c
Removed:
################################################################################
diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index d7c3377d4bd56..19467429cf4c3 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -419,17 +419,17 @@ static int getProfileFileSizeForMerging(FILE *ProfileFile,
* \p ProfileBuffer. Returns -1 on failure. On success, the caller is
* responsible for unmapping the mmap'd buffer in \p ProfileBuffer. */
static int mmapProfileForMerging(FILE *ProfileFile, uint64_t ProfileFileSize,
- char **ProfileBuffer) {
- *ProfileBuffer = mmap(NULL, ProfileFileSize, PROT_READ, MAP_SHARED | MAP_FILE,
- fileno(ProfileFile), 0);
- if (*ProfileBuffer == MAP_FAILED) {
- PROF_ERR("Unable to merge profile data, mmap failed: %s\n",
- strerror(errno));
+ ManagedMemory *ProfileBuffer) {
+ lprofGetFileContentBuffer(ProfileFile, ProfileFileSize, ProfileBuffer);
+
+ if (ProfileBuffer->Status == MS_INVALID) {
+ PROF_ERR("Unable to merge profile data: %s\n", "reading file failed");
return -1;
}
- if (__llvm_profile_check_compatibility(*ProfileBuffer, ProfileFileSize)) {
- (void)munmap(*ProfileBuffer, ProfileFileSize);
+ if (__llvm_profile_check_compatibility(ProfileBuffer->Addr,
+ ProfileFileSize)) {
+ (void)lprofReleaseBuffer(ProfileBuffer, ProfileFileSize);
PROF_WARN("Unable to merge profile data: %s\n",
"source profile file is not compatible.");
return -1;
@@ -444,7 +444,7 @@ static int mmapProfileForMerging(FILE *ProfileFile, uint64_t ProfileFileSize,
*/
static int doProfileMerging(FILE *ProfileFile, int *MergeDone) {
uint64_t ProfileFileSize;
- char *ProfileBuffer;
+ ManagedMemory ProfileBuffer;
/* Get the size of the profile on disk. */
if (getProfileFileSizeForMerging(ProfileFile, &ProfileFileSize) == -1)
@@ -460,9 +460,9 @@ static int doProfileMerging(FILE *ProfileFile, int *MergeDone) {
return -1;
/* Now start merging */
- if (__llvm_profile_merge_from_buffer(ProfileBuffer, ProfileFileSize)) {
+ if (__llvm_profile_merge_from_buffer(ProfileBuffer.Addr, ProfileFileSize)) {
PROF_ERR("%s\n", "Invalid profile data to merge");
- (void)munmap(ProfileBuffer, ProfileFileSize);
+ (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
return -1;
}
@@ -471,7 +471,7 @@ static int doProfileMerging(FILE *ProfileFile, int *MergeDone) {
(void)COMPILER_RT_FTRUNCATE(ProfileFile,
__llvm_profile_get_size_for_buffer());
- (void)munmap(ProfileBuffer, ProfileFileSize);
+ (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
*MergeDone = 1;
return 0;
@@ -672,13 +672,13 @@ static void initializeProfileForContinuousMode(void) {
} else {
/* The merged profile has a non-zero length. Check that it is compatible
* with the data in this process. */
- char *ProfileBuffer;
+ ManagedMemory ProfileBuffer;
if (mmapProfileForMerging(File, ProfileFileSize, &ProfileBuffer) == -1) {
lprofUnlockFileHandle(File);
fclose(File);
return;
}
- (void)munmap(ProfileBuffer, ProfileFileSize);
+ (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
}
} else {
File = fopen(Filename, FileOpenMode);
@@ -1257,12 +1257,12 @@ COMPILER_RT_VISIBILITY int __llvm_profile_set_file_object(FILE *File,
} else {
/* The merged profile has a non-zero length. Check that it is compatible
* with the data in this process. */
- char *ProfileBuffer;
+ ManagedMemory ProfileBuffer;
if (mmapProfileForMerging(File, ProfileFileSize, &ProfileBuffer) == -1) {
lprofUnlockFileHandle(File);
return 1;
}
- (void)munmap(ProfileBuffer, ProfileFileSize);
+ (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
}
mmapForContinuousMode(0, File);
lprofUnlockFileHandle(File);
diff --git a/compiler-rt/lib/profile/InstrProfilingUtil.c b/compiler-rt/lib/profile/InstrProfilingUtil.c
index c637b9d0b893c..0fae91cfb8950 100644
--- a/compiler-rt/lib/profile/InstrProfilingUtil.c
+++ b/compiler-rt/lib/profile/InstrProfilingUtil.c
@@ -21,6 +21,15 @@
#include <unistd.h>
#endif
+#ifdef _AIX
+#include <sys/statfs.h>
+// <sys/vmount.h> depends on `uint` to be a typedef from <sys/types.h> to
+// `uint_t`; however, <sys/types.h> does not always declare `uint`. We provide
+// the typedef prior to including <sys/vmount.h> to work around this issue.
+typedef uint_t uint;
+#include <sys/vmount.h>
+#endif
+
#ifdef COMPILER_RT_HAS_UNAME
#include <sys/utsname.h>
#endif
@@ -258,6 +267,121 @@ COMPILER_RT_VISIBILITY FILE *lprofOpenFileEx(const char *ProfileName) {
return f;
}
+#if defined(_AIX)
+// Return 1 (true) if the file descriptor Fd represents a file that is on a
+// local filesystem, otherwise return 0.
+static int isLocalFilesystem(int Fd) {
+ struct statfs Vfs;
+ if (fstatfs(Fd, &Vfs) != 0) {
+ PROF_ERR("%s: fstatfs(%d) failed: %s\n", __func__, Fd, strerror(errno));
+ return 0;
+ }
+
+ int Ret;
+ size_t BufSize = 2048u;
+ char *Buf;
+ int Tries = 3;
+ while (Tries--) {
+ Buf = malloc(BufSize);
+ // mntctl returns -1 if `Buf` is `NULL`.
+ Ret = mntctl(MCTL_QUERY, BufSize, Buf);
+ if (Ret != 0)
+ break;
+ BufSize = *(unsigned int *)Buf;
+ free(Buf);
+ }
+
+ if (Ret != -1) {
+ // Look for the correct vmount entry.
+ char *CurObjPtr = Buf;
+ while (Ret--) {
+ struct vmount *Vp = (struct vmount *)CurObjPtr;
+ _Static_assert(sizeof(Vfs.f_fsid) == sizeof(Vp->vmt_fsid),
+ "fsid length mismatch");
+ if (memcmp(&Vfs.f_fsid, &Vp->vmt_fsid, sizeof Vfs.f_fsid) == 0) {
+ int Answer = (Vp->vmt_flags & MNT_REMOTE) == 0;
+ free(Buf);
+ return Answer;
+ }
+ CurObjPtr += Vp->vmt_length;
+ }
+ }
+
+ free(Buf);
+ // There was an error in mntctl or vmount entry not found; "remote" is the
+ // conservative answer.
+ return 0;
+}
+#endif
+
+static int isMmapSafe(int Fd) {
+ if (getenv("LLVM_PROFILE_NO_MMAP")) // For testing purposes.
+ return 0;
+#ifdef _AIX
+ return isLocalFilesystem(Fd);
+#else
+ return 1;
+#endif
+}
+
+COMPILER_RT_VISIBILITY void lprofGetFileContentBuffer(FILE *F, uint64_t Length,
+ ManagedMemory *Buf) {
+ Buf->Status = MS_INVALID;
+ if (isMmapSafe(fileno(F))) {
+ Buf->Addr =
+ mmap(NULL, Length, PROT_READ, MAP_SHARED | MAP_FILE, fileno(F), 0);
+ if (Buf->Addr == MAP_FAILED)
+ PROF_ERR("%s: mmap failed: %s\n", __func__, strerror(errno))
+ else
+ Buf->Status = MS_MMAP;
+ return;
+ }
+
+ if (getenv("LLVM_PROFILE_VERBOSE"))
+ PROF_NOTE("%s\n", "could not use mmap; using fread instead");
+
+ void *Buffer = malloc(Length);
+ if (!Buffer) {
+ PROF_ERR("%s: malloc failed: %s\n", __func__, strerror(errno));
+ return;
+ }
+ if (ftell(F) != 0) {
+ PROF_ERR("%s: expecting ftell to return zero\n", __func__);
+ free(Buffer);
+ return;
+ }
+
+ // Read the entire file into memory.
+ size_t BytesRead = fread(Buffer, 1, Length, F);
+ if (BytesRead != (size_t)Length) {
+ PROF_ERR("%s: fread failed%s\n", __func__,
+ feof(F) ? ": end of file reached" : "");
+ free(Buffer);
+ return;
+ }
+
+ // Reading was successful, record the result in the Buf parameter.
+ Buf->Addr = Buffer;
+ Buf->Status = MS_MALLOC;
+}
+
+COMPILER_RT_VISIBILITY
+void lprofReleaseBuffer(ManagedMemory *Buf, size_t Length) {
+ switch (Buf->Status) {
+ case MS_MALLOC:
+ free(Buf->Addr);
+ break;
+ case MS_MMAP:
+ (void)munmap(Buf->Addr, Length);
+ break;
+ default:
+ PROF_ERR("%s: Buffer has invalid state: %d\n", __func__, Buf->Status);
+ break;
+ }
+ Buf->Addr = NULL;
+ Buf->Status = MS_INVALID;
+}
+
COMPILER_RT_VISIBILITY const char *lprofGetPathPrefix(int *PrefixStrip,
size_t *PrefixLen) {
const char *Prefix = getenv("GCOV_PREFIX");
diff --git a/compiler-rt/lib/profile/InstrProfilingUtil.h b/compiler-rt/lib/profile/InstrProfilingUtil.h
index 227c2aa0a7cae..234cf26cc0af7 100644
--- a/compiler-rt/lib/profile/InstrProfilingUtil.h
+++ b/compiler-rt/lib/profile/InstrProfilingUtil.h
@@ -31,6 +31,25 @@ int lprofUnlockFileHandle(FILE *F);
* lock for exclusive access. The caller will block
* if the lock is already held by another process. */
FILE *lprofOpenFileEx(const char *Filename);
+
+enum MemoryStatus {
+ MS_INVALID, // Addr is not a valid address
+ MS_MMAP, // Addr was mmap'ed
+ MS_MALLOC // Addr was malloc'ed
+};
+typedef struct {
+ void *Addr;
+ enum MemoryStatus Status;
+} ManagedMemory;
+
+/* Read the content of a file using mmap or fread into a buffer.
+ * Certain files (e.g. NFS mounted) cannot be opened reliably with mmap,
+ * so we use fread in those cases. The corresponding lprofReleaseBuffer
+ * will free/munmap the buffer.
+ */
+void lprofGetFileContentBuffer(FILE *F, uint64_t FileSize, ManagedMemory *Buf);
+void lprofReleaseBuffer(ManagedMemory *FileBuffer, size_t Length);
+
/* PS4 doesn't have setenv/getenv/fork. Define a shim. */
#if __ORBIS__
#include <sys/types.h>
diff --git a/compiler-rt/test/profile/Posix/instrprof-fork.c b/compiler-rt/test/profile/Posix/instrprof-fork.c
index 8df0abd73c4c3..a1b99b492aa4f 100644
--- a/compiler-rt/test/profile/Posix/instrprof-fork.c
+++ b/compiler-rt/test/profile/Posix/instrprof-fork.c
@@ -3,11 +3,15 @@
// RUN: %clang_pgogen=%t.profdir -o %t -O2 %s
// RUN: %run %t
// RUN: llvm-profdata show --all-functions --counts %t.profdir/default_*.profraw | FileCheck %s
+// RUN: rm -fr %t.profdir
+// RUN: env LLVM_PROFILE_NO_MMAP=1 %run %t
+// RUN: llvm-profdata show --all-functions --counts %t.profdir/default_*.profraw | FileCheck %s
+
//
// CHECK: func1:
-// CHECK: Block counts: [4]
+// CHECK: Block counts: [21]
// CHECK: func2:
-// CHECK: Block counts: [1]
+// CHECK: Block counts: [10]
#include <sys/wait.h>
#include <unistd.h>
@@ -16,17 +20,23 @@ __attribute__((noinline)) void func1() {}
__attribute__((noinline)) void func2() {}
int main(void) {
- // child | parent
- int status; // func1 func2 | func1 func2
- func1(); // +1 | +1 (*)
- pid_t pid = fork(); // |
- if (pid == -1) // |
- return 1; // |
- if (pid == 0) // |
- func2(); // +1 |
- func1(); // +1 | +1
- if (pid) // ------------+------------
- wait(&status); // 2 1 | 2 0
- return 0; // (*) the child inherits counter values prior to fork
- // from the parent in non-continuous mode.
+ // child | parent
+ // func1 func2 | func1 func2
+ func1(); // +10 | +1 (*)
+ int i = 10; // |
+ while (i-- > 0) { // |
+ pid_t pid = fork(); // |
+ if (pid == -1) // |
+ return 1; // |
+ if (pid == 0) { // |
+ func2(); // +10 |
+ func1(); // +10 |
+ return 0; // |
+ } // |
+ } // ------------+------------
+ int status; // 20 10 | 1 0
+ i = 10; // (*) the child inherits counter values prior to fork
+ while (i-- > 0) // from the parent in non-continuous mode.
+ wait(&status);
+ return 0;
}
diff --git a/compiler-rt/test/profile/instrprof-no-mmap-during-merging.c b/compiler-rt/test/profile/instrprof-no-mmap-during-merging.c
new file mode 100644
index 0000000000000..0f269388388d8
--- /dev/null
+++ b/compiler-rt/test/profile/instrprof-no-mmap-during-merging.c
@@ -0,0 +1,25 @@
+// RUN: mkdir -p %t.d && cd %t.d
+// RUN: rm -f *.profraw
+// RUN: %clang_pgogen %s -o a.out
+
+// Need to run a.out twice. On the second time, a merge will occur, which will
+// trigger an mmap.
+// RUN: ./a.out
+// RUN: llvm-profdata show default_*.profraw --all-functions --counts --memop-sizes 2>&1 | FileCheck %s -check-prefix=PROFDATA
+// RUN: env LLVM_PROFILE_NO_MMAP=1 LLVM_PROFILE_VERBOSE=1 ./a.out 2>&1 | FileCheck %s
+// RUN: llvm-profdata show default_*.profraw --all-functions --counts --memop-sizes 2>&1 | FileCheck %s -check-prefix=PROFDATA2
+
+// CHECK: could not use mmap; using fread instead
+// PROFDATA: Block counts: [1]
+// PROFDATA: [ 0, 0, 1 ]
+// PROFDATA: Maximum function count: 1
+// PROFDATA2: Block counts: [2]
+// PROFDATA2: [ 0, 0, 2 ]
+// PROFDATA2: Maximum function count: 2
+
+int ar[8];
+int main() {
+ __builtin_memcpy(ar, ar + 2, ar[0]);
+ __builtin_memcpy(ar, ar + 2, ar[2]);
+ return ar[2];
+}
More information about the llvm-commits
mailing list