lkml.org 
[lkml]   [2004]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch] voluntary-preempt-2.6.9-rc1-bk4-Q8

* Lee Revell <rlrevell@joe-job.com> wrote:

> > these all seem to be single-packet processing latencies - it would be
> > quite hard to make those codepaths preemptible.
>
> I suspected as much, these are not a problem. The large latencies
> from reading the /proc filesystem are a bit worrisome (trace1.txt), I
> will report these again if they still happen with Q8.

conntrack's ct_seq ops indeed seems to have latency problems - the quick
workaround is to disable conntrack.

The reason for the latency is that ct_seq_start() does a read_lock() on
ip_conntrack_lock and only ct_seq_stop() releases it - possibly
milliseconds later. But the whole conntrack /proc code is quite flawed:

READ_LOCK(&ip_conntrack_lock);

if (*pos >= ip_conntrack_htable_size)
return NULL;
bucket = kmalloc(sizeof(unsigned int), GFP_KERNEL);
if (!bucket) {
return ERR_PTR(-ENOMEM);
}
*bucket = *pos;
return bucket;
#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.

Rusty, what's going on in this code?

Ingo
-
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/

\
 
 \ /
  Last update: 2005-03-22 14:05    [from the cache]
©2003-2011 Jasper Spaans