From: Yu Watanabe Date: Sun, 27 Feb 2022 06:39:16 +0000 (+0900) Subject: network: make Request object take Manager* X-Git-Tag: v251-rc1~164^2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e26d3d407cbdf9138139ca9526573c494ed55e81;p=thirdparty%2Fsystemd.git network: make Request object take Manager* Previously, even though all Request object are owned by Manager, they do not have direct reference to Manager, but through Link or NetDev object. But, as Link or NetDev can be NULL, we need to conditionalize how to access Manager from Request with the type of the request. This makes the way simpler, as now Request object has direct reference to Manager. This also rename request_drop() -> request_detach(), as in the previous commit, the reference counter is introduced, so even if a reference of a Request object from Manager is dropped, the object may still alive. The naming `request_drop()` sounds the object will freed by the function. But it may not. And `request_detach()` suggests the object will not be managed by Manager any more, and I think it is more appropreate. This is just a cleanup, and should not change any behavior. --- diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 3164f82c304..63b40da4328 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1295,7 +1295,7 @@ void address_cancel_request(Address *address) { .address = address, }; - request_drop(ordered_set_get(address->link->manager->request_queue, &req)); + request_detach(address->link->manager, &req); address_cancel_requesting(address); } diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 48f8e40fb40..e925739d06e 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -906,7 +906,7 @@ static void link_drop_requests(Link *link) { ORDERED_SET_FOREACH(req, link->manager->request_queue) if (req->link == link) - request_drop(req); + request_detach(link->manager, req); } static Link *link_drop(Link *link) { diff --git a/src/network/networkd-queue.c b/src/network/networkd-queue.c index 06a0cb066e2..70edc27fcdf 100644 --- a/src/network/networkd-queue.c +++ b/src/network/networkd-queue.c @@ -81,25 +81,38 @@ static Request *request_free(Request *req) { if (!req) return NULL; - if (req->link && req->link->manager) - /* To prevent from triggering assertions in hash functions, remove this request before - * freeing object below. */ - ordered_set_remove(req->link->manager->request_queue, req); + /* To prevent from triggering assertions in the hash and compare functions, remove this request + * before freeing userdata below. */ + if (req->manager) + ordered_set_remove(req->manager->request_queue, req); + if (req->consume_object) request_free_object(req->type, req->object); - link_unref(req->link); + + link_unref(req->link); /* link may be NULL, but link_unref() can handle it gracefully. */ return mfree(req); } DEFINE_TRIVIAL_REF_UNREF_FUNC(Request, request, request_free); -void request_drop(Request *req) { +void request_detach(Manager *manager, Request *req) { + assert(manager); + if (!req) return; - if (req->message_counter) + req = ordered_set_remove(manager->request_queue, req); + if (!req) + return; + + req->manager = NULL; + + if (req->message_counter) { + assert(*req->message_counter > 0); (*req->message_counter)--; + req->message_counter = NULL; /* To prevent double decrement. */ + } request_unref(req); } @@ -257,6 +270,7 @@ int netdev_queue_request( *req = (Request) { .n_ref = 1, + .manager = netdev->manager, .netdev = netdev_ref(netdev), .type = REQUEST_TYPE_NETDEV_INDEPENDENT, .consume_object = true, @@ -264,9 +278,6 @@ int netdev_queue_request( existing = ordered_set_get(netdev->manager->request_queue, req); if (existing) { - /* To prevent from removing the existing request. */ - req->netdev = netdev_unref(req->netdev); - if (ret) *ret = existing; return 0; @@ -276,6 +287,8 @@ int netdev_queue_request( if (r < 0) return r; + req->manager = netdev->manager; + if (ret) *ret = req; @@ -336,9 +349,6 @@ int link_queue_request( existing = ordered_set_get(link->manager->request_queue, req); if (existing) { - /* To prevent from removing the existing request. */ - req->link = link_unref(req->link); - if (ret) *ret = existing; return 0; @@ -348,6 +358,7 @@ int link_queue_request( if (r < 0) return r; + req->manager = link->manager; if (req->message_counter) (*req->message_counter)++; @@ -440,8 +451,8 @@ int manager_process_requests(sd_event_source *s, void *userdata) { if (req->link) link_enter_failed(req->link); } else if (r > 0) { - ordered_set_remove(manager->request_queue, req); - request_unref(req); + req->message_counter = NULL; + request_detach(manager, req); processed = true; } } diff --git a/src/network/networkd-queue.h b/src/network/networkd-queue.h index 7410e5c8612..14403efdd08 100644 --- a/src/network/networkd-queue.h +++ b/src/network/networkd-queue.h @@ -46,7 +46,9 @@ typedef enum RequestType { typedef struct Request { unsigned n_ref; - Link *link; + Manager *manager; /* must be non-NULL */ + Link *link; /* can be NULL */ + RequestType type; bool consume_object; union { @@ -74,7 +76,7 @@ Request *request_ref(Request *req); Request *request_unref(Request *req); DEFINE_TRIVIAL_CLEANUP_FUNC(Request*, request_unref); -void request_drop(Request *req); +void request_detach(Manager *manager, Request *req); int netdev_queue_request( NetDev *netdev, diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 1c5ace643ed..9be1190655e 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1560,7 +1560,7 @@ void route_cancel_request(Route *route, Link *link) { .route = route, }; - request_drop(ordered_set_get(link->manager->request_queue, &req)); + request_detach(link->manager, &req); route_cancel_requesting(route); }