Date: Tue, 20 Mar 2012 22:33:35 +1100 From: "Indan Zupancic" <indan@....nu> To: "Eric Dumazet" <eric.dumazet@...il.com> Cc: "Will Drewry" <wad@...omium.org>, linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org, linux-doc@...r.kernel.org, kernel-hardening@...ts.openwall.com, netdev@...r.kernel.org, x86@...nel.org, arnd@...db.de, davem@...emloft.net, hpa@...or.com, mingo@...hat.com, oleg@...hat.com, peterz@...radead.org, rdunlap@...otime.net, mcgrathr@...omium.org, tglx@...utronix.de, luto@....edu, eparis@...hat.com, serge.hallyn@...onical.com, djm@...drot.org, scarybeasts@...il.com, pmoore@...hat.com, akpm@...ux-foundation.org, corbet@....net, markus@...omium.org, coreyb@...ux.vnet.ibm.com, keescook@...omium.org Subject: Re: [PATCH] net: bpf_jit: Simplify code by always using offset8 or offset32. On Tue, March 20, 2012 13:59, Eric Dumazet wrote: > On Tue, 2012-03-20 at 13:24 +1100, Indan Zupancic wrote: > >> If it does then perhaps the fast path should be made faster by inlining >> the code instead of calling a function which may not be cached. >> > > inlining 400 times a sequence of code is waste of icache, you probably > missed this. Well, according to you most filters were small, inling 26 bytes a few times should be faster than calling an external function. Not all calls need to be inlined either. > > I spent a lot of time on working on this implementation, tried many > different strategies before choosing the one in place. > > Listen, I am tired of this thread, it seems you want to push changes > that have almost no value but still need lot of review. The latest patch didn't change generated code except for a few ancillary instructions. The one before that just added documentation. The first patch was indeed bad. > > Unless you make benchmarks and can make at least 5 % improvement of the > speed, or improve maintainability of this code, I am not interested. My next patch would have changed the compiler to always compile in two passes instead of looping till the result is stable. But never mind. > > We certainly _can_ one day have sizeof(struct sk_buff) > 256, and actual > code is ready for this. You want to break this for absolutely no valid > reason. I've the feeling you didn't read the latest patch, it doesn't assume sizeof(struct sk_buff) < 256, nor that fields aren't reordered. > > We _can_ change fields order anytime in struct sk_buff, even if you > state "its very unlikely that those fields are ever moved to the end > of sk_buff". And if the dev, len or data_len fields are really moved past the first 127 bytes the JIT code can be changed too. The JIT code already depends on some of struct sk_buff's field properties anyway. Greetings, Indan
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.