Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <cc8aa6a4-a187-f3ad-fec9-05f037a3886d@virtuozzo.com>
Date:   Thu, 11 May 2023 11:17:07 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [RFD] posix-timers: CRIU woes
Content-Language: en-US
To:     Andrey Vagin <avagin@openvz.org>,
        Thomas Gleixner <tglx@linutronix.de>
Cc:     Frederic Weisbecker <frederic@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Anna-Maria Behnsen <anna-maria@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>,
        syzbot+5c54bd3eb218bb595aa9@syzkaller.appspotmail.com,
        Dmitry Vyukov <dvyukov@google.com>,
        Sebastian Siewior <bigeasy@linutronix.de>,
        Michael Kerrisk <mtk.manpages@gmail.com>,
        Christian Brauner <brauner@kernel.org>,
        Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>,
        Pavel Emelyanov <xemul@openvz.org>
References: <20230425181827.219128101@linutronix.de>
 <20230425183312.932345089@linutronix.de> <ZFUXrCZtWyNG3Esi@lothringen>
 <87zg6i2xn3.ffs@tglx> <87v8h62vwp.ffs@tglx> <878rdy32ri.ffs@tglx>
 <87v8h126p2.ffs@tglx> <875y911xeg.ffs@tglx> <87ednpyyeo.ffs@tglx>
 <CANaxB-wV9iUT6=Y9nZCWbJhiscMrnAQh4fUXs7Tb8pr=-HwSYQ@mail.gmail.com>
