Skip to contentSkip to conversation

trace_syscalls: cleanup for syscalls_metadata

PATCH In review Lai Jiangshan 2 participants 4 messages Jan 6, 2010 – Jan 7, 2010 Tracing

lore
Share & tools
lore

Lai Jiangshan

author
#0 lore raw

PATCH 1/1 trace_syscalls: cleanup for syscalls_metadata

NR_syscalls is compile-time constant.
so we allocates memory for syscalls_metadata at compile-time.

Signed-off-by Lai Jiangshan <[email protected]>

---

kernel/trace/trace_syscalls.c +2 -9
index f694f66..c7db995 100644
@@ -17,7 +17,7 @@ static DECLARE_BITMAP(enabled_exit_syscalls, NR_syscalls);
 extern unsigned long __start_syscalls_metadata[];
 extern unsigned long __stop_syscalls_metadata[];
 
-static struct syscall_metadata **syscalls_metadata;
+static struct syscall_metadata *syscalls_metadata[NR_syscalls];
 
 static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
 {

@@ -45,7 +45,7 @@ static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
 
 static struct syscall_metadata *syscall_nr_to_meta(int nr)
 {
-	if (!syscalls_metadata || nr >= NR_syscalls || nr < 0)
+	if (nr >= NR_syscalls || nr < 0)
 		return NULL;
 
 	return syscalls_metadata[nr];

@@ -400,13 +400,6 @@ int __init init_ftrace_syscalls(void)
 	unsigned long addr;
 	int i;
 
-	syscalls_metadata = kzalloc(sizeof(*syscalls_metadata) *
-					NR_syscalls, GFP_KERNEL);
-	if (!syscalls_metadata) {
-		WARN_ON(1);
-		return -ENOMEM;
-	}
-
 	for (i = 0; i < NR_syscalls; i++) {
 		addr = arch_syscall_addr(i);
 		meta = find_syscall_meta(addr);
maintainer
#1 lore raw
Quoted 1 line · Lai Jiangshan

NR_syscalls is compile-time constant.

The question is, is that always true on all archs, and will that always be true?

Unless we can guarantee somewhere that NR_syscalls must be constant on all archs, we can't make this change. If it is guaranteed, then sure, this patch is fine.

Quoted 3 lines

so we allocates memory for syscalls_metadata at compile-time.

Signed-off-by Lai Jiangshan <[email protected]>

-- Steve

author
#2 lore raw
Quoted 10 lines · Steven Rostedt
Quoted 1 line · Lai Jiangshan

NR_syscalls is compile-time constant.

The question is, is that always true on all archs, and will that always be true?

Unless we can guarantee somewhere that NR_syscalls must be constant on all archs, we can't make this change. If it is guaranteed, then sure, this patch is fine.

It seems that there is no spec/document guarantees it.

I searched all NR_syscalls in current kernel source code, and I found it is a constant in header file (all archs).

trace_syscalls.c has already used it as a compile-time constant:

static DECLARE_BITMAP(enabled_enter_syscalls, NR_syscalls); static DECLARE_BITMAP(enabled_exit_syscalls, NR_syscalls);

Thanks, Lai

maintainer
#3 lore raw
Quoted 11 lines · Lai Jiangshan

Steven Rostedt wrote:

It seems that there is no spec/document guarantees it.

I searched all NR_syscalls in current kernel source code, and I found it is a constant in header file (all archs).

trace_syscalls.c has already used it as a compile-time constant:

static DECLARE_BITMAP(enabled_enter_syscalls, NR_syscalls); static DECLARE_BITMAP(enabled_exit_syscalls, NR_syscalls);

I guess no one complained about that yet. But that could be because no one noticed.

Still, is there a reason to make that change other than just a "clean up"? What advantage do we have by changing the array into a static array instead of allocating it at run time?

-- Steve