Messages in this thread | | | From | Vaibhav Nagarnaik <> | Date | Tue, 6 Sep 2011 11:56:30 -0700 | Subject | Re: [PATCH v3] trace: Add per_cpu ring buffer control files |
| |
On Fri, Sep 2, 2011 at 7:45 PM, Steven Rostedt <rostedt@goodmis.org> wrote: > On Mon, 2011-08-22 at 18:17 -0700, Vaibhav Nagarnaik wrote: >> + /* ring buffer pages to update, > 0 to add, < 0 to remove */ >> + int nr_pages_to_update; >> + struct list_head new_pages; /* new pages to add */ > > There's no reason for the added new_pages. And I'm not sure I even like > the 'nr_pages_to_update' either. These are only used for resizing and > are just wasting space otherwise. > > You could allocate an array of numbers for the nr_pages_to_update and > use that instead. As for the list, heck, you can still use a single list > and pass that around like the original code did. >
In this patch's context, I could still use the same logic of having a temporary list of new pages and passing the nr_pages_to_update as a parameter to the function. However, this comes in handy when considering the next 2 patches. The updates are meant to be per_cpu and so having separate list of new pages is better to handle. Same thing with nr_pages_to_update.
>> + update_pages_handler(cpu_buffer); >> + cpu_buffer->nr_pages = nr_pages; > > The two places that call update_pages_handler() also updates > cpu_buffer->nr_pages. Move that to the update_pages_handler() as well.
Sure. I will update that.
> I'm still very nervous about this patch. I'm going to hold off a release > cycle before even giving it up to Ingo.
Sure. I agree that the patches need a rigorous review and your confidence in them is a necessary element.
I am working on other projects now, so I might have a slower response time. But I will try to update the patch and send it to you in a timely manner.
Vaibhav Nagarnaik -- 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/
| |