From: Pratyush Yadav <pratyush@kernel.org>
To: David Matlack <dmatlack@google.com>
Cc: kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
linux-kselftest@vger.kernel.org,
Jason Gunthorpe <jgg@nvidia.com>,
Mike Rapoport <rppt@kernel.org>,
Pasha Tatashin <pasha.tatashin@soleen.com>,
Pratyush Yadav <pratyush@kernel.org>,
Samiullah Khawaja <skhawaja@google.com>,
Shuah Khan <shuah@kernel.org>
Subject: Re: [RFC PATCH] liveupdate: Allow multiple openers for /dev/liveupdate
Date: Wed, 15 Jul 2026 15:50:33 +0200 [thread overview]
Message-ID: <2vxzy0fcicpi.fsf@kernel.org> (raw)
In-Reply-To: <20260714190356.190328-1-dmatlack@google.com> (David Matlack's message of "Tue, 14 Jul 2026 19:03:56 +0000")
Hi David,
On Tue, Jul 14 2026, David Matlack wrote:
> Remove the single-opener restriction for /dev/liveupdate by removing the
> atomic in_use tracking and the exclusive open check in luo_open() that
> returned -EBUSY. Protect luo_session_deserialize() with a mutex guard so
> that concurrent open attempts by multiple processes safely executes
> deserialization only once. Update liveupdate selftest to verify that
> multiple concurrent openers succeed.
>
> LUO does not inherently require a single opener. There is some
> documentation about it simplifying state management, but the only thing
> it actually protects is the session deserialization during first open,
> which can be easily handled with a mutex.
>
> Relaxing the single-opener requirement avoids the kernel forcing a
> design pattern on userspace that it itself does not require, e.g.
> allowing multiple userspace processes to create and manage sessions.
Agreed. When the kernel had a global state machine in the early versions
of LUO, this might have been more relevant. With sessions, even if we
later add a state machine, it likely will be per-session instead of
being global. So I think letting userspace open /dev/liveupdate multiple
times makes a lot of sense.
Also, today's systemd only supports preserving individual files, and
does not hand out sessions. To get sessions, userspace must open
/dev/liveupdate and create a session. This opens up room for one bad
process to block every other process from creating sessions. It also
imposes a need for userspace to add a polling/retry logic for getting
sessions and serializes their execution around this point.
I don't see any architectural reasons for doing so from kernel's side.
If userspace wants to only have one owner of /dev/liveupdate, they are
free to do so by unlinking the device from devtmpfs after opening or
restricting its permissions.
So the idea has my vote :-)
Acked-by: Pratyush Yadav (Google) <pratyush@kernel.org>
That said, a comment on the code below.
>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
[...]
> diff --git a/kernel/liveupdate/luo_session.c b/kernel/liveupdate/luo_session.c
> index b79b2a488974..ca4d0639d39a 100644
> --- a/kernel/liveupdate/luo_session.c
> +++ b/kernel/liveupdate/luo_session.c
> @@ -584,13 +584,17 @@ static int luo_session_deserialize_one(struct luo_session_header *sh,
>
> int luo_session_deserialize(void)
> {
> - struct luo_session_header *sh = &luo_session_global.incoming;
> + static DEFINE_MUTEX(luo_session_deserialize_lock);
> static bool is_deserialized;
> + static int saved_err;
> +
> + struct luo_session_header *sh = &luo_session_global.incoming;
> struct luo_session_ser *ser;
> struct kho_block_set_it it;
> - static int saved_err;
> int err;
>
> + guard(mutex)(&luo_session_deserialize_lock);
Do we really need a new lock? Can we re-use sh->rwsem instead?
It can block session retrieve (but not file retrieve) for a short time
though since luo_session_retrieve() also takes it. But the block will be
short since only the first open of /dev/liveupdate does work. After that
it just checks is_deserialized and returns. And session retrieval should
not be very frequent anyway.
I don't have very strong opinion on this, but I reckon the less locks to
keep track of the better.
> +
> /* If has been deserialized, always return the same error code */
> if (is_deserialized)
> return saved_err;
[...]
--
Regards,
Pratyush Yadav
next prev parent reply other threads:[~2026-07-15 13:50 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-14 19:03 [RFC PATCH] liveupdate: Allow multiple openers for /dev/liveupdate David Matlack
2026-07-15 13:50 ` Pratyush Yadav [this message]
2026-07-15 16:07 ` David Matlack
2026-07-15 17:23 ` Jason Gunthorpe
2026-07-16 6:06 ` Mike Rapoport
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2vxzy0fcicpi.fsf@kernel.org \
--to=pratyush@kernel.org \
--cc=dmatlack@google.com \
--cc=jgg@nvidia.com \
--cc=kexec@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=pasha.tatashin@soleen.com \
--cc=rppt@kernel.org \
--cc=shuah@kernel.org \
--cc=skhawaja@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.