lkml.org 
[lkml]   [2009]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/7] ring-buffer: change test to be more latency friendly

    On Thu, 7 May 2009, Ingo Molnar wrote:

    >
    > * Steven Rostedt <rostedt@goodmis.org> wrote:
    >
    > > From: Steven Rostedt <srostedt@redhat.com>
    > >
    > > The ring buffer benchmark/test runs a producer for 10 seconds.
    > > This is done with preemption and interrupts enabled. But if the
    > > kernel is not compiled with CONFIG_PREEMPT, it basically stops
    > > everything but interrupts for 10 seconds.
    > >
    > > Although this is just a test and is not for production, this attribute
    > > can be quite annoying. It can also spawn badness elsewhere.
    >
    > Yep, this probably explains that lockdep splat i got in a networking
    > driver. Some functionality (a workqueue iirc) of the driver got
    > starved and a time-out timer triggered - where lockdep caught
    > locking badness.

    We probably need to notify the network people about that.

    >
    > > This patch solves the issues by calling "cond_resched" when the
    > > system is not compiled with CONFIG_PREEMPT. It also keeps track of
    > > the time spent to call cond_resched such that it does not go
    > > against the time calculations. That is, if the task schedules
    > > away, the time scheduled out is removed from the test data. Note,
    > > this only works for non PREEMPT because we do not know when the
    > > task is scheduled out if we have PREEMPT enabled.
    > >
    > > [ Impact: prevent test from stopping the world for 10 seconds ]
    > >
    > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
    > > ---
    > > kernel/trace/ring_buffer_benchmark.c | 31 +++++++++++++++++++++++++++++++
    > > 1 files changed, 31 insertions(+), 0 deletions(-)
    > >
    > > diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
    > > index dcd75e9..a26fc67 100644
    > > --- a/kernel/trace/ring_buffer_benchmark.c
    > > +++ b/kernel/trace/ring_buffer_benchmark.c
    > > @@ -185,6 +185,35 @@ static void ring_buffer_consumer(void)
    > > complete(&read_done);
    > > }
    > >
    > > +/*
    > > + * If we are a non preempt kernel, the 10 second run will
    > > + * stop everything while it runs. Instead, we will call cond_resched
    > > + * and also add any time that was lost by a rescedule.
    > > + */
    > > +#ifdef CONFIG_PREEMPT
    > > +static void sched_if_needed(struct timeval *start_tv, struct timeval *end_tv)
    > > +{
    > > +}
    > > +#else
    > > +static void sched_if_needed(struct timeval *start_tv, struct timeval *end_tv)
    > > +{
    > > + struct timeval tv;
    > > +
    > > + cond_resched();
    > > + do_gettimeofday(&tv);
    > > + if (tv.tv_usec < end_tv->tv_usec) {
    > > + tv.tv_usec += 1000000;
    > > + tv.tv_sec--;
    > > + }
    > > + start_tv->tv_sec += tv.tv_sec - end_tv->tv_sec;
    > > + start_tv->tv_usec += tv.tv_usec - end_tv->tv_usec;
    > > + if (start_tv->tv_usec > 1000000) {
    > > + start_tv->tv_usec -= 1000000;
    > > + start_tv->tv_sec++;
    > > + }
    > > +}
    > > +#endif
    >
    > This is _way_ too ugly. Why not just add a cond_resched() to the
    > inner loop and be done with it? cond_resched() is conditional
    > already, so it will only schedule 'if needed'.
    >
    > If the test's timing gets skewed, what's the big deal? If its being
    > preempted there will be impact _anyway_. (due to cache footprint
    > elimination, etc.) People obviously should only rely on the numbers
    > if the system is idle.

    OK, I'll nuke it. I find that the writer is more affected by the reader.
    With this code, I get the same timings as if I run it with reader
    disabled.

    But it is more a way to see how a change may affect the buffering than to
    really time the buffering itself. If that was the case, I would have added
    preemption around the timings.

    -- Steve



    \
     
     \ /
      Last update: 2009-05-07 16:03    [W:0.038 / U:0.888 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site