Re: svn commit: r254779 - head/sys/kern
On Sat, Aug 24, 2013 at 12:24:59PM +, Andre Oppermann wrote: A Author: andre A Date: Sat Aug 24 12:24:58 2013 A New Revision: 254779 A URL: http://svnweb.freebsd.org/changeset/base/254779 A A Log: A Avoid code duplication for mbuf initialization and use m_init() instead A in mb_ctor_mbuf() and mb_ctor_pack(). m_init() is inline, but it calls m_pkthdr_init() which isn't inline. It might be that compiler inline it due to m_pkthdr_init() living in the same file as mb_ctor_mbuf() and mb_ctor_pack(), but not sure. m_pkthdr_init() zeroes much more than deleted code did. Some zeroing operations are definitely superfluous job. The change is definitely not an no-op change. It might have pessimized the mbuf allocation performance. -- Totus tuus, Glebius. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r254779 - head/sys/kern
On 26.08.2013 12:50, Gleb Smirnoff wrote: On Sat, Aug 24, 2013 at 12:24:59PM +, Andre Oppermann wrote: A Author: andre A Date: Sat Aug 24 12:24:58 2013 A New Revision: 254779 A URL: http://svnweb.freebsd.org/changeset/base/254779 A A Log: A Avoid code duplication for mbuf initialization and use m_init() instead A in mb_ctor_mbuf() and mb_ctor_pack(). m_init() is inline, but it calls m_pkthdr_init() which isn't inline. It might be that compiler inline it due to m_pkthdr_init() living in the same file as mb_ctor_mbuf() and mb_ctor_pack(), but not sure. It depends on the optimization level. m_pkthdr_init() zeroes much more than deleted code did. Some zeroing operations are definitely superfluous job. It's exactly the same for mb_ctor_mbuf() and one more for mb_ctor_pack(). Overall it has become more because we've got a couple more (small) fields in the mbuf headers now. Looking at the size of the headers it may be faster to just bzero() it instead of doing it one by one. The overall problem is that replicating the same operations in multiple places is dangerous from a consistency point of view. The change is definitely not an no-op change. It might have pessimized the mbuf allocation performance. Heavy inlining is not always an advantage and cause i-cache bloat. But you're right I haven't benchmarked it (yet) and thus we don't know. I've been setting up my 10G benchmark environment but had to re-prioritize due to the impeding freeze on -current. If the benchmarking shows a conclusive pessimization I'm more than happy to compensate for it, either by inlining m_pkthdr_init() as well or some form of macro. Such changes can still be done during API freeze, but before BETA1. -- Andre ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r254779 - head/sys/kern
On Sat, Aug 24, 2013 at 12:24:59PM +, Andre Oppermann wrote: Author: andre Date: Sat Aug 24 12:24:58 2013 New Revision: 254779 URL: http://svnweb.freebsd.org/changeset/base/254779 Log: Avoid code duplication for mbuf initialization and use m_init() instead in mb_ctor_mbuf() and mb_ctor_pack(). Modified: head/sys/kern/kern_mbuf.c [...] -#ifdef MAC - /* If the label init fails, fail the alloc */ - error = mac_mbuf_init(m, how); - if (error) - return (error); -#endif - } else - m-m_data = m-m_dat; - return (0); + m = (struct mbuf *)mem; + flags = args-flags; + + error = m_init(m, NULL, size, how, type, flags); + + return (error); } This breaks head/. cc -c -O -pipe -std=c99 -g -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual -Wundef -Wno-pointer-sign -fformat-extensions -Wmissing-include-dirs -fdiagnostics-show-option -Wno-error-tautological-compare -Wno-error-empty-body -Wno-error-parentheses-equality -nostdinc -I. -I/src/sys -I/src/sys/contrib/altq -I/src/sys/contrib/libfdt -D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h -funwind-tables -mllvm -arm-enable-ehabi -ffreestanding -Werror /src/sys/kern/kern_mbuf.c /src/sys/kern/kern_mbuf.c:637:2: error: use of undeclared identifier 'error' error = m_init(m, NULL, size, how, type, flags); ^ /src/sys/kern/kern_mbuf.c:643:10: error: use of undeclared identifier 'error' return (error); ^ http://tinderbox.freebsd.org/tinderbox-head-build-HEAD-armv6-arm.full Glen pgpWbopOXqBhq.pgp Description: PGP signature
Re: svn commit: r254779 - head/sys/kern
On 24.08.2013 23:13, Glen Barber wrote: On Sat, Aug 24, 2013 at 12:24:59PM +, Andre Oppermann wrote: Author: andre Date: Sat Aug 24 12:24:58 2013 New Revision: 254779 URL: http://svnweb.freebsd.org/changeset/base/254779 Log: Avoid code duplication for mbuf initialization and use m_init() instead in mb_ctor_mbuf() and mb_ctor_pack(). Modified: head/sys/kern/kern_mbuf.c [...] -#ifdef MAC - /* If the label init fails, fail the alloc */ - error = mac_mbuf_init(m, how); - if (error) - return (error); -#endif - } else - m-m_data = m-m_dat; - return (0); + m = (struct mbuf *)mem; + flags = args-flags; + + error = m_init(m, NULL, size, how, type, flags); + + return (error); } This breaks head/. cc -c -O -pipe -std=c99 -g -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual -Wundef -Wno-pointer-sign -fformat-extensions -Wmissing-include-dirs -fdiagnostics-show-option -Wno-error-tautological-compare -Wno-error-empty-body -Wno-error-parentheses-equality -nostdinc -I. -I/src/sys -I/src/sys/contrib/altq -I/src/sys/contrib/libfdt -D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h -funwind-tables -mllvm -arm-enable-ehabi -ffreestanding -Werror /src/sys/kern/kern_mbuf.c /src/sys/kern/kern_mbuf.c:637:2: error: use of undeclared identifier 'error' error = m_init(m, NULL, size, how, type, flags); ^ /src/sys/kern/kern_mbuf.c:643:10: error: use of undeclared identifier 'error' return (error); ^ http://tinderbox.freebsd.org/tinderbox-head-build-HEAD-armv6-arm.full Sorry, and thanks for the report. Fixed in r254814. -- Andre ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
