Re: [PATCH] tracing: Merge separate resize loops

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 2012-05-18 at 20:00 -0700, Vaibhav Nagarnaik wrote:
> On Fri, May 18, 2012 at 6:56 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Fri, 2012-05-18 at 13:29 -0700, Vaibhav Nagarnaik wrote:
> >>
> >>               /* wait for all the updates to complete */
> >>               for_each_buffer_cpu(buffer, cpu) {
> >>                       cpu_buffer = buffer->buffers[cpu];
> >> -                     if (!cpu_buffer->nr_pages_to_update||
> >> -                         !cpu_online(cpu))
> >> +                     if (!cpu_buffer->nr_pages_to_update)
> >>                               continue;
> >>
> >
> > Why did you make this change? As we only need to wait for completions.
> >
> >> -                     wait_for_completion(&cpu_buffer->update_completion);
> >> -                     /* reset this value */
> >> +                     if (cpu_online(cpu))
> >> +                             wait_for_completion(&cpu_buffer->update_done);
> >>                       cpu_buffer->nr_pages_to_update = 0;
> >
> > Or was the original patch buggy, where we never set nr_page_to_update to
> > zero for the offline case?
> 
> I don't see a bug, since at the start of the resize operation, we
> always recalculate this value. It will be reset to 0, if there are no
> updates. I set it to zero at the end just as a precautionary measure.

But if there were updates on a offline CPU, then the original patch
would not have set this to zero at the end.

Or are you just saying that we don't need to set this to zero, as it
isn't used later on? And when we re-enter this function (where its the
only place, and what it calls, that uses nr_page_to_update), it gets
reset.

IOW, this reset is just a "clean up" of the nr_pages_to_update. Right?

-- Steve