Received-SPF: pass (google.com: domain of linux-kernel+bounces-156104-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Thomas Gleixner <tglx@linutronix.de>
To: Oleg Nesterov <oleg@redhat.com>
Cc: LKML <linux-kernel@vger.kernel.org>, Anna-Maria Behnsen
 <anna-maria@linutronix.de>, Frederic Weisbecker <frederic@kernel.org>,
 John Stultz <jstultz@google.com>, Peter Zijlstra <peterz@infradead.org>,
 Ingo Molnar <mingo@kernel.org>, Stephen Boyd <sboyd@kernel.org>, Eric
 Biederman <ebiederm@xmission.com>, Andrei Vagin <avagin@google.com>
Subject: Re: [patch V2 26/50] signal: Get rid of resched_timer logic
In-Reply-To: <87sezbu69x.ffs@tglx>
References: <20240410164558.316665885@linutronix.de>
 <20240410165552.572304080@linutronix.de>
 <20240418163811.GA23440@redhat.com> <20240418181821.GA26239@redhat.com>
 <20240419110632.GA3198@redhat.com> <87sezbu69x.ffs@tglx>
Date: Wed, 24 Apr 2024 03:48:53 +0200
Message-ID: <87plufttru.ffs@tglx>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

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.