[PATCH] trace_syscalls: cleanup for syscalls_metadata

[PATCH] trace_syscalls: cleanup for syscalls_metadata

[Lai Jiangshan]

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

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index f694f66..c7db995 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -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);

Re: [PATCH] trace_syscalls: cleanup for syscalls_metadata

[Steven Rostedt]

On Wed, 2010-01-06 at 20:39 +0800, Lai Jiangshan wrote:
> 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.

> so we allocates memory for syscalls_metadata at compile-time.
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>

-- Steve

Re: [PATCH] trace_syscalls: cleanup for syscalls_metadata

[Lai Jiangshan]

Steven Rostedt wrote:
> On Wed, 2010-01-06 at 20:39 +0800, Lai Jiangshan wrote:
>> 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

Re: [PATCH] trace_syscalls: cleanup for syscalls_metadata

[Steven Rostedt]

On Thu, 2010-01-07 at 09:01 +0800, Lai Jiangshan wrote:
> 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