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>

diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
index 1db7cf6b291e4cb15133f9e6467307da16b78240..fdabb8b622893bc9b7430f270cfb1430bed6b02a 100644 (file)
--- 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"
@@ -381,13 +382,25 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
 
       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 128fe872db1f2f42cf6290266b1f46e1488d6fa8..228f1e823c5e388db01d0deb335e25d3c44d1af2 100644 (file)
--- a/src/util/os_file.c
+++ b/src/util/os_file.c
@@ -133,12 +133,16 @@ os_read_file(const char *filename)
    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 @@ os_read_file(const char *filename)
    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 1972beba32b11ae2ab04a0b2f5f8a55642df9898..58639476f60a42645430abc66da7d7f2dae82983 100644 (file)
--- a/src/util/os_file.h
+++ b/src/util/os_file.h
@@ -32,10 +32,14 @@ char *
 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