2010-02-02 21:22:07

by Jason Baron

[permalink] [raw]
Subject: [PATCH 0/6] tracing: add compat syscall support

Hi,

This series adds 'compat' syscall tracing support, and updates x86_64 to use
this new functionality. Other arches still need to be implemented.

To provide arch support 3 interfaces need to be defined:
1) int is_compat_task(void);
- most arches seem to have this already
2) unsigned long arch_compat_syscall_addr(int nr);
- returns a pointer to the compat syscall entry corresponding to syscall 'nr'
3) int NR_syscalls_compat;
- number of entries in the compat syscall table.

Patches #1 and #2 in the series, implement the "NR_syscalls_compat" and the
"arch_compat_syscall_addr()" for x86.

Patches #3 and #4 add the necessary 'core' support for compat syscalls to the
event tracing code.

Finally, patches #5 and #6 convert 28 x86 compat syscalls to use the
'DEFINE_SYSCALL' macros, which is necessary for syscall tracing.

thanks,

-Jason


Jason Baron (6):
-x86: add NR_syscalls_compat, make ia32 syscall table visible
-x86: add arch_compat_syscall_addr()
-tracing: remove syscall bitmaps in preparation for compat support
-tracing: add tracing support for compat syscalls
-syscalls: add define syscall prefix macro
-x86: convert compat syscalls to use 'DEFINE_SYSCALL()' macros

arch/x86/ia32/ia32entry.S | 3 +
arch/x86/ia32/sys_ia32.c | 106 ++++++++++++++++++++--------------------
arch/x86/include/asm/compat.h | 2 +
arch/x86/kernel/ftrace.c | 9 ++++
include/linux/compat.h | 8 +++
include/linux/syscalls.h | 16 ++++++
include/trace/syscall.h | 8 +++
kernel/trace/trace.h | 2 +
kernel/trace/trace_syscalls.c | 101 ++++++++++++++++++++++++++++-----------
9 files changed, 174 insertions(+), 81 deletions(-)


2010-02-02 21:21:58

by Jason Baron

[permalink] [raw]
Subject: [PATCH 2/6] x86: add arch_compat_syscall_addr()

Add arch_compat_syscall_addr(int nr) for x86. This is in preparation for adding
compat syscall support to the event tracer.

Signed-off-by: Jason Baron <[email protected]>
---
arch/x86/kernel/ftrace.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 3096892..da2568c 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -488,9 +488,18 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
#ifdef CONFIG_FTRACE_SYSCALLS

extern unsigned long *sys_call_table;
+extern unsigned long *ia32_sys_call_table;

unsigned long __init arch_syscall_addr(int nr)
{
return (unsigned long)(&sys_call_table)[nr];
}
+
+#ifdef CONFIG_COMPAT
+unsigned long __init arch_compat_syscall_addr(int nr)
+{
+ return (unsigned long)(&ia32_sys_call_table)[nr];
+}
+#endif
+
#endif
--
1.6.5.1

2010-02-02 21:22:28

by Jason Baron

[permalink] [raw]
Subject: [PATCH 6/6] x86: convert compat syscalls to use 'DEFINE_SYSCALL()' macros

Convert x86 compat syscalls to use SYSCALL_PREFIX_DEFINE#() macros. This allows
us to hook the compat syscalls into the event tracer.

Signed-off-by: Jason Baron <[email protected]>
---
arch/x86/ia32/sys_ia32.c | 106 +++++++++++++++++++++++-----------------------
1 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/arch/x86/ia32/sys_ia32.c b/arch/x86/ia32/sys_ia32.c
index 422572c..751d950 100644
--- a/arch/x86/ia32/sys_ia32.c
+++ b/arch/x86/ia32/sys_ia32.c
@@ -50,15 +50,15 @@
#define AA(__x) ((unsigned long)(__x))


-asmlinkage long sys32_truncate64(char __user *filename,
- unsigned long offset_low,
- unsigned long offset_high)
+SYSCALL_PREFIX_DEFINE3(32_, truncate64, char __user *, filename,
+ unsigned long, offset_low,
+ unsigned long, offset_high)
{
return sys_truncate(filename, ((loff_t) offset_high << 32) | offset_low);
}

-asmlinkage long sys32_ftruncate64(unsigned int fd, unsigned long offset_low,
- unsigned long offset_high)
+SYSCALL_PREFIX_DEFINE3(32_, ftruncate64, unsigned int, fd, unsigned long, offset_low,
+ unsigned long, offset_high)
{
return sys_ftruncate(fd, ((loff_t) offset_high << 32) | offset_low);
}
@@ -95,8 +95,8 @@ static int cp_stat64(struct stat64 __user *ubuf, struct kstat *stat)
return 0;
}

