Re: [PATCH 4/5] ftrace: Make ftrace_hash_rec_enable return update bool

[Namhyung Kim]

On Wed, Mar 09, 2016 at 09:46:44PM +0100, Jiri Olsa wrote:
> Change __ftrace_hash_rec_update to return true in case
> we need to update dynamic ftrace call records. It return
> false in case no update is needed.
> 
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
>  kernel/trace/ftrace.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index eca592f977b2..123dddc660e9 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -1610,7 +1610,7 @@ static bool test_rec_ops_needs_regs(struct dyn_ftrace *rec)
>  	return  keep_regs;
>  }
>  
> -static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
> +static bool __ftrace_hash_rec_update(struct ftrace_ops *ops,
>  				     int filter_hash,
>  				     bool inc)
>  {
> @@ -1618,12 +1618,13 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
>  	struct ftrace_hash *other_hash;
>  	struct ftrace_page *pg;
>  	struct dyn_ftrace *rec;
> +	bool update = false;
>  	int count = 0;
>  	int all = 0;
>  
>  	/* Only update if the ops has been registered */
>  	if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
> -		return;
> +		return false;
>  
>  	/*
>  	 * In the filter_hash case:
> @@ -1650,7 +1651,7 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
>  		 * then there's nothing to do.
>  		 */
>  		if (ftrace_hash_empty(hash))
> -			return;
> +			return false;
>  	}
>  
>  	do_for_each_ftrace_rec(pg, rec) {
> @@ -1694,7 +1695,7 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
>  		if (inc) {
>  			rec->flags++;
>  			if (FTRACE_WARN_ON(ftrace_rec_count(rec) == FTRACE_REF_MAX))
> -				return;
> +				return false;
>  
>  			/*
>  			 * If there's only a single callback registered to a
> @@ -1720,7 +1721,7 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
>  				rec->flags |= FTRACE_FL_REGS;
>  		} else {
>  			if (FTRACE_WARN_ON(ftrace_rec_count(rec) == 0))
> -				return;
> +				return false;
>  			rec->flags--;
>  
>  			/*
> @@ -1753,22 +1754,27 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
>  			 */
>  		}
>  		count++;
> +
> +		update |= ftrace_test_record(rec, 1) != FTRACE_UPDATE_IGNORE;

Shouldn't it use 'inc' instead of 1 for the second argument of
the ftrace_test_record()?

Thanks,
Namhyung


> +
>  		/* Shortcut, if we handled all records, we are done. */
>  		if (!all && count == hash->count)
> -			return;
> +			return update;
>  	} while_for_each_ftrace_rec();
> +
> +	return update;
>  }
>  
> -static void ftrace_hash_rec_disable(struct ftrace_ops *ops,
> +static bool ftrace_hash_rec_disable(struct ftrace_ops *ops,
>  				    int filter_hash)
>  {
> -	__ftrace_hash_rec_update(ops, filter_hash, 0);
> +	return __ftrace_hash_rec_update(ops, filter_hash, 0);
>  }
>  
> -static void ftrace_hash_rec_enable(struct ftrace_ops *ops,
> +static bool ftrace_hash_rec_enable(struct ftrace_ops *ops,
>  				   int filter_hash)
>  {
> -	__ftrace_hash_rec_update(ops, filter_hash, 1);
> +	return __ftrace_hash_rec_update(ops, filter_hash, 1);
>  }
>  
>  static void ftrace_hash_rec_update_modify(struct ftrace_ops *ops,
> -- 
> 2.4.3
>