2013/8/23 Davide Italiano <davide_at_freebsd.org>: > On Fri, Aug 23, 2013 at 3:45 PM, John Baldwin <jhb_at_freebsd.org> wrote: >> On Friday, August 23, 2013 2:27:58 am Vitja Makarov wrote: >>> 2013/8/22 John Baldwin <jhb_at_freebsd.org>: >>> > On Thursday, August 22, 2013 12:18:48 am Vitja Makarov wrote: >>> >> 2013/8/21 John Baldwin <jhb_at_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) >>> >> > _at__at_ -2699,21 +2699,16 _at__at_ 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(). >> >> Ok, thanks. tvtohz() will be good to MFC (and I will do that), but for >> HEAD I think we can fix this to use a precise timeout. I've cc'd davide_at_ >> so he can take a look at that. >> >> -- >> John Baldwin > > Hi, > I think I can switch this code to new timeout KPI, but this will > require the timeout field of 'struct sockbuf' to be changed from 'int' > to 'sbintime_t' which breaks binary compatibility. Do you have any > strong objections about this? If any, I would like this to happen ABI > freeze, so it looks like this is the right moment. > I think that for socket's timeouts it's ok to have a HZ-precision. It would be much more important to implement high-precision timeouts for select() and friends, if it's not done yet (sorry I'm running 9.1). -- vitja.Received on Fri Aug 23 2013 - 12:04:06 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:40 UTC