[MERGED] osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Tom Tsou]

Tom Tsou has submitted this change and it was merged.

Change subject: Add '-t' command line option to enable SCHED_RR
..


Add '-t' command line option to enable SCHED_RR

SCHED_RR allows us to operate osmo-trx reliable even under exceptionally
high system load, as the realtime scheduler priority will have higher
priority than the other "regular" tasks on the system.

Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Closes: OS#2344
---
M Transceiver52M/osmo-trx.cpp
1 file changed, 29 insertions(+), 2 deletions(-)

Approvals:
  Tom Tsou: Looks good to me, approved
  Jenkins Builder: Verified
  Holger Freyther: Looks good to me, but someone else must approve



diff --git a/Transceiver52M/osmo-trx.cpp b/Transceiver52M/osmo-trx.cpp
index 31a99cf..61b3098 100644
--- a/Transceiver52M/osmo-trx.cpp
+++ b/Transceiver52M/osmo-trx.cpp
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -77,6 +78,7 @@
double rssi_offset;
bool swap_channels;
bool edge;
+   int sched_rr;
 };
 
 ConfigurationTable gConfig;
@@ -259,7 +261,8 @@
"  -rRandom Normal Burst test mode with TSC\n"
"  -ARandom Access Burst test mode with delay\n"
"  -RRSSI to dBm offset in dB (default=0)\n"
-   "  -SSwap channels (UmTRX only)\n",
+   "  -SSwap channels (UmTRX only)\n"
+   "  -tSCHED_RR real-time priority (1..32)\n",
"EMERG, ALERT, CRT, ERR, WARNING, NOTICE, INFO, DEBUG");
 }
 
@@ -283,8 +286,9 @@
config->rssi_offset = 0.0;
config->swap_channels = false;
config->edge = false;
+   config->sched_rr = -1;
 
-   while ((option = getopt(argc, argv, "ha:l:i:p:c:dmxgfo:s:b:r:A:R:Se")) 
!= -1) {
+   while ((option = getopt(argc, argv, 
"ha:l:i:p:c:dmxgfo:s:b:r:A:R:Set:")) != -1) {
switch (option) {
case 'h':
print_help();
@@ -343,6 +347,9 @@
case 'e':
config->edge = true;
break;
+   case 't':
+   config->sched_rr = atoi(optarg);
+   break;
default:
print_help();
exit(0);
@@ -386,6 +393,21 @@
exit(0);
 }
 
+static int set_sched_rr(int prio)
+{
+   struct sched_param param;
+   int rc;
+   memset(, 0, sizeof(param));
+   param.sched_priority = prio;
+   printf("Setting SCHED_RR priority(%d)\n", param.sched_priority);
+   rc = sched_setscheduler(getpid(), SCHED_RR, );
+   if (rc != 0) {
+   std::cerr << "Config: Setting SCHED_RR failed" << std::endl;
+   return -1;
+   }
+   return 0;
+}
+
 int main(int argc, char *argv[])
 {
int type, chans, ref;
@@ -424,6 +446,11 @@
 
handle_options(argc, argv, );
 
+   if (config.sched_rr != -1) {
+   if (set_sched_rr(config.sched_rr) < 0)
+   return EXIT_FAILURE;
+   }
+
setup_signal_handlers();
 
/* Check database sanity */

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 2
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Holger Freyther 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Tom Tsou 
Gerrit-Reviewer: neels 

osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Tom Tsou]

Patch Set 1: Code-Review+2

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Holger Freyther 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Tom Tsou 
Gerrit-Reviewer: neels 
Gerrit-HasComments: No

osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Holger Freyther]

Patch Set 1: Code-Review+1

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Holger Freyther 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Tom Tsou 
Gerrit-Reviewer: neels 
Gerrit-HasComments: No

osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Neels Hofmeyr]

Patch Set 1:

i.e. there is a group that is given realtime priority permissions, which the 
jenkins user is a member of.

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Tom Tsou 
Gerrit-Reviewer: neels 
Gerrit-HasComments: No

osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Neels Hofmeyr]

Patch Set 1:

on osmo-gsm-tester we use /etc/security/limits.d as documented in 
http://ftp.osmocom.org/docs/latest/osmo-gsm-tester-manual.pdf in 3.3.2
Allow Realtime Priority

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Tom Tsou 
Gerrit-Reviewer: neels 
Gerrit-HasComments: No

osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Max]

Patch Set 1:

Does osmo-gsm-tester use sudo to run osmo-trx?

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Tom Tsou 
Gerrit-Reviewer: neels 
Gerrit-HasComments: No

osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Tom Tsou]

Patch Set 1:

> so should we go for this kind of patch, or should I push another
 > version that does it unconditionally?  My preference would be to go
 > for the latter, to have a "sane default" configureation.

My only concern with default RR priority is that it requires proper permissions 
or root access.

Otherwise we should go with what works. There is code duplication on the 
priority setting. I'm fine with the raw POSIX version, but I would like to 
remove the priority scheduling calls to UHD if we go with that approach.

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Tom Tsou 
Gerrit-HasComments: No

osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Harald Welte]

Patch Set 1:

