Trivial cleanups relates to ftrace.
Wei Yang (6):
ftrace: define seq_file only for FMODE_READ
ftrace: use fls() to get the bits for dup_hash()
ftrace: simplify the dyn_ftrace->flags macro
ftrace: simplify the calculation of page number for
ftrace_page->records
ftrace: replace do_for_each_ftrace_rec() with for_ftrace_rec_iter()
ftrace: ftrace_global_list is renamed to ftrace_ops_list
include/linux/ftrace.h | 11 ++---
kernel/trace/ftrace.c | 108 ++++++++++++++++++++---------------------
2 files changed, 56 insertions(+), 63 deletions(-)
--
2.20.1 (Apple Git-117)
All the three macro are defined to be used for ftrace_rec_count(). This
can be achieved by (flags & FTRACE_REF_MAX) directly.
Since no other places would use those macros, remove them for clarity.
Also it fixes a typo in the comment.
Signed-off-by: Wei Yang <[email protected]>
---
include/linux/ftrace.h | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 3e3fe779a8c2..23c4d6526998 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -433,7 +433,7 @@ bool is_ftrace_trampoline(unsigned long addr);
* DIRECT - there is a direct function to call
*
* When a new ftrace_ops is registered and wants a function to save
- * pt_regs, the rec->flag REGS is set. When the function has been
+ * pt_regs, the rec->flags REGS is set. When the function has been
* set up to save regs, the REG_EN flag is set. Once a function
* starts saving regs it will do so until all ftrace_ops are removed
* from tracing that function.
@@ -451,12 +451,9 @@ enum {
};
#define FTRACE_REF_MAX_SHIFT 23
-#define FTRACE_FL_BITS 9
-#define FTRACE_FL_MASKED_BITS ((1UL << FTRACE_FL_BITS) - 1)
-#define FTRACE_FL_MASK (FTRACE_FL_MASKED_BITS << FTRACE_REF_MAX_SHIFT)
#define FTRACE_REF_MAX ((1UL << FTRACE_REF_MAX_SHIFT) - 1)
-#define ftrace_rec_count(rec) ((rec)->flags & ~FTRACE_FL_MASK)
+#define ftrace_rec_count(rec) ((rec)->flags & FTRACE_REF_MAX)
struct dyn_ftrace {
unsigned long ip; /* address of mcount call-site */
--
2.20.1 (Apple Git-117)
Based on the following two reasones, we could simplify the calculation:
- If the number after roundup count is not power of 2, we would
definitely have more than 1 empty page with a higher order.
- get_count_order() just return current order, so one lower order
could meet the requirement.
The calculation could be simplified by lower one order level when pages
are not power of 2.
Signed-off-by: Wei Yang <[email protected]>
---
kernel/trace/ftrace.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 9021e16fa079..15fcfa16895d 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3127,18 +3127,19 @@ static int ftrace_update_code(struct module *mod, struct ftrace_page *new_pgs)
static int ftrace_allocate_records(struct ftrace_page *pg, int count)
{
int order;
- int cnt;
+ int pages, cnt;
if (WARN_ON(!count))
return -EINVAL;
- order = get_count_order(DIV_ROUND_UP(count, ENTRIES_PER_PAGE));
+ pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);
+ order = get_count_order(pages);
/*
* We want to fill as much as possible. No more than a page
* may be empty.
*/
- while ((PAGE_SIZE << order) / ENTRY_SIZE >= count + ENTRIES_PER_PAGE)
+ if (!is_power_of_2(pages))
order--;
again:
--
2.20.1 (Apple Git-117)
The purpose of the operation is to get ftrace_iterator, which is embedded
in file or seq_file for FMODE_WRITE/FMODE_READ respectively. Since we
don't have a seq_file for FMODE_WRITE case, it is meaningless to cast
file->private_data to seq_file.
Let's move the definition when there is a valid seq_file.
Signed-off-by: Wei Yang <[email protected]>
---
kernel/trace/ftrace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index edc233122598..12cb535769bc 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5558,7 +5558,6 @@ static void __init set_ftrace_early_filters(void)
int ftrace_regex_release(struct inode *inode, struct file *file)
{
- struct seq_file *m = (struct seq_file *)file->private_data;
struct ftrace_iterator *iter;
struct ftrace_hash **orig_hash;
struct trace_parser *parser;
@@ -5566,6 +5565,7 @@ int ftrace_regex_release(struct inode *inode, struct file *file)
int ret;
if (file->f_mode & FMODE_READ) {
+ struct seq_file *m = file->private_data;
iter = m->private;
seq_release(inode, file);
} else
--
2.20.1 (Apple Git-117)
The effect here is to get the number of bits, lets use fls() to do
this job.
Signed-off-by: Wei Yang <[email protected]>
---
kernel/trace/ftrace.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 12cb535769bc..9021e16fa079 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1370,8 +1370,9 @@ static struct ftrace_hash *dup_hash(struct ftrace_hash *src, int size)
/*
* Make the hash size about 1/2 the # found
*/
- for (size /= 2; size; size >>= 1)
- bits++;
+ bits = fls(size);
+ if (bits)
+ bits--;
/* Don't allocate too much */
if (bits > FTRACE_HASH_MAX_BITS)
--
2.20.1 (Apple Git-117)
On Mon, 31 Aug 2020 11:10:59 +0800
Wei Yang <[email protected]> wrote:
> The purpose of the operation is to get ftrace_iterator, which is embedded
> in file or seq_file for FMODE_WRITE/FMODE_READ respectively. Since we
> don't have a seq_file for FMODE_WRITE case, it is meaningless to cast
> file->private_data to seq_file.
>
> Let's move the definition when there is a valid seq_file.
I didn't pull in this patch because I find the original more expressive,
and there's really no benefit in changing it.
-- Steve
>
> Signed-off-by: Wei Yang <[email protected]>
> ---
> kernel/trace/ftrace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index edc233122598..12cb535769bc 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5558,7 +5558,6 @@ static void __init set_ftrace_early_filters(void)
>
> int ftrace_regex_release(struct inode *inode, struct file *file)
> {
> - struct seq_file *m = (struct seq_file *)file->private_data;
> struct ftrace_iterator *iter;
> struct ftrace_hash **orig_hash;
> struct trace_parser *parser;
> @@ -5566,6 +5565,7 @@ int ftrace_regex_release(struct inode *inode, struct file *file)
> int ret;
>
> if (file->f_mode & FMODE_READ) {
> + struct seq_file *m = file->private_data;
> iter = m->private;
> seq_release(inode, file);
> } else
On Tue, Oct 06, 2020 at 10:36:38AM -0400, Steven Rostedt wrote:
>On Mon, 31 Aug 2020 11:10:59 +0800
>Wei Yang <[email protected]> wrote:
>
>> The purpose of the operation is to get ftrace_iterator, which is embedded
>> in file or seq_file for FMODE_WRITE/FMODE_READ respectively. Since we
>> don't have a seq_file for FMODE_WRITE case, it is meaningless to cast
>> file->private_data to seq_file.
>>
>> Let's move the definition when there is a valid seq_file.
>
>I didn't pull in this patch because I find the original more expressive,
>and there's really no benefit in changing it.
>
Got it, thanks
>-- Steve
>
>
>>
>> Signed-off-by: Wei Yang <[email protected]>
>> ---
>> kernel/trace/ftrace.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index edc233122598..12cb535769bc 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -5558,7 +5558,6 @@ static void __init set_ftrace_early_filters(void)
>>
>> int ftrace_regex_release(struct inode *inode, struct file *file)
>> {
>> - struct seq_file *m = (struct seq_file *)file->private_data;
>> struct ftrace_iterator *iter;
>> struct ftrace_hash **orig_hash;
>> struct trace_parser *parser;
>> @@ -5566,6 +5565,7 @@ int ftrace_regex_release(struct inode *inode, struct file *file)
>> int ret;
>>
>> if (file->f_mode & FMODE_READ) {
>> + struct seq_file *m = file->private_data;
>> iter = m->private;
>> seq_release(inode, file);
>> } else
--
Wei Yang
Help you, Help me