From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC286C36005 for ; Thu, 20 Mar 2025 13:40:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5FA71280002; Thu, 20 Mar 2025 09:40:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5829A280001; Thu, 20 Mar 2025 09:40:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3FCD8280002; Thu, 20 Mar 2025 09:40:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 1DD2C280001 for ; Thu, 20 Mar 2025 09:40:05 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 701E116148C for ; Thu, 20 Mar 2025 13:40:06 +0000 (UTC) X-FDA: 83242038012.03.CBDE69F Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.21]) by imf29.hostedemail.com (Postfix) with ESMTP id E24A5120020 for ; Thu, 20 Mar 2025 13:40:03 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=XmtgeeTf; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf29.hostedemail.com: domain of andriy.shevchenko@linux.intel.com has no SPF policy when checking 198.175.65.21) smtp.mailfrom=andriy.shevchenko@linux.intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1742478004; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=F1HtQq5nH5jH7qHdU74Umd8ms24Hg/lQNYGZqN3/vxQ=; b=s8MwXA6+n5oUsguT7AZXx0i+/FfC8iyxJ+Pa/FNiC5sdTgM2/1to50/sKW5AiIhhJkuVSQ YXKjbZ5DJpETqxI4WBGW8a0oE4KiabIdA9K7t+sEghSdD5YiCRl/CmuinRUdH2u/rO6wtY znrvc1aMAbK/JIU9acdlzsC+GNpTGNc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1742478004; a=rsa-sha256; cv=none; b=7FyecE8DTKdHMCsOxVdtNGmkcHN0An94x8YzU5rW8cqgaMkbtcpIHmtvNmCbI63IMgm7m/ xB15k+xJox1bYB6Pu3n3/d1zXzBf6F6CM4wg9c5JXR0zOP6NIaIGIKVkjTih3jnmPOT/3l NPuIeNV8WBgYhUYyy0v305HQalg35Rw= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=XmtgeeTf; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf29.hostedemail.com: domain of andriy.shevchenko@linux.intel.com has no SPF policy when checking 198.175.65.21) smtp.mailfrom=andriy.shevchenko@linux.intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1742478004; x=1774014004; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=LtWi7OjWEJ70WoRQ+ITfE9olr6zQDR0kAGYpbs7gw3Q=; b=XmtgeeTfF+ypoC0/BlNr5K9v2uXDJ7cLIDBz1Wy6Z3891e6mLuoP8rlY oWleWez9EX062OuqOtGRo+J3p9I1yVlIxJ3JnG/wnjXgd9oPiAVUwKPeP OEs+l+yWWgUwd0oFf/7fXnLwqy2pFHga5evOe+QwIpDn04eaqz6joWO38 O3bLTrCp3SfkoU26UTFhjOTqQgVmXTLUB9givXhYl3dbjiYc8aXLxvCLO /f04ii0x7MJFHlKy4p5UEud/UZF+qQJ8oc9hi6VQBpmB9hhxE9dr/SaHO KF5oZSgnyiG1YbxAquI0kvqt1tD5jDitQPSygm9xsWkVEO8KAPpZ1T8qh w==; X-CSE-ConnectionGUID: labg8CqSR9eBmBeuM4+ibQ== X-CSE-MsgGUID: 6vBOxavJQu+RNes69Awhgw== X-IronPort-AV: E=McAfee;i="6700,10204,11379"; a="43628200" X-IronPort-AV: E=Sophos;i="6.14,261,1736841600"; d="scan'208";a="43628200" Received: from fmviesa002.fm.intel.com ([10.60.135.142]) by orvoesa113.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Mar 2025 06:40:02 -0700 X-CSE-ConnectionGUID: pT1/HkoDQWWTbz5tShmYvQ== X-CSE-MsgGUID: qIaaMbFGQLCv5lsW4B3/Wg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.14,261,1736841600"; d="scan'208";a="146305344" Received: from smile.fi.intel.com ([10.237.72.58]) by fmviesa002.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Mar 2025 06:39:50 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.98) (envelope-from ) id 1tvG7b-00000004EWe-3yk8; Thu, 20 Mar 2025 15:39:43 +0200 Date: Thu, 20 Mar 2025 15:39:43 +0200 From: Andy Shevchenko To: Pasha Tatashin Cc: changyuanl@google.com, graf@amazon.com, rppt@kernel.org, rientjes@google.com, corbet@lwn.net, rdunlap@infradead.org, ilpo.jarvinen@linux.intel.com, kanie@linux.alibaba.com, ojeda@kernel.org, aliceryhl@google.com, masahiroy@kernel.org, akpm@linux-foundation.org, tj@kernel.org, yoann.congal@smile.fr, mmaurer@google.com, roman.gushchin@linux.dev, chenridong@huawei.com, axboe@kernel.dk, mark.rutland@arm.com, jannh@google.com, vincent.guittot@linaro.org, hannes@cmpxchg.org, dan.j.williams@intel.com, david@redhat.com, joel.granados@kernel.org, rostedt@goodmis.org, anna.schumaker@oracle.com, song@kernel.org, zhangguopeng@kylinos.cn, linux@weissschuh.net, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-mm@kvack.org, gregkh@linuxfoundation.org, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, rafael@kernel.org, dakr@kernel.org, bartosz.golaszewski@linaro.org, cw00.choi@samsung.com, myungjoo.ham@samsung.com, yesanishhere@gmail.com, Jonathan.Cameron@huawei.com, quic_zijuhu@quicinc.com, aleksander.lobakin@intel.com, ira.weiny@intel.com, leon@kernel.org, lukas@wunner.de, bhelgaas@google.com, wagi@kernel.org, djeffery@redhat.com, stuart.w.hayes@gmail.com, jgowans@amazon.com, jgg@nvidia.com Subject: Re: [RFC v1 1/3] luo: Live Update Orchestrator Message-ID: References: <20250320024011.2995837-1-pasha.tatashin@soleen.com> <20250320024011.2995837-2-pasha.tatashin@soleen.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250320024011.2995837-2-pasha.tatashin@soleen.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: E24A5120020 X-Stat-Signature: jh7nif3ukp6z36j3r7g618cfjeyuboky X-HE-Tag: 1742478003-928859 X-HE-Meta: U2FsdGVkX1+onyqaZLMAYd66Ospg05nbjfrAd+EdZ7gVaH8i+5cpgzJyl6ze0YS/TRMYdAlxmt7o1qzd7p49N6sPsiwOpyz6xXA9w6OWU/PItRRsUVqImtx5T8XODoqYj0WiAHW+QtCqbiWaUzX+24nMCNQYD8yw+DLieGFQEhE1AJLBLSstr+wmeNTNkkMhRLEHo95r0S89qipIc62d7R6J+hDr2p5cbTqHO9Rtu0d0TOVe+87yM+ZkuFfunfXLriMoksY6IsnnjJj1j1TaQllpwg9eUbVG0clqTqHRnufsYE+CQiXOa5yyxT1o3nlYsKuze2Eh5FjN9YlYt7Qjnu6IYynzuvcu0BRWjFXRNccssTNr10fuEOJ/eQ1HQYE5LQYDq4pu/t4UxVSr0cCJ8NtzwyUsLF1ZuKn5pndxlfFFE/Yt1JqF0YE33fOnJxZRzKqw113VPXdnIiHrzPcq0mpk41MgbPjosKsnqju4g5BnTXyegBFM0UZTf7UprL/o/Dwh9pmpgOQxt0q/8NZ09I9mPSjdhzJHC75Rf1nIAm6iHVFw7ESisLUQUg65ElPexQqArlQXIDQYZBaAi2oIqsP58w3fsgWn2041L2IIKUqCLuSo3kORFm9I4KgwWR7mrgJHEi8ls9vwx2h3jImoK3S2MlT1hITwBo7JwhCVQSmQbo3dBxfBEorHfN9NyCtqjmm7AMGc1xhu/MsfP1ga+Nz5qgGPkWIxCAnH0CbLR4sbWLPeZTTjbx6TztKs444O8rbr1Nv9Xyfd8xRF3+YB8M+qyAOQAuRoP7Ihf8gGlteKWxd+bdCZT2iOJsYsYAm3/m5Y5AGwDKMdIq3Ryi38SxPz/pYawwi6yXj02ov5nkh7+yj0zaYVnhQU0h/0CxwNPR3zziS1AKTzXaC5Bo/XwtIYcCqaLIw1dwMOrZTIarG0sXwrS3RAkLHRGsM9P2ZgG1iOcTb3Ff33RTpRhK4 qL5AIZSA 6LeTyStLBSxIkcn6DhE8hoPkPvwYLWFbiuLfcUIaWRG/9w9kWoTvTLn0uVG9FGn25gaJ/IjedNPD7zJfzejBvhjvCQaztHpzG5sk98agFiWGyA+Udyw7ERug2fnOKGpUJ2GFdO2m08L4Sck0paFjjY9+X2aoCdUZvXCIGAExCIHLF0QziGRpGLkYX4IMfzxnc6Em7EHmSfZR7GxJ9kz0MFv2GiSeu+H3M8W27m2EigqgveCbiP30+s0J2sjDOlLWEtPFLLg3JQt0ihh596C8anvZyJDgrQOvMI+9bnLtua273uhw= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, Mar 20, 2025 at 02:40:09AM +0000, Pasha Tatashin wrote: > Introduces the Live Update Orchestrator (LUO), a new kernel subsystem > designed to facilitate live updates. Live update is a method to reboot > the kernel while attempting to keep selected devices alive across the > reboot boundary, minimizing downtime. > > The primary use case is cloud environments, allowing hypervisor updates > without fully disrupting running virtual machines. VMs can be suspended > while the hypervisor kernel reboots, and devices attached to these VM > are kept operational by the LUO. > > Features introduced: > > - Core orchestration logic for managing the live update process. > - A state machine (NORMAL, PREPARED, UPDATED, *_FAILED) to track > the progress of live updates. > - Notifier chains for subsystems (device layer, interrupts, KVM, IOMMU, > etc.) to register callbacks for different live update events: > - LIVEUPDATE_PREPARE: Prepare for reboot (before blackout). > - LIVEUPDATE_REBOOT: Final serialization before kexec (blackout). > - LIVEUPDATE_FINISH: Cleanup after update (after blackout). > - LIVEUPDATE_CANCEL: Rollback actions on failure or user request. > - A sysfs interface (/sys/kernel/liveupdate/) for user-space control: > - `prepare`: Initiate preparation (write 1) or reset (write 0). > - `finish`: Finalize update in new kernel (write 1). > - `cancel`: Abort ongoing preparation or reboot (write 1). > - `reset`: Force state back to normal (write 1). > - `state`: Read-only view of the current LUO state. > - `enabled`: Read-only view of whether live update is enabled. > - Integration with KHO to pass orchestrator state to the new kernel. > - Version checking during startup of the new kernel to ensure > compatibility with the previous kernel's live update state. > > This infrastructure allows various kernel subsystems to coordinate and > participate in the live update process, serializing and restoring device > state across a kernel reboot. ... > +Date: March 2025 > +KernelVersion: 6.14.0 This is way too optimistic, it even won't make v6.15. And date can be chosen either v6.16-rc1 or v6.16 release in accordance with prediction tool ... > +#ifndef _LINUX_LIVEUPDATE_H > +#define _LINUX_LIVEUPDATE_H > +#include > +#include This is semi-random list of inclusions. Try to follow IWYU principle. See below. ... > +bool liveupdate_state_updated(void); Where bool is defined? ... > +/** > + * LIVEUPDATE_DECLARE_NOTIFIER - Declare a live update notifier with default > + * structure. > + * @_name: A base name used to generate the names of the notifier block > + * (e.g., ``_name##_liveupdate_notifier_block``) and the callback function > + * (e.g., ``_name##_liveupdate``). > + * @_priority: The priority of the notifier, specified using the > + * ``enum liveupdate_cb_priority`` values > + * (e.g., ``LIVEUPDATE_CB_PRIO_BEFORE_DEVICES``). > + * > + * This macro declares a static struct notifier_block and a corresponding > + * notifier callback function for use with the live update orchestrator. > + * It simplifies the process by automatically handling the dispatching of > + * live update events to separate handler functions for prepare, reboot, > + * finish, and cancel. > + * > + * This macro expects the following functions to be defined: > + * > + * ``_name##_liveupdate_prepare()``: Called on LIVEUPDATE_PREPARE. > + * ``_name##_liveupdate_reboot()``: Called on LIVEUPDATE_REBOOT. > + * ``_name##_liveupdate_finish()``: Called on LIVEUPDATE_FINISH. > + * ``_name##_liveupdate_cancel()``: Called on LIVEUPDATE_CANCEL. > + * > + * The generated callback function handles the switch statement for the > + * different live update events and calls the appropriate handler function. > + * It also includes warnings if the finish or cancel handlers return an error. > + * > + * For example, declartion can look like this: > + * > + * ``static int foo_liveupdate_prepare(void) { ... }`` > + * > + * ``static int foo_liveupdate_reboot(void) { ... }`` > + * > + * ``static int foo_liveupdate_finish(void) { ... }`` > + * > + * ``static int foo_liveupdate_cancel(void) { ... }`` > + * > + * ``LIVEUPDATE_DECLARE_NOTIFIER(foo, LIVEUPDATE_CB_PRIO_WITH_DEVICES);`` > + * Hmm... Have you run kernel-doc validator? There is missing Return section and it will warn about that. > + */ ... > + WARN_ONCE(rv, "cancel failed[%d]\n", rv); \ + bug.h ... > + #undef pr_fmt > + #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt Leftover from the development? > +#undef pr_fmt Not needed as long as pr_fmt9) is at the top of the file. > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt ... > +#include What for? Can you follow IWYU, please? Here again semi-random list of inclusions. > +#include > +#include > +#include > +#include > +#include > +#include > +#include Can you keep them ordered which will be easier to read and maintain? ... > +static int __init early_liveupdate_param(char *buf) > +{ > + return kstrtobool(buf, &luo_enabled); > +} > + Redundant blank line. > +early_param("liveupdate", early_liveupdate_param); ... > +/* Show the current live update state */ > +static ssize_t state_show(struct kobject *kobj, > + struct kobj_attribute *attr, It is still enough room even for the strict 80 limit case. > + char *buf) > +{ > + return sysfs_emit(buf, "%s\n", LUO_STATE_STR); > +} ... > + ret = blocking_notifier_call_chain(&luo_notify_list, > + event, > + NULL); There is room on the previous lines. Ditto for the similar cases all over the code. ... > +{ > + int ret; > + > + if (down_write_killable(&luo_state_rwsem)) { > + pr_warn(" %s, change state canceled by user\n", __func__); Why __func__ is so important in _this_ message? And why leading space? Ditto for the similar cases. > + return -EAGAIN; > + } > + > + if (!IS_STATE(LIVEUPDATE_STATE_NORMAL)) { > + pr_warn("Can't switch to [%s] from [%s] state\n", > + luo_state_str[LIVEUPDATE_STATE_PREPARED], > + LUO_STATE_STR); > + up_write(&luo_state_rwsem); > + > + return -EINVAL; > + } > + > + ret = luo_notify(LIVEUPDATE_PREPARE); > + if (!ret) > + luo_set_state(LIVEUPDATE_STATE_PREPARED); > + > + up_write(&luo_state_rwsem); > + > + return ret; > +} ... > +static ssize_t prepare_store(struct kobject *kobj, > + struct kobj_attribute *attr, > + const char *buf, > + size_t count) > +{ > + ssize_t ret; > + long val; > + if (kstrtol(buf, 0, &val) < 0) > + return -EINVAL; Shadower error code. > + if (val != 1 && val != 0) > + return -EINVAL; What's wrong with using kstrtobool() from the beginning? > + > + if (val) > + ret = luo_prepare(); > + else > + ret = luo_cancel(); > + if (!ret) > + ret = count; > + > + return ret; Can we go with the usual pattern "check for errors first"? if (ret) return ret; ... > +} ... > +static ssize_t finish_store(struct kobject *kobj, > + struct kobj_attribute *attr, > + const char *buf, > + size_t count) Same comments as per above. ... > +static struct attribute *luo_attrs[] = { > + &state_attribute.attr, > + &prepare_attribute.attr, > + &finish_attribute.attr, > + NULL, No comma for the terminator entry. > +}; ... > +static int __init luo_init(void) > +{ > + int ret; > + > + if (!luo_enabled || !kho_is_enabled()) { > + pr_info("disabled by user\n"); > + luo_enabled = false; > + > + return 0; > + } Can be written like if (!kho_is_enabled()) luo_enabled = false; if (!luo_enabled) { pr_info("disabled by user\n"); return 0; } > + ret = sysfs_create_group(kernel_kobj, &luo_attr_group); > + if (ret) > + pr_err("Failed to create group\n"); > + > + luo_sysfs_initialized = true; > + pr_info("Initialized\n"); > + > + return ret; Something is odd here between (non-)checking for errors and printed messages. > +} ... > +EXPORT_SYMBOL_GPL(liveupdate_state_normal); No namespace? ... > --- a/kernel/reboot.c > +++ b/kernel/reboot.c > @@ -18,6 +18,7 @@ > #include > #include > #include > +#include Can oyu preserve order (with given context at least)? -- With Best Regards, Andy Shevchenko