lkml.org 
[lkml]   [2023]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] ftrace: Add ftrace_page to list after the index is calculated
From

On 2023/3/8 22:53, Steven Rostedt wrote:
>
>> - } else {
>> - if (!ftrace_pages)
>> - goto out;
>> -
>> - if (WARN_ON(ftrace_pages->next)) {
>> - /* Hmm, we have free pages? */
>> - while (ftrace_pages->next)
>> - ftrace_pages = ftrace_pages->next;
>> - }
>> -
>> - ftrace_pages->next = start_pg;
> Basically, what you are saying is that once we add ftrace_pages->next to
> point to the new start_pg, it becomes visible to others and that could be a
> problem. And moving this code around is not really going to solve that, as
> then we would need to add memory barriers.
>
>> - }
>> -
>> p = start;
>> pg = start_pg;
>> while (p < end) {
>> @@ -6855,6 +6841,21 @@ static int ftrace_process_locs(struct module *mod,
>> /* We should have used all pages */
>> WARN_ON(pg->next);
>>
>> + /* Add pages to ftrace_pages list */
>> + if (!mod) {
>> + WARN_ON(ftrace_pages || ftrace_pages_start);
>> + /* First initialization */
>> + ftrace_pages_start = start_pg;
>> + } else {
>> + if (WARN_ON(ftrace_pages->next)) {
>> + /* Hmm, we have free pages? */
>> + while (ftrace_pages->next)
>> + ftrace_pages = ftrace_pages->next;
>> + }
>> +
>> + ftrace_pages->next = start_pg;
>> + }
>> +
>> /* Assign the last page to ftrace_pages */
>> ftrace_pages = pg;
>>
> Why not just test for it?
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 29baa97d0d53..9b2803c7a18f 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -1564,7 +1564,8 @@ static struct dyn_ftrace *lookup_rec(unsigned long start, unsigned long end)
> key.flags = end; /* overload flags, as it is unsigned long */
>
> for (pg = ftrace_pages_start; pg; pg = pg->next) {
> - if (end < pg->records[0].ip ||
> + if (pg->index == 0 ||
> + end < pg->records[0].ip ||
> start >= (pg->records[pg->index - 1].ip + MCOUNT_INSN_SIZE))
> continue;
> rec = bsearch(&key, pg->records, pg->index,
>
>
> -- Steve
Thanks for review!

At first I'm worried that it will cause lookup_rec to return an
incorrect result
if it issearching a module address going to be added.
But now I found that records are added at MODULE_STATE_UNFORMED, the module
address won't  be searched at this point in any case.

Let's do it this way. I'll send another patch.

\
 
 \ /
  Last update: 2023-03-27 00:51    [W:0.057 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site