lkml.org 
[lkml]   [2011]   [Sep]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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/

\
 
 \ /
  Last update: 2011-09-06 20:59    [W:0.187 / U:1.044 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site