From: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
To: ofono@lists.linux.dev
Cc: merlijn@wizzup.org, tony@atomide.com, pavel@ucw.cz,
Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Subject: [PATCH 2/2] qmimodem: Fix shared service creation logic
Date: Thu, 15 Sep 2022 23:54:35 +0300 [thread overview]
Message-ID: <1663275275-19800-3-git-send-email-ivo.g.dimitrov.75@gmail.com> (raw)
In-Reply-To: <1663275275-19800-1-git-send-email-ivo.g.dimitrov.75@gmail.com>
qmi_service_create_shared() tries to find already created service of the
same type and if it fails to find one, start a creation of a new service.
This creation takes some time, so if while it is not complete, any new
calls to qmi_service_create_shared() will still fail to find a service of
that type and will start creation. This can easily lead to client ids
exhaustion and service creation failures.
Fix that by adding logic that delays responses to any shared service
creation requests after the first one, until that request either fails or
succeeds.
---
drivers/qmimodem/qmi.c | 132 ++++++++++++++++++++++++++++++++++++-------------
1 file changed, 97 insertions(+), 35 deletions(-)
diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c
index fc4534a..d2d1bce 100644
--- a/drivers/qmimodem/qmi.c
+++ b/drivers/qmimodem/qmi.c
@@ -260,6 +260,10 @@ static gboolean __service_compare_shared(gpointer key, gpointer value,
struct qmi_service *service = value;
uint8_t type = GPOINTER_TO_UINT(user_data);
+ /* ignore those that are in process of creation */
+ if (GPOINTER_TO_UINT(key) & 0x80000000)
+ return FALSE;
+
if (service->type == type)
return TRUE;
@@ -736,6 +740,10 @@ static void service_notify(gpointer key, gpointer value, gpointer user_data)
struct qmi_result *result = user_data;
GList *list;
+ /* ignore those that are in process of creation */
+ if (GPOINTER_TO_UINT(key) & 0x80000000)
+ return;
+
for (list = g_list_first(service->notify_list); list;
list = g_list_next(list)) {
struct qmi_notify *notify = list->data;
@@ -1967,12 +1975,58 @@ static void service_create_data_free(gpointer user_data)
g_free(data);
}
+struct service_create_shared_data {
+ struct discovery super;
+ struct qmi_service *service;
+ struct qmi_device *device;
+ qmi_create_func_t func;
+ void *user_data;
+ qmi_destroy_func_t destroy;
+ guint timeout;
+};
+
+static gboolean service_create_shared_reply(gpointer user_data)
+{
+ struct service_create_shared_data *data = user_data;
+
+ data->timeout = 0;
+ data->func(data->service, data->user_data);
+
+ __qmi_device_discovery_complete(data->device, &data->super);
+
+ return FALSE;
+}
+
+static void service_create_shared_pending_reply(struct qmi_device *device,
+ unsigned int type,
+ struct qmi_service *service)
+{
+ gpointer key = GUINT_TO_POINTER(type | 0x80000000);
+ GList **shared = g_hash_table_lookup(device->service_list, key);
+ GList *l;
+
+ g_hash_table_steal(device->service_list, key);
+
+ for (l = *shared; l; l = l->next) {
+ struct service_create_shared_data *shared_data = l->data;
+
+ shared_data->service = qmi_service_ref(service);
+ shared_data->timeout = g_timeout_add(
+ 0, service_create_shared_reply, shared_data);
+ }
+
+ g_list_free(*shared);
+ g_free(shared);
+}
+
static gboolean service_create_reply(gpointer user_data)
{
struct service_create_data *data = user_data;
struct qmi_device *device = data->device;
struct qmi_request *req;
+ service_create_shared_pending_reply(device, data->type, NULL);
+
/* remove request from queues */
req = find_control_request(device, data->tid);
@@ -2039,6 +2093,8 @@ static void service_create_callback(uint16_t message, uint16_t length,
GUINT_TO_POINTER(hash_id), service);
done:
+ service_create_shared_pending_reply(device, data->type, service);
+
data->func(service, data->user_data);
qmi_service_unref(service);
@@ -2052,14 +2108,22 @@ static bool service_create(struct qmi_device *device,
struct service_create_data *data;
unsigned char client_req[] = { 0x01, 0x01, 0x00, type };
struct qmi_request *req;
+ GList **shared;
+ unsigned int type_val = type;
int i;
- data = g_try_new0(struct service_create_data, 1);
- if (!data)
+ if (!device->version_list)
return false;
- if (!device->version_list)
+ shared = g_try_new0(GList *, 1);
+ if (!shared)
+ return NULL;
+
+ data = g_try_new0(struct service_create_data, 1);
+ if (!data) {
+ g_free(shared);
return false;
+ }
data->super.destroy = service_create_data_free;
data->device = device;
@@ -2088,19 +2152,13 @@ static bool service_create(struct qmi_device *device,
__qmi_device_discovery_started(device, &data->super);
+ /* Mark service creation as pending */
+ g_hash_table_insert(device->service_list,
+ GUINT_TO_POINTER(type_val | 0x80000000), shared);
+
return true;
}
-struct service_create_shared_data {
- struct discovery super;
- struct qmi_service *service;
- struct qmi_device *device;
- qmi_create_func_t func;
- void *user_data;
- qmi_destroy_func_t destroy;
- guint timeout;
-};
-
static void service_create_shared_data_free(gpointer user_data)
{
struct service_create_shared_data *data = user_data;
@@ -2118,23 +2176,11 @@ static void service_create_shared_data_free(gpointer user_data)
g_free(data);
}
-static gboolean service_create_shared_reply(gpointer user_data)
-{
- struct service_create_shared_data *data = user_data;
-
- data->timeout = 0;
- data->func(data->service, data->user_data);
-
- __qmi_device_discovery_complete(data->device, &data->super);
-
- return FALSE;
-}
-
-bool qmi_service_create_shared(struct qmi_device *device,
- uint8_t type, qmi_create_func_t func,
- void *user_data, qmi_destroy_func_t destroy)
+bool qmi_service_create_shared(struct qmi_device *device, uint8_t type,
+ qmi_create_func_t func, void *user_data,
+ qmi_destroy_func_t destroy)
{
- struct qmi_service *service;
+ gpointer service;
unsigned int type_val = type;
if (!device || !func)
@@ -2143,27 +2189,43 @@ bool qmi_service_create_shared(struct qmi_device *device,
if (type == QMI_SERVICE_CONTROL)
return false;
- service = g_hash_table_find(device->service_list,
+ service = g_hash_table_lookup(device->service_list,
+ GUINT_TO_POINTER(type_val | 0x80000000));
+ if (!service) {
+ service = g_hash_table_find(device->service_list,
__service_compare_shared, GUINT_TO_POINTER(type_val));
+ } else
+ type_val |= 0x80000000;
+
if (service) {
struct service_create_shared_data *data;
data = g_try_new0(struct service_create_shared_data, 1);
if (!data)
- return false;
+ return NULL;
data->super.destroy = service_create_shared_data_free;
- data->service = qmi_service_ref(service);
data->device = device;
data->func = func;
data->user_data = user_data;
data->destroy = destroy;
- data->timeout = g_timeout_add(0,
- service_create_shared_reply, data);
+ if (!data)
+ return false;
+
+ if (!(type_val & 0x80000000)) {
+ data->service = qmi_service_ref(service);
+ data->timeout = g_timeout_add(
+ 0, service_create_shared_reply, data);
+ } else {
+ GList **l = service;
+
+ *l = g_list_prepend(*l, data);
+ }
+
__qmi_device_discovery_started(device, &data->super);
- return 0;
+ return true;
}
return service_create(device, type, func, user_data, destroy);
--
1.9.1
next prev parent reply other threads:[~2022-09-15 20:55 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-15 20:54 qmimodem: Service creation fixes Ivaylo Dimitrov
2022-09-15 20:54 ` [PATCH 1/2] qmimodem: Remove service create request on timeout Ivaylo Dimitrov
2022-09-15 20:54 ` Ivaylo Dimitrov [this message]
-- strict thread matches above, loose matches on Subject: below --
2022-09-16 9:31 [PATCH v2 0/2] qmimodem: Service creation fixes Ivaylo Dimitrov
2022-09-16 9:31 ` [PATCH 2/2] qmimodem: Fix shared service creation logic Ivaylo Dimitrov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1663275275-19800-3-git-send-email-ivo.g.dimitrov.75@gmail.com \
--to=ivo.g.dimitrov.75@gmail.com \
--cc=merlijn@wizzup.org \
--cc=ofono@lists.linux.dev \
--cc=pavel@ucw.cz \
--cc=tony@atomide.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).