Re: [patch] voluntary-preempt-2.6.9-rc1-bk4-Q8
From: Rusty Russell
Date: Thu Sep 02 2004 - 20:28:02 EST
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/