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 32FACC71148 for ; Fri, 13 Jun 2025 15:27:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C88C36B0098; Fri, 13 Jun 2025 11:27:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C60BE6B0099; Fri, 13 Jun 2025 11:27:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B2AB16B009A; Fri, 13 Jun 2025 11:27:33 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 967B36B0098 for ; Fri, 13 Jun 2025 11:27:33 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 43030160A1A for ; Fri, 13 Jun 2025 15:27:33 +0000 (UTC) X-FDA: 83550756786.28.5980948 Received: from tor.source.kernel.org (tor.source.kernel.org [172.105.4.254]) by imf18.hostedemail.com (Postfix) with ESMTP id B71231C0014 for ; Fri, 13 Jun 2025 15:27:31 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=OBosYd0U; spf=pass (imf18.hostedemail.com: domain of pratyush@kernel.org designates 172.105.4.254 as permitted sender) smtp.mailfrom=pratyush@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1749828451; 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=nJYsCyrZDIHx/tln3kYvDrRSj+BDkGVPh63pYasNkqs=; b=EcJxGwekphwsuGdBsWOo4Hytclf8H4vXzwRV1zD2fHzcnI5QiKbbjVLm769rca4UYg4q3n 7SvP5fdhI7/WKDn7qYAZA9YQE2mFuKq4GSdgJL0GYSx0R29eBH/HVqsRz2SuXldocxgekX tO/xhCOrnTDuKZcc+jqoSCirPIbqYXo= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=OBosYd0U; spf=pass (imf18.hostedemail.com: domain of pratyush@kernel.org designates 172.105.4.254 as permitted sender) smtp.mailfrom=pratyush@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1749828451; a=rsa-sha256; cv=none; b=fgYK8XZAci3qOZlRkdNA6lqDYpRGQa6iDKIvVCPnAMsDHKKVWvXLi4ZeAHgRrOvxuwrEt7 ZUKUhI0An9QxsTxv7rrewuj2Hz5bkr6XH8LLnHr8DTbgeeMqr8Tbxzb0SSAlmiSa1Y+QRj qb/QHxfo2J8j0XNWEQfjSMgQiLe1Fk0= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id A612362A13; Fri, 13 Jun 2025 15:27:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AEC73C4CEE3; Fri, 13 Jun 2025 15:27:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1749828450; bh=88xUdYBMPT3UbWHPTJRNtJ5le6acxvABX2a2MypWfUE=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=OBosYd0UCBZw4SAiVzH5bFP26U8lcMe2wbd/3tbGiUAQ9sfKtUT2UeGBUS50U9S4P F6Qt1zqdq8JNIyEFCbk/oIp6YWZXv/UUqkYVEqLxwjhuQ/66c4+ka28Qh4HVgBdVdV SB1Mnr5ycqi1qRMC7qKas9U652pK/xJngizVsheYzvkbsjBiYv+y+i/yT/A+ogx5fN YKgYRNzaqcMkMv6xGYhGfseiaMJ58y+G4SfCtefRuriPxTMAPoUXqBZfpRLZn/UICh 2ocRZSUrTAIXwmBzylEyCCg4nzKTpd2//Brxj1WhoTUAr+b6hyMJbfwbIpFQDpyln+ eCaXnAU7yNhyw== From: Pratyush Yadav To: Pasha Tatashin Cc: Pratyush Yadav , jasonmiu@google.com, graf@amazon.com, changyuanl@google.com, rppt@kernel.org, dmatlack@google.com, 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, andriy.shevchenko@linux.intel.com, leon@kernel.org, lukas@wunner.de, bhelgaas@google.com, wagi@kernel.org, djeffery@redhat.com, stuart.w.hayes@gmail.com Subject: Re: [RFC v2 08/16] luo: luo_files: add infrastructure for FDs In-Reply-To: References: <20250515182322.117840-1-pasha.tatashin@soleen.com> <20250515182322.117840-9-pasha.tatashin@soleen.com> Date: Fri, 13 Jun 2025 17:27:21 +0200 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-Rspamd-Queue-Id: B71231C0014 X-Rspamd-Server: rspam03 X-Rspam-User: X-Stat-Signature: dqn1dwcsathak13w5r7ndr7ro5oxk1k3 X-HE-Tag: 1749828451-804741 X-HE-Meta: U2FsdGVkX18AfvD4ZrAV6VU7+uLRmnvwvK8KXON+r16rQzD8cwomNVlwIMHs+GORFm7RB/YzThz28bs/k5L32RbcKRQia46zBWO1JFyS3oDvcAamO2qx28L7D1Y3JIJKgqnkdVP/IRpn98xN1YWL+JSC47Ns++EXUh1q9Lfh1jFZ6XxpahwL8ZlczY+GyDZKrMaFVFmIjoUveiKUvxslRcVGRea9VETzDyIMHEytSbkYUGhZWQheQdPPJPejf6iegLFjWf9PkX8ySVONJZWUveD2X5U4SFuwKvhVQaummTxWjD8cZE6hXkwSebm0t6/RQKi+E9JwHqO0IgHUl9mAEq5EBTh7MQdkW1CZMveMc/bV3aNNZ4wJ+yB5Ge+bJv/VYwrFmo5K5cU4fe7/XPI9sWOXdR+pqmEhSyBLnQ/vMY2BIDsXqiWqQq9v+5jRstF22ZVcfjY3sS4mBn+biVp1pd9oCsecPayBftrA8C7xXMcj+TDp0Mob210tk+OzLAXQod656FsDHHBbCp6oi7K4V3GPONLU2UUKa/YNIp5UTHWLRM0h8VNdf8q4/zjLLscH+B1vQ4vgH8IYcswDKCXvf4uPAsouFqobgDY5E5Y+IU/wmkMzr/odAvov4N4Fjb07D8LN8HRb2VDAMC9rhksqixdoSeAHakJdrmvI65gorQxR1Q8JfSw1+rqox6323EW6bdwyjCguw41id3iY/nI0DttdOoSwddBGKNxcB1GKd2smoCHbCD9TqhhL0RDv1zbmlxTm0E536RxDQ9l3bzc/p4ePgOktKTAL/7jCQoCRdFo40WwgVJJuF8fScELo3/N7+G5gjyXCSMDM2hGPI6X7OQeRctttLJ5CNKdbbuiejXNltyqo1ik/vFRcXcGXaa03zl/x3MQoKtebD+2VOpVqbN4WCthaoMExBEBqzTab5d4PKYu/cgyiNpB8eWJ8RpH5v3OgsPzTxWLBbyRVbYa DUoB662Q jdG9NQ3CzijIUZtwRYXb7SG/93JP4iQW6SF8QtvOo9mhauIY= 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 Sun, Jun 08 2025, Pasha Tatashin wrote: [...] >> > + down_write(&luo_filesystems_list_rwsem); >> > + if (luo_files_xa_in_recreated) >> > + goto exit_unlock; >> > + >> > + parent_node_offset = fdt_subnode_offset(luo_fdt_in, 0, >> > + LUO_FILES_NODE_NAME); >> > + >> > + fdt_for_each_subnode(file_node_offset, luo_fdt_in, parent_node_offset) { >> > + bool handler_found = false; >> > + u64 token; >> > + >> > + node_name = fdt_get_name(luo_fdt_in, file_node_offset, NULL); >> > + if (!node_name) { >> > + panic("Skipping FDT subnode at offset %d: Cannot get name\n", >> > + file_node_offset); >> >> Should failure to parse a specific FD really be a panic? Wouldn't it be >> better to continue and let userspace decide if it can live with the FD >> missing? > > This is not safe, the memory might be DMA or owned by a sensetive > process, and if we proceed liveupdate reboot without properly handling > memory, we can get corruptions, and memory leaks. Therefore, during > liveupdate boot if there are exceptions, we should panic. I don't get how it would result in memory leaks or corruptions, since KHO would have marked that memory as preserved, and the new kernel won't touch it until someone restores it. So it can at most lead to loss of data, and in that case, userspace can very well decide if it can live with that loss or not. Or are you assuming here that even data in KHO is broken? In that case, it would probably be a good idea to panic early. [...] >> > + } >> > + >> > + luo_file = kmalloc(sizeof(*luo_file), >> > + GFP_KERNEL | __GFP_NOFAIL); >> > + luo_file->fs = fs; >> > + luo_file->file = NULL; >> > + memcpy(&luo_file->private_data, data_ptr, sizeof(u64)); >> >> Why not make sure data_ptr is exactly sizeof(u64) when we parse it, and >> then simply do luo_file->private_data = (u64)*data_ptr ? > > Because FDT alignment is 4 bytes, we can't simply assign it. Hmm, good catch. Didn't think of that. > >> Because if the previous kernel wrote more than a u64 in data, then >> something is broken and we should catch that error anyway. >> >> > + luo_file->reclaimed = false; >> > + mutex_init(&luo_file->mutex); >> > + luo_file->state = LIVEUPDATE_STATE_UPDATED; >> > + ret = xa_err(xa_store(&luo_files_xa_in, token, luo_file, >> > + GFP_KERNEL | __GFP_NOFAIL)); >> [...] >> > +struct liveupdate_filesystem { >> > + int (*prepare)(struct file *file, void *arg, u64 *data); >> > + int (*freeze)(struct file *file, void *arg, u64 *data); >> > + void (*cancel)(struct file *file, void *arg, u64 data); >> > + void (*finish)(struct file *file, void *arg, u64 data, bool reclaimed); >> > + int (*retrieve)(void *arg, u64 data, struct file **file); >> > + bool (*can_preserve)(struct file *file, void *arg); >> > + const char *compatible; >> > + void *arg; >> >> What is the use for this arg? I would expect one file type/system to >> register one set of handlers. So they can keep their arg in a global in >> their code. I don't see why a per-filesystem arg is needed. > > I think, arg is useful in case we support a subsystem is registered > multiple times with some differences: i.e. based on mount point, or > file types handling. Let's keep it for now, but if needed, we can > remove that in future revisions. > >> What I do think is needed is a per-file arg. Each callback gets 'data', >> which is the serialized data, but there is no place to store runtime >> state, like some flags or serialization metadata. Sure, you could make >> place for it somewhere in the inode, but I think it would be a lot >> cleaner to be able to store it in struct luo_file. >> >> So perhaps rename private_data in struct luo_file to say >> serialized_data, and have a field called "private" that filesystems can >> use for their runtime state? > > I am not against this, but let's make this change when it is actually > needed by a registered filesystem. Okay, fair enough. -- Regards, Pratyush Yadav