Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2

[Arnd Bergmann]

On Thu, Nov 19, 2020 at 9:13 PM Willem de Bruijn
<willemdebruijn.kernel@xxxxxxxxx> wrote:
> On Thu, Nov 19, 2020 at 10:45 AM Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
> > On Thu, Nov 19, 2020 at 3:31 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> > The right shift would work indeed, but it's also a bit ugly unless
> > __estimate_accuracy() is changed to always use the same shift.
> >
> > I see that on 32-bit ARM, select_estimate_accuracy() calls
> > the external __aeabi_idiv() function to do the 32-bit division, so
> > changing it to a shift would speed up select as well.
> >
> > Changing select_estimate_accuracy() to take the relative timeout
> > as an argument to avoid the extra ktime_get_ts64() should
> > have a larger impact.
>
> It could be done by having poll_select_set_timeout take an extra u64*
> slack, call select_estimate_accuracy before adding in the current time
> and then pass the slack down to do_select and do_sys_poll, also
> through core_sys_select and compat_core_sys_select.
>
> It could be a patch independent from this new syscall. Since it changes
> poll_select_set_timeout it clearly has a conflict with the planned next
> revision of this. I can include it in the next patchset to decide whether
> it's worth it.

Yes, that sounds good, not sure how much rework this would require.

It would be easier to do if we first merged the native and compat
native versions of select/pselect/ppoll by moving the
in_compat_syscall() check into combined get_sigset()
and get_fd_set() helpers. I would assume you have enough
on your plate already and don't want to add that to it.

> > I don't see a problem with an s64 timeout if that makes the interface
> > simpler by avoiding differences between the 32-bit and 64-bit ABIs.
> >
> > More importantly, I think it should differ from poll/select by calculating
> > and writing back the remaining timeout.
> >
> > I don't know what the latest view on absolute timeouts at the syscall
> > ABI is, it would probably simplify the implementation, but make it
> > less consistent with the others. Futex uses absolute timeouts, but
> > is itself inconsistent about that.
>
> If the implementation internally uses poll_select_set_timeout and
> passes around timespec64 *, it won't matter much in terms of
> performance or implementation. Then there seems to be no downside to
> following the consistency argument.

Ok. So to clarify, you would stay with relative __kernel_timespec
pointers and not copy back the remaining time, correct?

       ARnd