1
0
mirror of https://github.com/samba-team/samba.git synced 2025-07-05 04:59:08 +03:00
Commit Graph

29 Commits

Author SHA1 Message Date
b51a2712c1 pthreadpool: Move creating of thread to new function
No functional change, but this simplifies error handling.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13170

Signed-off-by: Christof Schmitt <cs@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 949ccc3ea9)
2017-12-13 10:45:12 +01:00
7b1c746fe2 pthreadpool: Fix fork behaviour
glibc's pthread_cond_wait(&c, &m) increments m.__data.__nusers, making
pthread_mutex_destroy return EBUSY. Thus we can't allow any thread waiting for
a job across a fork. Also, the state of the condvar itself is unclear across a
fork. Right now to me it looks like an initialized but unused condvar can be
used in the child. Busy worker threads don't cause any trouble here, they don't
hold mutexes or condvars. Also, they can't reach the condvar because _prepare
holds all mutexes.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13006
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit ff98e3fb66)
2017-09-10 15:35:13 +02:00
1ad29ae69d lib/pthreadpool: fix a memory leak
When copying large files from the server to the client with aio enabled
we noticed that smbd kept growing RSS and VSZ.

valgrind was reporting:

==2503== 4,093,440 bytes in 6,560 blocks are possibly lost in loss record 460 of 460
==2503==    at 0x4C299CE: calloc (vg_replace_malloc.c:711)
==2503==    by 0x4011C24: _dl_allocate_tls (in /usr/lib64/ld-2.17.so)
==2503==    by 0x4E3C960: pthread_create@@GLIBC_2.2.5 (in /usr/lib64/libpthread-2.17.so)
==2503==    by 0x9B298AE: pthreadpool_add_job (in /usr/lib64/samba/libmessages-dgm-samba4.so)
==2503==    by 0x9B29FDC: pthreadpool_tevent_job_send (in /usr/lib64/samba/libmessages-dgm-samba4.so)
==2503==    by 0x56A78EF: ??? (in /usr/lib64/samba/libsmbd-base-samba4.so)
==2503==    by 0x55D86B7: smb_vfs_call_pread_send (in /usr/lib64/samba/libsmbd-base-samba4.so)
==2503==    by 0x55F7543: schedule_smb2_aio_read (in /usr/lib64/samba/libsmbd-base-samba4.so)
==2503==    by 0x5608F57: smbd_smb2_request_process_read (in /usr/lib64/samba/libsmbd-base-samba4.so)
==2503==    by 0x55FCB6C: smbd_smb2_request_dispatch (in /usr/lib64/samba/libsmbd-base-samba4.so)
==2503==    by 0x55FD7DC: ??? (in /usr/lib64/samba/libsmbd-base-samba4.so)
==2503==    by 0x641B977: ??? (in /usr/lib64/samba/libtevent.so.0.9.31)

The problem seems to be caused by worked threads that are not properly
started in detached state and thus their tls is not reclaimed upon
thread termination.

In pthreadpool.c we prepare a pthread attribute with
PTHREAD_CREATE_DETACHED, but we don't pass it to pthread_create().

Bug: https://bugzilla.samba.org/show_bug.cgi?id=12624

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Fri Mar 10 22:06:02 CET 2017 on sn-devel-144

(cherry picked from commit c9a7a065bb)

Autobuild-User(v4-6-test): Karolin Seeger <kseeger@samba.org>
Autobuild-Date(v4-6-test): Mon Mar 13 13:30:49 CET 2017 on sn-devel-144
2017-03-13 13:30:49 +01:00
e84521dc44 lib: Fix a pthreadpool race condition
Yes, there is one.... I've seen two flaky builds on sn-devel with
pthreadpool after the coverity checks went in. They were in the

		ret = pthread_mutex_unlock(&pool->mutex);
		assert(ret == 0);

