Messages in this thread | | | Subject | Re: [patch] voluntary-preempt-2.6.9-rc1-bk4-Q8 | From | Rusty Russell <> | Date | Fri, 03 Sep 2004 11:10:30 +1000 |
| |
On Thu, 2004-09-02 at 17:46, Ingo Molnar wrote: > Rusty, what's going on in this code?
Good question! Not my code, fortunately...
> #1: we kmalloc(GFP_KERNEL) with a spinlock held and softirqs off - ouch! > > #2: why does it do the kmalloc() anyway? It could store the position in > the seq pointer just fine. No need to alloc an integer pointer to > store the value in ... > > #3: to fix the latency, ct_seq_show() could take the ip_conntrack_lock > and could check the current index against ip_conntrack_htable_size. > There's not much point in making this non-preemptible, there's > a 4K granularity anyway.
The code tries to put an entire hash bucket into a single seq_read(). That's not going to work if the hash is really deep. On the other hand, not much will, and it's simple.
The lock is only needed on traversing: htable_size can't change after init anyway, so it should be done in ct_seq_show.
Fix should be fairly simple... Rusty. -- Anyone who quotes me in their signature is an idiot -- Rusty Russell
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |