On Wed, 27 Jan 2021 12:39:51 -0500
Steven Rostedt <[email protected]> wrote:
>
> kernel/tracepoint.c | 54 +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 7261fa0f5e3c..23088f6276a4 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -129,7 +129,7 @@ static struct tracepoint_func *
> func_add(struct tracepoint_func **funcs, struct tracepoint_func *tp_func,
> int prio)
> {
> - struct tracepoint_func *old, *new;
> + struct tracepoint_func *old, *new, *tp_funcs;
> int nr_probes = 0;
> int pos = -1;
>
> @@ -149,10 +149,28 @@ func_add(struct tracepoint_func **funcs, struct
> tracepoint_func *tp_func,
> return ERR_PTR(-EEXIST);
> }
> }
> - /* + 2 : one for new probe, one for NULL func */
> - new = allocate_probes(nr_probes + 2);
> - if (new == NULL)
> + /*
> + * The size of the tp_funcs will be the current size, plus
> + * one for the new probe, one for the NULL func, and one for
> + * the pointer to the "removal" array.
> + * And then double the size to create the "removal" array.
> + */
> + tp_funcs = allocate_probes((nr_probes + 3) * 2);
Note, I realize that this double allocation is unnecessary if we add a
single probe. But to make this different for 2 or more probes, would make
the logic more complex, so I just kept the logic the same for the single
probe even though it's not needed.
> + if (tp_funcs == NULL)
> return ERR_PTR(-ENOMEM);
> + /*
> + * When removing a probe and there are more probes left,
> + * we cannot rely on allocation to succeed to create the new
> + * RCU array. Thus, the array is doubled here, and on removal of
> + * a probe with other probes still in the array, the second half
> + * of the array is used.
> + *
> + * The first element of the array has its "func" field point to
> + * the start of the other half of the array.
> + */
> + tp_funcs->func = tp_funcs + nr_probes + 3;
> + tp_funcs[nr_probes + 3].func = tp_funcs;
> + new = tp_funcs + 1;
> if (old) {
> if (pos < 0) {
> pos = nr_probes;
> @@ -164,6 +182,14 @@ func_add(struct tracepoint_func **funcs, struct
> tracepoint_func *tp_func,
> memcpy(new + pos + 1, old + pos,
> (nr_probes - pos) * sizeof(struct
> tracepoint_func));
> }
> + /* Make old point back to the allocated array */
> + old--;
> + /*
> + * If this is the second half of the array,
> + * make it point back to the first half.
> + */
> + if (old->func < old)
> + old = old->func;
> } else
> pos = 0;
> new[pos] = *tp_func;
> @@ -202,14 +228,18 @@ static void *func_remove(struct tracepoint_func **funcs,
> /* N -> 0, (N > 1) */
> *funcs = NULL;
> debug_print_probes(*funcs);
> + /* Set old back to what it was allocated to */
> + old--;
> + if (old->func < old)
> + old = old->func;
> return old;
> } else {
> int j = 0;
> - /* N -> M, (N > 1, M > 0) */
> - /* + 1 for NULL */
> - new = allocate_probes(nr_probes - nr_del + 1);
> - if (new == NULL)
> - return ERR_PTR(-ENOMEM);
> +
> + /* Use the other half of the array for the new probes */
> + new = old - 1;
> + new = new->func;
> + new++;
> for (i = 0; old[i].func; i++)
> if (old[i].func != tp_func->func
> || old[i].data != tp_func->data)
> @@ -218,7 +248,7 @@ static void *func_remove(struct tracepoint_func **funcs,
> *funcs = new;
> }
> debug_print_probes(*funcs);
> - return old;
> + return NULL;
> }
>
> static void tracepoint_update_call(struct tracepoint *tp, struct
> tracepoint_func *tp_funcs, bool sync)
> @@ -309,8 +339,8 @@ static int tracepoint_remove_func(struct tracepoint *tp,
> rcu_assign_pointer(tp->funcs, tp_funcs);
> } else {
> rcu_assign_pointer(tp->funcs, tp_funcs);
> - tracepoint_update_call(tp, tp_funcs,
> - tp_funcs[0].func != old[0].func);
> + /* Need to sync, if going back to a single caller */
> + tracepoint_update_call(tp, tp_funcs, tp_funcs[1].func == NULL);
I may make this change a separate patch. As it changes the logic slightly
unrelated to the change being fixed, and was only needed for this patch, to
remove the reference to "old".
-- Steve
> }
> release_probes(old);
> return 0;
