derek/gem5
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

dev, virtio: Improvements to diod process handling

Browse Source 
* When dispatching multiple gem5 simulations at once, they race
for the socket id, resulting in a panic when calling 'bind'. To
avoid this problem, the socket id is now created before the diod
process is created.  In case of a race, a panic is called in the
gem5 process, whereas before the panic was called in the diod
process where it didn't have any effect.

* In some cases killing the diod process in terminateDiod() using
only SIGTERM failed, so a call using SIGKILL is added.

Change-Id: Ie10741e10af52c8d255210cd4bfe0e5d761485d3
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Reviewed-by: Sascha Bischoff <sascha.bischoff@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/2821
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>
This commit is contained in:
Anouk Van Laer
2017-02-17 11:20:49 +00:00
parent 0658925a8f
commit 1b48d293c4
2 changed files with 80 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

105
src/dev/virtio/fs9p.cc
View File

@@ -45,10 +45,13 @@
#include <sys/socket.h>
#include <sys/types.h>
#include <sys/un.h>
#include <sys/wait.h>
#include <unistd.h>
#include <csignal>
#include <fstream>
#include "base/callback.hh"
#include "base/output.hh"
#include "debug/VIO9P.hh"
#include "debug/VIO9PData.hh"
@@ -313,6 +316,10 @@ VirtIO9PDiod::VirtIO9PDiod(Params *params)
: VirtIO9PProxy(params),
fd_to_diod(-1), fd_from_diod(-1), diod_pid(-1)
{
// Register an exit callback so we can kill the diod process
Callback* cb = new MakeCallback<VirtIO9PDiod,
&VirtIO9PDiod::terminateDiod>(this);
registerExitCallback(cb);
}
VirtIO9PDiod::~VirtIO9PDiod()
@@ -346,12 +353,38 @@ VirtIO9PDiod::startDiod()
fd_to_diod = pipe_rfd[1];
fd_from_diod = pipe_wfd[0];
// Create Unix domain socket
int socket_id = socket(AF_UNIX, SOCK_STREAM, 0);
if (socket_id == -1) {
panic("Socket creation failed %i \n", errno);
}
// Bind the socket to a path which will not be read
struct sockaddr_un socket_address;
memset(&socket_address, 0, sizeof(struct sockaddr_un));
socket_address.sun_family = AF_UNIX;
const std::string socket_path = simout.resolve(p->socketPath);
fatal_if(!OutputDirectory::isAbsolute(socket_path), "Please make the" \
" output directory an absolute path, else diod will fail!\n");
// Prevent overflow in strcpy
fatal_if(sizeof(socket_address.sun_path) <= socket_path.length(),
"Incorrect length of socket path");
strncpy(socket_address.sun_path, socket_path.c_str(),
sizeof(socket_address.sun_path));
if (bind(socket_id, (struct sockaddr*) &socket_address,
sizeof(struct sockaddr_un)) == -1){
perror("Socket binding");
panic("Socket binding to %i failed - most likely the output dir" \
" and hence unused socket already exists \n", socket_id);
}
diod_pid = fork();
if (diod_pid == -1) {
panic("Fork failed: %i\n", errno);
} else if (diod_pid == 0) {
// Create the socket which will later by used by the diod process
close(STDIN_FILENO);
if (dup2(pipe_rfd[0], DIOD_RFD) == -1 ||
dup2(pipe_wfd[1], DIOD_WFD) == -1) {
@@ -359,33 +392,7 @@ VirtIO9PDiod::startDiod()
errno);
}
// Create Unix domain socket
int socket_id = socket(AF_UNIX, SOCK_STREAM, 0);
if (socket_id == -1) {
panic("Socket creation failed %i \n", errno);
}
// Bind the socket to a path which will not be read
struct sockaddr_un socket_address;
memset(&socket_address, 0, sizeof(struct sockaddr_un));
socket_address.sun_family = AF_UNIX;
const std::string socket_path = simout.resolve(p->socketPath);
fatal_if(!OutputDirectory::isAbsolute(socket_path), "Please make the" \
" output directory an absolute path, else diod will fail!\n");
// Prevent overflow in strcpy
fatal_if(sizeof(socket_address.sun_path) <= socket_path.length(),
"Incorrect length of socket path");
strncpy(socket_address.sun_path, socket_path.c_str(),
sizeof(socket_address.sun_path));
if (bind(socket_id, (struct sockaddr*) &socket_address,
sizeof(struct sockaddr_un)) == -1){
perror("Socket binding");
panic("Socket binding to %i failed - most likely the output dir" \
" and hence unused socket already exists \n", socket_id);
}
// Start diod
execlp(diod, diod,
"-f", // start in foreground
"-r", "3", // setup read FD
@@ -400,6 +407,8 @@ VirtIO9PDiod::startDiod()
} else {
close(pipe_rfd[0]);
close(pipe_wfd[1]);
inform("Started diod with PID %u, you might need to manually kill " \
" diod if gem5 crashes \n", diod_pid);
}
#undef DIOD_RFD
@@ -428,6 +437,46 @@ VirtIO9PDiod::DiodDataEvent::process(int revent)
parent.serverDataReady();
}
void
VirtIO9PDiod::terminateDiod()
{
assert(diod_pid != -1);
DPRINTF(VIO9P, "Trying to kill diod at pid %u \n", diod_pid);
if (kill(diod_pid, SIGTERM) != 0) {
perror("Killing diod process");
warn("Failed to kill diod using SIGTERM");
return;
}
// Check if kill worked
for (unsigned i = 0; i < 5; i++) {
int wait_return = waitpid(diod_pid, NULL, WNOHANG);
if (wait_return == diod_pid) {
// Managed to kill diod
return;
} else if (wait_return == 0) {
// Diod is not killed so sleep and try again
usleep(500);
} else {
// Failed in waitpid
perror("Waitpid");
warn("Failed in waitpid");
}
}
// Try again to kill diod with sigkill
inform("Trying to kill diod with SIGKILL as SIGTERM failed \n");
if (kill(diod_pid, SIGKILL) != 0) {
perror("Killing diod process");
warn("Failed to kill diod using SIGKILL");
} else {
// Managed to kill diod
return;
}
}
VirtIO9PDiod *
VirtIO9PDiodParams::create()
{

4
src/dev/virtio/fs9p.hh
View File

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2014-2015 ARM Limited
* Copyright (c) 2014-2017 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -305,6 +305,8 @@ class VirtIO9PDiod : public VirtIO9PProxy
ssize_t read(uint8_t *data, size_t len);
ssize_t write(const uint8_t *data, size_t len);
/** Kill the diod child process at the end of the simulation */
void terminateDiod();
private:
class DiodDataEvent : public PollEvent