Stas,
Now, when we fully understand the patch, will you fix description of
98cbcb14d? The following does not seem correct:
> IOW, any signal, arrived to the process, which does page fault
handling on fuse
> file, _before_ request_wait_answer() is called, will lead to request
> interruption, producing SIGBUS error in page fault handler
(filemap_fault).
Thanks,
Maxim
On 10/17/2016 04:59 AM, Stanislav Kinsburskiy wrote:
16.10.2016 05:21, Maxim Patlasov пишет:
Stas,
On 10/14/2016 03:30 AM, Stanislav Kinsburskiy wrote:
14.10.2016 02:23, Maxim Patlasov пишет:
Stas,
The series look fine, so:
Acked-by: Maxim Patlasov <mpatla...@virtuozzo.com>
But, please, refine the description of the second patch. It must
explain clearly why the patch fixes the problem:
block_sigs() blocks ordinary non-fatal signals as expected, but
surprisingly SIGTRAP is special: it does not matter whether it
comes before or after block_sigs(), the latter does not affect
SIGTRAP at all! And in contrast, wait_event_killable() is immune to
it -- only fatal sig can wake it up.
No, Maxim. You make a mistake here.
Yes, I agree.
There is nothing special with SIGTRAP (although it's being sometimes
sent via force_sig_info()).
OK.
The problem is described as it is: block_sigs() doesn't (!) clear
TIG_SIGPENDING flag. All it does is blocking future signals to arrive.
OK. But I disagree with your explanation why it doesn't clear the flag.
Moreover, __set_task_blocked() call recalc_sigpending(), which check
whether any of the signals to block is present in process pending
mask, and if so - set (!) TIF_SIGPENDING on the task.
Only if the correspondent bit is NOT set in blocked->sig[]:
> case 1: ready = signal->sig[0] &~ blocked->sig[0];
That's definitely not the case for block_sigs() who set all bits in
blocked->sig[] except sigmask(SIGKILL). So, in our case
recalc_sigpending() can only clear TIG_SIGPENDING flag, not set it.
Agreed.
IOW, any pending signal remains pending after call to blocks_sigs().
No. Conversely: all non-fatal signals does NOT remain pending after
call to blocks_sigs(). You can ascertain it yourself by debugging how
block_sigs() react on "kill -USR1".
And that's is the root of the issue (as it described in the patch
comment).
No. The root of the issue is in ptrace(2) calling ptrace_request(),
calling task_set_jobctl_pending(), setting JOBCTL_TRAP_STOP in
task->jobctl. So, when fuse calls block_sigs(), it eventually calls
recalc_sigpending() which calls recalc_sigpending_tsk() which looks
like this:
> if ((t->jobctl & JOBCTL_PENDING_MASK) ||
> PENDING(&t->pending, &t->blocked) ||
> PENDING(&t->signal->shared_pending, &t->blocked)) {
> set_tsk_thread_flag(t, TIF_SIGPENDING);
> return 1;
but as we know ptrace(2) already set JOBCTL_TRAP_STOP in task->jobctl:
> #define JOBCTL_TRAP_MASK (JOBCTL_TRAP_STOP | JOBCTL_TRAP_NOTIFY)
> #define JOBCTL_PENDING_MASK (JOBCTL_STOP_PENDING |
JOBCTL_TRAP_MASK)
Nice catch, thanks.
To sum it up, the patch from Al Viro that you backported doesn't
change fuse behavior w.r.t signals, but it nicely replace
signal_pending with fatal_signal_pending, and the latter solves our
case because it checks for SIGKILL explicitly:
> static inline int fatal_signal_pending(struct task_struct *p)
> {
> return signal_pending(p) && __fatal_signal_pending(p);
> }
> static inline int __fatal_signal_pending(struct task_struct *p)
> {
> return unlikely(sigismember(&p->pending.signal, SIGKILL));
> }
Thanks,
Maxim
Thanks,
Maxim
On 10/13/2016 03:03 AM, Stanislav Kinsburskiy wrote:
This patch fixes wrong SIGBUS result in page fault handler for
fuse file, when
process received a signal.
https://jira.sw.ru/browse/PSBM-53581
---
Stanislav Kinsburskiy (2):
new helper: wait_event_killable_exclusive()
fuse: handle only fatal signals while waiting request answer
fs/fuse/dev.c | 42
++++++++++++++++--------------------------
include/linux/wait.h | 26 ++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 26 deletions(-)
--
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