From:   Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
In-Reply-To: <CANaxB-wV9iUT6=Y9nZCWbJhiscMrnAQh4fUXs7Tb8pr=-HwSYQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a1NHN3hqMGdnV0JYalg2Tm1YeXBJN2pqd3FUblprYWdVQUwzT1Y0TU5WL2Nn?=
 =?utf-8?B?YjFuSGhGK3pLZUNzRGV1a0RRZTZ2VkJaRGdTbjY3dEFqODBOcFlnZDUrSzdX?=
 =?utf-8?B?akRyeWUxY2RHOXZUcWJ4bmZBenAvZzVUcEVOSEdLQjFEcCs0QVIxMmUvdThJ?=
 =?utf-8?B?YVRNb2QvVmZrcXZhVVJFclBRdzhPSlBDdU54MERvZTBvLzJBRE5meTZOUHc1?=
 =?utf-8?B?UXdXUU9vVVAxRE1uTFJzeDJ2eXlCWlJobjJQclQvUUtPTnpZUDk4SnI4bWFO?=
 =?utf-8?B?bnp5NjRBT0lycHc4Uk16WXhGTDNWd3htK1pWYkJ2K0o4a1p0c2M4dEFPRXFp?=
 =?utf-8?B?L0NBaW1XS3Q5RXJDSlJjOHpONDVUSHRSSkM3SENDU2dUKzUwUUtPSGVKTitx?=
 =?utf-8?B?cFZaR3BSR2xVMVFwTUJLeEdHSzN2TG1PaFhoemEvcllHWWYrd0hLb0tyeUpv?=
 =?utf-8?B?cFZIc0ZrS1lWaVg3eVBqdXBTYWk4RVo5N2RGRnV4dFlSTXV3cXNzWWViVGwr?=
 =?utf-8?B?eVdCRWZSb3g3T3hraHRCa3VMblhHVm5xMTlmcEdpaG0zc2RZMGFNUnpZYkgz?=
 =?utf-8?B?SWdqU3FNcGhTcWVqSnNNMis2TUxFdXJNUzRQNys0Vk1EWW5VYUhISWhpckdm?=
 =?utf-8?B?VWczKzlFY3JpQ3ZPdkR4eTI1SU9ldmltTUlXQkxlN3pKNGRqdWlvcmRHQjZm?=
 =?utf-8?B?bG5wY2FHbU03QjQ5T2FLaTNHcGhlSHR1Zmd6Qy9QMmNvWDg5d2FsSnB1dGZs?=
 =?utf-8?B?WGF3Y1BGSkVqbGVZVjJ2Q2RwbFp5OVlZb3EzZE5xeUx2aDVDallaYUlGdWI5?=
 =?utf-8?B?eFpBMXh1OWlxaVlPR2R6L3ZyNVZ4VElSMSs2SkI4eitjYXZreWR0QUduUXhl?=
 =?utf-8?B?VkxrT3h6YllVOGIzNzRVQmlUdEJ6SlJBa2NMZnRHTUJ3SE13Zk94RmVubmU4?=
 =?utf-8?B?NFRiZEdOOFJOY2hoK1h0Z3lKZHc4SVVwRyt4a01IWjRySTBuSlJiSEFmQnB2?=
 =?utf-8?B?d1dZOHhpN2I3WTlBSkRCTGhCYWUzYXBuZDRId1NreG1udDFXRUtySzJDQ3hX?=
 =?utf-8?B?ak5ERkE3L3d3TitaWE9sbmFBYlJSRHVoS09vZ0IyLytJL24yRTM3TTZHZXVx?=
 =?utf-8?B?K3R6YUtTSW1sd05rRFhnNWVUNjY3V2tPcE11Yzh2WmgvaXo1QnB1aFBUWVRw?=
 =?utf-8?B?b3lBQVJvMzVNVnNiTGVMZlFVOGw0KzVYTUFRRm8xQ1VETW9BQ1dyYUZSaTlo?=
 =?utf-8?B?RHZSQ2hEcEFKcW1iZXdOQmF1VXlrNmRLTXQrMElUb1MrQVZJaWllRGpMS2NU?=
 =?utf-8?B?a0s4YUhodGdrTnJIM1U1dC9TM2F5MkFQcnJnVnVabUpWQ2VxQU1ibTFoL3dJ?=
 =?utf-8?B?YzJwWDRETHlTOWVBbVIzb0dhcTJYeWRaY0JTanVJQjlHaEFoOC93VjRDd25Q?=
 =?utf-8?B?aWlBUDN5TjZWS0kyODlGZFc4WWJibE00TEZXK1p0Y1ZXRSsybkZPVWRDOCtp?=
 =?utf-8?B?bmczZGwrblVKWGNLbnZDMlYzYWtVVWU0Nk9sbEgzL0NKYXVrbnAybVFaUm1Z?=
 =?utf-8?B?bDNWcVhPWVFIS3djSElKQWZqUW9rbUVYYzJ1RXFTMEp3eWVoUzRGeTJQUDEz?=
 =?utf-8?B?cUJMWXlSVEZZc0lNK3pPWDZhbjFjcGpWVHY1NzZQbVVGL0hRNGRZVGZmVUI1?=
 =?utf-8?B?R01GTUkzT0k2ektwQ05TcytlSXNEUDZYeUZZZmN2NHErb1gxSnNYclpFZE41?=
 =?utf-8?B?Q21RekpobnpQMnVzWlZjWVRLdmpWTGowNG1rTkV2Q1hPR0haOTVPWGErMkVJ?=
 =?utf-8?B?Z1MvREVUYjhCSjhIeGNzSDZLVUs3QWFJMzBKd0JCbnIvc2NVUzBNdnpRbXpD?=
 =?utf-8?B?amJTWFg1ODFubmdUL3EzT2Q2NE5DcWZndUZldFVVRGF6SlZmNGN6NVZFS1dk?=
 =?utf-8?B?OHZwTU8rUElWREtyNXIvS1NiMmtTQnF1eVZxWTd3b0hkKzA2MWtkR0FnYVBM?=
 =?utf-8?B?Qm8zZUFKaE1mZUc3NlF4NXM2M3NvY2JyazV5VU1YK3ZnK1MxU1YxQlZ4S3FC?=
 =?utf-8?B?V0NyMTg3NTRhWUJwV0huNEsvV051Z3VnbHc4anY5dFhvbUovdkszYW9NWFVv?=
 =?utf-8?B?ZGZ6SW5zS1dySGQ0d1VZSW1odlBONGxUNVpIQmdreEt6V1R0NUJJcFVldU5P?=
 =?utf-8?B?WGN6aEtzbnZCbnpuUXlBOGRmcGZNVk9aK1ZSMThoTFU2TmVPRFppMG0xVlhF?=
 =?utf-8?B?T0NTRzBvcGxBYlNrU0JQMXFKODlBPT0=?=
