Messages in this thread | | | Subject | Re: [PATCH] tracing: Merge separate resize loops | From | Steven Rostedt <> | Date | Fri, 18 May 2012 23:19:13 -0400 |
| |
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
| |