Merge pull request #7 from kpouget/remoting

remoting: improve the frontend<>backend error handling

Author: kpouget

ggml/src/ggml-remotingbackend/backend-dispatched.cpp

@@ -41,7 +41,7 @@ uint32_t backend_dispatch_initialize(void *ggml_backend_reg_fct_p, void *ggml_ba
 
   size_t free, total;
   dev->iface.get_memory(dev, &free, &total);
-  WARNING("%s: free memory: %ld MB\n", __func__, (size_t) free/1024/1024);
+  INFO("%s: free memory: %ld MB", __func__, (size_t) free/1024/1024);
 
-  return APIR_BACKEND_INITIALIZE_SUCCESSS;
+  return APIR_BACKEND_INITIALIZE_SUCCESS;
 }

ggml/src/ggml-remotingbackend/backend-internal.h

@@ -5,6 +5,7 @@
 #include "ggml-impl.h"
 #include "ggml-backend-impl.h"
 #include "ggml-backend.h"
+#include "shared/api_remoting.h"
 
 extern ggml_backend_reg_t reg;
 extern ggml_backend_dev_t dev;
@@ -20,7 +21,7 @@ extern ggml_backend_t bck;
   } while(0)
 
 extern "C" {
-  uint32_t apir_backend_initialize();
+  ApirLoadLibraryReturnCode apir_backend_initialize();
   void apir_backend_deinit(void);
   uint32_t apir_backend_dispatcher(uint32_t cmd_type, struct virgl_apir_context *ctx,
 				   char *dec_cur, const char *dec_end,

ggml/src/ggml-remotingbackend/backend.cpp

@@ -7,6 +7,7 @@
 #include "backend-internal.h"
 #include "backend-dispatched.h"
 
+#include "shared/api_remoting.h"
 #include "shared/apir_backend.h"
 #include "shared/venus_cs.h"
 
@@ -26,9 +27,11 @@ extern "C" {
       buffer->iface.free_buffer(buffer);
     }
 
-    size_t free, total;
-    dev->iface.get_memory(dev, &free, &total);
-    WARNING("%s: free memory: %ld MB\n", __func__, (size_t) free/1024/1024);
+    if (dev) {
+      size_t free, total;
+      dev->iface.get_memory(dev, &free, &total);
+      INFO("%s: free memory: %ld MB", __func__, (size_t) free/1024/1024);
+    }
 
     show_timer(&graph_compute_timer);
     show_timer(&set_tensor_timer);
@@ -43,7 +46,7 @@ extern "C" {
     INFO("%s: bye-bye", __func__);
   }
 
-  uint32_t apir_backend_initialize() {
+  ApirLoadLibraryReturnCode apir_backend_initialize() {
     const char* dlsym_error;
 
     const char* library_name = getenv(GGML_BACKEND_LIBRARY_PATH_ENV);
@@ -53,56 +56,61 @@ extern "C" {
     INFO("%s: loading %s (%s|%s)", __func__, library_name, library_reg, library_init);
 
     if (!library_name) {
-      ERROR("Cannot open library: env var '%s' not defined\n", GGML_BACKEND_LIBRARY_PATH_ENV);
+      ERROR("cannot open the GGML library: env var '%s' not defined\n", GGML_BACKEND_LIBRARY_PATH_ENV);
 
-      return APIR_BACKEND_INITIALIZE_CANNOT_OPEN_GGML_LIBRARY;
+      return APIR_LOAD_LIBRARY_ENV_VAR_MISSING;
     }
 
     backend_library_handle = dlopen(library_name, RTLD_LAZY);
 
     if (!backend_library_handle) {
-      ERROR("Cannot open library: %s\n", dlerror());
+      ERROR("cannot open the GGML library: %s", dlerror());
 
-      return APIR_BACKEND_INITIALIZE_CANNOT_OPEN_GGML_LIBRARY;
+      return APIR_LOAD_LIBRARY_CANNOT_OPEN;
     }
 
     if (!library_reg) {
-      ERROR("Cannot register library: env var '%s' not defined\n", GGML_BACKEND_LIBRARY_REG_ENV);
+      ERROR("cannot register the GGML library: env var '%s' not defined", GGML_BACKEND_LIBRARY_REG_ENV);
 
-      return APIR_BACKEND_INITIALIZE_CANNOT_OPEN_GGML_LIBRARY;
+      return APIR_LOAD_LIBRARY_ENV_VAR_MISSING;
     }
 
     void *ggml_backend_reg_fct = dlsym(backend_library_handle, library_reg);
     dlsym_error = dlerror();
     if (dlsym_error) {
-      ERROR("Cannot load symbol: %s\n", dlsym_error);
+      ERROR("cannot find the GGML backend registration symbol '%s' (from %s): %s",
+	    library_reg, GGML_BACKEND_LIBRARY_REG_ENV, dlsym_error);
 
-      return APIR_BACKEND_INITIALIZE_MISSING_GGML_SYMBOLS;
+      return APIR_LOAD_LIBRARY_SYMBOL_MISSING;
     }
 
     if (!library_init) {
-      ERROR("Cannot initialize library: env var '%s' not defined\n", library_init);
+      ERROR("cannot initialize the GGML library: env var '%s' not defined", library_init);
 
-      return APIR_BACKEND_INITIALIZE_CANNOT_OPEN_GGML_LIBRARY;
+      return APIR_LOAD_LIBRARY_ENV_VAR_MISSING;
     }
 
     void *ggml_backend_init_fct = dlsym(backend_library_handle, library_init);
     dlsym_error = dlerror();
     if (dlsym_error) {
-      ERROR("Cannot load symbol: %s\n", dlsym_error);
+      ERROR("cannot find the GGML backend init symbol '%s' (from %s): %s",
+	    library_init, GGML_BACKEND_LIBRARY_INIT_ENV, dlsym_error);
 
-      return APIR_BACKEND_INITIALIZE_MISSING_GGML_SYMBOLS;
+      return APIR_LOAD_LIBRARY_SYMBOL_MISSING;
     }
 
     ggml_backend_metal_get_device_context_fct = (void (*)(ggml_backend_dev_t, bool *, bool *, bool *)) dlsym(backend_library_handle, GGML_BACKEND_LIBRARY_METAL_DEVICE_CONTEXT);
     dlsym_error = dlerror();
     if (dlsym_error) {
-      ERROR("Cannot load symbol: %s\n", dlsym_error);
+      ERROR("cannot find the GGML device context symbol '%s': %s\n",
+	    GGML_BACKEND_LIBRARY_METAL_DEVICE_CONTEXT, dlsym_error);
 
-      return APIR_BACKEND_INITIALIZE_MISSING_GGML_SYMBOLS;
+      return APIR_LOAD_LIBRARY_SYMBOL_MISSING;
     }
 
-    return backend_dispatch_initialize(ggml_backend_reg_fct, ggml_backend_init_fct);
+    uint32_t ret = backend_dispatch_initialize(ggml_backend_reg_fct, ggml_backend_init_fct);
+
+    return (ApirLoadLibraryReturnCode) (APIR_LOAD_LIBRARY_INIT_BASE_INDEX + ret);
   }
 
   uint32_t apir_backend_dispatcher(uint32_t cmd_type, struct virgl_apir_context *ctx,
@@ -122,8 +130,8 @@ extern "C" {
     struct vn_cs_decoder *dec = &_dec;
 
 
-    if (cmd_type > APIR_BACKEND_DISPATCH_TABLE_COUNT) {
-      ERROR("Received an invalid dispatch index (%d > %d)\n",
+    if (cmd_type >= APIR_BACKEND_DISPATCH_TABLE_COUNT) {
+      ERROR("Received an invalid dispatch index (%d >= %d)\n",
 	    cmd_type, APIR_BACKEND_DISPATCH_TABLE_COUNT);
       return APIR_BACKEND_FORWARD_INDEX_INVALID;
     }

ggml/src/ggml-remotingbackend/shared/api_remoting.h

@@ -1,11 +1,88 @@
-#define VIRGL_APIR_COMMAND_TYPE_LoadLibrary 331
-#define VIRGL_APIR_COMMAND_TYPE_Forward 332
+#pragma once
 
-static inline const char *api_remoting_command_name(int32_t type)
+/* the rest of this file must match virglrenderer/src/apir-protocol.h */
+
+#include <unistd.h>
+
+#define VENUS_COMMAND_TYPE_LENGTH 331
+
+#define APIR_PROTOCOL_MAJOR 0
+#define APIR_PROTOCOL_MINOR 1
+
+#define APIR_HANDSHAKE_MAGIC 0xab1e
+
+typedef enum {
+    APIR_COMMAND_TYPE_HandShake = 0,
+    APIR_COMMAND_TYPE_LoadLibrary = 1,
+    APIR_COMMAND_TYPE_Forward = 2,
+
+    APIR_COMMAND_TYPE_LENGTH = 3,
+} ApirCommandType;
+
+typedef uint64_t ApirCommandFlags;
+
+typedef enum {
+    APIR_LOAD_LIBRARY_SUCCESS = 0,
+    APIR_LOAD_LIBRARY_HYPERCALL_INITIALIZATION_ERROR = 1,
+    APIR_LOAD_LIBRARY_ALREADY_LOADED = 2,
+    APIR_LOAD_LIBRARY_ENV_VAR_MISSING = 3,
+    APIR_LOAD_LIBRARY_CANNOT_OPEN = 4,
+    APIR_LOAD_LIBRARY_SYMBOL_MISSING = 5,
+    APIR_LOAD_LIBRARY_INIT_BASE_INDEX = 6, // anything above this is a APIR backend library initialization return code
+} ApirLoadLibraryReturnCode;
+
+typedef enum {
+    APIR_FORWARD_SUCCESS = 0,
+    APIR_FORWARD_NO_DISPATCH_FCT = 1,
+    APIR_FORWARD_TIMEOUT = 2,
+
+    APIR_FORWARD_BASE_INDEX = 3, // anything above this is a APIR backend library forward return code
+} ApirForwardReturnCode;
+
+__attribute__((unused))
+static inline const char *apir_command_name(ApirCommandType type)
 {
   switch (type) {
-  case VIRGL_APIR_COMMAND_TYPE_LoadLibrary: return "LoadLibrary";
-  case VIRGL_APIR_COMMAND_TYPE_Forward: return "Forward";
+  case APIR_COMMAND_TYPE_HandShake: return "HandShake";
+  case APIR_COMMAND_TYPE_LoadLibrary: return "LoadLibrary";
+  case APIR_COMMAND_TYPE_Forward: return "Forward";
   default: return "unknown";
   }
 }
+
+__attribute__((unused))
+static const char *apir_load_library_error(ApirLoadLibraryReturnCode code) {
+#define APIR_LOAD_LIBRARY_ERROR(code_name) \
+  do {						 \
+    if (code == code_name) return #code_name;	 \
+  } while (0)					 \
+
+  APIR_LOAD_LIBRARY_ERROR(APIR_LOAD_LIBRARY_SUCCESS);
+  APIR_LOAD_LIBRARY_ERROR(APIR_LOAD_LIBRARY_HYPERCALL_INITIALIZATION_ERROR);
+  APIR_LOAD_LIBRARY_ERROR(APIR_LOAD_LIBRARY_ALREADY_LOADED);
+  APIR_LOAD_LIBRARY_ERROR(APIR_LOAD_LIBRARY_ENV_VAR_MISSING);
+  APIR_LOAD_LIBRARY_ERROR(APIR_LOAD_LIBRARY_CANNOT_OPEN);
+  APIR_LOAD_LIBRARY_ERROR(APIR_LOAD_LIBRARY_SYMBOL_MISSING);
+  APIR_LOAD_LIBRARY_ERROR(APIR_LOAD_LIBRARY_INIT_BASE_INDEX);
+
+  return "Unknown APIR_COMMAND_TYPE_LoadLibrary error";
+
+#undef APIR_LOAD_LIBRARY_ERROR
+}
+
+__attribute__((unused))
+static const char *apir_forward_error(ApirForwardReturnCode code) {
+#define APIR_FORWARD_ERROR(code_name) \
+  do {						 \
+    if (code == code_name) return #code_name;	 \
+  } while (0)					 \
+
+  APIR_FORWARD_ERROR(APIR_FORWARD_SUCCESS);
+  APIR_FORWARD_ERROR(APIR_FORWARD_NO_DISPATCH_FCT);
+  APIR_FORWARD_ERROR(APIR_FORWARD_TIMEOUT);
+  APIR_FORWARD_ERROR(APIR_FORWARD_BASE_INDEX);
+
+  return "Unknown APIR_COMMAND_TYPE_Forward error";
+
+#undef APIR_FORWARD_ERROR
+}

ggml/src/ggml-remotingbackend/shared/apir_backend.h

@@ -1,10 +1,11 @@
 #pragma once
 
-#define APIR_BACKEND_INITIALIZE_SUCCESSS 0
+#define APIR_BACKEND_INITIALIZE_SUCCESS 0
 #define APIR_BACKEND_INITIALIZE_CANNOT_OPEN_BACKEND_LIBRARY 1
 #define APIR_BACKEND_INITIALIZE_CANNOT_OPEN_GGML_LIBRARY 2
 #define APIR_BACKEND_INITIALIZE_MISSING_BACKEND_SYMBOLS 3
 #define APIR_BACKEND_INITIALIZE_MISSING_GGML_SYMBOLS 4
+
 #define APIR_BACKEND_INITIALIZE_BACKEND_FAILED 5
 // new entries here need to be added to the apir_backend_initialize_error function below
 
@@ -89,14 +90,14 @@ extern struct timer_data set_tensor_from_ptr_timer;
 
 static inline void start_timer(struct timer_data *timer) {
   struct timespec ts;
-  clock_gettime(CLOCK_REALTIME, &ts);  // Use CLOCK_MONOTONIC for elapsed time
+  clock_gettime(CLOCK_MONOTONIC, &ts);
   timer->start = (long long)ts.tv_sec * 1000000000LL + ts.tv_nsec;
 }
 
 // returns the duration in ns
 static inline long long stop_timer(struct timer_data *timer) {
   struct timespec ts;
-  clock_gettime(CLOCK_REALTIME, &ts);  // Use CLOCK_MONOTONIC for elapsed time
+  clock_gettime(CLOCK_MONOTONIC, &ts);
   long long timer_end = (long long)ts.tv_sec * 1000000000LL + ts.tv_nsec;
 
   long long duration = (timer_end - timer->start);
@@ -111,6 +112,10 @@ static inline void show_timer(struct timer_data *timer) {
   double itl = ms/timer->count;
   double speed = 1/itl * 1000;
 
+  if (!timer->total) {
+    return;
+  }
+
   INFO("%15s [%9.0f] ms for %4ld invocations | ITL %2.2f ms | throughput = %4.2f t/s (%4.2f ms/call)",
        timer->name, ms, timer->count, itl, speed, ms/timer->count);
 }
@@ -121,7 +126,7 @@ static const char *apir_backend_initialize_error(int code) {
     if (code == code_name) return #code_name;	 \
   } while (0)					 \
 
-  APIR_BACKEND_INITIALIZE_ERROR(APIR_BACKEND_INITIALIZE_SUCCESSS);
+  APIR_BACKEND_INITIALIZE_ERROR(APIR_BACKEND_INITIALIZE_SUCCESS);
   APIR_BACKEND_INITIALIZE_ERROR(APIR_BACKEND_INITIALIZE_CANNOT_OPEN_BACKEND_LIBRARY);
   APIR_BACKEND_INITIALIZE_ERROR(APIR_BACKEND_INITIALIZE_CANNOT_OPEN_GGML_LIBRARY);
   APIR_BACKEND_INITIALIZE_ERROR(APIR_BACKEND_INITIALIZE_MISSING_BACKEND_SYMBOLS);

ggml/src/ggml-remotingfrontend/ggml-metal-remoting.cpp

@@ -9,6 +9,7 @@ const struct ggml_backend_metal_device_context *get_metal_dev_context(const ggml
     return &metal_dev_ctx;
   }
 
+  has_metal_dev_ctx = true;
   struct virtgpu *gpu = DEV_TO_GPU(dev);
 
   apir_metal_get_device_context(gpu, &metal_dev_ctx);

ggml/src/ggml-remotingfrontend/ggml-remoting.h

@@ -117,7 +117,6 @@ ggml_backend_dev_t ggml_backend_remoting_get_device(size_t device);
 ggml_backend_buffer_type_t ggml_backend_remoting_host_buffer_type();
 ggml_backend_t ggml_backend_remoting_device_init(ggml_backend_dev_t dev, const char * params);
 ggml_backend_buffer_type_t ggml_backend_remoting_device_get_buffer_type(ggml_backend_dev_t dev);
-ggml_backend_t ggml_backend_remoting_device_init(ggml_backend_dev_t dev, const char * params);
 
 struct remoting_buffer_struct;
 typedef std::shared_ptr<remoting_buffer_struct> remoting_buffer;

ggml/src/ggml-remotingfrontend/virtgpu-forward-backend.cpp

@@ -10,6 +10,7 @@ ggml_status
 apir_backend_graph_compute(struct virtgpu *gpu, ggml_cgraph *cgraph) {
   struct vn_cs_encoder *encoder;
   struct vn_cs_decoder *decoder;
+  ApirForwardReturnCode ret;
 
   REMOTE_CALL_PREPARE(gpu, encoder, APIR_COMMAND_TYPE_BACKEND_GRAPH_COMPUTE);
 
@@ -37,13 +38,13 @@ apir_backend_graph_compute(struct virtgpu *gpu, ggml_cgraph *cgraph) {
 
   vn_encode_cgraph_data(&secondary_enc, cgraph_data);
 
-  REMOTE_CALL(gpu, encoder, decoder);
+  REMOTE_CALL(gpu, encoder, decoder, ret);
 
   ggml_status status = GGML_STATUS_ABORTED;
   vn_decode_ggml_status(decoder, &status);
   //INFO("Received status %u", status);
 
-  REMOTE_CALL_FINISH(gpu, encoder, decoder);
+  remote_call_finish(gpu, encoder, decoder);
 
   if (shmem != gpu->data_shmem) {
     virtgpu_shmem_destroy(gpu, shmem->shmem);