-asmlinkage long sys32_stat64(char __user *filename,
- struct stat64 __user *statbuf)
+SYSCALL_PREFIX_DEFINE2(32_, stat64, char __user *, filename,
+ struct stat64 __user *, statbuf)
{
struct kstat stat;
int ret = vfs_stat(filename, &stat);
@@ -106,8 +106,8 @@ asmlinkage long sys32_stat64(char __user *filename,
return ret;
}

-asmlinkage long sys32_lstat64(char __user *filename,
- struct stat64 __user *statbuf)
+SYSCALL_PREFIX_DEFINE2(32_, lstat64, char __user *, filename,
+ struct stat64 __user *, statbuf)
{
struct kstat stat;
int ret = vfs_lstat(filename, &stat);
@@ -116,7 +116,7 @@ asmlinkage long sys32_lstat64(char __user *filename,
return ret;
}

-asmlinkage long sys32_fstat64(unsigned int fd, struct stat64 __user *statbuf)
+SYSCALL_PREFIX_DEFINE2(32_, fstat64, unsigned int, fd, struct stat64 __user *, statbuf)
{
struct kstat stat;
int ret = vfs_fstat(fd, &stat);
@@ -125,8 +125,8 @@ asmlinkage long sys32_fstat64(unsigned int fd, struct stat64 __user *statbuf)
return ret;
}

-asmlinkage long sys32_fstatat(unsigned int dfd, char __user *filename,
- struct stat64 __user *statbuf, int flag)
+SYSCALL_PREFIX_DEFINE4(32_, fstatat, unsigned int, dfd, char __user *, filename,
+ struct stat64 __user *, statbuf, int, flag)
{
struct kstat stat;
int error;
@@ -152,7 +152,7 @@ struct mmap_arg_struct {
unsigned int offset;
};

-asmlinkage long sys32_mmap(struct mmap_arg_struct __user *arg)
+SYSCALL_PREFIX_DEFINE1(32_, mmap, struct mmap_arg_struct __user *, arg)
{
struct mmap_arg_struct a;

@@ -166,15 +166,15 @@ asmlinkage long sys32_mmap(struct mmap_arg_struct __user *arg)
a.offset>>PAGE_SHIFT);
}

-asmlinkage long sys32_mprotect(unsigned long start, size_t len,
- unsigned long prot)
+SYSCALL_PREFIX_DEFINE3(32_, mprotect, unsigned long, start, size_t, len,
+ unsigned long, prot)
{
return sys_mprotect(start, len, prot);
}

-asmlinkage long sys32_rt_sigaction(int sig, struct sigaction32 __user *act,
- struct sigaction32 __user *oact,
- unsigned int sigsetsize)
+SYSCALL_PREFIX_DEFINE4(32_, rt_sigaction, int, sig, struct sigaction32 __user *, act,
+ struct sigaction32 __user *, oact,
+ unsigned int, sigsetsize)
{
struct k_sigaction new_ka, old_ka;
int ret;
@@ -248,8 +248,8 @@ asmlinkage long sys32_rt_sigaction(int sig, struct sigaction32 __user *act,
return ret;
}

-asmlinkage long sys32_sigaction(int sig, struct old_sigaction32 __user *act,
- struct old_sigaction32 __user *oact)
+SYSCALL_PREFIX_DEFINE3(32_, sigaction, int, sig, struct old_sigaction32 __user *, act,
+ struct old_sigaction32 __user *, oact)
{
struct k_sigaction new_ka, old_ka;
int ret;
@@ -287,9 +287,9 @@ asmlinkage long sys32_sigaction(int sig, struct old_sigaction32 __user *act,
return ret;
}

-asmlinkage long sys32_rt_sigprocmask(int how, compat_sigset_t __user *set,
- compat_sigset_t __user *oset,
- unsigned int sigsetsize)
+SYSCALL_PREFIX_DEFINE4(32_, rt_sigprocmask, int, how, compat_sigset_t __user *, set,
+ compat_sigset_t __user *, oset,
+ unsigned int, sigsetsize)
{
sigset_t s;
compat_sigset_t s32;
@@ -327,7 +327,7 @@ asmlinkage long sys32_rt_sigprocmask(int how, compat_sigset_t __user *set,
return 0;
}

-asmlinkage long sys32_alarm(unsigned int seconds)
+SYSCALL_PREFIX_DEFINE1(32_, alarm, unsigned int, seconds)
{
return alarm_setitimer(seconds);
}
@@ -340,7 +340,7 @@ struct sel_arg_struct {
unsigned int tvp;
};

-asmlinkage long sys32_old_select(struct sel_arg_struct __user *arg)
+SYSCALL_PREFIX_DEFINE1(32_, old_select, struct sel_arg_struct __user *, arg)
{
struct sel_arg_struct a;

@@ -350,21 +350,21 @@ asmlinkage long sys32_old_select(struct sel_arg_struct __user *arg)
compat_ptr(a.exp), compat_ptr(a.tvp));
}

-asmlinkage long sys32_waitpid(compat_pid_t pid, unsigned int *stat_addr,
- int options)
+SYSCALL_PREFIX_DEFINE3(32_, waitpid, compat_pid_t, pid, unsigned int *, stat_addr,
+ int, options)
{
return compat_sys_wait4(pid, stat_addr, options, NULL);
}

/* 32-bit timeval and related flotsam. */

-asmlinkage long sys32_sysfs(int option, u32 arg1, u32 arg2)
+SYSCALL_PREFIX_DEFINE3(32_, sysfs, int, option, u32, arg1, u32, arg2)
{
return sys_sysfs(option, arg1, arg2);
}

-asmlinkage long sys32_sched_rr_get_interval(compat_pid_t pid,
- struct compat_timespec __user *interval)
+SYSCALL_PREFIX_DEFINE2(32_, sched_rr_get_interval, compat_pid_t, pid,
+ struct compat_timespec __user *, interval)
{
struct timespec t;
int ret;
@@ -378,8 +378,8 @@ asmlinkage long sys32_sched_rr_get_interval(compat_pid_t pid,
return ret;
}

-asmlinkage long sys32_rt_sigpending(compat_sigset_t __user *set,
- compat_size_t sigsetsize)
+SYSCALL_PREFIX_DEFINE2(32_, rt_sigpending, compat_sigset_t __user *, set,
+ compat_size_t, sigsetsize)
{
sigset_t s;
compat_sigset_t s32;
@@ -402,8 +402,8 @@ asmlinkage long sys32_rt_sigpending(compat_sigset_t __user *set,
return ret;
}

-asmlinkage long sys32_rt_sigqueueinfo(int pid, int sig,
- compat_siginfo_t __user *uinfo)
+SYSCALL_PREFIX_DEFINE3(32_, rt_sigqueueinfo, int, pid, int, sig,
+ compat_siginfo_t __user *, uinfo)
{
siginfo_t info;
int ret;
@@ -418,22 +418,22 @@ asmlinkage long sys32_rt_sigqueueinfo(int pid, int sig,
}

/* warning: next two assume little endian */
-asmlinkage long sys32_pread(unsigned int fd, char __user *ubuf, u32 count,
- u32 poslo, u32 poshi)
+SYSCALL_PREFIX_DEFINE5(32_, pread, unsigned int, fd, char __user *, ubuf, u32, count,
+ u32, poslo, u32, poshi)
{
return sys_pread64(fd, ubuf, count,
((loff_t)AA(poshi) << 32) | AA(poslo));
}

-asmlinkage long sys32_pwrite(unsigned int fd, char __user *ubuf, u32 count,
- u32 poslo, u32 poshi)
+SYSCALL_PREFIX_DEFINE5(32_, pwrite, unsigned int, fd, char __user *, ubuf, u32, count,
+ u32, poslo, u32, poshi)
{
return sys_pwrite64(fd, ubuf, count,
((loff_t)AA(poshi) << 32) | AA(poslo));
}


-asmlinkage long sys32_personality(unsigned long personality)
+SYSCALL_PREFIX_DEFINE1(32_, personality, unsigned long, personality)
{
int ret;

@@ -446,8 +446,8 @@ asmlinkage long sys32_personality(unsigned long personality)
return ret;
}

-asmlinkage long sys32_sendfile(int out_fd, int in_fd,
- compat_off_t __user *offset, s32 count)
+SYSCALL_PREFIX_DEFINE4(32_, sendfile, int, out_fd, int, in_fd,
+ compat_off_t __user *, offset, s32, count)
{
mm_segment_t old_fs = get_fs();
int ret;
@@ -466,7 +466,7 @@ asmlinkage long sys32_sendfile(int out_fd, int in_fd,
return ret;
}

-asmlinkage long sys32_olduname(struct oldold_utsname __user *name)
+SYSCALL_PREFIX_DEFINE1(32_, olduname, struct oldold_utsname __user *, name)
{
char *arch = "x86_64";
int err;
@@ -518,8 +518,8 @@ long sys32_uname(struct old_utsname __user *name)
return err ? -EFAULT : 0;
}

-asmlinkage long sys32_execve(char __user *name, compat_uptr_t __user *argv,
- compat_uptr_t __user *envp, struct pt_regs *regs)
+SYSCALL_PREFIX_DEFINE4(32_, execve, char __user *, name, compat_uptr_t __user *, argv,
+ compat_uptr_t __user *, envp, struct pt_regs *, regs)
{
long error;
char *filename;
@@ -533,8 +533,8 @@ asmlinkage long sys32_execve(char __user *name, compat_uptr_t __user *argv,
return error;
}

-asmlinkage long sys32_clone(unsigned int clone_flags, unsigned int newsp,
- struct pt_regs *regs)
+SYSCALL_PREFIX_DEFINE3(32_, clone, unsigned int, clone_flags, unsigned int, newsp,
+ struct pt_regs *, regs)
{
void __user *parent_tid = (void __user *)regs->dx;
void __user *child_tid = (void __user *)regs->di;
@@ -593,24 +593,24 @@ asmlinkage ssize_t sys32_readahead(int fd, unsigned off_lo, unsigned off_hi,
return sys_readahead(fd, ((u64)off_hi << 32) | off_lo, count);
}

-asmlinkage long sys32_sync_file_range(int fd, unsigned off_low, unsigned off_hi,
- unsigned n_low, unsigned n_hi, int flags)
+SYSCALL_PREFIX_DEFINE6(32_, sync_file_range, int, fd, unsigned, off_low, unsigned, off_hi,
+ unsigned, n_low, unsigned, n_hi, int, flags)
{
return sys_sync_file_range(fd,
((u64)off_hi << 32) | off_low,
((u64)n_hi << 32) | n_low, flags);
}

-asmlinkage long sys32_fadvise64(int fd, unsigned offset_lo, unsigned offset_hi,
- size_t len, int advice)
+SYSCALL_PREFIX_DEFINE5(32_, fadvise64, int, fd, unsigned, offset_lo, unsigned, offset_hi,
+ size_t, len, int, advice)
{
return sys_fadvise64_64(fd, ((u64)offset_hi << 32) | offset_lo,
len, advice);
}

-asmlinkage long sys32_fallocate(int fd, int mode, unsigned offset_lo,
- unsigned offset_hi, unsigned len_lo,
- unsigned len_hi)
+SYSCALL_PREFIX_DEFINE6(32_, fallocate, int, fd, int, mode, unsigned, offset_lo,
+ unsigned, offset_hi, unsigned, len_lo,
+ unsigned, len_hi)
{
return sys_fallocate(fd, mode, ((u64)offset_hi << 32) | offset_lo,
((u64)len_hi << 32) | len_lo);
--
1.6.5.1

2010-02-02 21:22:16

by Jason Baron

[permalink] [raw]
Subject: [PATCH 5/6] syscalls: add define syscall prefix macro

Add a new 'SYSCALL_PREFIX_DEFINE#()' style macro to include/linux/syscalls.h.
This allows us to create syscalls via:

SYSCALL_PREFIX_DEFINE1(32_, mmap, struct mmap_arg_struct __user *, arg);

The standard 'SYSCALL_DEFINE#()' macro forces 'sys_blah', but for the 32 compat
calls we want 'sys32_blah'.

Signed-off-by: Jason Baron <[email protected]>
---
include/linux/syscalls.h | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 36af9db..8ddd27e 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -210,6 +210,7 @@ struct perf_event_attr;
asmlinkage long sys_##sname(void)
#else
#define SYSCALL_DEFINE0(name) asmlinkage long sys_##name(void)
+#define SYSCALL_PREFIX_DEFINE0(pre, name) asmlinkage long sys_##name(void)
#endif

#define SYSCALL_DEFINE1(name, ...) SYSCALL_DEFINEx(1, _##name, __VA_ARGS__)
@@ -219,6 +220,13 @@ struct perf_event_attr;
#define SYSCALL_DEFINE5(name, ...) SYSCALL_DEFINEx(5, _##name, __VA_ARGS__)
#define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)

+#define SYSCALL_PREFIX_DEFINE1(pre, name, ...) SYSCALL_DEFINEx(1, pre##name, __VA_ARGS__)
+#define SYSCALL_PREFIX_DEFINE2(pre, name, ...) SYSCALL_DEFINEx(2, pre##name, __VA_ARGS__)
+#define SYSCALL_PREFIX_DEFINE3(pre, name, ...) SYSCALL_DEFINEx(3, pre##name, __VA_ARGS__)
+#define SYSCALL_PREFIX_DEFINE4(pre, name, ...) SYSCALL_DEFINEx(4, pre##name, __VA_ARGS__)
+#define SYSCALL_PREFIX_DEFINE5(pre, name, ...) SYSCALL_DEFINEx(5, pre##name, __VA_ARGS__)
+#define SYSCALL_PREFIX_DEFINE6(pre, name, ...) SYSCALL_DEFINEx(6, pre##name, __VA_ARGS__)
+
#ifdef CONFIG_PPC64
#define SYSCALL_ALIAS(alias, name) \
asm ("\t.globl " #alias "\n\t.set " #alias ", " #name "\n" \
--
1.6.5.1

2010-02-02 21:22:20

by Jason Baron

[permalink] [raw]
Subject: [PATCH 3/6] tracing: remove syscall bitmaps in preparation for compat support

In preparation for compat syscall tracing support, let's store the enabled
syscalls, with the struct syscall_metadata itself. That way we don't duplicate
enabled information when the compat table points to an entry in the regular
syscall table. Also, allows us to remove the bitmap data structures completely.

Signed-off-by: Jason Baron <[email protected]>
---
include/linux/syscalls.h | 8 +++++++
include/trace/syscall.h | 4 +++
kernel/trace/trace_syscalls.c | 42 +++++++++++++++++++---------------------
3 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 0b99a97..36af9db 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -183,6 +183,10 @@ struct perf_event_attr;
.nb_args = nb, \
.types = types_##sname, \
.args = args_##sname, \
+ .ftrace_enter = 0, \
+ .ftrace_exit = 0, \
+ .perf_enter = 0, \
+ .perf_exit = 0, \
.enter_event = &event_enter_##sname, \
.exit_event = &event_exit_##sname, \
};
@@ -196,6 +200,10 @@ struct perf_event_attr;
__syscall_meta__##sname = { \
.name = "sys_"#sname, \
.nb_args = 0, \
+ .ftrace_enter = 0, \
+ .ftrace_exit = 0, \
+ .perf_enter = 0, \
+ .perf_exit = 0, \
.enter_event = &event_enter__##sname, \
.exit_event = &event_exit__##sname, \
}; \
diff --git a/include/trace/syscall.h b/include/trace/syscall.h
index 0387100..8f5ac38 100644
--- a/include/trace/syscall.h
+++ b/include/trace/syscall.h
@@ -25,6 +25,10 @@ struct syscall_metadata {
int nb_args;
const char **types;
const char **args;
+ char ftrace_enter;
+ char ftrace_exit;
+ char perf_enter;
+ char perf_exit;

struct ftrace_event_call *enter_event;
struct ftrace_event_call *exit_event;
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 6cce6a8..8411d6c 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -11,8 +11,6 @@
static DEFINE_MUTEX(syscall_trace_lock);
static int sys_refcount_enter;
static int sys_refcount_exit;
-static DECLARE_BITMAP(enabled_enter_syscalls, NR_syscalls);
-static DECLARE_BITMAP(enabled_exit_syscalls, NR_syscalls);

extern unsigned long __start_syscalls_metadata[];
extern unsigned long __stop_syscalls_metadata[];
@@ -254,13 +252,14 @@ void ftrace_syscall_enter(struct pt_regs *regs, long id)
syscall_nr = syscall_get_nr(current, regs);
if (syscall_nr < 0)
return;
- if (!test_bit(syscall_nr, enabled_enter_syscalls))
- return;

sys_data = syscall_nr_to_meta(syscall_nr);
if (!sys_data)
return;

+ if (!sys_data->ftrace_enter)
+ return;
+
size = sizeof(*entry) + sizeof(unsigned long) * sys_data->nb_args;

event = trace_current_buffer_lock_reserve(&buffer,
@@ -288,13 +287,14 @@ void ftrace_syscall_exit(struct pt_regs *regs, long ret)
syscall_nr = syscall_get_nr(current, regs);
if (syscall_nr < 0)
return;
- if (!test_bit(syscall_nr, enabled_exit_syscalls))
- return;

sys_data = syscall_nr_to_meta(syscall_nr);
if (!sys_data)
return;

+ if (!sys_data->ftrace_exit)
+ return;
+
event = trace_current_buffer_lock_reserve(&buffer,
sys_data->exit_event->id, sizeof(*entry), 0, 0);
if (!event)
@@ -321,7 +321,7 @@ int reg_event_syscall_enter(struct ftrace_event_call *call)
if (!sys_refcount_enter)
ret = register_trace_sys_enter(ftrace_syscall_enter);
if (!ret) {
- set_bit(num, enabled_enter_syscalls);
+ ((struct syscall_metadata *)call->data)->ftrace_enter = 1;
sys_refcount_enter++;
}
mutex_unlock(&syscall_trace_lock);
@@ -337,7 +337,7 @@ void unreg_event_syscall_enter(struct ftrace_event_call *call)
return;
mutex_lock(&syscall_trace_lock);
sys_refcount_enter--;
- clear_bit(num, enabled_enter_syscalls);
+ ((struct syscall_metadata *)call->data)->ftrace_enter = 0;
if (!sys_refcount_enter)
unregister_trace_sys_enter(ftrace_syscall_enter);
mutex_unlock(&syscall_trace_lock);
@@ -355,7 +355,7 @@ int reg_event_syscall_exit(struct ftrace_event_call *call)
if (!sys_refcount_exit)
ret = register_trace_sys_exit(ftrace_syscall_exit);
if (!ret) {
- set_bit(num, enabled_exit_syscalls);
+ ((struct syscall_metadata *)call->data)->ftrace_exit = 1;
sys_refcount_exit++;
}
mutex_unlock(&syscall_trace_lock);
@@ -371,7 +371,7 @@ void unreg_event_syscall_exit(struct ftrace_event_call *call)
return;
mutex_lock(&syscall_trace_lock);
sys_refcount_exit--;
- clear_bit(num, enabled_exit_syscalls);
+ ((struct syscall_metadata *)call->data)->ftrace_exit = 0;
if (!sys_refcount_exit)
unregister_trace_sys_exit(ftrace_syscall_exit);
mutex_unlock(&syscall_trace_lock);
@@ -423,8 +423,6 @@ core_initcall(init_ftrace_syscalls);

#ifdef CONFIG_PERF_EVENTS

-static DECLARE_BITMAP(enabled_prof_enter_syscalls, NR_syscalls);
-static DECLARE_BITMAP(enabled_prof_exit_syscalls, NR_syscalls);
static int sys_prof_refcount_enter;
static int sys_prof_refcount_exit;

@@ -438,13 +436,13 @@ static void prof_syscall_enter(struct pt_regs *regs, long id)
int size;

syscall_nr = syscall_get_nr(current, regs);
- if (!test_bit(syscall_nr, enabled_prof_enter_syscalls))
- return;
-
sys_data = syscall_nr_to_meta(syscall_nr);
if (!sys_data)
return;

+ if (!sys_data->perf_enter)
+ return;
+
/* get the size after alignment with the u32 buffer size field */
size = sizeof(unsigned long) * sys_data->nb_args + sizeof(*rec);
size = ALIGN(size + sizeof(u32), sizeof(u64));
@@ -479,7 +477,7 @@ int prof_sysenter_enable(struct ftrace_event_call *call)
pr_info("event trace: Could not activate"
"syscall entry trace point");
} else {
- set_bit(num, enabled_prof_enter_syscalls);
+ ((struct syscall_metadata *)call->data)->perf_enter = 1;
sys_prof_refcount_enter++;
}
mutex_unlock(&syscall_trace_lock);
@@ -494,7 +492,7 @@ void prof_sysenter_disable(struct ftrace_event_call *call)

mutex_lock(&syscall_trace_lock);
sys_prof_refcount_enter--;
- clear_bit(num, enabled_prof_enter_syscalls);
+ ((struct syscall_metadata *)call->data)->perf_enter = 0;
if (!sys_prof_refcount_enter)
unregister_trace_sys_enter(prof_syscall_enter);
mutex_unlock(&syscall_trace_lock);
@@ -510,13 +508,13 @@ static void prof_syscall_exit(struct pt_regs *regs, long ret)
int size;

syscall_nr = syscall_get_nr(current, regs);
- if (!test_bit(syscall_nr, enabled_prof_exit_syscalls))
- return;
-
sys_data = syscall_nr_to_meta(syscall_nr);
if (!sys_data)
return;

+ if (!sys_data->perf_exit)
+ return;
+
/* We can probably do that at build time */
size = ALIGN(sizeof(*rec) + sizeof(u32), sizeof(u64));
size -= sizeof(u32);
@@ -554,7 +552,7 @@ int prof_sysexit_enable(struct ftrace_event_call *call)
pr_info("event trace: Could not activate"
"syscall entry trace point");
} else {
- set_bit(num, enabled_prof_exit_syscalls);
+ ((struct syscall_metadata *)call->data)->perf_exit = 1;
sys_prof_refcount_exit++;
}
mutex_unlock(&syscall_trace_lock);
@@ -569,7 +567,7 @@ void prof_sysexit_disable(struct ftrace_event_call *call)

mutex_lock(&syscall_trace_lock);
sys_prof_refcount_exit--;
- clear_bit(num, enabled_prof_exit_syscalls);
+ ((struct syscall_metadata *)call->data)->perf_exit = 0;
if (!sys_prof_refcount_exit)
unregister_trace_sys_exit(prof_syscall_exit);
mutex_unlock(&syscall_trace_lock);
--
1.6.5.1

2010-02-02 21:22:31

by Jason Baron

[permalink] [raw]
Subject: [PATCH 4/6] tracing: add tracing support for compat syscalls

Add core support to event tracing for compat syscalls. The basic idea is that we
check if we have a compat task via 'is_compat_task()'. If so, we lookup in the
new compat_syscalls_metadata table, the corresponding struct syscall_metadata, to
check syscall arguments and whether or not we are enabled.

Signed-off-by: Jason Baron <[email protected]>
---
include/linux/compat.h | 8 +++++
include/trace/syscall.h | 4 ++
kernel/trace/trace.h | 2 +
kernel/trace/trace_syscalls.c | 63 +++++++++++++++++++++++++++++++++++-----
4 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index ef68119..ab2cda2 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -353,5 +353,13 @@ asmlinkage long compat_sys_newfstatat(unsigned int dfd, char __user * filename,
asmlinkage long compat_sys_openat(unsigned int dfd, const char __user *filename,
int flags, int mode);

+#else /* CONFIG_COMPAT */
+
+#define NR_syscalls_compat 0
+static inline int is_compat_task(void)
+{
+ return 0;
+}
+
#endif /* CONFIG_COMPAT */
#endif /* _LINUX_COMPAT_H */
diff --git a/include/trace/syscall.h b/include/trace/syscall.h
index 8f5ac38..1cc1d1e 100644
--- a/include/trace/syscall.h
+++ b/include/trace/syscall.h
@@ -22,6 +22,7 @@
struct syscall_metadata {
const char *name;
int syscall_nr;
+ int compat_syscall_nr;
int nb_args;
const char **types;
const char **args;
@@ -36,6 +37,9 @@ struct syscall_metadata {

#ifdef CONFIG_FTRACE_SYSCALLS
extern unsigned long arch_syscall_addr(int nr);
+#ifdef CONFIG_COMPAT
+extern unsigned long arch_compat_syscall_addr(int nr);
+#endif
extern int init_syscall_trace(struct ftrace_event_call *call);

extern int syscall_enter_define_fields(struct ftrace_event_call *call);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index ce077fb..0ed0d2e 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -94,12 +94,14 @@ extern struct tracer boot_tracer;
struct syscall_trace_enter {
struct trace_entry ent;
int nr;
+ int compat;
unsigned long args[];
};

struct syscall_trace_exit {
struct trace_entry ent;
int nr;
+ int compat;
long ret;
};

diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 8411d6c..6f721be 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -3,6 +3,7 @@
#include <linux/kernel.h>
#include <linux/ftrace.h>
#include <linux/perf_event.h>
+#include <linux/compat.h>
#include <asm/syscall.h>

#include "trace_output.h"
@@ -16,6 +17,7 @@ extern unsigned long __start_syscalls_metadata[];
extern unsigned long __stop_syscalls_metadata[];

static struct syscall_metadata **syscalls_metadata;
+static struct syscall_metadata **compat_syscalls_metadata;

static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
{
@@ -41,8 +43,14 @@ static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
return NULL;
}

-static struct syscall_metadata *syscall_nr_to_meta(int nr)
+static struct syscall_metadata *syscall_nr_to_meta(int nr, int compat)
{
+ if (compat) {
+ if (!compat_syscalls_metadata || nr >= NR_syscalls_compat ||
+ nr < 0)
+ return NULL;
+ return compat_syscalls_metadata[nr];
+ }
if (!syscalls_metadata || nr >= NR_syscalls || nr < 0)
return NULL;

@@ -60,7 +68,7 @@ print_syscall_enter(struct trace_iterator *iter, int flags)

trace = (typeof(trace))ent;
syscall = trace->nr;
- entry = syscall_nr_to_meta(syscall);
+ entry = syscall_nr_to_meta(syscall, trace->compat);

if (!entry)
goto end;
@@ -113,7 +121,7 @@ print_syscall_exit(struct trace_iterator *iter, int flags)

trace = (typeof(trace))ent;
syscall = trace->nr;
- entry = syscall_nr_to_meta(syscall);
+ entry = syscall_nr_to_meta(syscall, trace->compat);

if (!entry) {
trace_seq_printf(s, "\n");
@@ -248,12 +256,16 @@ void ftrace_syscall_enter(struct pt_regs *regs, long id)
struct ring_buffer *buffer;
int size;
int syscall_nr;
+ int compat = 0;

syscall_nr = syscall_get_nr(current, regs);
if (syscall_nr < 0)
return;

- sys_data = syscall_nr_to_meta(syscall_nr);
+ if (is_compat_task())
+ compat = 1;
+
+ sys_data = syscall_nr_to_meta(syscall_nr, compat);
if (!sys_data)
return;

@@ -269,6 +281,7 @@ void ftrace_syscall_enter(struct pt_regs *regs, long id)

entry = ring_buffer_event_data(event);
entry->nr = syscall_nr;
+ entry->compat = compat;
syscall_get_arguments(current, regs, 0, sys_data->nb_args, entry->args);

if (!filter_current_check_discard(buffer, sys_data->enter_event,
@@ -283,12 +296,16 @@ void ftrace_syscall_exit(struct pt_regs *regs, long ret)
struct ring_buffer_event *event;
struct ring_buffer *buffer;
int syscall_nr;
+ int compat = 0;

syscall_nr = syscall_get_nr(current, regs);
if (syscall_nr < 0)
return;

- sys_data = syscall_nr_to_meta(syscall_nr);
+ if (is_compat_task())
+ compat = 1;
+
+ sys_data = syscall_nr_to_meta(syscall_nr, compat);
if (!sys_data)
return;

@@ -302,6 +319,7 @@ void ftrace_syscall_exit(struct pt_regs *regs, long ret)

entry = ring_buffer_event_data(event);
entry->nr = syscall_nr;
+ entry->compat = compat;
entry->ret = syscall_get_return_value(current, regs);

if (!filter_current_check_discard(buffer, sys_data->exit_event,
@@ -416,7 +434,26 @@ int __init init_ftrace_syscalls(void)
meta->syscall_nr = i;
syscalls_metadata[i] = meta;
}
-
+#ifdef CONFIG_COMPAT
+ if (NR_syscalls_compat) {
+ compat_syscalls_metadata = kzalloc(sizeof(*syscalls_metadata) *
+ NR_syscalls_compat, GFP_KERNEL);
+ if (!compat_syscalls_metadata) {
+ WARN_ON(1);
+ kfree(syscalls_metadata);
+ return -ENOMEM;
+ }
+ for (i = 0; i < NR_syscalls_compat; i++) {
+ addr = arch_compat_syscall_addr(i);
+ meta = find_syscall_meta(addr);
+ if (!meta)
+ continue;
+
+ meta->compat_syscall_nr = i;
+ compat_syscalls_metadata[i] = meta;
+ }
+ }
+#endif
return 0;
}
core_initcall(init_ftrace_syscalls);
@@ -434,9 +471,13 @@ static void prof_syscall_enter(struct pt_regs *regs, long id)
int syscall_nr;
int rctx;
int size;
+ int compat = 0;

syscall_nr = syscall_get_nr(current, regs);
- sys_data = syscall_nr_to_meta(syscall_nr);
+ if (is_compat_task())
+ compat = 1;
+
+ sys_data = syscall_nr_to_meta(syscall_nr, compat);
if (!sys_data)
return;

@@ -458,6 +499,7 @@ static void prof_syscall_enter(struct pt_regs *regs, long id)
return;

rec->nr = syscall_nr;
+ rec->compat = compat;
syscall_get_arguments(current, regs, 0, sys_data->nb_args,
(unsigned long *)&rec->args);
ftrace_perf_buf_submit(rec, size, rctx, 0, 1, flags);
@@ -506,9 +548,13 @@ static void prof_syscall_exit(struct pt_regs *regs, long ret)
int syscall_nr;
int rctx;
int size;
+ int compat = 0;

syscall_nr = syscall_get_nr(current, regs);
- sys_data = syscall_nr_to_meta(syscall_nr);
+ if (is_compat_task())
+ compat = 1;
+
+ sys_data = syscall_nr_to_meta(syscall_nr, compat);
if (!sys_data)
return;

@@ -533,6 +579,7 @@ static void prof_syscall_exit(struct pt_regs *regs, long ret)
return;

rec->nr = syscall_nr;
+ rec->compat = compat;
rec->ret = syscall_get_return_value(current, regs);

ftrace_perf_buf_submit(rec, size, rctx, 0, 1, flags);
--
1.6.5.1

2010-02-02 21:21:54

by Jason Baron

[permalink] [raw]
Subject: [PATCH 1/6] x86: add NR_syscalls_compat, make ia32 syscall table visible

Add 'NR_syscalls_compat' global for x86. Also, make 'ia32_sys_call_table' into a global.

Signed-off-by: Jason Baron <[email protected]>
---
arch/x86/ia32/ia32entry.S | 3 +++
arch/x86/include/asm/compat.h | 2 ++
2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 53147ad..acc6fbf 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -503,6 +503,7 @@ END(ia32_ptregs_common)

.section .rodata,"a"
.align 8
+.globl ia32_sys_call_table
ia32_sys_call_table:
.quad sys_restart_syscall
.quad sys_exit
@@ -843,3 +844,5 @@ ia32_sys_call_table:
.quad sys_perf_event_open
.quad compat_sys_recvmmsg
ia32_syscall_end:
+.globl NR_syscalls_compat
+NR_syscalls_compat: .int ((ia32_syscall_end - ia32_sys_call_table)/8)
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index 9a9c7bd..b5b78e6 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -215,4 +215,6 @@ static inline int is_compat_task(void)
return current_thread_info()->status & TS_COMPAT;
}

+extern int NR_syscalls_compat;
+
#endif /* _ASM_X86_COMPAT_H */
--
1.6.5.1

2010-02-03 12:52:55

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH 4/6] tracing: add tracing support for compat syscalls

On Tue, Feb 02, 2010 at 04:21:51PM -0500, Jason Baron wrote:
> Add core support to event tracing for compat syscalls. The basic idea is that we
> check if we have a compat task via 'is_compat_task()'. If so, we lookup in the
> new compat_syscalls_metadata table, the corresponding struct syscall_metadata, to
> check syscall arguments and whether or not we are enabled.
>
> Signed-off-by: Jason Baron <[email protected]>
> ---
> include/linux/compat.h | 8 +++++
> include/trace/syscall.h | 4 ++
> kernel/trace/trace.h | 2 +
> kernel/trace/trace_syscalls.c | 63 +++++++++++++++++++++++++++++++++++-----
> 4 files changed, 69 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index ef68119..ab2cda2 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -353,5 +353,13 @@ asmlinkage long compat_sys_newfstatat(unsigned int dfd, char __user * filename,
> asmlinkage long compat_sys_openat(unsigned int dfd, const char __user *filename,
> int flags, int mode);
>
> +#else /* CONFIG_COMPAT */
> +
> +#define NR_syscalls_compat 0
> +static inline int is_compat_task(void)
> +{
> + return 0;
> +}
> +
> #endif /* CONFIG_COMPAT */
> #endif /* _LINUX_COMPAT_H */

This part will break compilation on s390, since it has already the same
function defined for !CONFIG_COMPAT.
In order to avoid the whole ifdef mess some s390 files do include
asm/compat.h instead of linux/compat.h. This will get messy :)

2010-02-03 13:01:11

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH 5/6] syscalls: add define syscall prefix macro

On Tue, Feb 02, 2010 at 04:21:55PM -0500, Jason Baron wrote:
> Add a new 'SYSCALL_PREFIX_DEFINE#()' style macro to include/linux/syscalls.h.
> This allows us to create syscalls via:
>
> SYSCALL_PREFIX_DEFINE1(32_, mmap, struct mmap_arg_struct __user *, arg);
>
> The standard 'SYSCALL_DEFINE#()' macro forces 'sys_blah', but for the 32 compat
> calls we want 'sys32_blah'.

Not really. That's what you want for x86. But the generic name for compat syscalls
is compat_sys_whatever. The arch specific compat syscalls don't follow a common
naming scheme (yet).
Especially if you consider the idea to get automated correct sign extension via
hpa's planned script for compat syscalls it would be good if you would just name
that define something like SYSCALL_COMPAT.. or COMPAT_SYSCALL..
That way it would be easy to add a hook in there.

2010-02-08 15:44:39

by Jason Baron

[permalink] [raw]
Subject: Re: [PATCH 4/6] tracing: add tracing support for compat syscalls

On Wed, Feb 03, 2010 at 01:52:51PM +0100, Heiko Carstens wrote:
> On Tue, Feb 02, 2010 at 04:21:51PM -0500, Jason Baron wrote:
> > Add core support to event tracing for compat syscalls. The basic idea is that we
> > check if we have a compat task via 'is_compat_task()'. If so, we lookup in the
> > new compat_syscalls_metadata table, the corresponding struct syscall_metadata, to
> > check syscall arguments and whether or not we are enabled.
> >
> > Signed-off-by: Jason Baron <[email protected]>
> > ---
> > include/linux/compat.h | 8 +++++
> > include/trace/syscall.h | 4 ++
> > kernel/trace/trace.h | 2 +
> > kernel/trace/trace_syscalls.c | 63 +++++++++++++++++++++++++++++++++++-----
> > 4 files changed, 69 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/linux/compat.h b/include/linux/compat.h
> > index ef68119..ab2cda2 100644
> > --- a/include/linux/compat.h
> > +++ b/include/linux/compat.h
> > @@ -353,5 +353,13 @@ asmlinkage long compat_sys_newfstatat(unsigned int dfd, char __user * filename,
> > asmlinkage long compat_sys_openat(unsigned int dfd, const char __user *filename,
> > int flags, int mode);
> >
> > +#else /* CONFIG_COMPAT */
> > +
> > +#define NR_syscalls_compat 0
> > +static inline int is_compat_task(void)
> > +{
> > + return 0;
> > +}
> > +
> > #endif /* CONFIG_COMPAT */
> > #endif /* _LINUX_COMPAT_H */
>
> This part will break compilation on s390, since it has already the same
> function defined for !CONFIG_COMPAT.

If multiple arches want this definition, then it seems the the common
include/linux/compat.h is good place for it.

> In order to avoid the whole ifdef mess some s390 files do include
> asm/compat.h instead of linux/compat.h. This will get messy :)

is there a way for s390 to use the common include/linux/compat.h file?
what other arch defs are required?

thanks,

-Jason

2010-02-08 15:49:50

by Jason Baron

[permalink] [raw]
Subject: Re: [PATCH 5/6] syscalls: add define syscall prefix macro

On Wed, Feb 03, 2010 at 02:01:04PM +0100, Heiko Carstens wrote:
> On Tue, Feb 02, 2010 at 04:21:55PM -0500, Jason Baron wrote:
> > Add a new 'SYSCALL_PREFIX_DEFINE#()' style macro to include/linux/syscalls.h.
> > This allows us to create syscalls via:
> >
> > SYSCALL_PREFIX_DEFINE1(32_, mmap, struct mmap_arg_struct __user *, arg);
> >
> > The standard 'SYSCALL_DEFINE#()' macro forces 'sys_blah', but for the 32 compat
> > calls we want 'sys32_blah'.
>
> Not really. That's what you want for x86. But the generic name for compat syscalls
> is compat_sys_whatever. The arch specific compat syscalls don't follow a common
> naming scheme (yet).
> Especially if you consider the idea to get automated correct sign extension via
> hpa's planned script for compat syscalls it would be good if you would just name
> that define something like SYSCALL_COMPAT.. or COMPAT_SYSCALL..
> That way it would be easy to add a hook in there.

So I was trying to keep the names of the arch ia32 compat sys calls the
same, ie 'sys32_blah'. However, I agree a common naming scheme makes
more sense. what about 'arch_compat_sys_blah'? So as to distinguish from
the common compat syscalls 'compat_sys_blah'.

thanks,

-Jason

2010-02-08 16:13:58

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 5/6] syscalls: add define syscall prefix macro

On Monday 08 February 2010, Jason Baron wrote:
> So I was trying to keep the names of the arch ia32 compat sys calls the
> same, ie 'sys32_blah'. However, I agree a common naming scheme makes
> more sense. what about 'arch_compat_sys_blah'? So as to distinguish from
> the common compat syscalls 'compat_sys_blah'.

Why do you need to distinguish them? I would hope that we never need to
have a kernel with both a generic and an arch specific compat version
of the same syscall.

Arnd

2010-02-08 16:34:58

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH 4/6] tracing: add tracing support for compat syscalls

On Mon, Feb 08, 2010 at 10:43:25AM -0500, Jason Baron wrote:
> On Wed, Feb 03, 2010 at 01:52:51PM +0100, Heiko Carstens wrote:
> > On Tue, Feb 02, 2010 at 04:21:51PM -0500, Jason Baron wrote:
> > > Add core support to event tracing for compat syscalls. The basic idea is that we
> > > check if we have a compat task via 'is_compat_task()'. If so, we lookup in the
> > > new compat_syscalls_metadata table, the corresponding struct syscall_metadata, to
> > > check syscall arguments and whether or not we are enabled.
> > >
> > > Signed-off-by: Jason Baron <[email protected]>
> > > ---
> > > include/linux/compat.h | 8 +++++
> > > include/trace/syscall.h | 4 ++
> > > kernel/trace/trace.h | 2 +
> > > kernel/trace/trace_syscalls.c | 63 +++++++++++++++++++++++++++++++++++-----
> > > 4 files changed, 69 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/include/linux/compat.h b/include/linux/compat.h
> > > index ef68119..ab2cda2 100644
> > > --- a/include/linux/compat.h
> > > +++ b/include/linux/compat.h
> > > @@ -353,5 +353,13 @@ asmlinkage long compat_sys_newfstatat(unsigned int dfd, char __user * filename,
> > > asmlinkage long compat_sys_openat(unsigned int dfd, const char __user *filename,
> > > int flags, int mode);
> > >
> > > +#else /* CONFIG_COMPAT */
> > > +
> > > +#define NR_syscalls_compat 0
> > > +static inline int is_compat_task(void)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > #endif /* CONFIG_COMPAT */
> > > #endif /* _LINUX_COMPAT_H */
> >
> > This part will break compilation on s390, since it has already the same
> > function defined for !CONFIG_COMPAT.
>
> If multiple arches want this definition, then it seems the the common
> include/linux/compat.h is good place for it.
>
> > In order to avoid the whole ifdef mess some s390 files do include
> > asm/compat.h instead of linux/compat.h. This will get messy :)
>
> is there a way for s390 to use the common include/linux/compat.h file?
> what other arch defs are required?

Yes of course it is possible (I think :). I'll send a patch.

2010-02-08 16:55:42

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 5/6] syscalls: add define syscall prefix macro

On 02/08/10 07:48, Jason Baron wrote:
> On Wed, Feb 03, 2010 at 02:01:04PM +0100, Heiko Carstens wrote:
>> On Tue, Feb 02, 2010 at 04:21:55PM -0500, Jason Baron wrote:
>>> Add a new 'SYSCALL_PREFIX_DEFINE#()' style macro to include/linux/syscalls.h.
>>> This allows us to create syscalls via:
>>>
>>> SYSCALL_PREFIX_DEFINE1(32_, mmap, struct mmap_arg_struct __user *, arg);
>>>
>>> The standard 'SYSCALL_DEFINE#()' macro forces 'sys_blah', but for the 32 compat
>>> calls we want 'sys32_blah'.
>>
>> Not really. That's what you want for x86. But the generic name for compat syscalls
>> is compat_sys_whatever. The arch specific compat syscalls don't follow a common
>> naming scheme (yet).
>> Especially if you consider the idea to get automated correct sign extension via
>> hpa's planned script for compat syscalls it would be good if you would just name
>> that define something like SYSCALL_COMPAT.. or COMPAT_SYSCALL..
>> That way it would be easy to add a hook in there.
>
> So I was trying to keep the names of the arch ia32 compat sys calls the
> same, ie 'sys32_blah'. However, I agree a common naming scheme makes
> more sense. what about 'arch_compat_sys_blah'? So as to distinguish from
> the common compat syscalls 'compat_sys_blah'.

Hi,
I can't find my copy of the original posting... sorry about that.

Are these new SYSCALL macros just a shorthand/shortcut to cut down on typing
(and/or errors)?

The added level of indirection makes problems for scripts/kernel-doc (well, if any of
the syscalls have kernel-doc notation, that is). I'd prefer not to see 2 levels of macros
for defining a syscall, but if it has to live, please look into updating scripts/kernel-doc
also.

thanks,
--
~Randy

2010-02-08 17:12:13

by Jason Baron

[permalink] [raw]
Subject: Re: [PATCH 5/6] syscalls: add define syscall prefix macro

On Mon, Feb 08, 2010 at 08:53:28AM -0800, Randy Dunlap wrote:
> On 02/08/10 07:48, Jason Baron wrote:
> > On Wed, Feb 03, 2010 at 02:01:04PM +0100, Heiko Carstens wrote:
> >> On Tue, Feb 02, 2010 at 04:21:55PM -0500, Jason Baron wrote:
> >>> Add a new 'SYSCALL_PREFIX_DEFINE#()' style macro to include/linux/syscalls.h.
> >>> This allows us to create syscalls via:
> >>>
> >>> SYSCALL_PREFIX_DEFINE1(32_, mmap, struct mmap_arg_struct __user *, arg);
> >>>
> >>> The standard 'SYSCALL_DEFINE#()' macro forces 'sys_blah', but for the 32 compat
> >>> calls we want 'sys32_blah'.
> >>
> >> Not really. That's what you want for x86. But the generic name for compat syscalls
> >> is compat_sys_whatever. The arch specific compat syscalls don't follow a common
> >> naming scheme (yet).
> >> Especially if you consider the idea to get automated correct sign extension via
> >> hpa's planned script for compat syscalls it would be good if you would just name
> >> that define something like SYSCALL_COMPAT.. or COMPAT_SYSCALL..
> >> That way it would be easy to add a hook in there.
> >
> > So I was trying to keep the names of the arch ia32 compat sys calls the
> > same, ie 'sys32_blah'. However, I agree a common naming scheme makes
> > more sense. what about 'arch_compat_sys_blah'? So as to distinguish from
> > the common compat syscalls 'compat_sys_blah'.
>
> Hi,
> I can't find my copy of the original posting... sorry about that.
>

http://marc.info/?l=linux-kernel&m=126514576917582&w=2

> Are these new SYSCALL macros just a shorthand/shortcut to cut down on typing
> (and/or errors)?
>

Most of the copat syscall are currently not using the 'SYCALL_DEFINE()'
family of macros. For my purposes, the use of these macros allows them
to be tied into the event tracing system. These macros append 'sys_' to
the begging of the system call name. However, for compat syscall which
are named 'sys32_blah', these macros are not sufficient. Thus, the need
to introduce some new macros for the compat syscalls.

> The added level of indirection makes problems for scripts/kernel-doc (well, if any of
> the syscalls have kernel-doc notation, that is). I'd prefer not to see 2 levels of macros
> for defining a syscall, but if it has to live, please look into updating scripts/kernel-doc
> also.
>

good point. I'll make sure the docs work properly.

thanks,

-Jason

2010-02-08 17:54:15

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 5/6] syscalls: add define syscall prefix macro

On Mon, Feb 08, 2010 at 05:12:36PM +0100, Arnd Bergmann wrote:
> On Monday 08 February 2010, Jason Baron wrote:
> > So I was trying to keep the names of the arch ia32 compat sys calls the
> > same, ie 'sys32_blah'. However, I agree a common naming scheme makes
> > more sense. what about 'arch_compat_sys_blah'? So as to distinguish from
> > the common compat syscalls 'compat_sys_blah'.
>
> Why do you need to distinguish them? I would hope that we never need to
> have a kernel with both a generic and an arch specific compat version
> of the same syscall.

Well, the usual alignment or number of arguments issues could lead to
just that. We should deal with it the same way as for native syscalls.

2010-02-08 20:48:31

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 5/6] syscalls: add define syscall prefix macro

On Monday 08 February 2010 18:54:07 Christoph Hellwig wrote:
> On Mon, Feb 08, 2010 at 05:12:36PM +0100, Arnd Bergmann wrote:
> > On Monday 08 February 2010, Jason Baron wrote:
> > > So I was trying to keep the names of the arch ia32 compat sys calls the
> > > same, ie 'sys32_blah'. However, I agree a common naming scheme makes
> > > more sense. what about 'arch_compat_sys_blah'? So as to distinguish from
> > > the common compat syscalls 'compat_sys_blah'.
> >
> > Why do you need to distinguish them? I would hope that we never need to
> > have a kernel with both a generic and an arch specific compat version
> > of the same syscall.
>
> Well, the usual alignment or number of arguments issues could lead to
> just that. We should deal with it the same way as for native syscalls.

Ah, right. The arch specific syscall may just be a wrapper calling the
generic function, so we need them both.

Arnd

2010-02-09 10:21:10

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH 4/6] tracing: add tracing support for compat syscalls

On Mon, Feb 08, 2010 at 10:43:25AM -0500, Jason Baron wrote:
> On Wed, Feb 03, 2010 at 01:52:51PM +0100, Heiko Carstens wrote:
> > In order to avoid the whole ifdef mess some s390 files do include
> > asm/compat.h instead of linux/compat.h. This will get messy :)
>
> is there a way for s390 to use the common include/linux/compat.h file?
> what other arch defs are required?

The patch below is the smallest solution so that s390 still compiles
and probably even works:

Subject: [PATCH] compat: have generic is_compat_task for !CONFIG_COMPAT

From: Heiko Carstens <[email protected]>

..instead of an arch specific one.

Signed-off-by: Heiko Carstens <[email protected]>
---
arch/s390/include/asm/compat.h | 7 -------
arch/s390/kernel/ptrace.c | 2 +-
arch/s390/kernel/setup.c | 2 +-
arch/s390/mm/mmap.c | 2 +-
drivers/s390/block/dasd_eckd.c | 2 +-
drivers/s390/block/dasd_ioctl.c | 1 +
drivers/s390/char/fs3270.c | 1 +
drivers/s390/char/vmcp.c | 1 +
drivers/s390/cio/chsc_sch.c | 1 +
drivers/s390/scsi/zfcp_cfdc.c | 1 +
include/linux/compat.h | 7 +++++++
11 files changed, 16 insertions(+), 11 deletions(-)

--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -171,13 +171,6 @@ static inline int is_compat_task(void)
return test_thread_flag(TIF_31BIT);
}

-#else
-
-static inline int is_compat_task(void)
-{
- return 0;
-}
-
#endif

static inline void __user *compat_alloc_user_space(long len)
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -36,8 +36,8 @@
#include <linux/regset.h>
#include <linux/tracehook.h>
#include <linux/seccomp.h>
+#include <linux/compat.h>
#include <trace/syscall.h>
-#include <asm/compat.h>
#include <asm/segment.h>
#include <asm/page.h>
#include <asm/pgtable.h>
--- a/arch/s390/kernel/setup.c
+++ b/arch/s390/kernel/setup.c
@@ -43,7 +43,7 @@
#include <linux/reboot.h>
#include <linux/topology.h>
#include <linux/ftrace.h>
-
+#include <linux/compat.h>
#include <asm/ipl.h>
#include <asm/uaccess.h>
#include <asm/system.h>
--- a/arch/s390/mm/mmap.c
+++ b/arch/s390/mm/mmap.c
@@ -27,8 +27,8 @@
#include <linux/personality.h>
#include <linux/mm.h>
#include <linux/module.h>
+#include <linux/compat.h>
#include <asm/pgalloc.h>
-#include <asm/compat.h>

/*
* Top of mmap area (just below the process stack).
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -14,6 +14,7 @@

#include <linux/stddef.h>
#include <linux/kernel.h>
+#include <linux/compat.h>
#include <linux/slab.h>
#include <linux/hdreg.h> /* HDIO_GETGEO */
#include <linux/bio.h>
@@ -23,7 +24,6 @@
#include <asm/debug.h>
#include <asm/idals.h>
#include <asm/ebcdic.h>
-#include <asm/compat.h>
#include <asm/io.h>
#include <asm/uaccess.h>
#include <asm/cio.h>
--- a/drivers/s390/block/dasd_ioctl.c
+++ b/drivers/s390/block/dasd_ioctl.c
@@ -17,6 +17,7 @@
#include <linux/fs.h>
#include <linux/blkpg.h>
#include <linux/smp_lock.h>
+#include <linux/compat.h>
#include <asm/compat.h>
#include <asm/ccwdev.h>
#include <asm/cmb.h>
--- a/drivers/s390/char/fs3270.c
+++ b/drivers/s390/char/fs3270.c
@@ -14,6 +14,7 @@
#include <linux/list.h>
#include <linux/types.h>
#include <linux/smp_lock.h>
+#include <linux/compat.h>

#include <asm/compat.h>
#include <asm/ccwdev.h>
--- a/drivers/s390/char/vmcp.c
+++ b/drivers/s390/char/vmcp.c
@@ -19,6 +19,7 @@
#include <linux/kernel.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
+#include <linux/compat.h>
#include <asm/compat.h>
#include <asm/cpcmd.h>
#include <asm/debug.h>
--- a/drivers/s390/cio/chsc_sch.c
+++ b/drivers/s390/cio/chsc_sch.c
@@ -8,6 +8,7 @@
*/

#include <linux/device.h>
+#include <linux/compat.h>
#include <linux/module.h>
#include <linux/uaccess.h>
#include <linux/miscdevice.h>
--- a/drivers/s390/scsi/zfcp_cfdc.c
+++ b/drivers/s390/scsi/zfcp_cfdc.c
@@ -12,6 +12,7 @@

#include <linux/types.h>
#include <linux/miscdevice.h>
+#include <linux/compat.h>
#include <asm/compat.h>
#include <asm/ccwdev.h>
#include "zfcp_def.h"
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -353,5 +353,12 @@ asmlinkage long compat_sys_newfstatat(un
asmlinkage long compat_sys_openat(unsigned int dfd, const char __user *filename,
int flags, int mode);

+#else /* CONFIG_COMPAT */
+
+static inline int is_compat_task(void)
+{
+ return 0;
+}
+
#endif /* CONFIG_COMPAT */
#endif /* _LINUX_COMPAT_H */

2010-02-09 14:55:35

by Jason Baron

[permalink] [raw]
Subject: Re: [PATCH 4/6] tracing: add tracing support for compat syscalls

On Tue, Feb 09, 2010 at 11:21:00AM +0100, Heiko Carstens wrote:
> > is there a way for s390 to use the common include/linux/compat.h file?
> > what other arch defs are required?
>
> The patch below is the smallest solution so that s390 still compiles
> and probably even works:
>
> Subject: [PATCH] compat: have generic is_compat_task for !CONFIG_COMPAT
>
> From: Heiko Carstens <[email protected]>
>
> ..instead of an arch specific one.
>
> Signed-off-by: Heiko Carstens <[email protected]>
> ---
> arch/s390/include/asm/compat.h | 7 -------
> arch/s390/kernel/ptrace.c | 2 +-
> arch/s390/kernel/setup.c | 2 +-
> arch/s390/mm/mmap.c | 2 +-
> drivers/s390/block/dasd_eckd.c | 2 +-
> drivers/s390/block/dasd_ioctl.c | 1 +
> drivers/s390/char/fs3270.c | 1 +
> drivers/s390/char/vmcp.c | 1 +
> drivers/s390/cio/chsc_sch.c | 1 +
> drivers/s390/scsi/zfcp_cfdc.c | 1 +
> include/linux/compat.h | 7 +++++++
> 11 files changed, 16 insertions(+), 11 deletions(-)
>

looks good. I'll include this patch when I repost the revised series.

thanks!

-Jason

2010-02-10 03:00:20

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 5/6] syscalls: add define syscall prefix macro

On Mon, 2010-02-08 at 12:11 -0500, Jason Baron wrote:

> Most of the copat syscall are currently not using the 'SYCALL_DEFINE()'
> family of macros. For my purposes, the use of these macros allows them
> to be tied into the event tracing system. These macros append 'sys_' to
> the begging of the system call name. However, for compat syscall which
> are named 'sys32_blah', these macros are not sufficient. Thus, the need
> to introduce some new macros for the compat syscalls.
>
> > The added level of indirection makes problems for scripts/kernel-doc (well, if any of
> > the syscalls have kernel-doc notation, that is). I'd prefer not to see 2 levels of macros
> > for defining a syscall, but if it has to live, please look into updating scripts/kernel-doc
> > also.
> >
>
> good point. I'll make sure the docs work properly.

Could you also make sure 'make TAGS' is updated too.
See scripts/tags.sh and search for SYSCALL

-- Steve