[suggestion] GDB: switch to epoll
locutus3009 opened this issue · 3 comments
Hi!
Internally, I have some patch for simavr that switches GDB mode from constant select-ing to epoll(7): upol GDB init, I create a epoll object using epoll_create1, add listen socket to the epoll, and spawn a new thread that usually sleeps in epoll_wait if there are no new commands from GDB client.
This slightly increases performance however I am not sure that my implementation falls in line with your thoughts on a future of this project. Moreover, I am not sure that epoll would work at all for OS X and MinGW targets.
The example implementation is below (this is not a production-ready code):
sim_gdb.c:
/* Extend avr_gdb_t struct */
typedef struct avr_gdb_t {
avr_t * avr;
int epollfd; // listen socket
int listen; // listen socket
int s; // current gdb connection
pthread_t thread;
pthread_mutex_t mutex;
avr_gdb_watchpoints_t breakpoints;
avr_gdb_watchpoints_t watchpoints;
} avr_gdb_t;
...
/* Change GDB sleep handler */
static int
gdb_network_handler(
avr_gdb_t * g,
uint32_t dosleep )
{
/* Thanks to futex(7), this will not call Kernel if there are no GDB commands */
pthread_mutex_lock(&g->mutex);
pthread_mutex_unlock(&g->mutex);
/* I don't personally need any timeout since I want to make simulator as fast as possible */
return 0;
}
...
/* Move and re-organize GDB implementation */
static void *
gdb_thread(
void * param)
{
avr_t * avr = param;
struct epoll_event event;
printf("GDB: listener thread started\n");
int ret;
while (1) {
ret = epoll_wait(avr->gdb->epollfd, &event, 1, -1);
if (ret < 0) {
printf("GDB: epoll_wait() failed, errno=%d\n", errno);
continue;
}
if (ret == 0) {
printf("GDB: epoll_wait() return 0 ready fd\n");
abort();
}
AVR_LOG(avr, LOG_DEBUG, "GDB: Fetch epoll, event_mask:0x%x\n", event.events);
ret = 0;
if (event.events & EPOLLIN) {
if (event.data.fd == avr->gdb->listen) {
printf("Accept GDB client\n");
avr->gdb->s = accept(avr->gdb->listen, NULL, NULL);
if (avr->gdb->s == -1) {
perror("gdb_network_handler accept");
sleep(5);
abort();
__builtin_unreachable();
}
int i = 1;
setsockopt (avr->gdb->s, IPPROTO_TCP, TCP_NODELAY, &i, sizeof (i));
avr->gdb->avr->state = cpu_Stopped;
printf("%s connection opened\n", __func__);
ret = 0;
struct epoll_event temp_event;
temp_event.events = EPOLLIN | EPOLLHUP;
temp_event.data.fd = avr->gdb->s;
if (epoll_ctl(avr->gdb->epollfd, EPOLL_CTL_ADD, avr->gdb->s, &temp_event)) {
AVR_LOG(avr, LOG_ERROR, "GDB: epoll_ctl failed: %s", strerror(errno));
abort();
}
} else if (event.data.fd == avr->gdb->s) {
uint8_t buffer[1024];
pthread_mutex_lock(&avr->gdb->mutex);
ssize_t r = recv(avr->gdb->s, buffer, sizeof(buffer)-1, 0);
if (r == 0) {
printf("%s connection closed\n", __func__);
close(avr->gdb->s);
gdb_watch_clear(&avr->gdb->breakpoints);
gdb_watch_clear(&avr->gdb->watchpoints);
avr->state = cpu_Running; // resume
avr->gdb->s = -1;
break;
}
if (r == -1) {
perror("gdb_network_handler recv");
sleep(1);
abort();
}
buffer[r] = 0;
uint8_t * src = buffer;
while (*src == '+' || *src == '-')
src++;
// control C -- lets send the guy a nice status packet
if (*src == 3) {
src++;
avr->state = cpu_StepDone;
printf("GDB hit control-c\n");
}
if (*src == '$') {
// strip checksum
uint8_t * end = buffer + r - 1;
while (end > src && *end != '#')
*end-- = 0;
*end = 0;
src++;
DBG(printf("GDB command = '%s'\n", src);)
send(avr->gdb->s, "+", 1, 0);
gdb_handle_command(avr->gdb, (char*)src);
}
pthread_mutex_unlock(&avr->gdb->mutex);
} else {
printf("Input from unknown client\n");
abort();
}
}
if (event.events & EPOLLERR) {
printf("GDB: EPOLLERR\n");
}
if (event.events & EPOLLHUP) {
printf("GDB: EPOLLHUP\n");
/* Provide cleanups */
break;
}
}
return NULL;
}
...
/* Update GDB initialization */
int
avr_gdb_init(
avr_t * avr )
{
if (avr->gdb)
return 0; // GDB server already is active
avr_gdb_t * g = malloc(sizeof(avr_gdb_t));
memset(g, 0, sizeof(avr_gdb_t));
avr->gdb = NULL;
if ( network_init() ) {
AVR_LOG(avr, LOG_ERROR, "GDB: Can't initialize network");
goto error;
}
if ((g->listen = socket(PF_INET, SOCK_STREAM, 0)) < 0) {
AVR_LOG(avr, LOG_ERROR, "GDB: Can't create socket: %s", strerror(errno));
goto error;
}
if ((g->epollfd = epoll_create1(0)) < 0) {
AVR_LOG(avr, LOG_ERROR, "GDB: Can't create epoll: %s", strerror(errno));
goto error;
}
int optval = 1;
setsockopt(g->listen, SOL_SOCKET, SO_REUSEADDR, &optval, sizeof(optval));
struct sockaddr_in address = { 0 };
address.sin_family = AF_INET;
address.sin_port = htons (avr->gdb_port);
if (bind(g->listen, (struct sockaddr *) &address, sizeof(address))) {
AVR_LOG(avr, LOG_ERROR, "GDB: Can not bind socket: %s", strerror(errno));
goto error;
}
struct epoll_event event;
event.events = EPOLLIN | EPOLLHUP;
event.data.fd = g->listen;
if (epoll_ctl(g->epollfd, EPOLL_CTL_ADD, g->listen, &event)) {
AVR_LOG(avr, LOG_ERROR, "GDB: epoll_ctl failed: %s", strerror(errno));
goto error;
}
if (listen(g->listen, 1)) {
perror("listen");
goto error;
}
printf("avr_gdb_init listening on port %d\n", avr->gdb_port);
g->avr = avr;
g->s = -1;
avr->gdb = g;
// change default run behaviour to use the slightly slower versions
avr->run = avr_callback_run_gdb;
avr->sleep = avr_callback_sleep_gdb;
pthread_mutex_init(&g->mutex, NULL);
pthread_create(&g->thread, NULL, gdb_thread, avr);
return 0;
error:
if (g->listen >= 0)
close(g->listen);
if (g->epollfd >= 0)
close(g->epollfd);
free(g);
return -1;
}You wrote:
I am not sure that epoll would work at all for OS X and MinGW targets.
According to man epoll:
CONFORMING TO
The epoll API is Linux-specific. Some other systems provide similar mechanisms, for example, FreeBSD has kqueue, and Solaris has /dev/poll.
See also Stack Overflow: Does OS X not support epoll function?.
Thanks, my fault. I tend to prefer systems with epoll interface (linux and some other kernels), and I completely ruled out the other possibilities.
Yes epoll is definitely superior for that sort of things than select() however fro the use case we have (we manually debug something, on occasion) the fact it is "more efficient" is a bit pointless given gdb is a pile of bloat anyway ;-)