From patchwork Thu Aug 29 23:17:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Samuel Thibault X-Patchwork-Id: 1155555 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-104911-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ens-lyon.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="hV5p7bBI"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46KJRp3t0pz9sDB for ; Fri, 30 Aug 2019 09:17:42 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id :mime-version:content-transfer-encoding; q=dns; s=default; b=Pxs fiv9lq51DmombjOQGcWJl69M1ixQWpTq3gqI6+HuuQTPBXoAZmRIuBxqRnqTjXFK fj+CLuc2XgcEdq1EGgZEHv72Su1Xi3DHjY7laQ9NygqiBhuFvpGqgU+xi2tvCyF+ YfV/Q+4v9Mo0pG7HoYFkDWEsmwx46/BRit0sL3dc= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id :mime-version:content-transfer-encoding; s=default; bh=iXKQgt5Np Qyag+sDlo8LGkNnYDg=; b=hV5p7bBIfANQt7IaTpRBWmiNUeQQOqZ9jZVclr/NM RrhHZ0zrb23GRfFrz7YuNVk6XcEnyXed7LleADf4XyOuLx+YlrSqqLh6McvjZ78i wwKJEtyOsmaDXkmErHDTca+wYSJZtj1dx1Lu3Hdc4e1Ku3uhcutlG3rYyIc43oVw ek= Received: (qmail 9448 invoked by alias); 29 Aug 2019 23:17:36 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 9440 invoked by uid 89); 29 Aug 2019 23:17:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_NEUTRAL autolearn=ham version=3.3.1 spammy= X-HELO: hera.aquilenet.fr From: Samuel Thibault To: libc-alpha@sourceware.org Cc: Richard Braun , commit-hurd@gnu.org Subject: [hurd,commited] hurd: Fix timeout handling in _hurd_select Date: Fri, 30 Aug 2019 01:17:27 +0200 Message-Id: <20190829231727.13328-1-samuel.thibault@ens-lyon.org> MIME-Version: 1.0 From: Richard Braun Rely on servers to implement timeouts, so that very short values (including 0) don't make mach_msg return before valid replies can be received. The purpose of this scheme is to guarantee a full client-server round-trip, whatever the timeout value. This change depends on the new io_select_timeout RPC being implemented by servers. * hurd/Makefile (user-interfaces): Add io_reply and io_request. * hurd/hurdselect.c: Include , and . (_hurd_select): Replace the call to __io_select with either __io_select_request or __io_select_timeout_request, depending on the timeout. Count the number of ready descriptors (replies for which at least one type bit is set). Implement the timeout locally when there is no file descriptor. --- ChangeLog | 9 +++- hurd/Makefile | 3 +- hurd/hurdselect.c | 116 ++++++++++++++++++++++++++++++---------------- 3 files changed, 87 insertions(+), 41 deletions(-) diff --git a/ChangeLog b/ChangeLog index d2ec5d5ac0..37cbe28169 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,5 @@ 2019-08-30 Samuel Thibault - * sysdeps/mach/hurd/getcwd.c (_hurd_canonicalize_directory_name_internal): Do not remove the heading slash if we got an unknown root directory. (__getcwd): Do not fail with @@ -11,6 +10,14 @@ * hurd/hurdselect.c (_hurd_select): Always call __io_select with no timeout. * sysdeps/mach/hurd/setitimer.c (setitimer_locked): Fix preemptor setup. + * hurd/Makefile (user-interfaces): Add io_reply and io_request. + * hurd/hurdselect.c: Include , and + . + (_hurd_select): Replace the call to __io_select with either + __io_select_request or __io_select_timeout_request, depending on the + timeout. Count the number of ready descriptors (replies for which at + least one type bit is set). Implement the timeout locally when there is + no file descriptor. 2019-08-29 Mihailo Stojanovic diff --git a/hurd/Makefile b/hurd/Makefile index 99d33f9562..6d9cbf57a5 100644 --- a/hurd/Makefile +++ b/hurd/Makefile @@ -33,7 +33,8 @@ user-interfaces := $(addprefix hurd/,\ process process_request \ msg msg_reply msg_request \ exec exec_startup crash interrupt \ - fs fsys io term tioctl socket ifsock \ + fs fsys io io_reply io_request \ + term tioctl socket ifsock \ login password pfinet pci \ ) server-interfaces := hurd/msg faultexc diff --git a/hurd/hurdselect.c b/hurd/hurdselect.c index a5e6e26b9a..416e4a37bd 100644 --- a/hurd/hurdselect.c +++ b/hurd/hurdselect.c @@ -16,14 +16,17 @@ License along with the GNU C Library; if not, see . */ +#include #include #include #include #include +#include #include #include #include #include +#include /* All user select types. */ #define SELECT_ALL (SELECT_READ | SELECT_WRITE | SELECT_URG) @@ -44,11 +47,13 @@ _hurd_select (int nfds, { int i; mach_port_t portset; - int got; + int got, ready; error_t err; fd_set rfds, wfds, xfds; int firstfd, lastfd; - mach_msg_timeout_t to = 0; + mach_msg_id_t reply_msgid; + mach_msg_timeout_t to; + struct timespec ts; struct { struct hurd_userlink ulink; @@ -73,16 +78,39 @@ _hurd_select (int nfds, return -1; } - if (timeout != NULL) +#define IO_SELECT_REPLY_MSGID (21012 + 100) /* XXX */ +#define IO_SELECT_TIMEOUT_REPLY_MSGID (21031 + 100) /* XXX */ + + if (timeout == NULL) + reply_msgid = IO_SELECT_REPLY_MSGID; + else { - if (timeout->tv_sec < 0 || timeout->tv_nsec < 0) + struct timeval now; + + if (timeout->tv_sec < 0 || timeout->tv_nsec < 0 || + timeout->tv_nsec >= 1000000000) { errno = EINVAL; return -1; } - to = (timeout->tv_sec * 1000 - + (timeout->tv_nsec + 999999) / 1000000); + err = __gettimeofday(&now, NULL); + if (err) + return -1; + + ts.tv_sec = now.tv_sec + timeout->tv_sec; + ts.tv_nsec = now.tv_usec * 1000 + timeout->tv_nsec; + + if (ts.tv_nsec >= 1000000000) + { + ts.tv_sec++; + ts.tv_nsec -= 1000000000; + } + + if (ts.tv_sec < 0) + ts.tv_sec = LONG_MAX; /* XXX */ + + reply_msgid = IO_SELECT_TIMEOUT_REPLY_MSGID; } if (sigmask && __sigprocmask (SIG_SETMASK, sigmask, &oset)) @@ -236,12 +264,13 @@ _hurd_select (int nfds, { int type = d[i].type; d[i].reply_port = __mach_reply_port (); - err = __io_select (d[i].io_port, d[i].reply_port, 0, &type); - switch (err) + if (timeout == NULL) + err = __io_select_request (d[i].io_port, d[i].reply_port, type); + else + err = __io_select_timeout_request (d[i].io_port, d[i].reply_port, + ts, type); + if (!err) { - case MACH_RCV_TIMED_OUT: - /* No immediate response. This is normal. */ - err = 0; if (firstfd == lastfd) /* When there's a single descriptor, we don't need a portset, so just pretend we have one, but really @@ -262,32 +291,24 @@ _hurd_select (int nfds, __mach_port_move_member (__mach_task_self (), d[i].reply_port, portset); } - break; - - default: - /* No other error should happen. Callers of select + } + else + { + /* No error should happen. Callers of select don't expect to see errors, so we simulate readiness of the erring object and the next call hopefully will get the error again. */ - type = SELECT_ALL; - /* FALLTHROUGH */ - - case 0: - /* We got an answer. */ - if ((type & SELECT_ALL) == 0) - /* Bogus answer; treat like an error, as a fake positive. */ - type = SELECT_ALL; - - /* This port is already ready already. */ - d[i].type &= type; d[i].type |= SELECT_RETURNED; ++got; - break; } _hurd_port_free (&d[i].cell->port, &d[i].ulink, d[i].io_port); } } + /* GOT is the number of replies (or errors), while READY is the number of + replies with at least one type bit set. */ + ready = 0; + /* Now wait for reply messages. */ if (!err && got == 0) { @@ -329,22 +350,35 @@ _hurd_select (int nfds, } success; #endif } msg; - mach_msg_option_t options = (timeout == NULL ? 0 : MACH_RCV_TIMEOUT); + mach_msg_option_t options; error_t msgerr; + + /* We rely on servers to implement the timeout, but when there are none, + do it on the client side. */ + if (timeout != NULL && firstfd == -1) + { + options = MACH_RCV_TIMEOUT; + to = timeout->tv_sec * 1000 + (timeout->tv_nsec + 999999) / 1000000; + } + else + { + options = 0; + to = MACH_MSG_TIMEOUT_NONE; + } + while ((msgerr = __mach_msg (&msg.head, MACH_RCV_MSG | MACH_RCV_INTERRUPT | options, 0, sizeof msg, portset, to, MACH_PORT_NULL)) == MACH_MSG_SUCCESS) { /* We got a message. Decode it. */ -#define IO_SELECT_REPLY_MSGID (21012 + 100) /* XXX */ #ifdef MACH_MSG_TYPE_BIT const union typeword inttype = { type: { MACH_MSG_TYPE_INTEGER_T, sizeof (integer_t) * 8, 1, 1, 0, 0 } }; #endif - if (msg.head.msgh_id == IO_SELECT_REPLY_MSGID + if (msg.head.msgh_id == reply_msgid && msg.head.msgh_size >= sizeof msg.error && !(msg.head.msgh_bits & MACH_MSGH_BITS_COMPLEX) #ifdef MACH_MSG_TYPE_BIT @@ -362,12 +396,13 @@ _hurd_select (int nfds, err = EINTR; goto poll; } + /* Keep in mind msg.success.result can be 0 if a timeout + occurred. */ if (msg.error.err - || msg.head.msgh_size != sizeof msg.success #ifdef MACH_MSG_TYPE_BIT || msg.success.result_type.word != inttype.word #endif - || (msg.success.result & SELECT_ALL) == 0) + || msg.head.msgh_size != sizeof msg.success) { /* Error or bogus reply. Simulate readiness. */ __mach_msg_destroy (&msg.head); @@ -384,6 +419,9 @@ _hurd_select (int nfds, && d[i].reply_port == msg.head.msgh_local_port) { d[i].type &= msg.success.result; + if (d[i].type) + ++ready; + d[i].type |= SELECT_RETURNED; ++got; } @@ -408,7 +446,7 @@ _hurd_select (int nfds, /* Interruption on our side (e.g. signal reception). */ err = EINTR; - if (got) + if (ready) /* At least one descriptor is known to be ready now, so we will return success. */ err = 0; @@ -452,9 +490,9 @@ _hurd_select (int nfds, } else { - /* Below we recalculate GOT to include an increment for each operation + /* Below we recalculate READY to include an increment for each operation allowed on each fd. */ - got = 0; + ready = 0; /* Set the user bitarrays. We only ever have to clear bits, as all desired ones are initially set. */ @@ -467,15 +505,15 @@ _hurd_select (int nfds, type = 0; if (type & SELECT_READ) - got++; + ready++; else if (readfds) FD_CLR (i, readfds); if (type & SELECT_WRITE) - got++; + ready++; else if (writefds) FD_CLR (i, writefds); if (type & SELECT_URG) - got++; + ready++; else if (exceptfds) FD_CLR (i, exceptfds); } @@ -484,5 +522,5 @@ _hurd_select (int nfds, if (sigmask && __sigprocmask (SIG_SETMASK, &oset, NULL)) return -1; - return got; + return ready; }