tracing: Simplify the iteration logic in f_start/f_next

f_next() looks overcomplicated, and it is not strictly correct
even if this doesn't matter.

Say, FORMAT_FIELD_SEPERATOR should not return NULL (means EOF)
if trace_get_fields() returns an empty list, we should simply
advance to FORMAT_PRINTFMT as we do when we find the end of list.

1. Change f_next() to return "struct list_head *" rather than
   "ftrace_event_field *", and change f_show() to do list_entry().

   This simplifies the code a bit, only f_show() needs to know
   about ftrace_event_field, and f_next() can play with ->prev
   directly

2. Change f_next() to not play with ->prev / return inside the
   switch() statement. It can simply set node = head/common_head,
   the prev-or-advance-to-the-next-magic below does all work.

While at it. f_start() looks overcomplicated too. I don't think
*pos == 0 makes sense as a separate case, just change this code
to do "while" instead of "do/while".

The patch also moves f_start() down, close to f_stop(). This is
purely cosmetic, just to make the locking added by the next patch
more clear/visible.

Link: http://lkml.kernel.org/r/20130718184710.GA4783@redhat.com
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 7a75cb22eab76150bbfae09a3189ef659d4da65f..76defd91f9b4f372d0db9717dec3d7a9892338d3 100644 (file)
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -826,59 +826,33 @@ enum {
 static void *f_next(struct seq_file *m, void *v, loff_t *pos)
 {
        struct ftrace_event_call *call = m->private;
-       struct ftrace_event_field *field;
        struct list_head *common_head = &ftrace_common_fields;
        struct list_head *head = trace_get_fields(call);
+       struct list_head *node = v;
 
        (*pos)++;
 
        switch ((unsigned long)v) {
        case FORMAT_HEADER:
-               if (unlikely(list_empty(common_head)))
-                       return NULL;
-
-               field = list_entry(common_head->prev,
-                                  struct ftrace_event_field, link);
-               return field;
+               node = common_head;
+               break;
 
        case FORMAT_FIELD_SEPERATOR:
-               if (unlikely(list_empty(head)))
-                       return NULL;
-
-               field = list_entry(head->prev, struct ftrace_event_field, link);
-               return field;
+               node = head;
+               break;
 
        case FORMAT_PRINTFMT:
                /* all done */
                return NULL;
        }
 
-       field = v;
-       if (field->link.prev == common_head)
+       node = node->prev;
+       if (node == common_head)
                return (void *)FORMAT_FIELD_SEPERATOR;
-       else if (field->link.prev == head)
+       else if (node == head)
                return (void *)FORMAT_PRINTFMT;
-
-       field = list_entry(field->link.prev, struct ftrace_event_field, link);
-
-       return field;
-}
-
-static void *f_start(struct seq_file *m, loff_t *pos)
-{
-       loff_t l = 0;
-       void *p;
-
-       /* Start by showing the header */
-       if (!*pos)
-               return (void *)FORMAT_HEADER;
-
-       p = (void *)FORMAT_HEADER;
-       do {
-               p = f_next(m, p, &l);
-       } while (p && l < *pos);
-
-       return p;
+       else
+               return node;
 }
 
 static int f_show(struct seq_file *m, void *v)
@@ -904,8 +878,7 @@ static int f_show(struct seq_file *m, void *v)
                return 0;
        }
 
-       field = v;
-
+       field = list_entry(v, struct ftrace_event_field, link);
        /*
         * Smartly shows the array type(except dynamic array).
         * Normal:
@@ -932,6 +905,17 @@ static int f_show(struct seq_file *m, void *v)
        return 0;
 }
 
+static void *f_start(struct seq_file *m, loff_t *pos)
+{
+       void *p = (void *)FORMAT_HEADER;
+       loff_t l = 0;
+
+       while (l < *pos && p)
+               p = f_next(m, p, &l);
+
+       return p;
+}
+
 static void f_stop(struct seq_file *m, void *p)
 {
 }