From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C1C62380FCB for ; Mon, 8 Jun 2026 14:19:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780928355; cv=none; b=BmkRahbfpQGfhN/9FR7yFtyPZe764F6zRq0E29tpNO4GSosw0au/oRBjXxqd4hMcztiTSMBH4ZGnWYg9NMhYAylh4jIpJogKKNEyto6PvWXaaDlJ+qyLqC7VqnaGwCXIgngAijV5OsT5Uh0wzSnIQUb0h4zqxG+6lHzaqVg8/lc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780928355; c=relaxed/simple; bh=KxFEOhg7P516zZ7TBq8eyubT7XnYEh7Vf5m7Ce2kDlk=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=nkAabVDcrFYXaljpZOoUmZogAjkl2ZKFnl9KAuMFueJc57sqtE7YDcTriLa+nqaoruZq8DCM1brJhyljL96+5MlbZuOjc1Ro2LDxlfo/9wVBuKFS3bA8m1VGfe/jfXQI8KBnkdrXB0N5e3Z3SteaC/Bwga0aA5oJjLVuIOReTRQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=OLD1qgx1; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="OLD1qgx1" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 592671F00893; Mon, 8 Jun 2026 14:19:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1780928354; bh=gtvAtyIWkgcGh5bt/21vAHxAw/x3unUhXw573vaCzPc=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=OLD1qgx1Q2ALx/ykbqbh/knGOvOBQnjApDdNeZJB8b/ezkWqap+NYtRL28TkoF4Gb 1rzfXZl7/bSff2jC9w5cHHfSythG4ByZ1aaIv5EBpq9w9g5qpsuL5/hIKF1mRO7L11 lgaSK3xDhJJ4+Y3EtjOsO0RZGVSMzd61OBjSVlZCT8WVg5fKHqF9n0+gfgti40KCsV 3R1zbLvvsJp5W6VQ60PfUnoMWGjgsCtB4gqk+Se2bL7BjB1LjECmWZH3K7wLr5oQfm w+7j6qBuiIWmNa599w6HPx7wVtYEE+sBgaobJ+bzASqAecdaY9yul/2LxhSh4xhnRy 78L+TzX8YNGEw== From: Pratyush Yadav To: David Matlack Cc: Pratyush Yadav , kexec@lists.infradead.org, linux-kernel@vger.kernel.org, Andrew Morton , Mike Rapoport , Pasha Tatashin Subject: Re: [PATCH 1/2] liveupdate: Reference count outgoing FLB data In-Reply-To: (David Matlack's message of "Tue, 2 Jun 2026 17:25:34 +0000") References: <20260528174140.1921129-1-dmatlack@google.com> <20260528174140.1921129-2-dmatlack@google.com> <2vxzfr34dfty.fsf@kernel.org> Date: Mon, 08 Jun 2026 16:19:12 +0200 Message-ID: <2vxzse6xt8rj.fsf@kernel.org> User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On Tue, Jun 02 2026, David Matlack wrote: > On 2026-06-02 07:15 PM, Pratyush Yadav wrote: >> Hi David, >> >> On Thu, May 28 2026, David Matlack wrote: >> >> > Increment the outgoing FLB refcount in liveupdate_flb_get_outgoing() so >> > that the FLB structure cannot be freed while the caller is actively >> > using it. Add an additional liveupdate_flb_put_outgoing() function so >> > the caller can explicitly indicate when it is done using the outgoing >> > FLB. >> > >> > During a Live Update, the kernel may need to fetch the outgoing FLB >> > outside of the scope of a file handler's preserve() and unpreserve() >> > callbacks. In that situation there is no way for the caller to protect >> > itself against the outgoing FLB from being freed while it is using it. >> > Incrementing the reference count in liveupdate_flb_get_outgoing() >> > ensures it cannot be freed. >> >> We grab a reference to the FLB's module when the first file using the >> FLB is preserved. So the FLB should never go away while preserved files >> exist. Once all preserved files go away, you normally shouldn't be doing >> anything with the FLB anyway. >> >> Can you please elaborate on the use case and why this is a problem? >> Using the FLB outside of the standard LUO file callbacks sounds >> problematic. > > The scenario I had in mind was to remove a PCI device from the outgoing > FLB if the device is forcibly removed while the file is still preserved, > for example someone writes 1 to /sys/bus/pci/devices/.../remove or a > device is physically hot-unplugged. > > Specifically this call here from the patch below: > > +void pci_liveupdate_cleanup_device(struct pci_dev *dev) > +{ > + /* > + * It should be safe to READ_ONCE() outside of the rwsem during cleanup > + * since there should no longer be any references to @dev on the system. > + */ > + if (READ_ONCE(dev->liveupdate.outgoing)) { > + pci_WARN(dev, 1, "Destroying outgoing-preserved device!\n"); > + pci_liveupdate_unpreserve(dev); > + } > +} > > https://lore.kernel.org/linux-pci/20260522202410.3104264-3-dmatlack@google.com/ > > I can do this without adding reference counting to > liveupdate_flb_get_outgoing(), but the reference counting makes it > obvious that the outgoing FLB will not be freed while I am using it > here, and also aligns with liveupdate_flb_get_incoming(). The lifecycle of FLB is bound to _preserved_ files. So it is only valid as long as preserved files exist. So I think you should only get the FLB object when you are inside a file preservation callback for a file which the FLB is registered. Anywhere outside of that, you are not guaranteed to get anything sane. This refcounting scheme breaks the inherent "file-lifecycle-bound" part of FLB, since now anyone can grab a reference and hold the FLB as long as they like, even when no preserved files exist. For the normal case, your the VFIO driver gets probed, it registers its file handler, then when the device is preserved by VFIO, the VFIO file handler's callbacks can get the FLB and do whatever. LUO guarantees the FLB exists. Anywhere outside of that, you should _not_ touch the FLB because of the reasons above. Now for hot-unplug, I think that case is not supported right now. When a preserved file exists, LUO can only remove it when the user closes the session. Trying to clean up the file from any other context will leave dangling references to the file and we currently do not handle those. Trying to hold the file reference won't help much either since LUO callbacks will try to proceed as normal, and normal no longer applies. For example, say userspace preserved the file for your device in their session, then you hot-unplug the device, then userspace triggers a kexec. What is the freeze() callback supposed to do? Sure, the FLB object still exists, but the device doesn't. Similarly, if you force remove the module, the freeze() callback itself no longer exists, and you likely get a panic. We might at some point support "invalidating" preserved files. I imagine when you hot-unplug with a preserved device, you tell LUO to invalidate all preserved files with that device. They would still exist in their sessions, but all operations on them fail immediately, including freeze(), which prevents live update from proceeding until user cleans them up. So unless I am missing something, I think this refcounting is a band-aid and the real problem is to properly track these "invalidated" files. Also, I think the refcounting on the incoming path is also a mistake. Unfortunately for incoming, there is a need for accessing the FLB outside of the file handling callbacks, since subsystems needs to use it to initialize itself. But I suppose we can have a accessor that subsystems can call once on boot/init to get their object. Then they use it to initialize their state and refer to the state directly, with all later calls going through the usual file handler callbacks. If you are interested in solving this problem, we can have a chat to talk in more detail, or perhaps have a discussion at one of the bi-weeklies? > >> > >> > This change also aligns the outgoing FLB lifecycle management with the >> > incoming FLB, since the latter uses the same get/put semantics. >> > >> > Fixes: cab056f2aae7 ("liveupdate: luo_flb: introduce File-Lifecycle-Bound global state") >> > Assisted-by: Gemini:gemini-3-pro-preview >> > Signed-off-by: David Matlack >> [...] >> >> -- >> Regards, >> Pratyush Yadav -- Regards, Pratyush Yadav