Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1733232 > unrolled thread

Re: [PATCH v2 0/2] enable hires timer to timeout datagram socket

Started byThomas Gleixner <tglx@linutronix.de>
First post2017-09-16 11:50 +0200
Last post2017-09-25 22:50 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2 0/2] enable hires timer to timeout datagram socket Thomas Gleixner <tglx@linutronix.de> - 2017-09-16 11:50 +0200
    Re: [PATCH v2 0/2] enable hires timer to timeout datagram socket Vallish Vaidyeshwara <vallish@amazon.com> - 2017-09-21 00:50 +0200
      Re: [PATCH v2 0/2] enable hires timer to timeout datagram socket Thomas Gleixner <tglx@linutronix.de> - 2017-09-25 22:50 +0200

#1733232 — Re: [PATCH v2 0/2] enable hires timer to timeout datagram socket

FromThomas Gleixner <tglx@linutronix.de>
Date2017-09-16 11:50 +0200
SubjectRe: [PATCH v2 0/2] enable hires timer to timeout datagram socket
Message-ID<uqhTz-886-1@gated-at.bofh.it>
On Fri, 8 Sep 2017, Eric Dumazet wrote:
> On Fri, 2017-09-08 at 11:55 -0700, Eduardo Valentin wrote:
> > Hello,
> > 
> > On Fri, Sep 08, 2017 at 10:26:45AM -0700, David Miller wrote:
> > > From: David Woodhouse <dwmw2@infradead.org>
> > > Date: Fri, 08 Sep 2017 18:23:22 +0100
> > > 
> > > > I don't know that anyone's ever tried saying "show me the chapter
> > and
> > > > verse of the documentation"
> > > 
> > > Do you know why I brought this up?  Because the person I am replying
> > > to told me that the syscall documentation should have suggested this
> > > or that.
> > > 
> > > That's why.
> > 
> > :-) My intention was for sure not to upset anybody.
> > 
> > Just to reiterate, the point of patch is simple, there was a change in
> > behavior in the system call from one kernel version to the other. As I
> > mentioned, I agree that the userspace could use other means to achieve
> > the same, but still the system call behavior has changed.
> > 
> > > 
> > > So let's concentrate on the other aspects of my reply, ok?
> > 
> > I agree. I would prefer to understand here what is the technical
> > reason not to accept these patches other than "use other system
> > calls".
> 
> So if we need to replace all 'legacy' timers to high resolution timer,
> because some application was _relying_ on jiffies being kind of precise,
> maybe it is better to revert the change done on legacy timers.

Which would be a major step back in terms of timer performance and system
disturbance caused by massive recascading operations.

> Or continue the migration and make them use high res internally.
> 
> select() and poll() are the standard way to have precise timeouts,
> it is silly we have to maintain a timeout handling in the datagram fast
> path.

A few years ago we switched select/poll over to use hrtimers because the
wheel timers were too inaccurate for some operations, so it feels
consequent to switch the timeout in the datagram rcv path over as well. I
agree that the whole timeout magic there feels silly, but unfortunately
it's a documented property of sockets.

Thanks,

	tglx

[toc] | [next] | [standalone]


#1736218

FromVallish Vaidyeshwara <vallish@amazon.com>
Date2017-09-21 00:50 +0200
Message-ID<urVYD-MK-31@gated-at.bofh.it>
In reply to#1733232
On Sat, Sep 16, 2017 at 11:47:56AM +0200, Thomas Gleixner wrote:
> On Fri, 8 Sep 2017, Eric Dumazet wrote:
> > On Fri, 2017-09-08 at 11:55 -0700, Eduardo Valentin wrote:
> > > Hello,
> > > 
> > > On Fri, Sep 08, 2017 at 10:26:45AM -0700, David Miller wrote:
> > > > From: David Woodhouse <dwmw2@infradead.org>
> > > > Date: Fri, 08 Sep 2017 18:23:22 +0100
> > > > 
> > > > > I don't know that anyone's ever tried saying "show me the chapter
> > > and
> > > > > verse of the documentation"
> > > > 
> > > > Do you know why I brought this up?  Because the person I am replying
> > > > to told me that the syscall documentation should have suggested this
> > > > or that.
> > > > 
> > > > That's why.
> > > 
> > > :-) My intention was for sure not to upset anybody.
> > > 
> > > Just to reiterate, the point of patch is simple, there was a change in
> > > behavior in the system call from one kernel version to the other. As I
> > > mentioned, I agree that the userspace could use other means to achieve
> > > the same, but still the system call behavior has changed.
> > > 
> > > > 
> > > > So let's concentrate on the other aspects of my reply, ok?
> > > 
> > > I agree. I would prefer to understand here what is the technical
> > > reason not to accept these patches other than "use other system
> > > calls".
> > 
> > So if we need to replace all 'legacy' timers to high resolution timer,
> > because some application was _relying_ on jiffies being kind of precise,
> > maybe it is better to revert the change done on legacy timers.
> 
> Which would be a major step back in terms of timer performance and system
> disturbance caused by massive recascading operations.
> 
> > Or continue the migration and make them use high res internally.
> > 
> > select() and poll() are the standard way to have precise timeouts,
> > it is silly we have to maintain a timeout handling in the datagram fast
> > path.
> 
> A few years ago we switched select/poll over to use hrtimers because the
> wheel timers were too inaccurate for some operations, so it feels
> consequent to switch the timeout in the datagram rcv path over as well. I
> agree that the whole timeout magic there feels silly, but unfortunately
> it's a documented property of sockets.
> 
> Thanks,
> 
> 	tglx
> 

Hello Thomas,

Thanks for your comments. This patch has been NACK'ed by David Miller. Is
there any other approach to solve this problem with out application code
being recompiled?

Thanks.
-Vallish

[toc] | [prev] | [next] | [standalone]


#1739278

FromThomas Gleixner <tglx@linutronix.de>
Date2017-09-25 22:50 +0200
Message-ID<utIud-33u-11@gated-at.bofh.it>
In reply to#1736218
On Wed, 20 Sep 2017, Vallish Vaidyeshwara wrote:
> On Sat, Sep 16, 2017 at 11:47:56AM +0200, Thomas Gleixner wrote:
> > > So if we need to replace all 'legacy' timers to high resolution timer,
> > > because some application was _relying_ on jiffies being kind of precise,
> > > maybe it is better to revert the change done on legacy timers.
> > 
> > Which would be a major step back in terms of timer performance and system
> > disturbance caused by massive recascading operations.
> > 
> > > Or continue the migration and make them use high res internally.
> > > 
> > > select() and poll() are the standard way to have precise timeouts,
> > > it is silly we have to maintain a timeout handling in the datagram fast
> > > path.
> > 
> > A few years ago we switched select/poll over to use hrtimers because the
> > wheel timers were too inaccurate for some operations, so it feels
> > consequent to switch the timeout in the datagram rcv path over as well. I
> > agree that the whole timeout magic there feels silly, but unfortunately
> > it's a documented property of sockets.
> > 
> 
> Thanks for your comments. This patch has been NACK'ed by David Miller. Is
> there any other approach to solve this problem with out application code
> being recompiled?

We have only three options here:

   1) Do a massive revert of the timer wheel changes and lose all the
      benefits of that rework.

   2) Make that timer list -> hrtimer change in the datagram code

   3) Ignore it

#1 Would be pretty ironic as networking would take the biggest penalty of
   the revert.

#2 Is IMO the proper solution as it cures a user space visible regression,
   though the patch itself could be made way simpler

#3 Shrug

Dave, Eric?

Thanks,

	tglx

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web