* [PATCH 0/3] linux-user: Fix some minor nits @ 2022-01-14 15:37 Peter Maydell 2022-01-14 15:37 ` [PATCH 1/3] linux-user: Remove unnecessary 'aligned' attribute from TaskState Peter Maydell ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Peter Maydell @ 2022-01-14 15:37 UTC (permalink / raw) To: qemu-devel; +Cc: Laurent Vivier This patchset fixes up some minor nits in the linux-user code that I noticed while I was reading code to assist with reviewing the bsd-user signal handling. thanks -- PMM Peter Maydell (3): linux-user: Remove unnecessary 'aligned' attribute from TaskState linux-user: Rename user_force_sig tracepoint to match function name linux-user: Return void from queue_signal() linux-user/qemu.h | 6 +----- linux-user/signal-common.h | 4 ++-- linux-user/signal.c | 7 +++---- linux-user/trace-events | 2 +- 4 files changed, 7 insertions(+), 12 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] linux-user: Remove unnecessary 'aligned' attribute from TaskState 2022-01-14 15:37 [PATCH 0/3] linux-user: Fix some minor nits Peter Maydell @ 2022-01-14 15:37 ` Peter Maydell 2022-01-27 13:20 ` Laurent Vivier 2022-01-14 15:37 ` [PATCH 2/3] linux-user: Rename user_force_sig tracepoint to match function name Peter Maydell 2022-01-14 15:37 ` [PATCH 3/3] linux-user: Return void from queue_signal() Peter Maydell 2 siblings, 1 reply; 9+ messages in thread From: Peter Maydell @ 2022-01-14 15:37 UTC (permalink / raw) To: qemu-devel; +Cc: Laurent Vivier The linux-user struct TaskState has an 'aligned(16)' attribute. When the struct was first added in commit 851e67a1b46f in 2003, there was a justification in a comment (still present in the source today): /* NOTE: we force a big alignment so that the stack stored after is aligned too */ because the final field in the struct was "uint8_t stack[0];" But that field was removed in commit 48e15fc2d in 2010 which switched us to allocating the stack and the TaskState separately. Because we allocate the structure with g_new0() rather than as a local variable, the attribute made no difference to the alignment of the structure anyway. Remove the unnecessary attribute, and the corresponding comment. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/qemu.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/linux-user/qemu.h b/linux-user/qemu.h index 5c713fa8ab2..bd0559759ae 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -96,10 +96,6 @@ struct emulated_sigtable { target_siginfo_t info; }; -/* - * NOTE: we force a big alignment so that the stack stored after is - * aligned too - */ typedef struct TaskState { pid_t ts_tid; /* tid (or pid) of this task */ #ifdef TARGET_ARM @@ -160,7 +156,7 @@ typedef struct TaskState { /* This thread's sigaltstack, if it has one */ struct target_sigaltstack sigaltstack_used; -} __attribute__((aligned(16))) TaskState; +} TaskState; abi_long do_brk(abi_ulong new_brk); -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] linux-user: Remove unnecessary 'aligned' attribute from TaskState 2022-01-14 15:37 ` [PATCH 1/3] linux-user: Remove unnecessary 'aligned' attribute from TaskState Peter Maydell @ 2022-01-27 13:20 ` Laurent Vivier 0 siblings, 0 replies; 9+ messages in thread From: Laurent Vivier @ 2022-01-27 13:20 UTC (permalink / raw) To: Peter Maydell, qemu-devel Le 14/01/2022 à 16:37, Peter Maydell a écrit : > The linux-user struct TaskState has an 'aligned(16)' attribute. When > the struct was first added in commit 851e67a1b46f in 2003, there was > a justification in a comment (still present in the source today): > > /* NOTE: we force a big alignment so that the stack stored after is > aligned too */ > > because the final field in the struct was "uint8_t stack[0];" > But that field was removed in commit 48e15fc2d in 2010 which > switched us to allocating the stack and the TaskState separately. > Because we allocate the structure with g_new0() rather than as > a local variable, the attribute made no difference to the alignment > of the structure anyway. > > Remove the unnecessary attribute, and the corresponding comment. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > linux-user/qemu.h | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/linux-user/qemu.h b/linux-user/qemu.h > index 5c713fa8ab2..bd0559759ae 100644 > --- a/linux-user/qemu.h > +++ b/linux-user/qemu.h > @@ -96,10 +96,6 @@ struct emulated_sigtable { > target_siginfo_t info; > }; > > -/* > - * NOTE: we force a big alignment so that the stack stored after is > - * aligned too > - */ > typedef struct TaskState { > pid_t ts_tid; /* tid (or pid) of this task */ > #ifdef TARGET_ARM > @@ -160,7 +156,7 @@ typedef struct TaskState { > > /* This thread's sigaltstack, if it has one */ > struct target_sigaltstack sigaltstack_used; > -} __attribute__((aligned(16))) TaskState; > +} TaskState; > > abi_long do_brk(abi_ulong new_brk); > Applied to my linux-user-for-7.0 branch. Thanks, Laurent ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] linux-user: Rename user_force_sig tracepoint to match function name 2022-01-14 15:37 [PATCH 0/3] linux-user: Fix some minor nits Peter Maydell 2022-01-14 15:37 ` [PATCH 1/3] linux-user: Remove unnecessary 'aligned' attribute from TaskState Peter Maydell @ 2022-01-14 15:37 ` Peter Maydell 2022-01-14 15:46 ` Philippe Mathieu-Daudé via 2022-01-18 11:41 ` Laurent Vivier 2022-01-14 15:37 ` [PATCH 3/3] linux-user: Return void from queue_signal() Peter Maydell 2 siblings, 2 replies; 9+ messages in thread From: Peter Maydell @ 2022-01-14 15:37 UTC (permalink / raw) To: qemu-devel; +Cc: Laurent Vivier In commit c599d4d6d6e9bfdb64 in 2016 we renamed the old force_sig() function to dump_core_and_abort(), but we forgot to rename the associated tracepoint. Rename the tracepoint to to match the function it's called from. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/signal.c | 2 +- linux-user/trace-events | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index f813b4f18e4..bfbbeab9ad2 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -734,7 +734,7 @@ static void QEMU_NORETURN dump_core_and_abort(int target_sig) struct sigaction act; host_sig = target_to_host_signal(target_sig); - trace_user_force_sig(env, target_sig, host_sig); + trace_user_dump_core_and_abort(env, target_sig, host_sig); gdb_signalled(env, target_sig); /* dump core if supported by target binary format */ diff --git a/linux-user/trace-events b/linux-user/trace-events index e7d2f54e940..f33717f248a 100644 --- a/linux-user/trace-events +++ b/linux-user/trace-events @@ -9,7 +9,7 @@ user_setup_frame(void *env, uint64_t frame_addr) "env=%p frame_addr=0x%"PRIx64 user_setup_rt_frame(void *env, uint64_t frame_addr) "env=%p frame_addr=0x%"PRIx64 user_do_rt_sigreturn(void *env, uint64_t frame_addr) "env=%p frame_addr=0x%"PRIx64 user_do_sigreturn(void *env, uint64_t frame_addr) "env=%p frame_addr=0x%"PRIx64 -user_force_sig(void *env, int target_sig, int host_sig) "env=%p signal %d (host %d)" +user_dump_core_and_abort(void *env, int target_sig, int host_sig) "env=%p signal %d (host %d)" user_handle_signal(void *env, int target_sig) "env=%p signal %d" user_host_signal(void *env, int host_sig, int target_sig) "env=%p signal %d (target %d)" user_queue_signal(void *env, int target_sig) "env=%p signal %d" -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] linux-user: Rename user_force_sig tracepoint to match function name 2022-01-14 15:37 ` [PATCH 2/3] linux-user: Rename user_force_sig tracepoint to match function name Peter Maydell @ 2022-01-14 15:46 ` Philippe Mathieu-Daudé via 2022-01-18 11:41 ` Laurent Vivier 1 sibling, 0 replies; 9+ messages in thread From: Philippe Mathieu-Daudé via @ 2022-01-14 15:46 UTC (permalink / raw) To: Peter Maydell, qemu-devel; +Cc: Laurent Vivier On 14/1/22 16:37, Peter Maydell wrote: > In commit c599d4d6d6e9bfdb64 in 2016 we renamed the old force_sig() > function to dump_core_and_abort(), but we forgot to rename the > associated tracepoint. Rename the tracepoint to to match the > function it's called from. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > linux-user/signal.c | 2 +- > linux-user/trace-events | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] linux-user: Rename user_force_sig tracepoint to match function name 2022-01-14 15:37 ` [PATCH 2/3] linux-user: Rename user_force_sig tracepoint to match function name Peter Maydell 2022-01-14 15:46 ` Philippe Mathieu-Daudé via @ 2022-01-18 11:41 ` Laurent Vivier 1 sibling, 0 replies; 9+ messages in thread From: Laurent Vivier @ 2022-01-18 11:41 UTC (permalink / raw) To: Peter Maydell, qemu-devel, qemu-trivial Le 14/01/2022 à 16:37, Peter Maydell a écrit : > In commit c599d4d6d6e9bfdb64 in 2016 we renamed the old force_sig() > function to dump_core_and_abort(), but we forgot to rename the > associated tracepoint. Rename the tracepoint to to match the > function it's called from. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > linux-user/signal.c | 2 +- > linux-user/trace-events | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/linux-user/signal.c b/linux-user/signal.c > index f813b4f18e4..bfbbeab9ad2 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -734,7 +734,7 @@ static void QEMU_NORETURN dump_core_and_abort(int target_sig) > struct sigaction act; > > host_sig = target_to_host_signal(target_sig); > - trace_user_force_sig(env, target_sig, host_sig); > + trace_user_dump_core_and_abort(env, target_sig, host_sig); > gdb_signalled(env, target_sig); > > /* dump core if supported by target binary format */ > diff --git a/linux-user/trace-events b/linux-user/trace-events > index e7d2f54e940..f33717f248a 100644 > --- a/linux-user/trace-events > +++ b/linux-user/trace-events > @@ -9,7 +9,7 @@ user_setup_frame(void *env, uint64_t frame_addr) "env=%p frame_addr=0x%"PRIx64 > user_setup_rt_frame(void *env, uint64_t frame_addr) "env=%p frame_addr=0x%"PRIx64 > user_do_rt_sigreturn(void *env, uint64_t frame_addr) "env=%p frame_addr=0x%"PRIx64 > user_do_sigreturn(void *env, uint64_t frame_addr) "env=%p frame_addr=0x%"PRIx64 > -user_force_sig(void *env, int target_sig, int host_sig) "env=%p signal %d (host %d)" > +user_dump_core_and_abort(void *env, int target_sig, int host_sig) "env=%p signal %d (host %d)" > user_handle_signal(void *env, int target_sig) "env=%p signal %d" > user_host_signal(void *env, int host_sig, int target_sig) "env=%p signal %d (target %d)" > user_queue_signal(void *env, int target_sig) "env=%p signal %d" Applied to my trivial-patches branch. Thanks, Laurent ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] linux-user: Return void from queue_signal() 2022-01-14 15:37 [PATCH 0/3] linux-user: Fix some minor nits Peter Maydell 2022-01-14 15:37 ` [PATCH 1/3] linux-user: Remove unnecessary 'aligned' attribute from TaskState Peter Maydell 2022-01-14 15:37 ` [PATCH 2/3] linux-user: Rename user_force_sig tracepoint to match function name Peter Maydell @ 2022-01-14 15:37 ` Peter Maydell 2022-01-14 15:57 ` Philippe Mathieu-Daudé via 2022-01-18 11:42 ` Laurent Vivier 2 siblings, 2 replies; 9+ messages in thread From: Peter Maydell @ 2022-01-14 15:37 UTC (permalink / raw) To: qemu-devel; +Cc: Laurent Vivier The linux-user queue_signal() function always returns 1, and none of its callers check the return value. Give it a void return type instead. The return value is a leftover from the old pre-2016 linux-user signal handling code, which really did have a queue of signals and so might return a failure indication if too many signals were queued at once. The current design avoids having to ever have more than one signal queued via queue_signal() at once, so it can never fail. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/signal-common.h | 4 ++-- linux-user/signal.c | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/linux-user/signal-common.h b/linux-user/signal-common.h index 42aa479080b..2113165a758 100644 --- a/linux-user/signal-common.h +++ b/linux-user/signal-common.h @@ -59,8 +59,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka, void process_pending_signals(CPUArchState *cpu_env); void signal_init(void); -int queue_signal(CPUArchState *env, int sig, int si_type, - target_siginfo_t *info); +void queue_signal(CPUArchState *env, int sig, int si_type, + target_siginfo_t *info); void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info); void target_to_host_siginfo(siginfo_t *info, const target_siginfo_t *tinfo); int target_to_host_signal(int sig); diff --git a/linux-user/signal.c b/linux-user/signal.c index bfbbeab9ad2..32854bb3752 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -780,8 +780,8 @@ static void QEMU_NORETURN dump_core_and_abort(int target_sig) /* queue a signal so that it will be send to the virtual CPU as soon as possible */ -int queue_signal(CPUArchState *env, int sig, int si_type, - target_siginfo_t *info) +void queue_signal(CPUArchState *env, int sig, int si_type, + target_siginfo_t *info) { CPUState *cpu = env_cpu(env); TaskState *ts = cpu->opaque; @@ -794,7 +794,6 @@ int queue_signal(CPUArchState *env, int sig, int si_type, ts->sync_signal.pending = sig; /* signal that a new signal is pending */ qatomic_set(&ts->signal_pending, 1); - return 1; /* indicates that the signal was queued */ } -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] linux-user: Return void from queue_signal() 2022-01-14 15:37 ` [PATCH 3/3] linux-user: Return void from queue_signal() Peter Maydell @ 2022-01-14 15:57 ` Philippe Mathieu-Daudé via 2022-01-18 11:42 ` Laurent Vivier 1 sibling, 0 replies; 9+ messages in thread From: Philippe Mathieu-Daudé via @ 2022-01-14 15:57 UTC (permalink / raw) To: Peter Maydell, qemu-devel; +Cc: Laurent Vivier On 14/1/22 16:37, Peter Maydell wrote: > The linux-user queue_signal() function always returns 1, and none of > its callers check the return value. Give it a void return type > instead. > > The return value is a leftover from the old pre-2016 linux-user > signal handling code, which really did have a queue of signals and so > might return a failure indication if too many signals were queued at > once. The current design avoids having to ever have more than one > signal queued via queue_signal() at once, so it can never fail. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > linux-user/signal-common.h | 4 ++-- > linux-user/signal.c | 5 ++--- > 2 files changed, 4 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] linux-user: Return void from queue_signal() 2022-01-14 15:37 ` [PATCH 3/3] linux-user: Return void from queue_signal() Peter Maydell 2022-01-14 15:57 ` Philippe Mathieu-Daudé via @ 2022-01-18 11:42 ` Laurent Vivier 1 sibling, 0 replies; 9+ messages in thread From: Laurent Vivier @ 2022-01-18 11:42 UTC (permalink / raw) To: Peter Maydell, qemu-devel, qemu-trivial Le 14/01/2022 à 16:37, Peter Maydell a écrit : > The linux-user queue_signal() function always returns 1, and none of > its callers check the return value. Give it a void return type > instead. > > The return value is a leftover from the old pre-2016 linux-user > signal handling code, which really did have a queue of signals and so > might return a failure indication if too many signals were queued at > once. The current design avoids having to ever have more than one > signal queued via queue_signal() at once, so it can never fail. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > linux-user/signal-common.h | 4 ++-- > linux-user/signal.c | 5 ++--- > 2 files changed, 4 insertions(+), 5 deletions(-) > > diff --git a/linux-user/signal-common.h b/linux-user/signal-common.h > index 42aa479080b..2113165a758 100644 > --- a/linux-user/signal-common.h > +++ b/linux-user/signal-common.h > @@ -59,8 +59,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka, > > void process_pending_signals(CPUArchState *cpu_env); > void signal_init(void); > -int queue_signal(CPUArchState *env, int sig, int si_type, > - target_siginfo_t *info); > +void queue_signal(CPUArchState *env, int sig, int si_type, > + target_siginfo_t *info); > void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info); > void target_to_host_siginfo(siginfo_t *info, const target_siginfo_t *tinfo); > int target_to_host_signal(int sig); > diff --git a/linux-user/signal.c b/linux-user/signal.c > index bfbbeab9ad2..32854bb3752 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -780,8 +780,8 @@ static void QEMU_NORETURN dump_core_and_abort(int target_sig) > > /* queue a signal so that it will be send to the virtual CPU as soon > as possible */ > -int queue_signal(CPUArchState *env, int sig, int si_type, > - target_siginfo_t *info) > +void queue_signal(CPUArchState *env, int sig, int si_type, > + target_siginfo_t *info) > { > CPUState *cpu = env_cpu(env); > TaskState *ts = cpu->opaque; > @@ -794,7 +794,6 @@ int queue_signal(CPUArchState *env, int sig, int si_type, > ts->sync_signal.pending = sig; > /* signal that a new signal is pending */ > qatomic_set(&ts->signal_pending, 1); > - return 1; /* indicates that the signal was queued */ > } > > Applied to my trivial-patches branch. Thanks, Laurent ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-01-27 13:23 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-14 15:37 [PATCH 0/3] linux-user: Fix some minor nits Peter Maydell 2022-01-14 15:37 ` [PATCH 1/3] linux-user: Remove unnecessary 'aligned' attribute from TaskState Peter Maydell 2022-01-27 13:20 ` Laurent Vivier 2022-01-14 15:37 ` [PATCH 2/3] linux-user: Rename user_force_sig tracepoint to match function name Peter Maydell 2022-01-14 15:46 ` Philippe Mathieu-Daudé via 2022-01-18 11:41 ` Laurent Vivier 2022-01-14 15:37 ` [PATCH 3/3] linux-user: Return void from queue_signal() Peter Maydell 2022-01-14 15:57 ` Philippe Mathieu-Daudé via 2022-01-18 11:42 ` Laurent Vivier
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.