On Wed, Oct 2, 2013 at 9:57 PM, Eric Dumazet <eric.duma...@gmail.com> wrote:
> On Wed, 2013-10-02 at 21:53 -0700, Eric Dumazet wrote:
>> On Wed, 2013-10-02 at 21:44 -0700, Alexei Starovoitov wrote:
>>
>> > I think ifdef config_x86 is a bit ugly inside struct sk_filter, but
>> > don't mind whichever way.
>>
>> Its not fair to make sk_filter bigger, because it means that simple (non
>> JIT) filter might need an extra cache line.
>>
>> You could presumably use the following layout instead :
>>
>> struct sk_filter
>> {
>> atomic_t refcnt;
>> struct rcu_head rcu;
>> struct work_struct work;
>>
>> unsigned int len ____cacheline_aligned; /* Number of
>> filter blocks */
>> unsigned int (*bpf_func)(const struct sk_buff *skb,
>> const struct sock_filter
>> *filter);
>> struct sock_filter insns[0];
>> };
>
> And since @len is not used by sk_run_filter() use :
>
> struct sk_filter {
> atomic_t refcnt;
> int len; /* number of filter blocks */
> struct rcu_head rcu;
> struct work_struct work;
>
> unsigned int (*bpf_func)(const struct sk_buff *skb,
> const struct sock_filter *filter)
> ____cacheline_aligned;
> struct sock_filter insns[0];
> };
yes. make sense to avoid first insn cache miss inside sk_run_filter()
at the expense
of 8-byte gap between work and bpf_func (on x86_64 w/o lockdep)
Probably even better to overlap work and insns fields.
Pro: sk_filter size the same, no impact on non-jit case
Con: would be harder to understand the code
another problem is that kfree(sk_filter) inside
sk_filter_release_rcu() needs to move inside bpf_jit_free().
so self nack. Let me fix these issues and respin
Thanks
Alexei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
