[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87plufttru.ffs@tglx>
Date: Wed, 24 Apr 2024 03:48:53 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Oleg Nesterov <oleg@...hat.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Anna-Maria Behnsen
<anna-maria@...utronix.de>, Frederic Weisbecker <frederic@...nel.org>,
John Stultz <jstultz@...gle.com>, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>, Stephen Boyd <sboyd@...nel.org>, Eric
Biederman <ebiederm@...ssion.com>, Andrei Vagin <avagin@...gle.com>
Subject: Re: [patch V2 26/50] signal: Get rid of resched_timer logic
On Tue, Apr 23 2024 at 23:18, Thomas Gleixner wrote:
> On Fri, Apr 19 2024 at 13:06, Oleg Nesterov wrote:
>> Otoh, I have no idea how CRIU restores the posix timers. If a process has
>> a pending blocked SI_TIMER signal, then I guess it actually needs to enqueue
>> this signal at restore time, but resched_timer will be never true?
>
> It can't restore the correct sys_si_private value because that is
> nowhere exposed to user space.
It is exposed via PTRACE_PEEKSIGINFO, but it's useless.
> There is no special treatment for SI_TIMER, so the signal restore might
> just end up queueing a random extra SI_TIMER signal if there was one
> pending.
It does. The sys_si_private value is not going to match the timer side
value and obviously the missing prealloc flag prevents it from trying to
rearm the timer.
> I checked the CRIU source and it looks like this just "works" by
> reconstructing and rearming the timer with the last expiry value. As
> that is in the past it will fire immediately and queue the signal.
It's not necessarily in the past, but it will fire eventually and in the
case of a blocked signal there will be two SI_TIMER signals queued.
So the patch is not completely wrong except that there is nothing which
prevents setting sys_si_private via rt_sigqueueinfo(), but that's
obviously a solvable problem. With that solved the condition:
*resched_timer =
(first->flags & SIGQUEUE_PREALLOC) &&
(info->si_code == SI_TIMER) &&
(info->si_sys_private);
really can be reduced to:
info->code == SI_TIMER && info->si_sys_private
In fact it makes a lot of sense _not_ to allow user space to set
info->si_sys_private because that's a kernel internal value and should
never be exposed to user space in the first place.
Let me look what it needs or if this can be solved slightly differently.
Powered by blists - more mailing lists