X-OriginatorOrg: virtuozzo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 91f3243c-1b88-4f5a-07c4-08db51ce3adb
X-MS-Exchange-CrossTenant-AuthSource: VE1PR08MB4989.eurprd08.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 May 2023 03:17:19.8311
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: qLGzFtJ2wF+xurBWRfLs0yLNePZ5RiFEwm3/ZiX8ur2/pGvbG3OMHwGi3kaiWrbTYZCSjPFnypYhE4KBxTIDHsZF+6qlu4EAnYN9/VmXYxo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBBPR08MB6043
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 10.05.2023 16:16, Andrey Vagin wrote:
> On Tue, May 9, 2023 at 2:42 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> Hi!
>>
>> This is a summary of several mails so that the CRIU people have the full
>> picture.
>>
>> A recent syzbot report made me look at the timer ID management, which
>> was modified 7 years ago to accomodate CRIU:
>>
>>      5ed67f05f66c ("posix timers: Allocate timer id per process (v2)")
>>
>> and introduced that reported issue along with a bogus loop termination
>> problem. See
>>
>>      https://lore.kernel.org/lkml/000000000000a3723305f9d98fc3@google.com/
>>      https://lore.kernel.org/lkml/20230425183312.932345089@linutronix.de
>>
>> for details.
>>
>> The intent to make the timer IDs per process is definitely correct, but
>> the implementation is beyond suboptimal. I really regret that I did not
>> catch this back then when picking those changes up.
>>
>> The way it works is that each process keeps a 'next_timer_id' which it
>> uses to allocate the next timer. That allows CRIU to reconstruct timers
>> with the same ID by walking the ID space via
>>
>>    do {
>>       timer_create(&timer, ...., &id);
>>       if (id == original_id)
>>          goto success;
>>       timer_delete(&timer);
>>    } while (idspace_not_exhausted());
>>
>> That works by some definition of works, but it is problematic in two ways:
>>
>>   1) As the timer ID space is up to INT_MAX, a process which creates and
>>      deletes timers frequently, can easily move up close to the INT_MAX
>>      id space over time.
>>
>>      If such a process is checkpointed and restored, then the above loop
>>      will run for at least an hour to restore a single timer.
>>
>>      And no, this is not only a hypothetical issue. There are legitimate
>>      scenarios where threads are created and the control thread arms a
>>      posix CPU timer on them. Such threads can be torn down on a regular
>>      base due to thread pool consolidations. As CRIU does not happen
>>      every 5 minutes it's not completely unlikely that such a process
>>      surives quite some time on a particular host and thereby approaches
>>      the ID space limit.
>>
>>      Sure we can restrict the ID space to a way smaller number so the
>>      search wraps around earlier, but what's a sensible limit?
>>
>>      Though restricting the ID space has its own issue vs. backwards
>>      compability. A process which created a timer on an older kernel with
>>      an ID larger than the newer kernels ID limit cannot longer be
>>      restored on that newer kernel.
>>
>>      Aside of that it does not solve the other problem this created:
>>
>>   2) That change created an user space ABI, which means that the kernel
>>      side has to stick with this next ID search mechanism forever.
>>
>>      That prevents to get rid of that global lock and hash table by
>>      sticking an xarray into task::signal which makes a lot of sense in
>>      terms of cache locality and gets rid of the extra timer list
>>      management in task::signal. Making this change would be very useful
>>      to address some other issues in the posix-timer code without
>>      creating yet more duct tape horrors.
>>
>>      Such a change obviously has to aim for a dense ID space to keep the
>>      memory overhead low, but that breaks existing CRIU userspace because
>>      dense ID space and next ID search does not fit together.
>>
>>      Next ID search is obviously creating non-recoverable holes in the
>>      case that timers are deleted afterwards.
>>
>>      A dense ID space approach can create holes too, but they are
>>      recoverable and well within the resource limits, because the process
>>      has to be able to create enough timers in the first place in order
>>      to release those in the middle.
>>
>>      With the next ID search brute force recovery is not possible on a
>>      kernel with dense ID as there is no way to create all intermediate
>>      timers first before reaching the one at the far end due to resource
>>      limits.
>>
>> So because of that half thought out user space ABI we are now up the
>> regression creek without a paddle, unless CRIU can accomodate to a
>> different restore mechanism to lift this restriction from the kernel.
>>
>> Thoughts?
> 
> Hi Thomas,
> 
> If you give us a new API to create timers with specified id-s, we will
> figure out how to live with it. It isn't good to ask users to update
> CRIU to work on new kernels, but here are reasons and event improvements
> for CRIU, so I think it's worth it.

I agree, any API to create timers with specified id-s would work for new 
CRIU versions.

> 
> As for API, we can use one bit of sigevent.sigev_notify to request a
> timer with a specified id.

Yes, I agree, this kind of API is a good option.

> 
> Thanks,
> Andrei


-- 
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.