Now we have two similar infrastructure to iterate ftrace_page and
dyn_ftrace:
* do_for_each_ftrace_rec()
* for_ftrace_rec_iter()
The 2nd one, for_ftrace_rec_iter(), looks more generic, so preserve it
and replace do_for_each_ftrace_rec() with it.
Signed-off-by: Wei Yang <[email protected]>
---
kernel/trace/ftrace.c | 94 ++++++++++++++++++++-----------------------
1 file changed, 44 insertions(+), 50 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 15fcfa16895d..4def668f45ba 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1501,21 +1501,6 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs)
return ret;
}
-/*
- * This is a double for. Do not use 'break' to break out of the loop,
- * you must use a goto.
- */
-#define do_for_each_ftrace_rec(pg, rec) \
- for (pg = ftrace_pages_start; pg; pg = pg->next) { \
- int _____i; \
- for (_____i = 0; _____i < pg->index; _____i++) { \
- rec = &pg->records[_____i];
-
-#define while_for_each_ftrace_rec() \
- } \
- }
-
-
static int ftrace_cmp_recs(const void *a, const void *b)
{
const struct dyn_ftrace *key = a;
@@ -1638,7 +1623,7 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops,
{
struct ftrace_hash *hash;
struct ftrace_hash *other_hash;
- struct ftrace_page *pg;
+ struct ftrace_rec_iter *iter;
struct dyn_ftrace *rec;
bool update = false;
int count = 0;
@@ -1676,11 +1661,13 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops,
return false;
}
- do_for_each_ftrace_rec(pg, rec) {
+ for_ftrace_rec_iter(iter) {
int in_other_hash = 0;
int in_hash = 0;
int match = 0;
+ rec = ftrace_rec_iter_record(iter);
+
if (rec->flags & FTRACE_FL_DISABLED)
continue;
@@ -1797,7 +1784,7 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops,
/* Shortcut, if we handled all records, we are done. */
if (!all && count == hash->count)
return update;
- } while_for_each_ftrace_rec();
+ }
return update;
}
@@ -1862,7 +1849,7 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
struct ftrace_hash *old_hash,
struct ftrace_hash *new_hash)
{
- struct ftrace_page *pg;
+ struct ftrace_rec_iter *iter;
struct dyn_ftrace *rec, *end = NULL;
int in_old, in_new;
@@ -1881,7 +1868,8 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
return -EINVAL;
/* Update rec->flags */
- do_for_each_ftrace_rec(pg, rec) {
+ for_ftrace_rec_iter(iter) {
+ rec = ftrace_rec_iter_record(iter);
if (rec->flags & FTRACE_FL_DISABLED)
continue;
@@ -1899,7 +1887,7 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
rec->flags |= FTRACE_FL_IPMODIFY;
} else /* Removed entry */
rec->flags &= ~FTRACE_FL_IPMODIFY;
- } while_for_each_ftrace_rec();
+ }
return 0;
@@ -1907,7 +1895,8 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
end = rec;
/* Roll back what we did above */
- do_for_each_ftrace_rec(pg, rec) {
+ for_ftrace_rec_iter(iter) {
+ rec = ftrace_rec_iter_record(iter);
if (rec->flags & FTRACE_FL_DISABLED)
continue;
@@ -1924,7 +1913,7 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
rec->flags &= ~FTRACE_FL_IPMODIFY;
else
rec->flags |= FTRACE_FL_IPMODIFY;
- } while_for_each_ftrace_rec();
+ }
err_out:
return -EBUSY;
@@ -2517,8 +2506,8 @@ __ftrace_replace_code(struct dyn_ftrace *rec, bool enable)
void __weak ftrace_replace_code(int mod_flags)
{
+ struct ftrace_rec_iter *iter;
struct dyn_ftrace *rec;
- struct ftrace_page *pg;
bool enable = mod_flags & FTRACE_MODIFY_ENABLE_FL;
int schedulable = mod_flags & FTRACE_MODIFY_MAY_SLEEP_FL;
int failed;
@@ -2526,7 +2515,8 @@ void __weak ftrace_replace_code(int mod_flags)
if (unlikely(ftrace_disabled))
return;
- do_for_each_ftrace_rec(pg, rec) {
+ for_ftrace_rec_iter(iter) {
+ rec = ftrace_rec_iter_record(iter);
if (rec->flags & FTRACE_FL_DISABLED)
continue;
@@ -2539,7 +2529,7 @@ void __weak ftrace_replace_code(int mod_flags)
}
if (schedulable)
cond_resched();
- } while_for_each_ftrace_rec();
+ }
}
struct ftrace_rec_iter {
@@ -2940,14 +2930,15 @@ int ftrace_shutdown(struct ftrace_ops *ops, int command)
*/
if (rcu_dereference_protected(ftrace_ops_list,
lockdep_is_held(&ftrace_lock)) == &ftrace_list_end) {
- struct ftrace_page *pg;
+ struct ftrace_rec_iter *iter;
struct dyn_ftrace *rec;
- do_for_each_ftrace_rec(pg, rec) {
+ for_ftrace_rec_iter(iter) {
+ rec = ftrace_rec_iter_record(iter);
if (FTRACE_WARN_ON_ONCE(rec->flags & ~FTRACE_FL_DISABLED))
pr_warn(" %pS flags:%lx\n",
(void *)rec->ip, rec->flags);
- } while_for_each_ftrace_rec();
+ }
}
ops->old_hash.filter_hash = NULL;
@@ -3917,6 +3908,7 @@ add_rec_by_index(struct ftrace_hash *hash, struct ftrace_glob *func_g,
int clear_filter)
{
long index = simple_strtoul(func_g->search, NULL, 0);
+ struct ftrace_rec_iter *iter;
struct ftrace_page *pg;
struct dyn_ftrace *rec;
@@ -3924,16 +3916,17 @@ add_rec_by_index(struct ftrace_hash *hash, struct ftrace_glob *func_g,
if (--index < 0)
return 0;
- do_for_each_ftrace_rec(pg, rec) {
+ for_ftrace_rec_iter(iter) {
+ pg = iter->pg;
if (pg->index <= index) {
+ iter->index = pg->index;
index -= pg->index;
- /* this is a double loop, break goes to the next page */
- break;
+ continue;
}
rec = &pg->records[index];
enter_record(hash, rec, clear_filter);
return 1;
- } while_for_each_ftrace_rec();
+ }
return 0;
}
@@ -3978,7 +3971,7 @@ ftrace_match_record(struct dyn_ftrace *rec, struct ftrace_glob *func_g,
static int
match_records(struct ftrace_hash *hash, char *func, int len, char *mod)
{
- struct ftrace_page *pg;
+ struct ftrace_rec_iter *iter;
struct dyn_ftrace *rec;
struct ftrace_glob func_g = { .type = MATCH_FULL };
struct ftrace_glob mod_g = { .type = MATCH_FULL };
@@ -4010,7 +4003,8 @@ match_records(struct ftrace_hash *hash, char *func, int len, char *mod)
goto out_unlock;
}
- do_for_each_ftrace_rec(pg, rec) {
+ for_ftrace_rec_iter(iter) {
+ rec = ftrace_rec_iter_record(iter);
if (rec->flags & FTRACE_FL_DISABLED)
continue;
@@ -4023,7 +4017,7 @@ match_records(struct ftrace_hash *hash, char *func, int len, char *mod)
}
found = 1;
}
- } while_for_each_ftrace_rec();
+ }
out_unlock:
mutex_unlock(&ftrace_lock);
@@ -5945,7 +5939,7 @@ ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer)
{
struct ftrace_glob func_g;
struct dyn_ftrace *rec;
- struct ftrace_page *pg;
+ struct ftrace_rec_iter *iter;
struct ftrace_func_entry *entry;
int fail = 1;
int not;
@@ -5963,7 +5957,8 @@ ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer)
return -ENODEV;
}
- do_for_each_ftrace_rec(pg, rec) {
+ for_ftrace_rec_iter(iter) {
+ rec = ftrace_rec_iter_record(iter);
if (rec->flags & FTRACE_FL_DISABLED)
continue;
@@ -5985,7 +5980,7 @@ ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer)
}
}
}
- } while_for_each_ftrace_rec();
+ }
out:
mutex_unlock(&ftrace_lock);
@@ -6407,7 +6402,7 @@ void ftrace_release_mod(struct module *mod)
void ftrace_module_enable(struct module *mod)
{
struct dyn_ftrace *rec;
- struct ftrace_page *pg;
+ struct ftrace_rec_iter *iter;
mutex_lock(&ftrace_lock);
@@ -6430,17 +6425,16 @@ void ftrace_module_enable(struct module *mod)
if (ftrace_start_up)
ftrace_arch_code_modify_prepare();
- do_for_each_ftrace_rec(pg, rec) {
+ for_ftrace_rec_iter(iter) {
int cnt;
- /*
- * do_for_each_ftrace_rec() is a double loop.
- * module text shares the pg. If a record is
- * not part of this module, then skip this pg,
- * which the "break" will do.
- */
+ rec = ftrace_rec_iter_record(iter);
+
if (!within_module_core(rec->ip, mod) &&
- !within_module_init(rec->ip, mod))
- break;
+ !within_module_init(rec->ip, mod)) {
+ /* skip current ftrace_page */
+ iter->index = iter->pg->index;
+ continue;
+ }
cnt = 0;
@@ -6464,7 +6458,7 @@ void ftrace_module_enable(struct module *mod)
}
}
- } while_for_each_ftrace_rec();
+ }
out_loop:
if (ftrace_start_up)
--
2.20.1 (Apple Git-117)
On Mon, 31 Aug 2020 11:11:03 +0800
Wei Yang <[email protected]> wrote:
> Now we have two similar infrastructure to iterate ftrace_page and
> dyn_ftrace:
>
> * do_for_each_ftrace_rec()
> * for_ftrace_rec_iter()
>
> The 2nd one, for_ftrace_rec_iter(), looks more generic, so preserve it
> and replace do_for_each_ftrace_rec() with it.
>
I also didn't pull in this patch. The reason is that the
for_ftrace_rec_iter() is specific for external usage (for archs to use) and
requires two function calls to do the iterations.
The do_for_each_ftrace_rec() is a faster, light weight version, but for
internal use only.
I rather keep both, as each has their own, but slightly different, use
cases.
-- Steve
On Tue, Oct 06, 2020 at 10:42:17AM -0400, Steven Rostedt wrote:
>On Mon, 31 Aug 2020 11:11:03 +0800
>Wei Yang <[email protected]> wrote:
>
>> Now we have two similar infrastructure to iterate ftrace_page and
>> dyn_ftrace:
>>
>> * do_for_each_ftrace_rec()
>> * for_ftrace_rec_iter()
>>
>> The 2nd one, for_ftrace_rec_iter(), looks more generic, so preserve it
>> and replace do_for_each_ftrace_rec() with it.
>>
>
>I also didn't pull in this patch. The reason is that the
>for_ftrace_rec_iter() is specific for external usage (for archs to use) and
>requires two function calls to do the iterations.
>
>The do_for_each_ftrace_rec() is a faster, light weight version, but for
>internal use only.
>
>I rather keep both, as each has their own, but slightly different, use
>cases.
>
Got it, thanks
>-- Steve
--
Wei Yang
Help you, Help me