Re: [PATCH 4/7] netprio_cgroup: reimplement priomap expansion
From: Tejun Heo <hidden>
Date: 2012-11-20 14:38:38
Also in:
lkml, netdev
Hello, Daniel. On Tue, Nov 20, 2012 at 09:46:22AM +0100, Daniel Wagner wrote:
struct netprio_map {
struct rcu_head rcu;
struct netprio_aux *aux; /* auxiliary config array */
u32 priomap_len;
u32 priomap[];
};
Is there a specific reason why aux and priomap is handled
differently? Couldn't you just use same approach for both variables,
e.g. re/allocating only them here and leave the allocation struct
netprio_map in cgrp_css_alloc()?->aux is no longer added, so the consistency issue doesn't exist anymore. The reason why they were handled differently before (or rather why I didn't change priomap[] to be allocated separately) was that pointer chasing tends to be more expensive than offsetting. I don't know how much effect it would have in this case but things sitting in packet in/out paths can be very hot so didn't wanna disturb it.
Also the algorithm to figure out the size of the array might be a bit too aggressive in my opinion. So you always start at PRIOMAP_MIN_SIZE and then try to double the size until target_idx fits. Wouldn't it make sense to start to look for the new size beginning at old->priomap_len and then do the power-of-two increase?
The only downside of always starting from PRIOMAP_MIN_SIZE is iterating several more times in the sizing loop which isn't really anything to worry about. The loop is structured that way because I wanted to keep the size of the whole thing power-of-two. Due to the fields before priomap[], if we size priomap_len power-of-two, we'll always end up with something slightly over power-of-two, which is usually the worst size to allocate. Thanks. -- tejun