From 87b31b0e13a0b9c33124087be07c7122acff05fd Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Mon, 22 Jun 2015 06:38:04 +0300 Subject: [PATCH v6 01/10] winbindd: set file descriptor limit according to configuration Set the winbindd process file descriptor limit according to the values that affect it in the configuration: - Maximum number of clients - Number of outgoing connections per domain BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397 Signed-off-by: Uri Simchoni VL -- I'm not sure here. See mail. Comments? --- source3/include/local.h | 4 ++++ source3/winbindd/winbindd.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/source3/include/local.h b/source3/include/local.h index 5963eb0..7f97d4e 100644 --- a/source3/include/local.h +++ b/source3/include/local.h @@ -204,4 +204,8 @@ /* Maximum size of RPC data we will accept for one call. */ #define MAX_RPC_DATA_SIZE (15*1024*1024) +/* A guestimate of how many domains winbindd will be contacting */ +#ifndef WINBIND_MAX_DOMAINS_HINT +#define WINBIND_MAX_DOMAINS_HINT 10 +#endif #endif diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c index 4ffe79c..defc9cc 100644 --- a/source3/winbindd/winbindd.c +++ b/source3/winbindd/winbindd.c @@ -48,6 +48,7 @@ static bool client_is_idle(struct winbindd_cli_state *state); static void remove_client(struct winbindd_cli_state *state); +static void winbindd_setup_max_fds(void); static bool opt_nocache = False; static bool interactive = False; @@ -145,6 +146,7 @@ static bool reload_services_file(const char *lfile) reopen_logs(); load_interfaces(); + winbindd_setup_max_fds(); return(ret); } @@ -1130,6 +1132,35 @@ char *get_winbind_priv_pipe_dir(void) return state_path(WINBINDD_PRIV_SOCKET_SUBDIR); } +static void winbindd_setup_max_fds(void) +{ + int num_fds = MAX_OPEN_FUDGEFACTOR; + int actual_fds; + + num_fds += lp_winbind_max_clients(); + /* Add some more to account for 2 sockets open + when the client transitions from unprivileged + to privileged socket + */ + num_fds += lp_winbind_max_clients() / 10; + + /* Add one socket per child process + (yeah there are child processes other than the + domain children but only domain children can vary + with configuration + */ + num_fds += lp_winbind_max_domain_connections() * + (lp_allow_trusted_domains() ? WINBIND_MAX_DOMAINS_HINT : 1); + + actual_fds = set_maxfiles(num_fds); + + if (actual_fds < num_fds) { + DEBUG(1, ("winbindd_setup_max_fds: Information only: " + "requested %d open files, %d are available.\n", + num_fds, actual_fds)); + } +} + static bool winbindd_setup_listeners(void) { struct winbindd_listen_state *pub_state = NULL; -- 1.9.1 From 824ac59aa66228af1f7264bc69f127adc6d7adfa Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Thu, 25 Jun 2015 08:59:20 +0300 Subject: [PATCH v6 02/10] winbindd: cleanup client connection if the client closes the connection This patch allows for early cleanup of client connections if the client has given up. Before this patch, any received request would be processed, and then only upon transmitting the result to the client would winbindd find out the client is no longer with us, possibly leading to a situation where the same client tries over and over and increases the number of client connections. This patch monitors the client socket for readability while the request is being processed, and closes the client connection if the socket becomes readable. The client is not supposed to be writing anything to the socket while it is waiting, so readability means either that the client has closed the connection, or that it has broken the protocol. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397 Signed-off-by: Uri Simchoni Reviewed-by: Volker Lendecke --- source3/winbindd/winbindd.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c index defc9cc..a03b5b4 100644 --- a/source3/winbindd/winbindd.c +++ b/source3/winbindd/winbindd.c @@ -42,6 +42,7 @@ #include "source4/lib/messaging/irpc.h" #include "source4/lib/messaging/messaging.h" #include "lib/param/param.h" +#include "lib/async_req/async_sock.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_WINBIND @@ -809,11 +810,15 @@ static void request_finished(struct winbindd_cli_state *state); static void winbind_client_request_read(struct tevent_req *req); static void winbind_client_response_written(struct tevent_req *req); +static void winbind_client_activity(struct tevent_req *req); static void request_finished(struct winbindd_cli_state *state) { struct tevent_req *req; + /* free client socket monitoring request */ + TALLOC_FREE(state->io_req); + TALLOC_FREE(state->request); req = wb_resp_write_send(state, winbind_event_context(), @@ -966,9 +971,32 @@ static void winbind_client_request_read(struct tevent_req *req) remove_client(state); return; } + + req = wait_for_read_send(state, winbind_event_context(), state->sock); + if (req == NULL) { + DEBUG(0, ("winbind_client_request_read[%d:%s]:" + " wait_for_read_send failed - removing client\n", + (int)state->pid, state->cmd_name)); + remove_client(state); + return; + } + tevent_req_set_callback(req, winbind_client_activity, state); + state->io_req = req; + process_request(state); } +static void winbind_client_activity(struct tevent_req *req) +{ + struct winbindd_cli_state *state = + tevent_req_callback_data(req, struct winbindd_cli_state); + int err; + + wait_for_read_recv(req, &err); + + remove_client(state); +} + /* Remove a client connection from client connection list */ static void remove_client(struct winbindd_cli_state *state) -- 1.9.1 From 7c9ce7edf2e41210ecc1ec83d9ffe7ca537913b5 Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Thu, 25 Jun 2015 09:46:24 +0300 Subject: [PATCH v6 03/10] async_req: check for errors when monitoring socket for readability Add an option to wait_for_read_send(), so that the request, upon calling back, report whether the socket actually contains data or is in EOF/error state. EOF is signalled via the EPIPE error. This is useful for clients which do not expect data to arrive but wait for readability to detect a closed socket (i.e. they do not intend to actually read the socket when it's readable). Actual data arrival would indicate a bug in this case, so the check can be used to print an error message. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397 Signed-off-by: Uri Simchoni --- lib/async_req/async_sock.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- lib/async_req/async_sock.h | 4 ++-- source3/smbd/process.c | 4 ++-- source3/winbindd/winbindd.c | 3 ++- 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/lib/async_req/async_sock.c b/lib/async_req/async_sock.c index d2cda15..bc3780c 100644 --- a/lib/async_req/async_sock.c +++ b/lib/async_req/async_sock.c @@ -534,6 +534,8 @@ ssize_t read_packet_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, struct wait_for_read_state { struct tevent_fd *fde; + int fd; + bool check_errors; }; static void wait_for_read_cleanup(struct tevent_req *req, @@ -544,8 +546,8 @@ static void wait_for_read_done(struct tevent_context *ev, void *private_data); struct tevent_req *wait_for_read_send(TALLOC_CTX *mem_ctx, - struct tevent_context *ev, - int fd) + struct tevent_context *ev, int fd, + bool check_errors) { struct tevent_req *req; struct wait_for_read_state *state; @@ -562,6 +564,9 @@ struct tevent_req *wait_for_read_send(TALLOC_CTX *mem_ctx, if (tevent_req_nomem(state->fde, req)) { return tevent_req_post(req, ev); } + + state->fd = fd; + state->check_errors = check_errors; return req; } @@ -581,10 +586,44 @@ static void wait_for_read_done(struct tevent_context *ev, { struct tevent_req *req = talloc_get_type_abort( private_data, struct tevent_req); + struct wait_for_read_state *state = + tevent_req_data(req, struct wait_for_read_state); + ssize_t nread; + char c; - if (flags & TEVENT_FD_READ) { + if ((flags & TEVENT_FD_READ) == 0) { + return; + } + + if (!state->check_errors) { tevent_req_done(req); + return; } + + nread = recv(state->fd, &c, 1, MSG_PEEK); + + if (nread == 0) { + tevent_req_error(req, EPIPE); + return; + } + + if ((nread == -1) && (errno == EINTR)) { + /* come back later */ + return; + } + + if ((nread == -1) && (errno == ENOTSOCK)) { + /* Ignore this specific error on pipes */ + tevent_req_done(req); + return; + } + + if (nread == -1) { + tevent_req_error(req, errno); + return; + } + + tevent_req_done(req); } bool wait_for_read_recv(struct tevent_req *req, int *perr) diff --git a/lib/async_req/async_sock.h b/lib/async_req/async_sock.h index 1b76fab..abbf822 100644 --- a/lib/async_req/async_sock.h +++ b/lib/async_req/async_sock.h @@ -53,8 +53,8 @@ ssize_t read_packet_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, uint8_t **pbuf, int *perrno); struct tevent_req *wait_for_read_send(TALLOC_CTX *mem_ctx, - struct tevent_context *ev, - int fd); + struct tevent_context *ev, int fd, + bool check_errors); bool wait_for_read_recv(struct tevent_req *req, int *perr); #endif diff --git a/source3/smbd/process.c b/source3/smbd/process.c index c83f3bc..4d047a9 100644 --- a/source3/smbd/process.c +++ b/source3/smbd/process.c @@ -2846,7 +2846,7 @@ static struct tevent_req *smbd_echo_read_send( state->ev = ev; state->xconn = xconn; - subreq = wait_for_read_send(state, ev, xconn->transport.sock); + subreq = wait_for_read_send(state, ev, xconn->transport.sock, false); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } @@ -2921,7 +2921,7 @@ static void smbd_echo_read_waited(struct tevent_req *subreq) } subreq = wait_for_read_send(state, state->ev, - xconn->transport.sock); + xconn->transport.sock, false); if (tevent_req_nomem(subreq, req)) { return; } diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c index a03b5b4..07faadf 100644 --- a/source3/winbindd/winbindd.c +++ b/source3/winbindd/winbindd.c @@ -972,7 +972,8 @@ static void winbind_client_request_read(struct tevent_req *req) return; } - req = wait_for_read_send(state, winbind_event_context(), state->sock); + req = wait_for_read_send(state, winbind_event_context(), state->sock, + false); if (req == NULL) { DEBUG(0, ("winbind_client_request_read[%d:%s]:" " wait_for_read_send failed - removing client\n", -- 1.9.1 From 8e52b26dfd61c79e775912a7283665ec1c1527a8 Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Thu, 25 Jun 2015 10:12:37 +0300 Subject: [PATCH v6 04/10] winbindd: verify that client has closed the connection A recent change was to remove a client if the client socket has become readable. In this change, a check is added to determine the source of the readbility (actual readability, closed connection, or some other error), and a suitable debug message is printed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397 Signed-off-by: Uri Simchoni --- source3/winbindd/winbindd.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c index 07faadf..fb47781 100644 --- a/source3/winbindd/winbindd.c +++ b/source3/winbindd/winbindd.c @@ -973,7 +973,7 @@ static void winbind_client_request_read(struct tevent_req *req) } req = wait_for_read_send(state, winbind_event_context(), state->sock, - false); + true); if (req == NULL) { DEBUG(0, ("winbind_client_request_read[%d:%s]:" " wait_for_read_send failed - removing client\n", @@ -992,8 +992,28 @@ static void winbind_client_activity(struct tevent_req *req) struct winbindd_cli_state *state = tevent_req_callback_data(req, struct winbindd_cli_state); int err; + bool has_data; - wait_for_read_recv(req, &err); + has_data = wait_for_read_recv(req, &err); + + if (has_data) { + DEBUG(0, ("winbind_client_activity[%d:%s]:" + "unexpected data from client - removing client\n", + (int)state->pid, state->cmd_name)); + } else { + if (err == EPIPE) { + DEBUG(6, ("winbind_client_activity[%d:%s]: " + "client has closed connection - removing " + "client\n", + (int)state->pid, state->cmd_name)); + } else { + DEBUG(2, ("winbind_client_activity[%d:%s]: " + "client socket error (%s) - removing " + "client\n", + (int)state->pid, state->cmd_name, + strerror(err))); + } + } remove_client(state); } -- 1.9.1 From 8daf73bb7f4c95bfd617a786de73fec0b17b6453 Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Wed, 3 Jun 2015 00:36:27 +0300 Subject: [PATCH v6 05/10] winbind client: avoid vicious cycle created by client retry This patch cancels the retry policy of the winbind client. When winbindd fails to respond to a request within 30 seconds, the winbind client closes the connection and retries up to 10 times. In some cases, delayed response is a result of multiple requests from multiple clients piling up on the winbind domain child process. Retrying just piles more and more requests, creating a vicious cycle. Even in the case of a single request taking long to complete, there's no point in retrying because the retry request would just wait for the current request to complete. Better to wait patiently. There's one possible benefit in the retry, namely that winbindd typically caches the results, and therefore a retry might take a cached result, so the net effect of the retry may be to increase the timeout to 300 seconds. But a more straightforward way to have a 300 second timeout is to modify the timeout. Therefore the timeout is modified from 30 seconds to 300 seconds (IMHO 300 seconds is too much, but we have "winbind rquest timeout" with a default of 60 to make sure the request completes or fails within 60 seconds) BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397 Signed-off-by: Uri Simchoni Reviewed-by: Jeremy Allison Reviewed-by: Volker Lendecke --- nsswitch/wb_common.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c index 3194df1..139f010 100644 --- a/nsswitch/wb_common.c +++ b/nsswitch/wb_common.c @@ -535,7 +535,7 @@ static int winbind_read_sock(struct winbindd_context *ctx, if (ret == 0) { /* Not ready for read yet... */ - if (total_time >= 30) { + if (total_time >= 300) { /* Timeout */ winbind_close_sock(ctx); return -1; @@ -719,20 +719,16 @@ NSS_STATUS winbindd_request_response(struct winbindd_context *ctx, struct winbindd_response *response) { NSS_STATUS status = NSS_STATUS_UNAVAIL; - int count = 0; struct winbindd_context *wb_ctx = ctx; if (ctx == NULL) { wb_ctx = &wb_global_ctx; } - while ((status == NSS_STATUS_UNAVAIL) && (count < 10)) { - status = winbindd_send_request(wb_ctx, req_type, 0, request); - if (status != NSS_STATUS_SUCCESS) - return(status); - status = winbindd_get_response(wb_ctx, response); - count += 1; - } + status = winbindd_send_request(wb_ctx, req_type, 0, request); + if (status != NSS_STATUS_SUCCESS) + return (status); + status = winbindd_get_response(wb_ctx, response); return status; } @@ -743,20 +739,16 @@ NSS_STATUS winbindd_priv_request_response(struct winbindd_context *ctx, struct winbindd_response *response) { NSS_STATUS status = NSS_STATUS_UNAVAIL; - int count = 0; struct winbindd_context *wb_ctx = ctx; if (ctx == NULL) { wb_ctx = &wb_global_ctx; } - while ((status == NSS_STATUS_UNAVAIL) && (count < 10)) { - status = winbindd_send_request(wb_ctx, req_type, 1, request); - if (status != NSS_STATUS_SUCCESS) - return(status); - status = winbindd_get_response(wb_ctx, response); - count += 1; - } + status = winbindd_send_request(wb_ctx, req_type, 1, request); + if (status != NSS_STATUS_SUCCESS) + return (status); + status = winbindd_get_response(wb_ctx, response); return status; } -- 1.9.1 From a61a88790a0c1006e2418aa5a8ace76cb58456be Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Mon, 6 Jul 2015 21:29:17 +0300 Subject: [PATCH v6 06/10] winbindd: periodically remove timed out clients Periodically scan winbind client list and close connections in which either the client is idle, or the request is taking too long to complete. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397 Signed-off-by: Uri Simchoni Reviewed-by: Jeremy Allison --- source3/winbindd/winbindd.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c index fb47781..66ea68b 100644 --- a/source3/winbindd/winbindd.c +++ b/source3/winbindd/winbindd.c @@ -47,6 +47,8 @@ #undef DBGC_CLASS #define DBGC_CLASS DBGC_WINBIND +#define SCRUB_CLIENTS_INTERVAL 5 + static bool client_is_idle(struct winbindd_cli_state *state); static void remove_client(struct winbindd_cli_state *state); static void winbindd_setup_max_fds(void); @@ -1143,6 +1145,20 @@ static void remove_timed_out_clients(void) } } +static void winbindd_scrub_clients_handler(struct tevent_context *ev, + struct tevent_timer *te, + struct timeval current_time, + void *private_data) +{ + remove_timed_out_clients(); + if (tevent_add_timer(ev, ev, + timeval_current_ofs(SCRUB_CLIENTS_INTERVAL, 0), + winbindd_scrub_clients_handler, NULL) == NULL) { + DEBUG(0, ("winbindd: failed to reschedule client scrubber\n")); + exit(1); + } +} + struct winbindd_listen_state { bool privileged; int fd; @@ -1276,6 +1292,8 @@ static bool winbindd_setup_listeners(void) } tevent_fd_set_auto_close(fde); + winbindd_scrub_clients_handler(winbind_event_context(), NULL, + timeval_current(), NULL); return true; failed: TALLOC_FREE(pub_state); -- 1.9.1 From 41d9390e73deac1469dd15f96df5ec00f1b7734a Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Mon, 6 Jul 2015 12:13:15 +0300 Subject: [PATCH v6 07/10] doc: clarify "winbind max clients" Add clarification about the nature of "winbind max clients" parameter. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397 Signed-off-by: Uri Simchoni Reviewed-by: Volker Lendecke --- docs-xml/smbdotconf/winbind/winbindmaxclients.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs-xml/smbdotconf/winbind/winbindmaxclients.xml b/docs-xml/smbdotconf/winbind/winbindmaxclients.xml index d37ee25..c11d0b6 100644 --- a/docs-xml/smbdotconf/winbind/winbindmaxclients.xml +++ b/docs-xml/smbdotconf/winbind/winbindmaxclients.xml @@ -6,6 +6,12 @@ This parameter specifies the maximum number of clients the winbindd 8 daemon can connect with. + The parameter is not a hard limit. + The winbindd + 8 daemon configures + itself to be able to accept at least that many connections, + and if the limit is reached, an attempt is made to disconnect + idle clients. -- 1.9.1 From cc2e9843f970cc7d53d1ea028283a82c25c371bb Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Mon, 13 Jul 2015 21:08:16 +0300 Subject: [PATCH v6 08/10] winbindd: add service routines to support a sorted client list Add some routines that support keeping the client list sorted (by last access time) and traversing the list from oldest to newest BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397 Signed-off-by: Uri Simchoni --- source3/winbindd/winbindd_proto.h | 4 ++++ source3/winbindd/winbindd_util.c | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h index 5ad69e2..9920a3f 100644 --- a/source3/winbindd/winbindd_proto.h +++ b/source3/winbindd/winbindd_proto.h @@ -430,8 +430,12 @@ char *fill_domain_username_talloc(TALLOC_CTX *ctx, const char *user, bool can_assume); struct winbindd_cli_state *winbindd_client_list(void); +struct winbindd_cli_state *winbindd_client_list_tail(void); +struct winbindd_cli_state * +winbindd_client_list_prev(struct winbindd_cli_state *cli); void winbindd_add_client(struct winbindd_cli_state *cli); void winbindd_remove_client(struct winbindd_cli_state *cli); +void winbindd_promote_client(struct winbindd_cli_state *cli); int winbindd_num_clients(void); NTSTATUS lookup_usergroups_cached(struct winbindd_domain *domain, TALLOC_CTX *mem_ctx, diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c index d73327c..0108504 100644 --- a/source3/winbindd/winbindd_util.c +++ b/source3/winbindd/winbindd_util.c @@ -1206,6 +1206,21 @@ struct winbindd_cli_state *winbindd_client_list(void) return _client_list; } +/* Return list-tail of all connected clients */ + +struct winbindd_cli_state *winbindd_client_list_tail(void) +{ + return DLIST_TAIL(_client_list); +} + +/* Return previous (read:newer) client in list */ + +struct winbindd_cli_state * +winbindd_client_list_prev(struct winbindd_cli_state *cli) +{ + return DLIST_PREV(cli); +} + /* Add a connection to the list */ void winbindd_add_client(struct winbindd_cli_state *cli) @@ -1222,6 +1237,13 @@ void winbindd_remove_client(struct winbindd_cli_state *cli) _num_clients--; } +/* Move a client to head or list */ + +void winbindd_promote_client(struct winbindd_cli_state *cli) +{ + DLIST_PROMOTE(_client_list, cli); +} + /* Return number of open clients */ int winbindd_num_clients(void) -- 1.9.1 From 0665b9c7c08544cfd4d918032b8ec24a9b2874fa Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Mon, 13 Jul 2015 21:33:41 +0300 Subject: [PATCH v6 09/10] winbindd: keep client list sorted by access time Keep client list sorted by last access time, newest to oldest. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397 Signed-off-by: Uri Simchoni --- source3/winbindd/winbindd.c | 5 ++--- source3/winbindd/winbindd_util.c | 2 ++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c index 66ea68b..620fd3f 100644 --- a/source3/winbindd/winbindd.c +++ b/source3/winbindd/winbindd.c @@ -701,7 +701,8 @@ static void process_request(struct winbindd_cli_state *state) state->cmd_name = "unknown request"; state->recv_fn = NULL; - state->last_access = time(NULL); + /* client is newest */ + winbindd_promote_client(state); /* Process command */ @@ -930,8 +931,6 @@ static void new_connection(int listen_sock, bool privileged) return; } - state->last_access = time(NULL); - state->privileged = privileged; req = wb_req_read_send(state, winbind_event_context(), state->sock, diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c index 0108504..233b5c9 100644 --- a/source3/winbindd/winbindd_util.c +++ b/source3/winbindd/winbindd_util.c @@ -1225,6 +1225,7 @@ winbindd_client_list_prev(struct winbindd_cli_state *cli) void winbindd_add_client(struct winbindd_cli_state *cli) { + cli->last_access = time(NULL); DLIST_ADD(_client_list, cli); _num_clients++; } @@ -1241,6 +1242,7 @@ void winbindd_remove_client(struct winbindd_cli_state *cli) void winbindd_promote_client(struct winbindd_cli_state *cli) { + cli->last_access = time(NULL); DLIST_PROMOTE(_client_list, cli); } -- 1.9.1 From c86308226661692c83faa2ac6c598270865342cd Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Mon, 13 Jul 2015 21:42:57 +0300 Subject: [PATCH v6 10/10] winbindd: shorten client list scan Counting on the client list being sorted by last access time, the list scan for removing timed-out clients is shortened - once the list is scanned oldest to newest, and once a non-timed-out client is found, the scan can stop. Also, finding the oldest idle client for removing an idle client is simplified - oldest idle client is last idle client. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397 Signed-off-by: Uri Simchoni --- source3/winbindd/winbindd.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c index 620fd3f..c878ce2 100644 --- a/source3/winbindd/winbindd.c +++ b/source3/winbindd/winbindd.c @@ -1086,16 +1086,13 @@ static bool client_is_idle(struct winbindd_cli_state *state) { static bool remove_idle_client(void) { struct winbindd_cli_state *state, *remove_state = NULL; - time_t last_access = 0; int nidle = 0; for (state = winbindd_client_list(); state; state = state->next) { if (client_is_idle(state)) { nidle++; - if (!last_access || state->last_access < last_access) { - last_access = state->last_access; - remove_state = state; - } + /* list is sorted by access time */ + remove_state = state; } } @@ -1117,14 +1114,14 @@ static bool remove_idle_client(void) static void remove_timed_out_clients(void) { - struct winbindd_cli_state *state, *next = NULL; + struct winbindd_cli_state *state, *prev = NULL; time_t curr_time = time(NULL); int timeout_val = lp_winbind_request_timeout(); - for (state = winbindd_client_list(); state; state = next) { + for (state = winbindd_client_list_tail(); state; state = prev) { time_t expiry_time; - next = state->next; + prev = winbindd_client_list_prev(state); expiry_time = state->last_access + timeout_val; if (curr_time > expiry_time) { @@ -1140,6 +1137,10 @@ static void remove_timed_out_clients(void) (unsigned int)state->pid)); } remove_client(state); + } else { + /* list is sorted, previous clients in + list are newer */ + break; } } } -- 1.9.1