[{"id":522,"web_url":"https://patchwork.libcamera.org/comment/522/","msgid":"<72f6f395-7630-d999-e6ee-082a04cd3707@ideasonboard.com>","date":"2019-01-23T11:30:25","subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: event_dispatcher_poll:\n\tHandle interrupted ppoll() calls","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 23/01/2019 08:59, Laurent Pinchart wrote:\n> The ppoll() call can be interrupted if a signal is delivered. Handle the\n> EINTR error code gracefully by restarting the call. This fixes the\n> event-dispatcher test failure.\n> \n\nTwo discussion points below - but otherwise:\n\nReviewed-by: Kieran Bingham \n\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/event_dispatcher_poll.cpp | 58 +++++++++++--------\n> src/libcamera/include/event_dispatcher_poll.h | 1 +\n> 2 files changed, 34 insertions(+), 25 deletions(-)\n> \n> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n> index eefac54ca6da..6e0609c34ddc 100644\n> --- a/src/libcamera/event_dispatcher_poll.cpp\n> +++ b/src/libcamera/event_dispatcher_poll.cpp\n> @@ -128,32 +128,11 @@ void EventDispatcherPoll::processEvents()\n> \tfor (auto notifier : notifiers_)\n> \t\tpollfds.push_back({ notifier.first, notifier.second.events(), 0 });\n> \n> -\t/* Compute the timeout. */\n> -\tTimer *nextTimer = !timers_.empty() ? timers_.front() : nullptr;\n> -\tstruct timespec timeout;\n> -\n> -\tif (nextTimer) {\n> -\t\tclock_gettime(CLOCK_MONOTONIC, &timeout);\n> -\t\tuint64_t now = timeout.tv_sec * 1000000000ULL + timeout.tv_nsec;\n> -\n> -\t\tif (nextTimer->deadline() > now) {\n> -\t\t\tuint64_t delta = nextTimer->deadline() - now;\n> -\t\t\ttimeout.tv_sec = delta / 1000000000ULL;\n> -\t\t\ttimeout.tv_nsec = delta % 1000000000ULL;\n> -\t\t} else {\n> -\t\t\ttimeout.tv_sec = 0;\n> -\t\t\ttimeout.tv_nsec = 0;\n> -\t\t}\n> -\n> -\t\tLOG(Event, Debug)\n> -\t\t\t<< \"timeout \" << timeout.tv_sec << \".\"\n> -\t\t\t<< std::setfill('0') << std::setw(9)\n> -\t\t\t<< timeout.tv_nsec;\n> -\t}\n> -\n> \t/* Wait for events and process notifiers and timers. */\n> -\tret = ppoll(pollfds.data(), pollfds.size(),\n> -\t\t nextTimer ? &timeout : nullptr, nullptr);\n> +\tdo {\n> +\t\tret = poll(&pollfds);\n\nEeep - you almost caught me here ... :\n\n\tint poll(struct pollfd *fds, nfds_t nfds, int timeout);\n\nNo problem conflicting with the std library namespace I hope?\n\n\n> +\t} while (ret == -1 && errno == EINTR);\n> +\n> \tif (ret < 0) {\n> \t\tret = -errno;\n> \t\tLOG(Event, Warning) << \"poll() failed with \" << strerror(-ret);\n> @@ -178,6 +157,35 @@ short EventDispatcherPoll::EventNotifierSetPoll::events() const\n> \treturn events;\n> }\n> \n> +int EventDispatcherPoll::poll(std::vector *pollfds)\n> +{\n> +\t/* Compute the timeout. */\n> +\tTimer *nextTimer = !timers_.empty() ? timers_.front() : nullptr;\n> +\tstruct timespec timeout;\n> +\n> +\tif (nextTimer) {\n> +\t\tclock_gettime(CLOCK_MONOTONIC, &timeout);\n> +\t\tuint64_t now = timeout.tv_sec * 1000000000ULL + timeout.tv_nsec;\n> +\n> +\t\tif (nextTimer->deadline() > now) {\n> +\t\t\tuint64_t delta = nextTimer->deadline() - now;\n> +\t\t\ttimeout.tv_sec = delta / 1000000000ULL;\n> +\t\t\ttimeout.tv_nsec = delta % 1000000000ULL;\n> +\t\t} else {\n> +\t\t\ttimeout.tv_sec = 0;\n> +\t\t\ttimeout.tv_nsec = 0;\n> +\t\t}\n> +\n> +\t\tLOG(Event, Debug)\n> +\t\t\t<< \"timeout \" << timeout.tv_sec << \".\"\n> +\t\t\t<< std::setfill('0') << std::setw(9)\n> +\t\t\t<< timeout.tv_nsec;\n> +\t}\n> +\n> +\treturn ppoll(pollfds->data(), pollfds->size(),\n> +\t\t nextTimer ? &timeout : nullptr, nullptr);\n\nI see comparisons (particularly from a table in Michael Kerrisk's kernel\ninterface book) looking at how epoll is much better than poll/select\nwhen there are more than about 10 fds to consider...\n\nI wonder how many FDs we'll end up monitoring in our event code...\nEspecially as external code will likely use our event loop.\n\nOne day it might be worth looking at the comparisons, but I don't think\ntoday is that day.\n\n\n> +}\n> +\n> void EventDispatcherPoll::processNotifiers(const std::vector &pollfds)\n> {\n> \tstatic const struct {\n> diff --git a/src/libcamera/include/event_dispatcher_poll.h b/src/libcamera/include/event_dispatcher_poll.h\n> index a41926e11a11..ac3efde082b4 100644\n> --- a/src/libcamera/include/event_dispatcher_poll.h\n> +++ b/src/libcamera/include/event_dispatcher_poll.h\n> @@ -41,6 +41,7 @@ private:\n> \tstd::map notifiers_;\n> \tstd::list timers_;\n> \n> +\tint poll(std::vector *pollfds);\n> \tvoid processNotifiers(const std::vector &pollfds);\n> \tvoid processTimers();\n> };\n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7438760C7D\n\tfor ;\n\tWed, 23 Jan 2019 12:30:28 +0100 (CET)","from [192.168.0.21]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D405523D;\n\tWed, 23 Jan 2019 12:30:27 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548243028;\n\tbh=w/bsK9uDm12qdDrqftpHUn6WvXsEkH+f0Wo/ryasoK0=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=Qq07/wNLx++O5EwIAmmYpLmg2JpiH/uwIPMOBnoa3SyBPTSX/jV6AaM0te+0x+Uuu\n\tA6V2gIyip4uA3fGtX2bfCMKTTxaGEPq/16XW7gsYIAHUfnazTMX/18jYU6iem/Ioom\n\tE2U3j19OYWUl4ARR8bv+PgMTV4CiaJzLFY2DW8Iw=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190123085923.12524-1-laurent.pinchart@ideasonboard.com>\n\t<20190123085923.12524-3-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<72f6f395-7630-d999-e6ee-082a04cd3707@ideasonboard.com>","Date":"Wed, 23 Jan 2019 11:30:25 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<20190123085923.12524-3-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: event_dispatcher_poll:\n\tHandle interrupted ppoll() calls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 23 Jan 2019 11:30:28 -0000"}},{"id":526,"web_url":"https://patchwork.libcamera.org/comment/526/","msgid":"<83804cc2-ac58-eb01-d9f1-2c68be1fca38@ideasonboard.com>","date":"2019-01-23T13:25:49","subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: event_dispatcher_poll:\n\tHandle interrupted ppoll() calls","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"In addendum\n\nOn 23/01/2019 11:30, Kieran Bingham wrote:\n> Hi Laurent,\n\n\n\n>> +\treturn ppoll(pollfds->data(), pollfds->size(),\n>> +\t\t nextTimer ? &timeout : nullptr, nullptr);\n> \n> I see comparisons (particularly from a table in Michael Kerrisk's kernel\n> interface book) looking at how epoll is much better than poll/select\n> when there are more than about 10 fds to consider...\n> \n> I wonder how many FDs we'll end up monitoring in our event code...\n> Especially as external code will likely use our event loop.\n> \n> One day it might be worth looking at the comparisons, but I don't think\n> today is that day.\n\nI think that might deserve it's own implementation, so that would be:\n\nclass EventDispatcherEpoll;\n\n- not this class anyway :)\n--\nKieran\n\n","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8FD2160C7D\n\tfor ;\n\tWed, 23 Jan 2019 14:25:53 +0100 (CET)","from [192.168.0.21]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E388323D;\n\tWed, 23 Jan 2019 14:25:52 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548249953;\n\tbh=c749/n0XmCnkoOMvEW/KxFSBGN6kjAhtTIsK14lscZ8=;\n\th=Reply-To:Subject:From:To:References:Date:In-Reply-To:From;\n\tb=EpNFEqZKGBBFx9X6LeA6Z6cOuWMK0uc/e2M7j1hAR7SvcozbBmhyuxLqcoCepUs5S\n\tt73SkGx49VzRfdlOe8YiSyXG5e6ZRwGn/TcHh+KvMpZVC9Z/uaLnhGVgKWkmcOFO1/\n\tZxyrf1qG2hCpKAQpj0ESG5l7YwCqmPyXuQ982MCo=","Reply-To":"kieran.bingham@ideasonboard.com","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190123085923.12524-1-laurent.pinchart@ideasonboard.com>\n\t<20190123085923.12524-3-laurent.pinchart@ideasonboard.com>\n\t<72f6f395-7630-d999-e6ee-082a04cd3707@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<83804cc2-ac58-eb01-d9f1-2c68be1fca38@ideasonboard.com>","Date":"Wed, 23 Jan 2019 13:25:49 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<72f6f395-7630-d999-e6ee-082a04cd3707@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: event_dispatcher_poll:\n\tHandle interrupted ppoll() calls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 23 Jan 2019 13:25:53 -0000"}},{"id":544,"web_url":"https://patchwork.libcamera.org/comment/544/","msgid":"<20190123164338.GH31885@pendragon.ideasonboard.com>","date":"2019-01-23T16:43:38","subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: event_dispatcher_poll:\n\tHandle interrupted ppoll() calls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Jan 23, 2019 at 11:30:25AM +0000, Kieran Bingham wrote:\n> On 23/01/2019 08:59, Laurent Pinchart wrote:\n> > The ppoll() call can be interrupted if a signal is delivered. Handle the\n> > EINTR error code gracefully by restarting the call. This fixes the\n> > event-dispatcher test failure.\n> \n> Two discussion points below - but otherwise:\n> \n> Reviewed-by: Kieran Bingham \n> \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/event_dispatcher_poll.cpp | 58 +++++++++++--------\n> > src/libcamera/include/event_dispatcher_poll.h | 1 +\n> > 2 files changed, 34 insertions(+), 25 deletions(-)\n> > \n> > diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n> > index eefac54ca6da..6e0609c34ddc 100644\n> > --- a/src/libcamera/event_dispatcher_poll.cpp\n> > +++ b/src/libcamera/event_dispatcher_poll.cpp\n> > @@ -128,32 +128,11 @@ void EventDispatcherPoll::processEvents()\n> > \tfor (auto notifier : notifiers_)\n> > \t\tpollfds.push_back({ notifier.first, notifier.second.events(), 0 });\n> > \n> > -\t/* Compute the timeout. */\n> > -\tTimer *nextTimer = !timers_.empty() ? timers_.front() : nullptr;\n> > -\tstruct timespec timeout;\n> > -\n> > -\tif (nextTimer) {\n> > -\t\tclock_gettime(CLOCK_MONOTONIC, &timeout);\n> > -\t\tuint64_t now = timeout.tv_sec * 1000000000ULL + timeout.tv_nsec;\n> > -\n> > -\t\tif (nextTimer->deadline() > now) {\n> > -\t\t\tuint64_t delta = nextTimer->deadline() - now;\n> > -\t\t\ttimeout.tv_sec = delta / 1000000000ULL;\n> > -\t\t\ttimeout.tv_nsec = delta % 1000000000ULL;\n> > -\t\t} else {\n> > -\t\t\ttimeout.tv_sec = 0;\n> > -\t\t\ttimeout.tv_nsec = 0;\n> > -\t\t}\n> > -\n> > -\t\tLOG(Event, Debug)\n> > -\t\t\t<< \"timeout \" << timeout.tv_sec << \".\"\n> > -\t\t\t<< std::setfill('0') << std::setw(9)\n> > -\t\t\t<< timeout.tv_nsec;\n> > -\t}\n> > -\n> > \t/* Wait for events and process notifiers and timers. */\n> > -\tret = ppoll(pollfds.data(), pollfds.size(),\n> > -\t\t nextTimer ? &timeout : nullptr, nullptr);\n> > +\tdo {\n> > +\t\tret = poll(&pollfds);\n> \n> Eeep - you almost caught me here ... :\n> \n> \tint poll(struct pollfd *fds, nfds_t nfds, int timeout);\n> \n> No problem conflicting with the std library namespace I hope?\n\nNo, the compiler will pick the right one, and if me ever need to call\nthe libc function, we can write ::poll().\n\n> > +\t} while (ret == -1 && errno == EINTR);\n> > +\n> > \tif (ret < 0) {\n> > \t\tret = -errno;\n> > \t\tLOG(Event, Warning) << \"poll() failed with \" << strerror(-ret);\n> > @@ -178,6 +157,35 @@ short EventDispatcherPoll::EventNotifierSetPoll::events() const\n> > \treturn events;\n> > }\n> > \n> > +int EventDispatcherPoll::poll(std::vector *pollfds)\n> > +{\n> > +\t/* Compute the timeout. */\n> > +\tTimer *nextTimer = !timers_.empty() ? timers_.front() : nullptr;\n> > +\tstruct timespec timeout;\n> > +\n> > +\tif (nextTimer) {\n> > +\t\tclock_gettime(CLOCK_MONOTONIC, &timeout);\n> > +\t\tuint64_t now = timeout.tv_sec * 1000000000ULL + timeout.tv_nsec;\n> > +\n> > +\t\tif (nextTimer->deadline() > now) {\n> > +\t\t\tuint64_t delta = nextTimer->deadline() - now;\n> > +\t\t\ttimeout.tv_sec = delta / 1000000000ULL;\n> > +\t\t\ttimeout.tv_nsec = delta % 1000000000ULL;\n> > +\t\t} else {\n> > +\t\t\ttimeout.tv_sec = 0;\n> > +\t\t\ttimeout.tv_nsec = 0;\n> > +\t\t}\n> > +\n> > +\t\tLOG(Event, Debug)\n> > +\t\t\t<< \"timeout \" << timeout.tv_sec << \".\"\n> > +\t\t\t<< std::setfill('0') << std::setw(9)\n> > +\t\t\t<< timeout.tv_nsec;\n> > +\t}\n> > +\n> > +\treturn ppoll(pollfds->data(), pollfds->size(),\n> > +\t\t nextTimer ? &timeout : nullptr, nullptr);\n> \n> I see comparisons (particularly from a table in Michael Kerrisk's kernel\n> interface book) looking at how epoll is much better than poll/select\n> when there are more than about 10 fds to consider...\n> \n> I wonder how many FDs we'll end up monitoring in our event code...\n> Especially as external code will likely use our event loop.\n> \n> One day it might be worth looking at the comparisons, but I don't think\n> today is that day.\n\nThat's a good point. Out of scope for this patch as you mentioned, but a\ngood point. If someone wants to have a good, be my guest :-)\n\n> > +}\n> > +\n> > void EventDispatcherPoll::processNotifiers(const std::vector &pollfds)\n> > {\n> > \tstatic const struct {\n> > diff --git a/src/libcamera/include/event_dispatcher_poll.h b/src/libcamera/include/event_dispatcher_poll.h\n> > index a41926e11a11..ac3efde082b4 100644\n> > --- a/src/libcamera/include/event_dispatcher_poll.h\n> > +++ b/src/libcamera/include/event_dispatcher_poll.h\n> > @@ -41,6 +41,7 @@ private:\n> > \tstd::map notifiers_;\n> > \tstd::list timers_;\n> > \n> > +\tint poll(std::vector *pollfds);\n> > \tvoid processNotifiers(const std::vector &pollfds);\n> > \tvoid processTimers();\n> > };","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A51560B2D\n\tfor ;\n\tWed, 23 Jan 2019 17:43:40 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6399E23E;\n\tWed, 23 Jan 2019 17:43:39 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548261819;\n\tbh=PIbC/R5b9D9wIYlphr93mCmbxEHi2UiDO/5c/A6Yk3M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b1mykeEwrLBYRTk7Zk80etEk8RTuwbeG2p1pIdk/kqXKp3naEEFLCHa6KuGC8Db9V\n\t/7o2xwT++F+BC/g6DtHgG6wT74s2RsPeztAco/MKfQ3Qh6phVghS2EuNqB4oTm1W0V\n\ts0neAs0tPx7RsJJrNMIEiZTBQsSqO77i7jqaEhCM=","Date":"Wed, 23 Jan 2019 18:43:38 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190123164338.GH31885@pendragon.ideasonboard.com>","References":"<20190123085923.12524-1-laurent.pinchart@ideasonboard.com>\n\t<20190123085923.12524-3-laurent.pinchart@ideasonboard.com>\n\t<72f6f395-7630-d999-e6ee-082a04cd3707@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<72f6f395-7630-d999-e6ee-082a04cd3707@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: event_dispatcher_poll:\n\tHandle interrupted ppoll() calls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 23 Jan 2019 16:43:40 -0000"}}]