util: Change os_same_file_description return type from bool to int
This allows communicating that it wasn't possible to determine whether
the two file descriptors reference the same file description. When
that's the case, log a warning in the amdgpu winsys.
In turn, remove the corresponding debugging output from the fallback
os_same_file_description implementation. It depends on the caller if
false negatives are problematic or not.
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Tested-by: Marge Bot <https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3879>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3879>
(cherry picked from commit f5a8958910f53d924d062cbf024cebe4134f757a)
diff --git a/.pick_status.json b/.pick_status.json
index 14baf63..ca9b2c0 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -39415,7 +39415,7 @@
"description": "util: Change os_same_file_description return type from bool to int",
"nominated": false,
"nomination_type": null,
- "resolution": 4,
+ "resolution": 1,
"master_sha": null,
"because_sha": null
},
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
index 4bbd506..8b56f69 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
@@ -31,6 +31,7 @@
#include "amdgpu_public.h"
#include "util/os_file.h"
+#include "util/os_misc.h"
#include "util/u_cpu_detect.h"
#include "util/u_hash_table.h"
#include "util/hash_table.h"
@@ -380,13 +381,25 @@
simple_mtx_lock(&aws->sws_list_lock);
for (sws_iter = aws->sws_list; sws_iter; sws_iter = sws_iter->next) {
- if (os_same_file_description(sws_iter->fd, ws->fd)) {
+ r = os_same_file_description(sws_iter->fd, ws->fd);
+
+ if (r == 0) {
close(ws->fd);
FREE(ws);
ws = sws_iter;
pipe_reference(NULL, &ws->reference);
simple_mtx_unlock(&aws->sws_list_lock);
goto unlock;
+ } else if (r < 0) {
+ static bool logged;
+
+ if (!logged) {
+ os_log_message("amdgpu: os_same_file_description couldn't "
+ "determine if two DRM fds reference the same "
+ "file description.\n"
+ "If they do, bad things may happen!\n");
+ logged = true;
+ }
}
}
simple_mtx_unlock(&aws->sws_list_lock);
diff --git a/src/util/os_file.c b/src/util/os_file.c
index 128fe87..228f1e8 100644
--- a/src/util/os_file.c
+++ b/src/util/os_file.c
@@ -133,12 +133,16 @@
return buf;
}
-bool
+int
os_same_file_description(int fd1, int fd2)
{
pid_t pid = getpid();
- return syscall(SYS_kcmp, pid, pid, KCMP_FILE, fd1, fd2) == 0;
+ /* Same file descriptor trivially implies same file description */
+ if (fd1 == fd2)
+ return 0;
+
+ return syscall(SYS_kcmp, pid, pid, KCMP_FILE, fd1, fd2);
}
#else
@@ -152,15 +156,15 @@
return NULL;
}
-bool
+int
os_same_file_description(int fd1, int fd2)
{
+ /* Same file descriptor trivially implies same file description */
if (fd1 == fd2)
- return true;
+ return 0;
- debug_warn_once("Can't tell if different file descriptors reference the same"
- " file description, false negatives might cause trouble!\n");
- return false;
+ /* Otherwise we can't tell */
+ return -1;
}
#endif
diff --git a/src/util/os_file.h b/src/util/os_file.h
index 1972beb..5863947 100644
--- a/src/util/os_file.h
+++ b/src/util/os_file.h
@@ -32,10 +32,14 @@
os_read_file(const char *filename);
/*
- * Returns true if the two file descriptors passed in can be determined to
- * reference the same file description, false otherwise
+ * Try to determine if two file descriptors reference the same file description
+ *
+ * Return values:
+ * - 0: They reference the same file description
+ * - > 0: They do not reference the same file description
+ * - < 0: Unable to determine whether they reference the same file description
*/
-bool
+int
os_same_file_description(int fd1, int fd2);
#ifdef __cplusplus
