From: Pavel Filipenský Date: Tue, 21 Jun 2022 16:19:16 +0000 (+0200) Subject: s3:winbind: Avoid unnecessary locking in wb_parent_idmap_setup_send() X-Git-Tag: talloc-2.4.0~591 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f5d77cb627d906bbebfbf863130bc5d1e36337c9;p=thirdparty%2Fsamba.git s3:winbind: Avoid unnecessary locking in wb_parent_idmap_setup_send() A function in tevent environment can span over several context loop iterations. Every iteration 'unschedules' the current code and a different functions can access not yet fully initialized structures. A locking is used to avoid this. In tevent, we use tevent queues as a locking mechanism. Every function trying to access lock protected data, puts itself to a queue. The function must remove itself from the queue only after the complete work is done. A good coding practise is to lock only the smallest code path and not to use the locking if not needed. wb_parent_idmap_setup_send() uses queue "wb_parent_idmap_config_queue" for: - testing if the setup is ready - setting up all idmap domains But "testing if the setup is ready" can be coded as an atomic operation without needing a lock. Signed-off-by: Pavel Filipenský Reviewed-by: Andreas Schneider Autobuild-User(master): Andreas Schneider Autobuild-Date(master): Fri Nov 4 10:06:28 UTC 2022 on sn-devel-184 --- diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h index 093a9fad11e..5044fee0c68 100644 --- a/source3/winbindd/winbindd.h +++ b/source3/winbindd/winbindd.h @@ -191,6 +191,7 @@ struct wb_parent_idmap_config_dom { struct wb_parent_idmap_config { struct tevent_queue *queue; uint32_t num_doms; + bool initialized; struct wb_parent_idmap_config_dom *doms; }; diff --git a/source3/winbindd/winbindd_idmap.c b/source3/winbindd/winbindd_idmap.c index e6d2c284cf2..75f7d37d86d 100644 --- a/source3/winbindd/winbindd_idmap.c +++ b/source3/winbindd/winbindd_idmap.c @@ -131,6 +131,7 @@ static void wb_parent_idmap_setup_cleanup(struct tevent_req *req, } state->cfg->num_doms = 0; + state->cfg->initialized = false; TALLOC_FREE(state->cfg->doms); state->cfg = NULL; } @@ -159,6 +160,11 @@ struct tevent_req *wb_parent_idmap_setup_send(TALLOC_CTX *mem_ctx, .dom_idx = 0, }; + if (state->cfg->initialized) { + tevent_req_done(req); + return tevent_req_post(req, ev); + } + if (state->cfg->queue == NULL) { state->cfg->queue = tevent_queue_create(NULL, "wb_parent_idmap_config_queue"); @@ -330,6 +336,7 @@ static void wb_parent_idmap_setup_lookupname_next(struct tevent_req *req) * We're done, so start the idmap child */ setup_child(NULL, &static_idmap_child, "log.winbindd", "idmap"); + static_parent_idmap_config.initialized = true; tevent_req_done(req); return; }