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 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
