lkml.org 
[lkml]   [2019]   [Oct]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/5] tracing: Handle the trace array ref counter in new functions
On Wed, 16 Oct 2019 16:42:02 -0700
Divya Indi <divya.indi@oracle.com> wrote:

> Hi Steve,
>
> Thanks again for taking the time to review and providing feedback. Please find my comments inline.
>
> On 10/15/19 4:04 PM, Steven Rostedt wrote:
> > Sorry for taking so long to getting to these patches.
> >
> > On Wed, 14 Aug 2019 10:55:26 -0700
> > Divya Indi <divya.indi@oracle.com> wrote:
> >
> >> For functions returning a trace array Eg: trace_array_create(), we need to
> >> increment the reference counter associated with the trace array to ensure it
> >> does not get freed when in use.
> >>
> >> Once we are done using the trace array, we need to call
> >> trace_array_put() to make sure we are not holding a reference to it
> >> anymore and the instance/trace array can be removed when required.
> > I think it would be more in line with other parts of the kernel if we
> > don't need to do the trace_array_put() before calling
> > trace_array_destroy().
>
> The reason we went with this approach is
>
> instance_mkdir - ref_ctr = 0 // Does not return a trace array ptr.
> trace_array_create - ref_ctr = 1 // Since this returns a trace array ptr.
> trace_array_lookup - ref_ctr = 1 // Since this returns a trace array ptr.
>
> if we make trace_array_destroy to expect ref_ctr to be 1, we risk destroying the trace array while in use.
>
> We could make it -
>
> instance_mkdir - ref_ctr = 1
> trace_array_create - ref_ctr = 2
> trace_array_lookup - ref_ctr = 2+ // depending on no of lookups
>
> but, we'd still need the trace_array_put() (?)
>
> We can also have one function doing create (if does not exist) or lookup (if exists), but that would require
> some redundant code since instance_mkdir needs to return -EXIST when a trace array already exists.
>
> Let me know your thoughts on this.
>

Can't we just move the trace_array_put() in the instance_rmdir()?

static int instance_rmdir(const char *name)
{
struct trace_array *tr;
int ret;

mutex_lock(&event_mutex);
mutex_lock(&trace_types_lock);

ret = -ENODEV;
list_for_each_entry(tr, &ftrace_trace_arrays, list) {
if (tr->name && strcmp(tr->name, name) == 0) {
__trace_array_put(tr);
ret = __remove_instance(tr);
if (ret)
tr->ref++;
break;
}
}

mutex_unlock(&trace_types_lock);
mutex_unlock(&event_mutex);

return ret;
}

-- Steve

\
 
 \ /
  Last update: 2019-10-23 04:53    [W:0.147 / U:1.900 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site