From: David Disseldorp Date: Wed, 9 Jul 2014 22:18:10 +0000 (+0200) Subject: printing: traverse_read the printer list for share updates X-Git-Tag: samba-4.0.22~19 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c82338f5aa013fbd0d394f5a8dced3a4eea04d31;p=thirdparty%2Fsamba.git printing: traverse_read the printer list for share updates The printcap update procedure involves the background printer process obtaining the printcap information from the printing backend, writing this to printer_list.tdb, and then notifying all smbd processes of the new list. The processes then all attempt to simultaneously traverse printer_list.tdb, in order to update their local share lists. With a large number of printers, and a large number of per-client smbd processes, this traversal results in significant lock contention, mostly due to the fact that the traversal is unnecessarily done with an exclusive (write) lock on the printer_list.tdb database. This commit changes the share update code path to perform a read-only traversal. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10652 Reported-by: Alex K Reported-by: Franz Pförtsch Signed-off-by: David Disseldorp Reviewed-by: Andreas Schneider (cherry picked from commit 1e83435eac2cef03fccb4cf69ef5e0bfbd710410) --- diff --git a/source3/printing/load.c b/source3/printing/load.c index 136d055088f..2ba3b2e106d 100644 --- a/source3/printing/load.c +++ b/source3/printing/load.c @@ -71,5 +71,5 @@ void load_printers(struct tevent_context *ev, /* load all printcap printers */ if (lp_load_printers() && lp_servicenumber(PRINTERS_NAME) >= 0) - pcap_printer_fn(lp_add_one_printer, NULL); + pcap_printer_read_fn(lp_add_one_printer, NULL); } diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c index dd7ba624590..25dd4c70cde 100644 --- a/source3/printing/pcap.c +++ b/source3/printing/pcap.c @@ -229,11 +229,11 @@ void pcap_printer_fn_specific(const struct pcap_cache *pc, return; } -void pcap_printer_fn(void (*fn)(const char *, const char *, const char *, void *), void *pdata) +void pcap_printer_read_fn(void (*fn)(const char *, const char *, const char *, void *), void *pdata) { NTSTATUS status; - status = printer_list_run_fn(fn, pdata); + status = printer_list_read_run_fn(fn, pdata); if (!NT_STATUS_IS_OK(status)) { DEBUG(3, ("Failed to run fn for all printers!\n")); } diff --git a/source3/printing/pcap.h b/source3/printing/pcap.h index 70562137ac8..6c062c3294e 100644 --- a/source3/printing/pcap.h +++ b/source3/printing/pcap.h @@ -39,7 +39,7 @@ bool pcap_cache_add(const char *name, const char *comment, const char *location) bool pcap_cache_loaded(void); bool pcap_cache_replace(const struct pcap_cache *cache); void pcap_printer_fn_specific(const struct pcap_cache *, void (*fn)(const char *, const char *, const char *, void *), void *); -void pcap_printer_fn(void (*fn)(const char *, const char *, const char *, void *), void *); +void pcap_printer_read_fn(void (*fn)(const char *, const char *, const char *, void *), void *); void pcap_cache_reload(struct tevent_context *ev, struct messaging_context *msg_ctx, diff --git a/source3/printing/printer_list.c b/source3/printing/printer_list.c index 0fc94733a08..2355e29b495 100644 --- a/source3/printing/printer_list.c +++ b/source3/printing/printer_list.c @@ -279,7 +279,8 @@ done: typedef int (printer_list_trv_fn_t)(struct db_record *, void *); static NTSTATUS printer_list_traverse(printer_list_trv_fn_t *fn, - void *private_data) + void *private_data, + bool read_only) { struct db_context *db; NTSTATUS status; @@ -289,7 +290,11 @@ static NTSTATUS printer_list_traverse(printer_list_trv_fn_t *fn, return NT_STATUS_INTERNAL_DB_CORRUPTION; } - status = dbwrap_traverse(db, fn, private_data, NULL); + if (read_only) { + status = dbwrap_traverse_read(db, fn, private_data, NULL); + } else { + status = dbwrap_traverse(db, fn, private_data, NULL); + } return status; } @@ -359,7 +364,7 @@ NTSTATUS printer_list_clean_old(void) state.status = NT_STATUS_OK; - status = printer_list_traverse(printer_list_clean_fn, &state); + status = printer_list_traverse(printer_list_clean_fn, &state, false); if (NT_STATUS_EQUAL(status, NT_STATUS_UNSUCCESSFUL) && !NT_STATUS_IS_OK(state.status)) { status = state.status; @@ -412,8 +417,8 @@ static int printer_list_exec_fn(struct db_record *rec, void *private_data) return 0; } -NTSTATUS printer_list_run_fn(void (*fn)(const char *, const char *, const char *, void *), - void *private_data) +NTSTATUS printer_list_read_run_fn(void (*fn)(const char *, const char *, const char *, void *), + void *private_data) { struct printer_list_exec_state state; NTSTATUS status; @@ -422,7 +427,7 @@ NTSTATUS printer_list_run_fn(void (*fn)(const char *, const char *, const char * state.private_data = private_data; state.status = NT_STATUS_OK; - status = printer_list_traverse(printer_list_exec_fn, &state); + status = printer_list_traverse(printer_list_exec_fn, &state, true); if (NT_STATUS_EQUAL(status, NT_STATUS_UNSUCCESSFUL) && !NT_STATUS_IS_OK(state.status)) { status = state.status; diff --git a/source3/printing/printer_list.h b/source3/printing/printer_list.h index fb2e007ae6c..b12c1923e72 100644 --- a/source3/printing/printer_list.h +++ b/source3/printing/printer_list.h @@ -100,6 +100,6 @@ NTSTATUS printer_list_mark_reload(void); */ NTSTATUS printer_list_clean_old(void); -NTSTATUS printer_list_run_fn(void (*fn)(const char *, const char *, const char *, void *), - void *private_data); +NTSTATUS printer_list_read_run_fn(void (*fn)(const char *, const char *, const char *, void *), + void *private_data); #endif /* _PRINTER_LIST_H_ */