This was done becaese fix-stack-ztest was added to the stack
in series after fix-pthreads because fix-stack-ztest depends
on many of the pthreads changes.
While ztest does run in user space we run it with the same stack
restrictions it would have in kernel space. This ensures that any
stack related issues which would be hit in the kernel can be caught
and debugged in user space instead.
This patch is a first pass to limit the stack usage of every ztest
function to 1024 bytes. Subsequent updates can further reduce this
Due to limited stack space recursive functions are frowned upon in
the Linux kernel. However, they often are the most elegant solution
to a problem. The following code preserves the recursive function
traverse_visitbp() but moves the local variables AND function
arguments to the heap to minimize the stack frame size. Enough
space is initially allocated on the stack for 20 levels of recursion.
This change does ugly-up-the-code but it reduces the worst case
usage from roughly 4160 bytes to 960 bytes on x86_64 archs.
These changes are now taken care of by the fix-stack-traverse_impl
topic branch which not only solves the uninit problem but also
moves these locals off the stack and on to the heap.
Move dsl_dataset_t local variable from the stack to the heap.
This reduces the stack usage of this function from 2048 bytes
to 176 bytes for x84_64 arches.
There are 3 fixes in thie commit. First, update ztest_run() to store
the thread id and not the address of the kthread_t. This will be freed
on thread exit and is not safe to use. This is pretty close to how
things were done in the original ztest code before I got there.
Second, for extra paranoia update thread_exit() to return a special
TS_MAGIC value via pthread_exit(). This value is then verified in
pthread_join() to ensure the thread exited cleanly. This can be
done cleanly because the kthread doesn't provide a return code
mechanism we need to worry about.
Third, replace the ztest deadman thread with a signal handler. We
cannot use the previous approach because the correct behavior for
pthreads is to wait for all threads to exit before terminating the
process. Since the deadman thread won't call exit by design we
end up hanging in kernel_exit(). To avoid this we just setup a
SIGALRM signal handle and register a deadman alarm. IMHO this
is simpler and cleaner anyway.
Accidentally dropped the zeroing of this structure in the
gcc-missing-braces topic branch which was causing a fall positive
space leak in ztest. Ensure the structure is zero'ed before use.
This may not strictly be needed but it does keep gcc happy. We
should keep our eye on this though if the extra bcopy significantly
impacts performance. It may.
There was previous discussion of a race with joinable threads but to
be honest I can neither exactly remember the race, or recrease the
issue. I believe it may have had to do with pthread_create() returning
without having set kt->tid since this was done in the created thread.
If that was the race then I've 'fixed' it by ensuring the thread id
is set in the thread AND as the first pthread_create() argument. Why
this wasn't done originally I'm not sure, with luck Ricardo remembers.
Additionally, explicitly set a PAGESIZE guard frame at the end of the
stack to aid in detecting stack overflow. And add some conditional
logic to set STACK_SIZE correctly for Solaris.
There are cases where under Linux it is not safe to sleep in
taskq_dispatch(). Rather than adding Linux specific code to
detect these cases I opted to keep it simple and just never
allow a sleep here. The impact of this should be minimal.
I missed a instanse of removing the & operator when reducing the
stack usage in this function. This unfortunately doesn't cause
a compile warning but it is does cause ztest failures. Anyway,
update the topic branch to correct this mistake.
Brian Behlendorf