Skip to content

Commit 015deb9

Browse files
kpougetkpouget
authored
ggml-virtgpu: make the code thread safe (ggml-org#19204)
* ggml-virtgpu: regenerate_remoting.py: add the ability to deprecate a function * ggml-virtgpu: deprecate buffer_type is_host remoting not necessary * ggml-virtgpu: stop using static vars as cache The static init isn't thread safe. * ggml-virtgpu: protect the use of the shared memory to transfer data * ggml-virtgpu: make the remote calls thread-safe * ggml-virtgpu: backend: don't continue if couldn't allocate the tensor memory * ggml-virtgpu: add a cleanup function for consistency * ggml-virtgpu: backend: don't crash if buft->iface.get_max_size is missing * fix style and ordering * Remove the static variable in apir_device_get_count * ggml-virtgpu: improve the logging * fix review minor formatting changes
1 parent 2ceda3f commit 015deb9

27 files changed

Lines changed: 399 additions & 239 deletions

‎ggml/include/ggml-virtgpu.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
extern "C" {
88
#endif
99

10-
#define GGML_REMOTING_FRONTEND_NAME "RemotingFrontend"
11-
1210
GGML_BACKEND_API ggml_backend_reg_t ggml_backend_virtgpu_reg();
1311

1412
#ifdef __cplusplus

‎ggml/src/ggml-virtgpu/apir_cs_ggml-rpc-front.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ apir_rpc_tensor apir_serialize_tensor(const ggml_tensor * tensor) {
3636
result.data = reinterpret_cast<uint64_t>(tensor->data);
3737
if (tensor->data) {
3838
if (!tensor->buffer) {
39-
GGML_ABORT("tensor has data but not buffer");
39+
GGML_ABORT("%s: tensor has data but not buffer", __func__);
4040
}
4141
// tensor->data is serialized as an offset to the buffer base address
4242
result.data -= reinterpret_cast<uint64_t>(BUFFER_TO_GGML_CONTEXT(tensor->buffer)->base);

‎ggml/src/ggml-virtgpu/backend/backend-dispatched-backend.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ uint32_t backend_backend_graph_compute(apir_encoder * enc, apir_decoder * dec, v
2727

2828
const void * shmem_data = ctx->iface->get_shmem_ptr(ctx->ctx_id, shmem_res_id);
2929
if (!shmem_data) {
30-
GGML_LOG_ERROR("Couldn't get the shmem addr from virgl\n");
30+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK "%s: Couldn't get the shmem addr from virgl\n", __func__);
3131
apir_decoder_set_fatal(dec);
3232
return 1;
3333
}
@@ -45,7 +45,7 @@ uint32_t backend_backend_graph_compute(apir_encoder * enc, apir_decoder * dec, v
4545
if (dev->iface.supports_op(dev, op)) {
4646
continue;
4747
}
48-
GGML_LOG_ERROR("Graph node %d (%s) not supported by the backend\n", idx, ggml_op_desc(op));
48+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK "%s: Graph node %d (%s) not supported by the backend\n", idx, ggml_op_desc(op));
4949

5050
status = GGML_STATUS_ABORTED;
5151
apir_encode_ggml_status(enc, &status);

‎ggml/src/ggml-virtgpu/backend/backend-dispatched-buffer-type.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,22 @@ uint32_t backend_buffer_type_get_max_size(apir_encoder * enc, apir_decoder * dec
3636
ggml_backend_buffer_type_t buft;
3737
buft = apir_decode_ggml_buffer_type(dec);
3838

39-
size_t value = buft->iface.get_max_size(buft);
39+
size_t value = SIZE_MAX;
40+
if (buft->iface.get_max_size) {
41+
value = buft->iface.get_max_size(buft);
42+
}
43+
4044
apir_encode_size_t(enc, &value);
4145

4246
return 0;
4347
}
4448

49+
/* APIR_COMMAND_TYPE_BUFFER_TYPE_IS_HOST is deprecated. Keeping the handler for backward compatibility. */
4550
uint32_t backend_buffer_type_is_host(apir_encoder * enc, apir_decoder * dec, virgl_apir_context * ctx) {
4651
GGML_UNUSED(ctx);
47-
ggml_backend_buffer_type_t buft;
48-
buft = apir_decode_ggml_buffer_type(dec);
52+
GGML_UNUSED(dec);
53+
const bool is_host = false;
4954

50-
bool is_host = buft->iface.is_host(buft);
5155
apir_encode_bool_t(enc, &is_host);
5256

5357
return 0;

‎ggml/src/ggml-virtgpu/backend/backend-dispatched-buffer.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ uint32_t backend_buffer_set_tensor(apir_encoder * enc, apir_decoder * dec, virgl
4040
void * shmem_data = ctx->iface->get_shmem_ptr(ctx->ctx_id, shmem_res_id);
4141

4242
if (!shmem_data) {
43-
GGML_LOG_ERROR("Couldn't get the shmem addr from virgl\n");
43+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK "%s: Couldn't get the shmem addr from virgl\n", __func__);
4444
return 1;
4545
}
4646

@@ -71,7 +71,7 @@ uint32_t backend_buffer_get_tensor(apir_encoder * enc, apir_decoder * dec, virgl
7171

7272
void * shmem_data = ctx->iface->get_shmem_ptr(ctx->ctx_id, shmem_res_id);
7373
if (!shmem_data) {
74-
GGML_LOG_ERROR("Couldn't get the shmem addr from virgl\n");
74+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK "%s: Couldn't get the shmem addr from virgl\n", __func__);
7575
return 1;
7676
}
7777

@@ -121,7 +121,7 @@ uint32_t backend_buffer_free_buffer(apir_encoder * enc, apir_decoder * dec, virg
121121
buffer = apir_decode_ggml_buffer(dec);
122122

123123
if (!apir_untrack_backend_buffer(buffer)) {
124-
GGML_LOG_WARN("%s: unknown buffer %p\n", __func__, (void *) buffer);
124+
GGML_LOG_WARN(GGML_VIRTGPU_BCK "%s: unknown buffer %p\n", __func__, (void *) buffer);
125125
return 1;
126126
}
127127

‎ggml/src/ggml-virtgpu/backend/backend-dispatched-device.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ uint32_t backend_device_buffer_from_ptr(apir_encoder * enc, apir_decoder * dec,
124124

125125
void * shmem_ptr = ctx->iface->get_shmem_ptr(ctx->ctx_id, shmem_res_id);
126126
if (!shmem_ptr) {
127-
GGML_LOG_ERROR("Couldn't get the shmem addr from virgl\n");
127+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK "%s: Couldn't get the shmem addr from virgl\n", __func__);
128128
apir_decoder_set_fatal(dec);
129129
return 1;
130130
}

‎ggml/src/ggml-virtgpu/backend/backend-dispatched.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,26 @@ uint64_t timer_count = 0;
1717

1818
uint32_t backend_dispatch_initialize(void * ggml_backend_reg_fct_p) {
1919
if (reg != NULL) {
20-
GGML_LOG_WARN("%s: already initialized\n", __func__);
20+
GGML_LOG_WARN(GGML_VIRTGPU_BCK "%s: already initialized\n", __func__);
2121
return APIR_BACKEND_INITIALIZE_ALREADY_INITED;
2222
}
2323
ggml_backend_reg_t (*ggml_backend_reg_fct)(void) = (ggml_backend_reg_t (*)()) ggml_backend_reg_fct_p;
2424

2525
reg = ggml_backend_reg_fct();
2626
if (reg == NULL) {
27-
GGML_LOG_ERROR("%s: backend registration failed\n", __func__);
27+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK "%s: backend registration failed\n", __func__);
2828
return APIR_BACKEND_INITIALIZE_BACKEND_REG_FAILED;
2929
}
3030

3131
if (!reg->iface.get_device_count(reg)) {
32-
GGML_LOG_ERROR("%s: backend initialization failed: no device found\n", __func__);
32+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK "%s: backend initialization failed: no device found\n", __func__);
3333
return APIR_BACKEND_INITIALIZE_NO_DEVICE;
3434
}
3535

3636
dev = reg->iface.get_device(reg, 0);
3737

3838
if (!dev) {
39-
GGML_LOG_ERROR("%s: backend initialization failed: no device received\n", __func__);
39+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK "%s: backend initialization failed: no device received\n", __func__);
4040
return APIR_BACKEND_INITIALIZE_NO_DEVICE;
4141
}
4242

‎ggml/src/ggml-virtgpu/backend/backend-dispatched.gen.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ uint32_t backend_device_buffer_from_ptr(apir_encoder * enc, apir_decoder * dec,
1616
uint32_t backend_buffer_type_get_name(apir_encoder * enc, apir_decoder * dec, virgl_apir_context * ctx);
1717
uint32_t backend_buffer_type_get_alignment(apir_encoder * enc, apir_decoder * dec, virgl_apir_context * ctx);
1818
uint32_t backend_buffer_type_get_max_size(apir_encoder * enc, apir_decoder * dec, virgl_apir_context * ctx);
19+
/* APIR_COMMAND_TYPE_BUFFER_TYPE_IS_HOST is deprecated. Keeping the handler for backward compatibility. */
1920
uint32_t backend_buffer_type_is_host(apir_encoder * enc, apir_decoder * dec, virgl_apir_context * ctx);
2021
uint32_t backend_buffer_type_alloc_buffer(apir_encoder * enc, apir_decoder * dec, virgl_apir_context * ctx);
2122
uint32_t backend_buffer_type_get_alloc_size(apir_encoder * enc, apir_decoder * dec, virgl_apir_context * ctx);
@@ -62,7 +63,7 @@ static inline const char * backend_dispatch_command_name(ApirBackendCommandType
6263
case APIR_COMMAND_TYPE_BUFFER_TYPE_GET_MAX_SIZE:
6364
return "backend_buffer_type_get_max_size";
6465
case APIR_COMMAND_TYPE_BUFFER_TYPE_IS_HOST:
65-
return "backend_buffer_type_is_host";
66+
return "backend_buffer_type_is_host (DEPRECATED)";
6667
case APIR_COMMAND_TYPE_BUFFER_TYPE_ALLOC_BUFFER:
6768
return "backend_buffer_type_alloc_buffer";
6869
case APIR_COMMAND_TYPE_BUFFER_TYPE_GET_ALLOC_SIZE:
@@ -110,7 +111,7 @@ static const backend_dispatch_t apir_backend_dispatch_table[APIR_BACKEND_DISPATC
110111
/* APIR_COMMAND_TYPE_BUFFER_TYPE_GET_NAME = */ backend_buffer_type_get_name,
111112
/* APIR_COMMAND_TYPE_BUFFER_TYPE_GET_ALIGNMENT = */ backend_buffer_type_get_alignment,
112113
/* APIR_COMMAND_TYPE_BUFFER_TYPE_GET_MAX_SIZE = */ backend_buffer_type_get_max_size,
113-
/* APIR_COMMAND_TYPE_BUFFER_TYPE_IS_HOST = */ backend_buffer_type_is_host,
114+
/* APIR_COMMAND_TYPE_BUFFER_TYPE_IS_HOST = */ backend_buffer_type_is_host /* DEPRECATED */,
114115
/* APIR_COMMAND_TYPE_BUFFER_TYPE_ALLOC_BUFFER = */ backend_buffer_type_alloc_buffer,
115116
/* APIR_COMMAND_TYPE_BUFFER_TYPE_GET_ALLOC_SIZE = */ backend_buffer_type_get_alloc_size,
116117

‎ggml/src/ggml-virtgpu/backend/backend-dispatched.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
#include "shared/apir_cs.h"
1212
#include "shared/apir_cs_ggml.h"
1313

14+
#define GGML_VIRTGPU_BCK "ggml-virtgpu-backend: "
15+
1416
struct virgl_apir_context {
1517
uint32_t ctx_id;
1618
virgl_apir_callbacks * iface;

‎ggml/src/ggml-virtgpu/backend/backend.cpp‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,8 @@ void apir_backend_deinit(uint32_t virgl_ctx_id) {
3535
buffer->iface.free_buffer(buffer);
3636
}
3737

38-
if (dev) {
39-
size_t free, total;
40-
dev->iface.get_memory(dev, &free, &total);
41-
GGML_LOG_INFO("%s: free memory: %ld MB\n", __func__, (size_t) free / 1024 / 1024);
42-
}
43-
4438
if (backend_library_handle) {
45-
GGML_LOG_INFO("%s: The GGML backend library was loaded. Unloading it.\n", __func__);
39+
GGML_LOG_INFO(GGML_VIRTGPU_BCK "The GGML backend library was loaded. Unloading it.\n");
4640
dlclose(backend_library_handle);
4741
backend_library_handle = NULL;
4842
}
@@ -65,7 +59,7 @@ ApirLoadLibraryReturnCode apir_backend_initialize(uint32_t virgl_ctx_id, struct
6559
if (apir_logfile) {
6660
ggml_log_set(log_to_file_callback, apir_logfile);
6761
} else {
68-
GGML_LOG_INFO("Could not open the log file at '%s'\n", apir_log_to_file);
62+
GGML_LOG_INFO(GGML_VIRTGPU_BCK "Could not open the log file at '%s'\n", apir_log_to_file);
6963
}
7064
}
7165

@@ -74,30 +68,38 @@ ApirLoadLibraryReturnCode apir_backend_initialize(uint32_t virgl_ctx_id, struct
7468
const char * library_reg = virgl_library_reg ? virgl_library_reg : GGML_DEFAULT_BACKEND_REG;
7569

7670
if (!library_name) {
77-
GGML_LOG_ERROR("cannot open the GGML library: env var '%s' not defined\n", APIR_LLAMA_CPP_GGML_LIBRARY_PATH_ENV);
71+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK
72+
"%s: cannot open the GGML library: env var '%s' not defined\n",
73+
__func__, APIR_LLAMA_CPP_GGML_LIBRARY_PATH_ENV);
74+
7875

7976
return APIR_LOAD_LIBRARY_ENV_VAR_MISSING;
8077
}
8178

8279
backend_library_handle = dlopen(library_name, RTLD_LAZY);
8380

8481
if (!backend_library_handle) {
85-
GGML_LOG_ERROR("cannot open the GGML library: %s\n", dlerror());
82+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK
83+
"%s: cannot open the GGML library: %s\n", __func__, dlerror());
8684

8785
return APIR_LOAD_LIBRARY_CANNOT_OPEN;
8886
}
8987

9088
if (!library_reg) {
91-
GGML_LOG_ERROR("cannot register the GGML library: env var '%s' not defined\n", APIR_LLAMA_CPP_GGML_LIBRARY_REG_ENV);
89+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK
90+
"%s: cannot register the GGML library: env var '%s' not defined\n",
91+
__func__, APIR_LLAMA_CPP_GGML_LIBRARY_REG_ENV);
9292

9393
return APIR_LOAD_LIBRARY_ENV_VAR_MISSING;
9494
}
9595

9696
void * ggml_backend_reg_fct = dlsym(backend_library_handle, library_reg);
9797
dlsym_error = dlerror();
9898
if (dlsym_error) {
99-
GGML_LOG_ERROR("cannot find the GGML backend registration symbol '%s' (from %s): %s\n", library_reg,
100-
APIR_LLAMA_CPP_GGML_LIBRARY_REG_ENV, dlsym_error);
99+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK
100+
"%s: cannot find the GGML backend registration symbol '%s' (from %s): %s\n",
101+
__func__, library_reg, APIR_LLAMA_CPP_GGML_LIBRARY_REG_ENV, dlsym_error);
102+
101103

102104
return APIR_LOAD_LIBRARY_SYMBOL_MISSING;
103105
}
@@ -134,7 +136,9 @@ uint32_t apir_backend_dispatcher(uint32_t virgl_ctx_id,
134136
};
135137

136138
if (cmd_type >= APIR_BACKEND_DISPATCH_TABLE_COUNT) {
137-
GGML_LOG_ERROR("Received an invalid dispatch index (%d >= %d)\n", cmd_type, APIR_BACKEND_DISPATCH_TABLE_COUNT);
139+
GGML_LOG_ERROR(GGML_VIRTGPU_BCK
140+
"%s: Received an invalid dispatch index (%d >= %d)\n",
141+
__func__, cmd_type, APIR_BACKEND_DISPATCH_TABLE_COUNT);
138142
return APIR_BACKEND_FORWARD_INDEX_INVALID;
139143
}
140144

0 commit comments

Comments
 (0)