PR for DC of #3589 and #3590 #3596
Conversation
On call of frr_pthread_new, save the os_name if given, if not given use the name passed in( shortening to fit in available space ) and finally if the name was not passed in use the default value. Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
When we start a thread we always call fpt_run and since the last commit we know os_name is filled with something, therefore we can just set the name on startup. This creates this output now for zebra: sharpd@donna ~/frr2> ps -L -p 25643 PID LWP TTY TIME CMD 25643 25643 ? 00:00:00 zebra 25643 25644 ? 00:00:00 Zebra dplane 25643 25684 ? 00:00:00 zebra_apic sharpd@donna ~/frr2> I removed the abstraction to frr_pthread_set_name because it was snprintf'ing into the same buffer which was the real bug here( the first character of os_name became null). In the next commit I'll remove that api because it is unneeded and was a horrible hack to get this to work for the one place it was wanted. Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
…me of the thread The current invocation of frr_pthread_set_name was causing it reset the os_name. There is no need for this, we now always create the pthread appropriately to have both name and os_name. So convert this function to a simple call through of the pthread call now. Before(any of these changes): sharpd@robot ~/frr1> ps -L -p 16895 PID LWP TTY TIME CMD 16895 16895 ? 00:01:39 bgpd 16895 16896 ? 00:00:54 16895 16897 ? 00:00:07 bgpd_ka After: sharpd@donna ~/frr1> ps -L -p 1752 PID LWP TTY TIME CMD 1752 1752 ? 00:00:00 bgpd 1752 1753 ? 00:00:00 bgpd_io 1752 1754 ? 00:00:00 bgpd_ka Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
When using getrusage, we have multiple choices about what
to call for data gathering about this particular thread of execution.
RUSAGE_SELF -> This means gather all cpu run time for all pthreads associated
with this process.
RUSAGE_THREAD -> This means gather all cpu run time for this particular
pthread.
Clearly with data gathering for slow thread as well as `show thread cpu`
it would be preferable to gather only data about the current running
pthread. This probably was the original behavior of using RUSAGE_SELF
when we didn't have multiple pthreads. So it didn't matter so much.
Prior to this change, 10 iterations of 1 million routes install/remove
from zebra would give us this cpu time for the dataplane pthread:
Showing statistics for pthread Zebra dplane thread
--------------------------------------------------
CPU (user+system): Real (wall-clock):
Active Runtime(ms) Invoked Avg uSec Max uSecs Avg uSec Max uSecs Type Thread
0 280902.149 326541 860 2609982 550 2468910 E dplane_thread_loop
After this change we are seeing this:
Showing statistics for pthread Zebra dplane thread
--------------------------------------------------
CPU (user+system): Real (wall-clock):
Active Runtime(ms) Invoked Avg uSec Max uSecs Avg uSec Max uSecs Type Thread
0 58045.560 334944 173 277226 539 2502268 E dplane_thread_loop
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an EXPERIMENTAL automated CI system. Get source and apply patch from patchwork: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedDebian 9 deb pkg check: Successful IPv4 protocols on Ubuntu 14.04: FailedWarnings Generated during build:Debian 8 amd64 build: Successful with additional warnings:Debian Package lintian failed for Debian 8 amd64 build: Ubuntu 14.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 14.04 amd64 build: Ubuntu 16.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 16.04 amd64 build: Ubuntu 18.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 18.04 amd64 build: Debian 9 amd64 build: Successful with additional warnings:Debian Package lintian failed for Debian 9 amd64 build: Ubuntu 16.04 i386 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 16.04 i386 build: CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-6368/ This is a comment from an EXPERIMENTAL automated CI system. Warnings Generated during build:Debian 8 amd64 build: Successful with additional warnings:Debian Package lintian failed for Debian 8 amd64 build: Ubuntu 14.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 14.04 amd64 build: Ubuntu 16.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 16.04 amd64 build: Ubuntu 18.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 18.04 amd64 build: Debian 9 amd64 build: Successful with additional warnings:Debian Package lintian failed for Debian 9 amd64 build: Ubuntu 16.04 i386 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 16.04 i386 build: CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base |
4 commits from master to dev/7.0 branch, from the #3589 and #3590 PR's
pthread set name appropriately
rusage of SELF -vs- THREAD