From: Andrew Morton <akpm@linux-foundation.org>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
viro@zeniv.linux.org.uk, soheil.kdev@gmail.com, arnd@arndb.de,
shuochen@google.com, Willem de Bruijn <willemb@google.com>
Subject: Re: [PATCH v2] epoll: add nsec timeout support
Date: Mon, 16 Nov 2020 12:04:45 -0800 [thread overview]
Message-ID: <20201116120445.7359b0053778c1a4492d1057@linux-foundation.org> (raw)
In-Reply-To: <20201116161001.1606608-1-willemdebruijn.kernel@gmail.com>
On Mon, 16 Nov 2020 11:10:01 -0500 Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Add epoll_create1 flag EPOLL_NSTIMEO. When passed, this changes the
> interpretation of argument timeout in epoll_wait from msec to nsec.
>
> Use cases such as datacenter networking operate on timescales well
> below milliseconds. Shorter timeouts bounds their tail latency.
> The underlying hrtimer is already programmed with nsec resolution.
hm, maybe. It's not very nice to be using one syscall to alter the
interpretation of another syscall's argument in this fashion. For
example, one wonders how strace(1) is to properly interpret & display
this argument?
Did you consider adding epoll_wait2()/epoll_pwait2() syscalls which
take a nsec timeout? Seems simpler.
Either way, we'd be interested in seeing the proposed manpage updates
alongside this change.
> --- a/fs/eventpoll.c
> +++ b/fs/eventpoll.c
> @@ -225,6 +225,9 @@ struct eventpoll {
> unsigned int napi_id;
> #endif
>
> + /* Accept timeout in ns resolution (EPOLL_NSTIMEO) */
> + unsigned int nstimeout:1;
> +
Why a bitfield? This invites other developers to add new bitfields to
the same word. And if that happens, we'll need to consider the locking
rules for that word - I don't think the compiler provides any
protection against concurrent modifications to the bitfields which
share a machine word. If we add a rule
/*
* per eventpoll flags. Initialized at creation time, never changes
* thereafter
*/
then that would cover it. Or just make the thing a `bool'?
next prev parent reply other threads:[~2020-11-16 20:05 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-16 16:10 [PATCH v2] epoll: add nsec timeout support Willem de Bruijn
2020-11-16 16:12 ` Soheil Hassas Yeganeh
2020-11-16 16:19 ` Matthew Wilcox
2020-11-16 17:00 ` Willem de Bruijn
2020-11-16 17:11 ` David Laight
2020-11-16 19:54 ` Willem de Bruijn
2020-11-16 20:00 ` Matthew Wilcox
2020-11-16 20:04 ` Andrew Morton [this message]
2020-11-16 23:36 ` Willem de Bruijn
2020-11-16 23:51 ` Willem de Bruijn
2020-11-17 0:37 ` Andrew Morton
2020-11-17 2:21 ` Willem de Bruijn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201116120445.7359b0053778c1a4492d1057@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=shuochen@google.com \
--cc=soheil.kdev@gmail.com \
--cc=viro@zeniv.linux.org.uk \
--cc=willemb@google.com \
--cc=willemdebruijn.kernel@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).