| Message ID | 20240605180408.691995506@goodmis.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 07bbe0833ed62f48785dffa8e429f35c1d906415 |
| Headers | show |
| Series | ftrace: Clean up and comment code | expand |
On Wed, Jun 05, 2024 at 02:03:36PM -0400, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > While adding comments to the function __ftrace_hash_rec_update() and > trying to describe in detail what the parameter for "ftrace_hash" does, I > realized that it basically does exactly the same thing (but differently) > if it is set or not! Typo: "ftrace_hash" should be "filter_hash", and likewise in the commit title. > If it is set, the idea was the ops->filter_hash was being updated, and the > code should focus on the functions that are in the ops->filter_hash and > add them. But it still had to pay attention to the functions in the > ops->notrace_hash, to ignore them. > > If it was cleared, it focused on the ops->notrace_hash, and would add > functions that were not in the ops->notrace_hash but would still keep > functions in the "ops->filter_hash". Basically doing the same thing. > > In reality, the __ftrace_hash_rec_update() only needs to either remove the > functions associated to the give ops (if "inc" is set) or remove them (if > "inc" is cleared). It has to pay attention to both the filter_hash and > notrace_hash regardless. AFAICT, we actually remove a latent bug here, but that bug wasn't reachable because we never call __ftrace_hash_rec_update() with "filter_hash" clear. Before this patch, if we did call __ftrace_hash_rec_update() with "filter_hash" clear, we'd setup: all = false; ... if (filter_hash) { ... } else { hash = ops->func_hash->notrace_hash; other_hash = ops->func_hash->filter_hash; } ... and then at the tail of the loop where we do: count++; [...] /* Shortcut, if we handled all records, we are done. */ if (!all && count == hash->count) { pr_info("HARK: stopping after %d recs\n", count); return update; } ... we'd be checking whether we've updated notrace_hash->count entries, when that could be smaller than the number of entries we actually need to update (e.g. if the notrace hash contains a single entry, but the filter_hash contained more). As above, we never actually hit that because we never call with "filter_hash" clear, so it might be good to explicitly say that before this patch we never actually call __ftrace_hash_rec_update() with "filter_hash" clear, and this is removing dead (and potentially broken) code. > Remove the "filter_hash" parameter from __filter_hash_rec_update() and > comment the function for what it really is doing. > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> FWIW, this looks good to me, and works in testing, so: Reviewed-by: Mark Rutland <mark.rutland@arm.com> Tested-by: Mark Rutland <mark.rutland@arm.com> I have one comment below, but the above stands regardless. [...] > +/* > + * This is the main engine to the ftrace updates to the dyn_ftrace records. > + * > + * It will iterate through all the available ftrace functions > + * (the ones that ftrace can have callbacks to) and set the flags > + * in the associated dyn_ftrace records. > + * > + * @inc: If true, the functions associated to @ops are added to > + * the dyn_ftrace records, otherwise they are removed. > + */ > static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, > - int filter_hash, > bool inc) > { > struct ftrace_hash *hash; > - struct ftrace_hash *other_hash; > + struct ftrace_hash *notrace_hash; > struct ftrace_page *pg; > struct dyn_ftrace *rec; > bool update = false; > @@ -1718,35 +1725,16 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, > return false; > > /* > - * In the filter_hash case: > * If the count is zero, we update all records. > * Otherwise we just update the items in the hash. > - * > - * In the notrace_hash case: > - * We enable the update in the hash. > - * As disabling notrace means enabling the tracing, > - * and enabling notrace means disabling, the inc variable > - * gets inversed. > */ > - if (filter_hash) { > - hash = ops->func_hash->filter_hash; > - other_hash = ops->func_hash->notrace_hash; > - if (ftrace_hash_empty(hash)) > - all = true; > - } else { > - inc = !inc; > - hash = ops->func_hash->notrace_hash; > - other_hash = ops->func_hash->filter_hash; > - /* > - * If the notrace hash has no items, > - * then there's nothing to do. > - */ > - if (ftrace_hash_empty(hash)) > - return false; > - } > + hash = ops->func_hash->filter_hash; > + notrace_hash = ops->func_hash->notrace_hash; > + if (ftrace_hash_empty(hash)) > + all = true; > > do_for_each_ftrace_rec(pg, rec) { > - int in_other_hash = 0; > + int in_notrace_hash = 0; > int in_hash = 0; > int match = 0; > > @@ -1758,26 +1746,17 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, > * Only the filter_hash affects all records. > * Update if the record is not in the notrace hash. > */ > - if (!other_hash || !ftrace_lookup_ip(other_hash, rec->ip)) > + if (!notrace_hash || !ftrace_lookup_ip(notrace_hash, rec->ip)) > match = 1; > } else { > in_hash = !!ftrace_lookup_ip(hash, rec->ip); > - in_other_hash = !!ftrace_lookup_ip(other_hash, rec->ip); > + in_notrace_hash = !!ftrace_lookup_ip(notrace_hash, rec->ip); > > /* > - * If filter_hash is set, we want to match all functions > - * that are in the hash but not in the other hash. > - * > - * If filter_hash is not set, then we are decrementing. > - * That means we match anything that is in the hash > - * and also in the other_hash. That is, we need to turn > - * off functions in the other hash because they are disabled > - * by this hash. > + * We want to match all functions that are in the hash but > + * not in the other hash. > */ > - if (filter_hash && in_hash && !in_other_hash) > - match = 1; > - else if (!filter_hash && in_hash && > - (in_other_hash || ftrace_hash_empty(other_hash))) > + if (in_hash && !in_notrace_hash) > match = 1; > } > if (!match) I wonder how much the (subsequent) shortcut for count == hash->count matters in practice, because if we were happy to get rid of that, we could get rid of 'all', 'count', 'in_hash', 'in_notrace_hash', and simplify the above down to: do_for_each_ftrace_rec(pg, rec) { if (skip_record(rec)) continue; /* * When the hash is empty we update all records. * Otherwise we just update the items in the hash. */ if (!ftrace_hash_empty(hash) && !ftrace_lookup_ip(hash, rec->ip)) continue; if (!ftrace_lookup_ip(notrace_hash, rec->ip)) continue; [...] /* do the actual updates here */ [...] } while_for_each_ftrace_rec(); ... which'd be easier to follow. FWIW, diff atop this patch below. It passes the selftests in my local testing, but I understand if we'd rather keep the shortcut. Mark. ---->8---- diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index f1aab82fa465..165e8dd4f894 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -1714,49 +1714,27 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, struct ftrace_page *pg; struct dyn_ftrace *rec; bool update = false; - int count = 0; - int all = false; /* Only update if the ops has been registered */ if (!(ops->flags & FTRACE_OPS_FL_ENABLED)) return false; - /* - * If the count is zero, we update all records. - * Otherwise we just update the items in the hash. - */ hash = ops->func_hash->filter_hash; notrace_hash = ops->func_hash->notrace_hash; - if (ftrace_hash_empty(hash)) - all = true; do_for_each_ftrace_rec(pg, rec) { - int in_notrace_hash = 0; - int in_hash = 0; - int match = 0; - if (skip_record(rec)) continue; - if (all) { - /* - * Only the filter_hash affects all records. - * Update if the record is not in the notrace hash. - */ - if (!notrace_hash || !ftrace_lookup_ip(notrace_hash, rec->ip)) - match = 1; - } else { - in_hash = !!ftrace_lookup_ip(hash, rec->ip); - in_notrace_hash = !!ftrace_lookup_ip(notrace_hash, rec->ip); + /* + * If the hash is empty, we update all records. + * Otherwise we just update the items in the hash. + */ + if (!ftrace_hash_empty(hash) && + !ftrace_lookup_ip(hash, rec->ip)) + continue; - /* - * We want to match all functions that are in the hash but - * not in the other hash. - */ - if (in_hash && !in_notrace_hash) - match = 1; - } - if (!match) + if (ftrace_lookup_ip(notrace_hash, rec->ip)) continue; if (inc) { @@ -1846,14 +1824,9 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, else rec->flags &= ~FTRACE_FL_CALL_OPS; - count++; - /* Must match FTRACE_UPDATE_CALLS in ftrace_modify_all_code() */ update |= ftrace_test_record(rec, true) != FTRACE_UPDATE_IGNORE; - /* Shortcut, if we handled all records, we are done. */ - if (!all && count == hash->count) - return update; } while_for_each_ftrace_rec(); return update;
On Thu, 6 Jun 2024 18:53:12 +0100 Mark Rutland <mark.rutland@arm.com> wrote: > On Wed, Jun 05, 2024 at 02:03:36PM -0400, Steven Rostedt wrote: > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > > > While adding comments to the function __ftrace_hash_rec_update() and > > trying to describe in detail what the parameter for "ftrace_hash" does, I > > realized that it basically does exactly the same thing (but differently) > > if it is set or not! > > Typo: "ftrace_hash" should be "filter_hash", and likewise in the commit > title. Let me go fix up linux-next :-p > > > If it is set, the idea was the ops->filter_hash was being updated, and the > > code should focus on the functions that are in the ops->filter_hash and > > add them. But it still had to pay attention to the functions in the > > ops->notrace_hash, to ignore them. > > > > If it was cleared, it focused on the ops->notrace_hash, and would add > > functions that were not in the ops->notrace_hash but would still keep > > functions in the "ops->filter_hash". Basically doing the same thing. > > > > In reality, the __ftrace_hash_rec_update() only needs to either remove the > > functions associated to the give ops (if "inc" is set) or remove them (if > > "inc" is cleared). It has to pay attention to both the filter_hash and > > notrace_hash regardless. > > AFAICT, we actually remove a latent bug here, but that bug wasn't > reachable because we never call __ftrace_hash_rec_update() with > "filter_hash" clear. > > Before this patch, if we did call __ftrace_hash_rec_update() with > "filter_hash" clear, we'd setup: > > all = false; > ... > if (filter_hash) { > ... > } else { > hash = ops->func_hash->notrace_hash; > other_hash = ops->func_hash->filter_hash; > } > > ... and then at the tail of the loop where we do: > > count++; > > [...] > > /* Shortcut, if we handled all records, we are done. */ > if (!all && count == hash->count) { > pr_info("HARK: stopping after %d recs\n", count); > return update; > } > > ... we'd be checking whether we've updated notrace_hash->count entries, > when that could be smaller than the number of entries we actually need > to update (e.g. if the notrace hash contains a single entry, but the > filter_hash contained more). I noticed this too as well as: if (filter_hash) { hash = ops->func_hash->filter_hash; other_hash = ops->func_hash->notrace_hash; if (ftrace_hash_empty(hash)) all = true; } else { inc = !inc; hash = ops->func_hash->notrace_hash; other_hash = ops->func_hash->filter_hash; /* * If the notrace hash has no items, * then there's nothing to do. */ if (ftrace_hash_empty(hash)) return false; } That "return false" is actually a mistake as well. But I tried to hit it, but found that I could not. I think I updated the code due to bugs where I prevent that from happening, but the real fix would have been this patch. :-p > > As above, we never actually hit that because we never call with > "filter_hash" clear, so it might be good to explicitly say that before > this patch we never actually call __ftrace_hash_rec_update() with > "filter_hash" clear, and this is removing dead (and potentially broken) > code. Right. > > > Remove the "filter_hash" parameter from __filter_hash_rec_update() and > > comment the function for what it really is doing. > > > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > > FWIW, this looks good to me, and works in testing, so: > > Reviewed-by: Mark Rutland <mark.rutland@arm.com> > Tested-by: Mark Rutland <mark.rutland@arm.com> > > I have one comment below, but the above stands regardless. > > [...] > > > +/* > > + * This is the main engine to the ftrace updates to the dyn_ftrace records. > > + * > > + * It will iterate through all the available ftrace functions > > + * (the ones that ftrace can have callbacks to) and set the flags > > + * in the associated dyn_ftrace records. > > + * > > + * @inc: If true, the functions associated to @ops are added to > > + * the dyn_ftrace records, otherwise they are removed. > > + */ > > static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, > > - int filter_hash, > > bool inc) > > { > > struct ftrace_hash *hash; > > - struct ftrace_hash *other_hash; > > + struct ftrace_hash *notrace_hash; > > struct ftrace_page *pg; > > struct dyn_ftrace *rec; > > bool update = false; > > @@ -1718,35 +1725,16 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, > > return false; > > > > /* > > - * In the filter_hash case: > > * If the count is zero, we update all records. > > * Otherwise we just update the items in the hash. > > - * > > - * In the notrace_hash case: > > - * We enable the update in the hash. > > - * As disabling notrace means enabling the tracing, > > - * and enabling notrace means disabling, the inc variable > > - * gets inversed. > > */ > > - if (filter_hash) { > > - hash = ops->func_hash->filter_hash; > > - other_hash = ops->func_hash->notrace_hash; > > - if (ftrace_hash_empty(hash)) > > - all = true; > > - } else { > > - inc = !inc; > > - hash = ops->func_hash->notrace_hash; > > - other_hash = ops->func_hash->filter_hash; > > - /* > > - * If the notrace hash has no items, > > - * then there's nothing to do. > > - */ > > - if (ftrace_hash_empty(hash)) > > - return false; > > - } > > + hash = ops->func_hash->filter_hash; > > + notrace_hash = ops->func_hash->notrace_hash; > > + if (ftrace_hash_empty(hash)) > > + all = true; > > > > do_for_each_ftrace_rec(pg, rec) { > > - int in_other_hash = 0; > > + int in_notrace_hash = 0; > > int in_hash = 0; > > int match = 0; > > > > @@ -1758,26 +1746,17 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, > > * Only the filter_hash affects all records. > > * Update if the record is not in the notrace hash. > > */ > > - if (!other_hash || !ftrace_lookup_ip(other_hash, rec->ip)) > > + if (!notrace_hash || !ftrace_lookup_ip(notrace_hash, rec->ip)) > > match = 1; > > } else { > > in_hash = !!ftrace_lookup_ip(hash, rec->ip); > > - in_other_hash = !!ftrace_lookup_ip(other_hash, rec->ip); > > + in_notrace_hash = !!ftrace_lookup_ip(notrace_hash, rec->ip); > > > > /* > > - * If filter_hash is set, we want to match all functions > > - * that are in the hash but not in the other hash. > > - * > > - * If filter_hash is not set, then we are decrementing. > > - * That means we match anything that is in the hash > > - * and also in the other_hash. That is, we need to turn > > - * off functions in the other hash because they are disabled > > - * by this hash. > > + * We want to match all functions that are in the hash but > > + * not in the other hash. > > */ > > - if (filter_hash && in_hash && !in_other_hash) > > - match = 1; > > - else if (!filter_hash && in_hash && > > - (in_other_hash || ftrace_hash_empty(other_hash))) > > + if (in_hash && !in_notrace_hash) > > match = 1; > > } > > if (!match) > > I wonder how much the (subsequent) shortcut for count == hash->count > matters in practice, because if we were happy to get rid of that, we > could get rid of 'all', 'count', 'in_hash', 'in_notrace_hash', and > simplify the above down to: > > do_for_each_ftrace_rec(pg, rec) { > if (skip_record(rec)) > continue; > > /* > * When the hash is empty we update all records. > * Otherwise we just update the items in the hash. > */ > if (!ftrace_hash_empty(hash) && > !ftrace_lookup_ip(hash, rec->ip)) > continue; > > if (!ftrace_lookup_ip(notrace_hash, rec->ip)) > continue; > > [...] > /* do the actual updates here */ > [...] > > } while_for_each_ftrace_rec(); > > ... which'd be easier to follow. > > FWIW, diff atop this patch below. It passes the selftests in my local > testing, but I understand if we'd rather keep the shortcut. I'll have to do benchmarks. This loop is what takes up a lot of time when you enable function tracing. It loops over 40,000 records (just do a wc -l available_filter_functions to get the true count). But if you want to send a formal patch, I could test it. Thanks, -- Steve > > Mark. > > ---->8---- > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index f1aab82fa465..165e8dd4f894 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -1714,49 +1714,27 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, > struct ftrace_page *pg; > struct dyn_ftrace *rec; > bool update = false; > - int count = 0; > - int all = false; > > /* Only update if the ops has been registered */ > if (!(ops->flags & FTRACE_OPS_FL_ENABLED)) > return false; > > - /* > - * If the count is zero, we update all records. > - * Otherwise we just update the items in the hash. > - */ > hash = ops->func_hash->filter_hash; > notrace_hash = ops->func_hash->notrace_hash; > - if (ftrace_hash_empty(hash)) > - all = true; > > do_for_each_ftrace_rec(pg, rec) { > - int in_notrace_hash = 0; > - int in_hash = 0; > - int match = 0; > - > if (skip_record(rec)) > continue; > > - if (all) { > - /* > - * Only the filter_hash affects all records. > - * Update if the record is not in the notrace hash. > - */ > - if (!notrace_hash || !ftrace_lookup_ip(notrace_hash, rec->ip)) > - match = 1; > - } else { > - in_hash = !!ftrace_lookup_ip(hash, rec->ip); > - in_notrace_hash = !!ftrace_lookup_ip(notrace_hash, rec->ip); > + /* > + * If the hash is empty, we update all records. > + * Otherwise we just update the items in the hash. > + */ > + if (!ftrace_hash_empty(hash) && > + !ftrace_lookup_ip(hash, rec->ip)) > + continue; > > - /* > - * We want to match all functions that are in the hash but > - * not in the other hash. > - */ > - if (in_hash && !in_notrace_hash) > - match = 1; > - } > - if (!match) > + if (ftrace_lookup_ip(notrace_hash, rec->ip)) > continue; > > if (inc) { > @@ -1846,14 +1824,9 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, > else > rec->flags &= ~FTRACE_FL_CALL_OPS; > > - count++; > - > /* Must match FTRACE_UPDATE_CALLS in ftrace_modify_all_code() */ > update |= ftrace_test_record(rec, true) != FTRACE_UPDATE_IGNORE; > > - /* Shortcut, if we handled all records, we are done. */ > - if (!all && count == hash->count) > - return update; > } while_for_each_ftrace_rec(); > > return update; >
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 9dcdefe9d1aa..f8d8de7adade 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -1383,10 +1383,8 @@ alloc_and_copy_ftrace_hash(int size_bits, struct ftrace_hash *hash) return NULL; } -static void -ftrace_hash_rec_disable_modify(struct ftrace_ops *ops, int filter_hash); -static void -ftrace_hash_rec_enable_modify(struct ftrace_ops *ops, int filter_hash); +static void ftrace_hash_rec_disable_modify(struct ftrace_ops *ops); +static void ftrace_hash_rec_enable_modify(struct ftrace_ops *ops); static int ftrace_hash_ipmodify_update(struct ftrace_ops *ops, struct ftrace_hash *new_hash); @@ -1474,11 +1472,11 @@ ftrace_hash_move(struct ftrace_ops *ops, int enable, * Remove the current set, update the hash and add * them back. */ - ftrace_hash_rec_disable_modify(ops, enable); + ftrace_hash_rec_disable_modify(ops); rcu_assign_pointer(*dst, new_hash); - ftrace_hash_rec_enable_modify(ops, enable); + ftrace_hash_rec_enable_modify(ops); return 0; } @@ -1701,12 +1699,21 @@ static bool skip_record(struct dyn_ftrace *rec) !(rec->flags & FTRACE_FL_ENABLED); } +/* + * This is the main engine to the ftrace updates to the dyn_ftrace records. + * + * It will iterate through all the available ftrace functions + * (the ones that ftrace can have callbacks to) and set the flags + * in the associated dyn_ftrace records. + * + * @inc: If true, the functions associated to @ops are added to + * the dyn_ftrace records, otherwise they are removed. + */ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, - int filter_hash, bool inc) { struct ftrace_hash *hash; - struct ftrace_hash *other_hash; + struct ftrace_hash *notrace_hash; struct ftrace_page *pg; struct dyn_ftrace *rec; bool update = false; @@ -1718,35 +1725,16 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, return false; /* - * In the filter_hash case: * If the count is zero, we update all records. * Otherwise we just update the items in the hash. - * - * In the notrace_hash case: - * We enable the update in the hash. - * As disabling notrace means enabling the tracing, - * and enabling notrace means disabling, the inc variable - * gets inversed. */ - if (filter_hash) { - hash = ops->func_hash->filter_hash; - other_hash = ops->func_hash->notrace_hash; - if (ftrace_hash_empty(hash)) - all = true; - } else { - inc = !inc; - hash = ops->func_hash->notrace_hash; - other_hash = ops->func_hash->filter_hash; - /* - * If the notrace hash has no items, - * then there's nothing to do. - */ - if (ftrace_hash_empty(hash)) - return false; - } + hash = ops->func_hash->filter_hash; + notrace_hash = ops->func_hash->notrace_hash; + if (ftrace_hash_empty(hash)) + all = true; do_for_each_ftrace_rec(pg, rec) { - int in_other_hash = 0; + int in_notrace_hash = 0; int in_hash = 0; int match = 0; @@ -1758,26 +1746,17 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, * Only the filter_hash affects all records. * Update if the record is not in the notrace hash. */ - if (!other_hash || !ftrace_lookup_ip(other_hash, rec->ip)) + if (!notrace_hash || !ftrace_lookup_ip(notrace_hash, rec->ip)) match = 1; } else { in_hash = !!ftrace_lookup_ip(hash, rec->ip); - in_other_hash = !!ftrace_lookup_ip(other_hash, rec->ip); + in_notrace_hash = !!ftrace_lookup_ip(notrace_hash, rec->ip); /* - * If filter_hash is set, we want to match all functions - * that are in the hash but not in the other hash. - * - * If filter_hash is not set, then we are decrementing. - * That means we match anything that is in the hash - * and also in the other_hash. That is, we need to turn - * off functions in the other hash because they are disabled - * by this hash. + * We want to match all functions that are in the hash but + * not in the other hash. */ - if (filter_hash && in_hash && !in_other_hash) - match = 1; - else if (!filter_hash && in_hash && - (in_other_hash || ftrace_hash_empty(other_hash))) + if (in_hash && !in_notrace_hash) match = 1; } if (!match) @@ -1883,24 +1862,21 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops, return update; } -static bool ftrace_hash_rec_disable(struct ftrace_ops *ops, - int filter_hash) +static bool ftrace_hash_rec_disable(struct ftrace_ops *ops) { - return __ftrace_hash_rec_update(ops, filter_hash, 0); + return __ftrace_hash_rec_update(ops, 0); } -static bool ftrace_hash_rec_enable(struct ftrace_ops *ops, - int filter_hash) +static bool ftrace_hash_rec_enable(struct ftrace_ops *ops) { - return __ftrace_hash_rec_update(ops, filter_hash, 1); + return __ftrace_hash_rec_update(ops, 1); } -static void ftrace_hash_rec_update_modify(struct ftrace_ops *ops, - int filter_hash, int inc) +static void ftrace_hash_rec_update_modify(struct ftrace_ops *ops, int inc) { struct ftrace_ops *op; - __ftrace_hash_rec_update(ops, filter_hash, inc); + __ftrace_hash_rec_update(ops, inc); if (ops->func_hash != &global_ops.local_hash) return; @@ -1914,20 +1890,18 @@ static void ftrace_hash_rec_update_modify(struct ftrace_ops *ops, if (op == ops) continue; if (op->func_hash == &global_ops.local_hash) - __ftrace_hash_rec_update(op, filter_hash, inc); + __ftrace_hash_rec_update(op, inc); } while_for_each_ftrace_op(op); } -static void ftrace_hash_rec_disable_modify(struct ftrace_ops *ops, - int filter_hash) +static void ftrace_hash_rec_disable_modify(struct ftrace_ops *ops) { - ftrace_hash_rec_update_modify(ops, filter_hash, 0); + ftrace_hash_rec_update_modify(ops, 0); } -static void ftrace_hash_rec_enable_modify(struct ftrace_ops *ops, - int filter_hash) +static void ftrace_hash_rec_enable_modify(struct ftrace_ops *ops) { - ftrace_hash_rec_update_modify(ops, filter_hash, 1); + ftrace_hash_rec_update_modify(ops, 1); } /* @@ -3050,7 +3024,7 @@ int ftrace_startup(struct ftrace_ops *ops, int command) return ret; } - if (ftrace_hash_rec_enable(ops, 1)) + if (ftrace_hash_rec_enable(ops)) command |= FTRACE_UPDATE_CALLS; ftrace_startup_enable(command); @@ -3092,7 +3066,7 @@ int ftrace_shutdown(struct ftrace_ops *ops, int command) /* Disabling ipmodify never fails */ ftrace_hash_ipmodify_disable(ops); - if (ftrace_hash_rec_disable(ops, 1)) + if (ftrace_hash_rec_disable(ops)) command |= FTRACE_UPDATE_CALLS; ops->flags &= ~FTRACE_OPS_FL_ENABLED;