in pthreadpool_parent() and pthreadpool_child(). No idea what that was,
I could not really reproduce that. A build attempt on FreeBSD also gave
an erratic error, this time it was an EINVAL in

		ret = pthread_mutex_lock(&pool->mutex);
		assert(ret == 0);

pthreadpool_parent(). EINVAL means that the mutex is not a proper
mutex. What happened: Someone (a detached thread) does the
pthreadpool_free behind our back, while we are in pthreadpool_parent,
preparing the fork. Unfortunately the mutex was already destroyed before
we came to lock it.

The fix is simple: Remove the obsolete struct pthreadpool from the
linked list before the mutex is destroyed.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2016-10-17 22:34:20 +02:00
1e676a6e2c pthreadpool: Rearrange locks a bit
Coverity ID 1373624 says we have a deadlock between pthreadpool_prepare and
pthreadpool_destroy. Coverity somehow misses that pthreadpool_free unlocks
pool->mutex, so I think this is a false positive. Nevertheless this re-arranges
the code a bit for more clarity, hoping that Coverity now can better track the
locks and unlocks. Also, the human reader might have to jump between routines a
bit less.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2016-10-14 21:45:08 +02:00
77b7dea2d9 pthreadpool: Use detached threads
So far we used joinable threads. This prevented pthreadpool_destroy with
blocked threads. This patch converts pthreadpool to detached threads. Now
pthreadpool_destroy does not have to wait for the idle threads to finish, it
can immediately return. pthreadpool_destroy will tell all threads to exit, and
the last active thread will actually free(pthreadpool).

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2016-10-05 00:06:21 +02:00
2d6b6c2446 pthreadpool: Make "shutdown" a bool
Just a small cleanup

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2016-10-05 00:06:21 +02:00
9395d958d6 pthreadpool: Signal job completion without the pool mutex
This essentially reverts 1c4284c739. We now call an alien function from
within pthreadpool, and we should not hold a mutex during that call. The alien
function could (and pthreadpool_tevent_job_signal actually does) lock a mutex.
We can't guarantee proper lock ordering here, so in theory we could deadlock. I
haven't seen it in the wild yet, but I could imagine that both _parent pieces
in pthreadpool and tevent could trigger such a deadlock.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Tue Aug 30 04:06:20 CEST 2016 on sn-devel-144
2016-08-30 04:06:20 +02:00
e7e18c432d pthreadpool: We always want asserts to abort()
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2016-08-30 00:13:10 +02:00
d7e51286e7 lib: add job data to to callback
The pthreadpool_tevent wrapper will need this

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2016-08-24 01:33:48 +02:00
5593467eb1 lib: Move pipe signalling to pthreadpool_pipe.c
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2016-08-24 01:33:48 +02:00
5066a6db4b s3:lib/pthreadpool: fix the build on older systems
Some systems required an explicit <signal.h>, which comes
via "system/wait.h"

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>

Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Tue Aug 16 19:09:55 CEST 2016 on sn-devel-144
2016-08-16 19:09:55 +02:00
57001580bb lib: Use replace.h properly in pthreadpool
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Fri Aug 12 04:26:09 CEST 2016 on sn-devel-144
2016-08-12 04:26:09 +02:00
c701e46431 lib: Fix CID 1338432 Unchecked return value
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2015-11-10 21:20:18 +01:00
1c4284c739 pthreadpool: Slightly serialize jobs
Using the new msg_source program with 1.500 instances against a single
msg_sink I found the msg_source process to spawn two worker threads for
synchronously sending the data towards the receiving socket. This should
not happen: Per destination node we only create one queue. We strictly
only add pthreadpool jobs one after the other, so a single helper thread
should be perfectly sufficient.

It turned out that under heavy overload the main sending thread was
scheduled before the thread that just had finished its send() job. So
the helper thread was not able to increment the pool->num_idle variable
indicating that we don't have to create a new thread when the new job
is added.

