Re: [PATCH 2/2] tracing - putting cond_resched into tace_pipe loop

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 2011-03-14 at 15:18 +0100, Jiri Olsa wrote:


> > I'm not sure that this needs the signal_pending() or the break, or even
> > the cond_resched(). Perhaps the first patch fixes the bug. But that
> > while loop does not block, and it should just spin enough to fill a
> > page. If it is not filling the page then that's a bug.
> 
> well, if there are no "event lost" messages and the flags are set
> to the "bin or raw" and you trace events only, then the page
> is not be filled.
> 
> But as there'll be allways lost events, the page will be populated
> and it will get out of the loop.

There's two loops, I'm talking about the inner one, which will break out
when there are no events left to process.

The "waitagain" calls into tracing_wait_pipe which has the
signal_pending() attribute. It should also block, or return -EAGAIN if
the caller is not blocking.

> 
> > 
> > Jiri,
> > 
> > Can you reproduce the bug with just he first patch? Actually, I can
> > reproduce it on vanilla, I'll apply your first patch and see if that
> > fixes things. If not, then we need to find out why and fix those.
> 
> The first patch fixies the 'lost evets' trace, so the loop is escaped
> when the page is full.
> (I was going to send some change for first patch WARN_ON -> WARN_ONCE)

Yes, I was going to recommend doing that too.

> 
> I think in case there are no 'lost events' and we have the conditions
> I describe, we need the cond_resched call.

No we don't.

As for non preempt kernels, honestly, I don't care about latencies. If
you are worried about latencies, don't run a non preempt kernel. I do
care if it goes into an infinite loop, or never processes signals, which
would be a bug. But long running times in the kernel on a non preempt
kernel. Well, that's just part of the game.

-- Steve

> 
> I'll send new patch version shortly.
> 
> thanks,
> jirka
> > 
> > -- Steve
> > 
> >