Messages in this thread | | | Date | Tue, 20 Mar 2012 22:33:35 +1100 | Subject | Re: [PATCH] net: bpf_jit: Simplify code by always using offset8 or offset32. | From | "Indan Zupancic" <> |
| |
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
| |