From 1b48d293c4e7bb07dbd73b1759f119c5d32563c7 Mon Sep 17 00:00:00 2001 From: Anouk Van Laer Date: Fri, 17 Feb 2017 11:20:49 +0000 Subject: [PATCH] dev, virtio: Improvements to diod process handling * 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 Reviewed-by: Sascha Bischoff Reviewed-on: https://gem5-review.googlesource.com/2821 Maintainer: Andreas Sandberg --- src/dev/virtio/fs9p.cc | 105 ++++++++++++++++++++++++++++++----------- src/dev/virtio/fs9p.hh | 4 +- 2 files changed, 80 insertions(+), 29 deletions(-) diff --git a/src/dev/virtio/fs9p.cc b/src/dev/virtio/fs9p.cc index 7dac79ca0..f2eb8ba52 100644 --- a/src/dev/virtio/fs9p.cc +++ b/src/dev/virtio/fs9p.cc @@ -45,10 +45,13 @@ #include #include #include +#include #include +#include #include +#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(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() { diff --git a/src/dev/virtio/fs9p.hh b/src/dev/virtio/fs9p.hh index 786f58464..46525603d 100644 --- a/src/dev/virtio/fs9p.hh +++ b/src/dev/virtio/fs9p.hh @@ -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 -- 2.30.2