tracing: probe-events: Cleanup entry-arg storing code

Cleanup __store_entry_arg() so that it is easier to understand.
The main complexity may come from combining the loops for finding
stored-entry-arg and max-offset and appending new entry.

This split those different loops into 3 parts, lookup the same
entry-arg, find the max offset and append new entry.

Link: https://lore.kernel.org/all/174323039929.348535.4705349977127704120.stgit@devnote2/
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 424751cdf31f9f10aa9ca6464be05879320db42d..abfab8957a6cdb28a36ad776b0f2bea5776bf124 100644 (file)
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -779,6 +779,36 @@ static int check_prepare_btf_string_fetch(char *typename,
 
 #ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
 
+static void store_entry_arg_at(struct fetch_insn *code, int argnum, int offset)
+{
+       code[0].op = FETCH_OP_ARG;
+       code[0].param = argnum;
+       code[1].op = FETCH_OP_ST_EDATA;
+       code[1].offset = offset;
+}
+
+static int get_entry_arg_max_offset(struct probe_entry_arg *earg)
+{
+       int i, max_offset = 0;
+
+       /*
+        * earg->code[] array has an operation sequence which is run in
+        * the entry handler.
+        * The sequence stopped by FETCH_OP_END and each data stored in
+        * the entry data buffer by FETCH_OP_ST_EDATA. The FETCH_OP_ST_EDATA
+        * stores the data at the data buffer + its offset, and all data are
+        * "unsigned long" size. The offset must be increased when a data is
+        * stored. Thus we need to find the last FETCH_OP_ST_EDATA in the
+        * code array.
+        */
+       for (i = 0; i < earg->size - 1 && earg->code[i].op != FETCH_OP_END; i++) {
+               if (earg->code[i].op == FETCH_OP_ST_EDATA)
+                       if (earg->code[i].offset > max_offset)
+                               max_offset = earg->code[i].offset;
+       }
+       return max_offset;
+}
+
 /*
  * Add the entry code to store the 'argnum'th parameter and return the offset
  * in the entry data buffer where the data will be stored.
@@ -786,8 +816,7 @@ static int check_prepare_btf_string_fetch(char *typename,
 static int __store_entry_arg(struct trace_probe *tp, int argnum)
 {
        struct probe_entry_arg *earg = tp->entry_arg;
-       bool match = false;
-       int i, offset;
+       int i, offset, last_offset = 0;
 
        if (!earg) {
                earg = kzalloc(sizeof(*tp->entry_arg), GFP_KERNEL);
@@ -804,78 +833,59 @@ static int __store_entry_arg(struct trace_probe *tp, int argnum)
                for (i = 0; i < earg->size; i++)
                        earg->code[i].op = FETCH_OP_END;
                tp->entry_arg = earg;
+               store_entry_arg_at(earg->code, argnum, 0);
+               return 0;
        }
 
        /*
-        * The entry code array is repeating the pair of
-        * [FETCH_OP_ARG(argnum)][FETCH_OP_ST_EDATA(offset of entry data buffer)]
-        * and the rest of entries are filled with [FETCH_OP_END].
+        * NOTE: if anyone change the following rule, please rewrite this.
+        * The entry code array is filled with the pair of
+        *
+        * [FETCH_OP_ARG(argnum)]
+        * [FETCH_OP_ST_EDATA(offset of entry data buffer)]
         *
-        * To reduce the redundant function parameter fetching, we scan the entry
-        * code array to find the FETCH_OP_ARG which already fetches the 'argnum'
-        * parameter. If it doesn't match, update 'offset' to find the last
-        * offset.
-        * If we find the FETCH_OP_END without matching FETCH_OP_ARG entry, we
-        * will save the entry with FETCH_OP_ARG and FETCH_OP_ST_EDATA, and
-        * return data offset so that caller can find the data offset in the entry
-        * data buffer.
+        * and the rest of entries are filled with [FETCH_OP_END].
+        * The offset should be incremented, thus the last pair should
+        * have the largest offset.
         */
-       offset = 0;
-       for (i = 0; i < earg->size - 1; i++) {
-               switch (earg->code[i].op) {
-               case FETCH_OP_END:
-                       earg->code[i].op = FETCH_OP_ARG;
-                       earg->code[i].param = argnum;
-                       earg->code[i + 1].op = FETCH_OP_ST_EDATA;
-                       earg->code[i + 1].offset = offset;
-                       return offset;
-               case FETCH_OP_ARG:
-                       match = (earg->code[i].param == argnum);
-                       break;
-               case FETCH_OP_ST_EDATA:
-                       offset = earg->code[i].offset;
-                       if (match)
-                               return offset;
-                       offset += sizeof(unsigned long);
-                       break;
-               default:
-                       break;
-               }
+
+       /* Search the offset for the sprcified argnum. */
+       for (i = 0; i < earg->size - 1 && earg->code[i].op != FETCH_OP_END; i += 2) {
+               if (WARN_ON_ONCE(earg->code[i].op != FETCH_OP_ARG))
+                       return -EINVAL;
+
+               if (earg->code[i].param != argnum)
+                       continue;
+
+               if (WARN_ON_ONCE(earg->code[i + 1].op != FETCH_OP_ST_EDATA))
+                       return -EINVAL;
+
+               return earg->code[i + 1].offset;
        }
-       return -ENOSPC;
+       /* Not found, append new entry if possible. */
+       if (i >= earg->size - 1)
+               return -ENOSPC;
+
+       /* The last entry must have the largest offset. */
+       if (i != 0) {
+               if (WARN_ON_ONCE(earg->code[i - 1].op != FETCH_OP_ST_EDATA))
+                       return -EINVAL;
+               last_offset = earg->code[i - 1].offset;
+       }
+
+       offset = last_offset + sizeof(unsigned long);
+       store_entry_arg_at(&earg->code[i], argnum, offset);
+       return offset;
 }
 
 int traceprobe_get_entry_data_size(struct trace_probe *tp)
 {
        struct probe_entry_arg *earg = tp->entry_arg;
-       int i, size = 0;
 
        if (!earg)
                return 0;
 
-       /*
-        * earg->code[] array has an operation sequence which is run in
-        * the entry handler.
-        * The sequence stopped by FETCH_OP_END and each data stored in
-        * the entry data buffer by FETCH_OP_ST_EDATA. The FETCH_OP_ST_EDATA
-        * stores the data at the data buffer + its offset, and all data are
-        * "unsigned long" size. The offset must be increased when a data is
-        * stored. Thus we need to find the last FETCH_OP_ST_EDATA in the
-        * code array.
-        */
-       for (i = 0; i < earg->size; i++) {
-               switch (earg->code[i].op) {
-               case FETCH_OP_END:
-                       goto out;
-               case FETCH_OP_ST_EDATA:
-                       size = earg->code[i].offset + sizeof(unsigned long);
-                       break;
-               default:
-                       break;
-               }
-       }
-out:
-       return size;
+       return get_entry_arg_max_offset(earg) + sizeof(unsigned long);
 }
 
 void store_trace_entry_data(void *edata, struct trace_probe *tp, struct pt_regs *regs)