From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 3971E686CA; Fri, 5 Feb 2021 21:58:12 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3971E686CA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1612551492; bh=l09Wu5LRlc5WvtD8TC/opiI4/Dm7zy6KHZm7GQLkK8w=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=QKDK8Ez5+BXXSgvgBAD3UYVp44laoZGgI6J8aOOh0ajsgVbEam/73NaB0YM5JT9NH 11j/JkEesOBTRreLh7TVaGzJg3xsRm5crvNRZtQylFfaBN0h6myzNivfY2D4IUdhxP J/ZF/cbamsd+3bgM0DILyXf1Ikj3Bpvqd6gAwHEg= Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id CCD39686CA for ; Fri, 5 Feb 2021 21:56:02 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org CCD39686CA Received: by mail-lf1-f52.google.com with SMTP id i187so11356023lfd.4 for ; Fri, 05 Feb 2021 10:56:02 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Q1NR9pJk1aPswmMwxddR+0JownpKy8ajOeLajjn9H14=; b=bXM9OkgrZs6qKC8Mmi1IMzQaz+J6vgIjiaaI0cAd5wPTzuAbGKHfPHYyT6lG/iRYCx tYxJaiKL370+R6i2GnASSvPZwR3PL3rODl35xSNHufLw8vlpRS+pHtCbGyFsGUNEt5fo D0YLG+MTs5Ohrst9zl2Odts3A65+GGexo+krv2m7AhZX54aw+Qh3I0ulflXllDHSQrMn XeafKtRYNKBozcety7fhavf6fwK9UIxao2qlXB1a6szFOCabqMsFt2C87TgkJ4JxW5ZU ntV/s++LuUalSRCUNMBE1gXSZTYtWZfsuINc/F6n03Ai25jAanuS6uARjc1Wws+5iikF yY4Q== X-Gm-Message-State: AOAM531UlEA53dkZy1gyW23meGR1QZch3nk0pZdcESqus4vV5UT8zRo1 hRn0MKysYNAFQRIkAXXCi64= X-Google-Smtp-Source: ABdhPJzFhfpAG3xvAshuBqOXyyGgZa4g6BbRXfWT2UapnAZvayM3NKq4olEc9bwAHpb++b9jwRWIng== X-Received: by 2002:a19:ccf:: with SMTP id 198mr3339569lfm.422.1612551362418; Fri, 05 Feb 2021 10:56:02 -0800 (PST) Received: from grain.localdomain ([5.18.103.226]) by smtp.gmail.com with ESMTPSA id k1sm1081049ljj.105.2021.02.05.10.56.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Feb 2021 10:56:01 -0800 (PST) Received: by grain.localdomain (Postfix, from userid 1000) id 93FF3560294; Fri, 5 Feb 2021 21:54:37 +0300 (MSK) To: tml Date: Fri, 5 Feb 2021 21:54:32 +0300 Message-Id: <20210205185436.638894-8-gorcunov@gmail.com> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20210205185436.638894-1-gorcunov@gmail.com> References: <20210205185436.638894-1-gorcunov@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v15 07/11] module_cache: use references as a main usage counter X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Cyrill Gorcunov via Tarantool-patches Reply-To: Cyrill Gorcunov Cc: Mons Anderson , Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" We use module:funcs_list as implicit reference counter, lets get rid of it and use explicit counting with module_ref/unref help. An interesting moment is module reload: we can't drop old instance from hash because hash now points to a new module. Part-of #4642 Suggested-by: Vladislav Shpilevoy Signed-off-by: Cyrill Gorcunov --- src/box/module_cache.c | 137 ++++++++++++++++++++++++++--------------- 1 file changed, 87 insertions(+), 50 deletions(-) diff --git a/src/box/module_cache.c b/src/box/module_cache.c index 3fc8261e2..44f34b375 100644 --- a/src/box/module_cache.c +++ b/src/box/module_cache.c @@ -129,6 +129,24 @@ module_cache_del(const char *name, const char *name_end) mh_strnptr_del(mod_hash, e, NULL); } +/** + * Mark module as out of cache. + */ +static void +module_set_orphan(struct module *module) +{ + module->package[0] = '\0'; +} + +/** + * Test if module is out of cache. + */ +static bool +module_is_orphan(struct module *module) +{ + return module->package[0] == '\0'; +} + /** * Arguments for luaT_module_find used by lua_cpcall(). */ @@ -206,6 +224,47 @@ module_find(const char *package, const char *package_end, return 0; } +/** + * Delete a module. + */ +static void +module_delete(struct module *module) +{ + struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT); + if (e != NULL) + --e->iparam; + dlclose(module->handle); + TRASH(module); + free(module); +} + +/** + * Increase reference to a module. + */ +static void +module_ref(struct module *module) +{ + assert(module->refs >= 0); + module->refs++; +} + +/** + * Decrease module reference and delete it if last one. + */ +static void +module_unref(struct module *module) +{ + assert(module->refs > 0); + if (module->refs-- == 1) { + if (!module_is_orphan(module)) { + size_t len = strlen(module->package); + module_cache_del(module->package, + &module->package[len]); + } + module_delete(module); + } +} + /** * Load dynamic shared object, ie module library. * @@ -301,6 +360,7 @@ module_load(const char *package, const char *package_end) struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT); if (e != NULL) ++e->iparam; + module_ref(module); return module; error: @@ -308,30 +368,6 @@ module_load(const char *package, const char *package_end) return NULL; } -/** - * Delete a module. - */ -static void -module_delete(struct module *module) -{ - struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT); - if (e != NULL) - --e->iparam; - dlclose(module->handle); - TRASH(module); - free(module); -} - -/** - * Check if a module is unused and delete it then. - */ -static void -module_gc(struct module *module) -{ - if (rlist_empty(&module->funcs_list) && module->refs == 0) - module_delete(module); -} - /** * Import a function from a module. */ @@ -366,28 +402,17 @@ module_sym_load(struct module_sym *mod_sym) if (module == NULL) return -1; if (module_cache_add(module) != 0) { - module_delete(module); + module_unref(module); return -1; } } else { + module_ref(cached); module = cached; } mod_sym->addr = module_sym(module, name.sym); if (mod_sym->addr == NULL) { - if (cached == NULL) { - /* - * In case if it was a first load we should - * clean the cache immediately otherwise - * the module continue being referenced even - * if there will be no use of it. - * - * Note the module_sym set an error thus be - * careful to not wipe it. - */ - module_cache_del(name.package, name.package_end); - module_delete(module); - } + module_unref(module); return -1; } @@ -403,12 +428,11 @@ module_sym_unload(struct module_sym *mod_sym) return; rlist_del(&mod_sym->item); - if (rlist_empty(&mod_sym->module->funcs_list)) { - struct func_name name; - func_split_name(mod_sym->name, &name); - module_cache_del(name.package, name.package_end); - } - module_gc(mod_sym->module); + /* + * Unref action might delete module + * so call it after rlist_del. + */ + module_unref(mod_sym->module); mod_sym->module = NULL; mod_sym->addr = NULL; @@ -454,10 +478,9 @@ module_sym_call(struct module_sym *mod_sym, struct port *args, */ struct module *module = mod_sym->module; assert(module != NULL); - ++module->refs; + module_ref(module); int rc = mod_sym->addr(&ctx, data, data + data_sz); - --module->refs; - module_gc(module); + module_unref(module); region_truncate(region, region_svp); if (rc != 0) { @@ -485,6 +508,9 @@ module_reload(const char *package, const char *package_end) if (new == NULL) return -1; + /* Extra ref until cache get updated */ + module_ref(old); + struct module_sym *mod_sym, *tmp; rlist_foreach_entry_safe(mod_sym, &old->funcs_list, item, tmp) { struct func_name name; @@ -499,6 +525,10 @@ module_reload(const char *package, const char *package_end) mod_sym->module = new; rlist_move(&new->funcs_list, &mod_sym->item); + + /* Move reference to a new place */ + module_ref(new); + module_unref(old); } if (module_cache_update(package, package_end, new) != 0) { @@ -511,10 +541,15 @@ module_reload(const char *package, const char *package_end) panic("module: can't update module cache (%s)", package); } - module_gc(old); + module_set_orphan(old); + module_unref(old); + + /* From explicit load above */ + module_unref(new); return 0; restore: + module_set_orphan(new); /* * Some old-dso func can't be load from new module, * restore old functions. @@ -533,11 +568,13 @@ module_reload(const char *package, const char *package_end) } mod_sym->module = old; rlist_move(&old->funcs_list, &mod_sym->item); + module_ref(old); + module_unref(new); } while (mod_sym != rlist_first_entry(&old->funcs_list, struct module_sym, item)); assert(rlist_empty(&new->funcs_list)); - module_delete(new); + module_unref(new); return -1; } @@ -559,7 +596,7 @@ module_free(void) while (mh_size(mod_hash) > 0) { mh_int_t i = mh_first(mod_hash); struct module *m = mh_strnptr_node(mod_hash, i)->val; - module_gc(m); + module_unref(m); } mh_strnptr_delete(mod_hash); mod_hash = NULL; -- 2.29.2