Date: Fri, 23 Aug 2013 10:27:58 +0400 From: Vitja Makarov <vitja.makarov@gmail.com> To: John Baldwin <jhb@freebsd.org>, freebsd-current@freebsd.org Subject: Re: Question about socket timeouts Message-ID: <CAKGHGPSYxmVoet8TnbxVFeEVk9CeD5iF6DK2do3w_ScnPU_SpQ@mail.gmail.com> In-Reply-To: <201308221408.08203.jhb@freebsd.org> References: <CAKGHGPS=HCYfXxPXuUz5G83j5sGieejPU-QHmi9TrmMhmweHLw@mail.gmail.com> <201308211401.46468.jhb@freebsd.org> <CAKGHGPQZC0vx7W9XW-nA8qSRzM_naz1M2_0QsScLRybz9qVZ2Q@mail.gmail.com> <201308221408.08203.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--089e013a05acf25bb004e4978110 Content-Type: text/plain; charset=ISO-8859-1 2013/8/22 John Baldwin <jhb@freebsd.org>: > On Thursday, August 22, 2013 12:18:48 am Vitja Makarov wrote: >> 2013/8/21 John Baldwin <jhb@freebsd.org>: >> > On Monday, August 19, 2013 11:13:02 pm Daniel Eischen wrote: >> >> On Mon, 19 Aug 2013, Adrian Chadd wrote: >> >> >> >> > Yes! Please file a PR! >> >> >> >> This sorta implies that both are acceptable (although, >> >> the Linux behavior seems more desirable). >> >> >> >> http://austingroupbugs.net/view.php?id=369 >> > >> > No, that says "round up", so it does mean that the requested timeout >> > should be the minimum amount slept. tvtohz() does this. Really odd >> > that the socket code is using its own version of this rather than >> > tvtohz(). >> > >> > Oh, I bet this just predates tvtohz(). Interesting that it keeps getting >> > bug fixes in its history that simply using tvtohz() would have solved. >> > >> > Try this: >> > >> > Index: uipc_socket.c >> > =================================================================== >> > --- uipc_socket.c (revision 254570) >> > +++ uipc_socket.c (working copy) >> > @@ -2699,21 +2699,16 @@ sosetopt(struct socket *so, struct sockopt *sopt) >> > if (error) >> > goto bad; >> > >> > - /* assert(hz > 0); */ >> > if (tv.tv_sec < 0 || tv.tv_sec > INT_MAX / hz || >> > tv.tv_usec < 0 || tv.tv_usec >= 1000000) { >> > error = EDOM; >> > goto bad; >> > } >> > - /* assert(tick > 0); */ >> > - /* assert(ULONG_MAX - INT_MAX >= 1000000); */ >> > - val = (u_long)(tv.tv_sec * hz) + tv.tv_usec / tick; >> > - if (val > INT_MAX) { >> > + val = tvtohz(&tv); >> > + if (val == INT_MAX) { >> > error = EDOM; >> > goto bad; >> > } >> > - if (val == 0 && tv.tv_usec != 0) >> > - val = 1; >> > >> > switch (sopt->sopt_name) { >> > case SO_SNDTIMEO: >> > >> >> That must help. But I want to see the issue solved in the next >> release. I can't apply patch to the production system. Btw in >> production environment we have kern.hz set to 1000 so it's not a >> problem there. > > Can you test this in some way in a test environment? > Ok, sorry for posting out of the list. Simple test program is attached. Without your patch timeout expires in about 20ms. With it it's ~40ms. 40 instead of 30 is beacuse of odd tick added by tvtohz(). -- vitja. --089e013a05acf25bb004e4978110--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAKGHGPSYxmVoet8TnbxVFeEVk9CeD5iF6DK2do3w_ScnPU_SpQ>