On Fri, 9 Feb 2018 10:17:45 +0900
Namhyung Kim [off-list ref] wrote:

quoted

+ # echo 1 > events/functions/ip_rcv/enable
+ # cat trace
+    <idle>-0     [003] ..s3   219.813582: __netif_receive_skb_core->ip_rcv(skb=ffff880118195e00, perm_addr=b4,b5,2f,ce,18,65)
+    <idle>-0     [003] ..s3   219.813595: __netif_receive_skb_core->ip_rcv(skb=ffff880118195e00, perm_addr=b4,b5,2f,ce,18,65)
+    <idle>-0     [003] ..s3   220.115053: __netif_receive_skb_core->ip_rcv(skb=ffff880118195c00, perm_addr=b4,b5,2f,ce,18,65)
+    <idle>-0     [003] ..s3   220.115293: __netif_receive_skb_core->ip_rcv(skb=ffff880118195c00, perm_addr=b4,b5,2f,ce,18,65)  

What about adding braces to indicate array type like below?

... ip_rcv(skb=ffff880118195c00, perm_addr={b4,b5,2f,ce,18,65})

That's a nice idea, I'll add it.

quoted

+	case FUNC_STATE_ARRAY:
 	case FUNC_STATE_BRACKET:
-		WARN_ON(!fevent->last_arg);
+		if (WARN_ON(!fevent->last_arg))
+			break;
 		ret = kstrtoul(token, 0, &val);
 		if (ret)
 			break;
-		val *= fevent->last_arg->size;
-		fevent->last_arg->indirect = val ^ INDIRECT_FLAG;
-		return FUNC_STATE_INDIRECT;
+		if (state == FUNC_STATE_BRACKET) {
+			val *= fevent->last_arg->size;
+			fevent->last_arg->indirect = val ^ INDIRECT_FLAG;
+			return FUNC_STATE_INDIRECT;
+		}
+		if (val <= 0)
+			break;  

The val is unsigned long type.

I probably should make it a cap it for the array, as arrays that are
too big will simply fail to allocate on the ring buffer.

But it should only check for zero.

quoted

+		fevent->last_arg->array = val;
+		type = kasprintf(GFP_KERNEL, "%s[%d]", fevent->last_arg->type, (unsigned)val);  

s/%d/%lu/  and no need to cast it.

Sure.

quoted

+		if (!type)
+			break;
+		kfree(fevent->last_arg->type);
+		fevent->last_arg->type = type;
+		/*
+		 * arg_offset has already been updated once by size.
+		 * This update needs to account for that (hence the "- 1").
+		 */
+		fevent->arg_offset += fevent->last_arg->size * (fevent->last_arg->array - 1);
+		return FUNC_STATE_ARRAY_SIZE;
+
+	case FUNC_STATE_ARRAY_SIZE:
+		if (token[0] != ']')
+			break;
+		return FUNC_STATE_ARRAY_END;
 
 	case FUNC_STATE_INDIRECT:
 		if (token[0] != ']')

@@ -453,6 +485,10 @@ static long long get_arg(struct func_arg *arg, unsigned long val)
 
 	val = val + (arg->indirect ^ INDIRECT_FLAG);
 
+	/* Arrays do their own indirect reads */
+	if (arg->array)
+		return val;
+  

Not sure about this.  After this change it would make 'x64[1] foo' and
'x64[1] foo[0]' equivalent, right?

Yeah, I may need to re-think this. I originally had the "array"
use the "indirect" code, but I'm thinking that isn't necessary.

Thanks for the input.

-- Steve