The ->hash_cmp and linked list ->cmp functions were sometimes
being used interchangeably and this really is not a good
thing. So let's modify the hash_cmp function pointer to return
a boolean and convert everything to use the new syntax.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Don't allocate threads in the stack, but use the standardized
`thread_get` and `thread_add_unused` to avoid creating corner cases in
the thread API.
This fixes a thread mutex memory leak in FreeBSD.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Two important changes:
* Centralize the thread teardown procedure;
* Save and restore thread mutex context to avoid losing the memory
pointer;
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
The master->unused list was unbounded during normal operation.
A full BGP feed on my machine left 11k threads on the unused
list, taking up over 2mb of data. This seemed a bit excessive,
reduce to a limit of 10.
Also fix a crash that this exposed where we assumed that a thread
structure was not deleted.
Future committers can make this configurable? or modify
the value to something better for their system. I am
dubious of the value of this.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We were storing Poll data for the read and write
memory information in MTYPE_THREAD, so a show run
would not be able to show actual amount of memory
associated with the `struct thread`.
Remove unnecessary NULL checks on malloc.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Thread statistics are collected and stored in a hashtable shared across
threads, but while the hashtable itself is protected by a mutex, the
records themselves were not being updated safely. Change all thread
history collection to use atomic operations.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The following types are nonstandard:
- u_char
- u_short
- u_int
- u_long
- u_int8_t
- u_int16_t
- u_int32_t
Replace them with the C99 standard types:
- uint8_t
- unsigned short
- unsigned int
- unsigned long
- uint8_t
- uint16_t
- uint32_t
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
There are some observed instances where we end up trying to cancel a rw
job based on a file descriptor that we don't have a reference on. The
specific cancel function for rw jobs assumes it's called with a file
descriptor that is valid within pollfds and will cause a segmentation
fault by buffer overrun if this is not the case.
Instead log it and move on. Since the fd does not exist this should
patch over the buggy behavior and provide additional information to help
in finding the root cause.
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
When we remove a thread from a pqueue, use the saved
index to go to the correct spot immediately instead of
having to search the whole queue for it.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
When displaying thread cpu data, display unsigned instead
of signed data when we get really really really large
numbers of invocations.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
list_free is occassionally being used to delete the
list and accidently not deleting all the nodes.
We keep running across this usage pattern. Let's
remove the temptation and only allow list_delete
to handle list deletion.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Convert the list_delete(struct list *) function to use
struct list **. This is to allow the list pointer to be nulled.
I keep running into uses of this list_delete function where we
forget to set the returned pointer to NULL and attempt to use
it and then experience a crash, usually after the developer
has long since left the building.
Let's make the api explicit in it setting the list pointer
to null.
Cynical Prediction: This code will expose a attempt
to use the NULL'ed list pointer in some obscure bit
of code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
list_delete does not set the list pointer to NULL
Thus when we accidently use it later we happily write
off into lala land instead of crashing imediately
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
1) Some hash key functions where converting pointers
directly to a 32 bit value via downcasting. Pointers
are 64 bit on a majority of our platforms.
2) Some hashes were being created with 256 entries,
downsize the hash creation size to more appropriate
values.
3) Add hash names to hash creation so we can watch
the hash via 'show debugging hashtable'
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
* Fix bad format specifier in thread.[ch]
* Move PRINTF_ATTRIBUTE macro to zebra.h
* Use PRINTF_ATTRIBUTE on termtable printers
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Add support for naming pthreads. Also, note that we don't have any
records yet if that's the case.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
This patch fixes up show thread commands so that they know about
and operate on all extant thread_masters, since we can now have multiple
running in any given application.
This change also eliminates a heap use after free that appears when
using a single cpu_record shared among multiple threads. Since struct
thread's have pointers to bits of memory that are freed when the global
statistics hash table is freed, later accesses are invalid. By moving
the stats hash to be unique to each thread_master this problem is
sidestepped.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