so should we go for this kind of patch, or should I push another version that 
does it unconditionally?  My preference would be to go for the latter, to have 
a "sane default" configureation.

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Tom Tsou 
Gerrit-HasComments: No

osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Harald Welte]

Patch Set 1:

Please see https://osmocom.org/issues/2344 for my test results.  With this 
patch (and associated '-t 1') I can sustain a load of >= 100 on the system 
running osmo-trx + osmo-bts.  Without the patch, it fails within seconds.

Hence my argument to even make it default, and to use a even "stronger" (lover 
numeric, higher conceptual) priority than osmo-bts. And make osmo-bts also use 
SCHED_RR unconditionally.

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Tom Tsou 
Gerrit-HasComments: No

osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Tom Tsou]

Patch Set 1:

> I would actually vote for making SCHED_RR a default, preferrably
 > with an even higher priority (e.g. 10) than what we'd use as
 > osmo-bts-trx, as osmo-trx is even more important to schedule than
 > osmo-bts.  What do you guys think?

These settings can be very tricky. osmo-trx already sets SCHED_RR and 
priorities through the UHD priority interface. 

https://github.com/EttusResearch/uhd/blob/master/host/lib/utils/thread_priority.cpp

But, importantly, the current settings were added years ago to be independent 
of underlying UHD libusb threads - which may not be ideal. UHD maintains a 
separate internal thread for driving libusb asynchronous I/O. That thread is 
indirectly controlled by the priority level at the time of construction - which 
is main() in osmo-trx.

So this patch could perform better than the current priority settings. Does 
testing confirm that?

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Tom Tsou 
Gerrit-HasComments: No

osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Harald Welte]

Patch Set 1:

I would actually vote for making SCHED_RR a default, preferrably with an even 
higher priority (e.g. 10) than what we'd use as osmo-bts-trx, as osmo-trx is 
even more important to schedule than osmo-bts.  What do you guys think?

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: No

[PATCH] osmo-trx[master]: Add '-t' command line option to enable SCHED_RR

[Harald Welte]

Review at  https://gerrit.osmocom.org/3080

Add '-t' command line option to enable SCHED_RR

SCHED_RR allows us to operate osmo-trx reliable even under exceptionally
high system load, as the realtime scheduler priority will have higher
priority than the other "regular" tasks on the system.

Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Closes: OS#2344
---
M Transceiver52M/osmo-trx.cpp
1 file changed, 29 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/80/3080/1

diff --git a/Transceiver52M/osmo-trx.cpp b/Transceiver52M/osmo-trx.cpp
index 31a99cf..61b3098 100644
--- a/Transceiver52M/osmo-trx.cpp
+++ b/Transceiver52M/osmo-trx.cpp
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -77,6 +78,7 @@
double rssi_offset;
bool swap_channels;
bool edge;
+   int sched_rr;
 };
 
 ConfigurationTable gConfig;
@@ -259,7 +261,8 @@
"  -rRandom Normal Burst test mode with TSC\n"
"  -ARandom Access Burst test mode with delay\n"
"  -RRSSI to dBm offset in dB (default=0)\n"
-   "  -SSwap channels (UmTRX only)\n",
+   "  -SSwap channels (UmTRX only)\n"
+   "  -tSCHED_RR real-time priority (1..32)\n",
"EMERG, ALERT, CRT, ERR, WARNING, NOTICE, INFO, DEBUG");
 }
 
@@ -283,8 +286,9 @@
config->rssi_offset = 0.0;
config->swap_channels = false;
config->edge = false;
+   config->sched_rr = -1;
 
-   while ((option = getopt(argc, argv, "ha:l:i:p:c:dmxgfo:s:b:r:A:R:Se")) 
!= -1) {
+   while ((option = getopt(argc, argv, 
"ha:l:i:p:c:dmxgfo:s:b:r:A:R:Set:")) != -1) {
switch (option) {
case 'h':
print_help();
@@ -343,6 +347,9 @@
case 'e':
config->edge = true;
break;
+   case 't':
+   config->sched_rr = atoi(optarg);
+   break;
default:
print_help();
exit(0);
@@ -386,6 +393,21 @@
exit(0);
 }
 
+static int set_sched_rr(int prio)
+{
+   struct sched_param param;
+   int rc;
+   memset(, 0, sizeof(param));
+   param.sched_priority = prio;
+   printf("Setting SCHED_RR priority(%d)\n", param.sched_priority);
+   rc = sched_setscheduler(getpid(), SCHED_RR, );
+   if (rc != 0) {
+   std::cerr << "Config: Setting SCHED_RR failed" << std::endl;
+   return -1;
+   }
+   return 0;
+}
+
 int main(int argc, char *argv[])
 {
int type, chans, ref;
@@ -424,6 +446,11 @@
 
handle_options(argc, argv, );
 
+   if (config.sched_rr != -1) {
+   if (set_sched_rr(config.sched_rr) < 0)
+   return EXIT_FAILURE;
+   }
+
setup_signal_handlers();
 
/* Check database sanity */

-- 
To view, visit https://gerrit.osmocom.org/3080
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia2452b9763960b2be37fbeee9d832554da68a53f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Harald Welte