This patch moves the signalling write under the mutex. This means that
indicating readiness via the pipe and the pool->num_idle variable happen both
under the same mutex lock and thus are atomic. No superfluous threads anymore.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2014-08-23 00:24:18 +02:00
c5d07df6ab pthreadpool: Allow multiple jobs to be received
This can avoid syscalls when multiple jobs are finished simultaneously

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2014-03-27 06:06:11 +01:00
84aa2ddd86 pthreadpool: Avoid a malloc/free per job
pthreadpool_add_job is in our hottest code path for r/w intensive workloads, so
we should avoid anything CPU-intensive. pthreadpool used to malloc each job and
free it in the worker thread. This patch adds a FIFO queue for jobs that helper
threads copy from, avoiding constant malloc/free. This cuts user space
CPU in the local-bench-pthreadpool benchmark by roughly 10% on my system.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2014-03-27 06:06:11 +01:00
ccc187ff5e pthreadpool: Fix pthreadpools with fork
The current could would crash if a pthreadpool was created, deleted and the
process then fork()s. "pthreadpools" is NULL in this case, but the
pthread_atfork handlers are in place. This fixes walking the pthreadpools list
in reverse.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Kamen Mazdrashki <kamenim@samba.org>
Reviewed-by: Simo Sorce <simo@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
2014-03-03 14:29:24 +01:00
58865d96f1 pthreadpool: Fix a comment, "quit"->"shutdown"
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: David Disseldorp <ddiss@samba.org>

Autobuild-User(master): David Disseldorp <ddiss@samba.org>
Autobuild-Date(master): Tue Jan 28 19:06:40 CET 2014 on sn-devel-104
2014-01-28 19:06:40 +01:00
7027c6aca9 pthreadpool: Fix CID 710828 Sizeof not portable
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ira Cooper <ira@samba.org>
2013-05-12 15:56:32 +02:00
8a907c9c65 s3: Fix the pthreadpool build on OS/X
OS/X does not have clock_gettime, and without replace.h we do not
get the replacement macro
2012-04-09 18:05:02 +02:00
711c18c230 Change the signature of pthreadpool_finished_job() to return 0
on success, errno on fail and return the jobid in a separate variable.

I need this fix for my vfs_aio_pthread.c module.

Autobuild-User: Jeremy Allison <jra@samba.org>
Autobuild-Date: Thu Dec 22 12:12:33 CET 2011 on sn-devel-104
2011-12-22 12:12:33 +01:00
7b5fb7d9e8 replace: Add don't include unistd.h directly and add uid_wrapper. 2011-10-27 13:32:02 +02:00
2a789c8442 build: Fix waf build on MacOS X
The -framework CoreFoundation is required by the charset_macosxfs module

The system/time.h header is required to access the replacement clock_gettime()

Andrew Bartlett

Autobuild-User: Andrew Bartlett <abartlet@samba.org>
Autobuild-Date: Fri Sep 23 10:58:02 CEST 2011 on sn-devel-104
2011-09-23 10:58:02 +02:00
4778d35c34 s3/pthreadpool: replace bad portable gettimeofday by clock_gettime
Signed-off-by: Stefan Metzmacher <metze@samba.org>
2011-06-06 11:48:10 +02:00
a8a6433fec s3: Properly clean up in pthreadpool_init in case of failure
Autobuild-User: Volker Lendecke <vlendec@samba.org>
Autobuild-Date: Wed Apr 27 23:57:19 CEST 2011 on sn-devel-104
2011-04-27 23:57:19 +02:00
dbc36befb5 s3: Allow unlimited parallelism in pthreadpool 2011-04-26 12:41:56 +02:00
f4a0f856f3 s3: pthreadpool_sig_fd->pthreadpool_signal_fd 2011-04-25 09:50:32 +02:00
62689d8166 s3: Many pthreadpool fixes
In particular, this makes it fork-safe
2011-04-25 09:50:32 +02:00