Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [RFC PATCH] liveupdate: Allow multiple openers for /dev/liveupdate
@ 2026-07-14 19:03 David Matlack
  2026-07-15 13:50 ` Pratyush Yadav
  0 siblings, 1 reply; 4+ messages in thread
From: David Matlack @ 2026-07-14 19:03 UTC (permalink / raw)
  To: kexec, linux-kernel, linux-kselftest
  Cc: Jason Gunthorpe, Mike Rapoport, Pasha Tatashin, Pratyush Yadav,
	Samiullah Khawaja, Shuah Khan, David Matlack

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.

Signed-off-by: David Matlack <dmatlack@google.com>
---
Cc: Jason Gunthorpe <jgg@nvidia.com>
Cc: Samiullah Khawaja <skhawaja@google.com>

 kernel/liveupdate/luo_core.c                  | 46 +++----------------
 kernel/liveupdate/luo_session.c               |  8 +++-
 .../testing/selftests/liveupdate/liveupdate.c | 14 +++---
 3 files changed, 18 insertions(+), 50 deletions(-)

diff --git a/kernel/liveupdate/luo_core.c b/kernel/liveupdate/luo_core.c
index 1b2bda22902d..931bb79da276 100644
--- a/kernel/liveupdate/luo_core.c
+++ b/kernel/liveupdate/luo_core.c
@@ -254,19 +254,8 @@ bool liveupdate_enabled(void)
  * which allows a userspace agent to manage the LUO state machine and its
  * associated resources, such as preservable file descriptors.
  *
- * To ensure that the state machine is controlled by a single entity, access
- * to this device is exclusive: only one process is permitted to have
- * ``/dev/liveupdate`` open at any given time. Subsequent open attempts will
- * fail with -EBUSY until the first process closes its file descriptor.
- * This singleton model simplifies state management by preventing conflicting
- * commands from multiple userspace agents.
  */
 
-struct luo_device_state {
-	struct miscdevice miscdev;
-	atomic_t in_use;
-};
-
 static int luo_ioctl_create_session(struct luo_ucmd *ucmd)
 {
 	struct liveupdate_ioctl_create_session *argp = ucmd->cmd;
@@ -329,28 +318,9 @@ static int luo_ioctl_retrieve_session(struct luo_ucmd *ucmd)
 
 static int luo_open(struct inode *inodep, struct file *filep)
 {
-	struct luo_device_state *ldev = container_of(filep->private_data,
-						     struct luo_device_state,
-						     miscdev);
-
-	if (atomic_cmpxchg(&ldev->in_use, 0, 1))
-		return -EBUSY;
-
 	/* Always return -EIO to user if deserialization fail */
-	if (luo_session_deserialize()) {
-		atomic_set(&ldev->in_use, 0);
+	if (luo_session_deserialize())
 		return -EIO;
-	}
-
-	return 0;
-}
-
-static int luo_release(struct inode *inodep, struct file *filep)
-{
-	struct luo_device_state *ldev = container_of(filep->private_data,
-						     struct luo_device_state,
-						     miscdev);
-	atomic_set(&ldev->in_use, 0);
 
 	return 0;
 }
@@ -419,17 +389,13 @@ static long luo_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
 static const struct file_operations luo_fops = {
 	.owner		= THIS_MODULE,
 	.open		= luo_open,
-	.release	= luo_release,
 	.unlocked_ioctl	= luo_ioctl,
 };
 
-static struct luo_device_state luo_dev = {
-	.miscdev = {
-		.minor = MISC_DYNAMIC_MINOR,
-		.name  = "liveupdate",
-		.fops  = &luo_fops,
-	},
-	.in_use = ATOMIC_INIT(0),
+static struct miscdevice luo_dev = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name  = "liveupdate",
+	.fops  = &luo_fops,
 };
 
 static int __init liveupdate_ioctl_init(void)
@@ -437,6 +403,6 @@ static int __init liveupdate_ioctl_init(void)
 	if (!liveupdate_enabled())
 		return 0;
 
-	return misc_register(&luo_dev.miscdev);
+	return misc_register(&luo_dev);
 }
 late_initcall(liveupdate_ioctl_init);
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);
+
 	/* If has been deserialized, always return the same error code */
 	if (is_deserialized)
 		return saved_err;
diff --git a/tools/testing/selftests/liveupdate/liveupdate.c b/tools/testing/selftests/liveupdate/liveupdate.c
index 502fb3567e38..a8e1a8911849 100644
--- a/tools/testing/selftests/liveupdate/liveupdate.c
+++ b/tools/testing/selftests/liveupdate/liveupdate.c
@@ -11,7 +11,7 @@
  * /dev/liveupdate character device and its session management capabilities.
  *
  * Tests include:
- * - Device access: basic open/close, and enforcement of exclusive access.
+ * - Device access: basic open/close, and support for multiple openers.
  * - Session management: creation of unique sessions, and duplicate name detection.
  * - Resource preservation: successfully preserving individual and multiple memfds,
  *   verifying contents remain accessible.
@@ -70,13 +70,12 @@ TEST_F(liveupdate_device, basic_open_close)
 }
 
 /*
- * Test Case: Exclusive Open Enforcement
+ * Test Case: Multiple Open Support
  *
- * Verifies that the /dev/liveupdate device can only be opened by one process
- * at a time. It checks that a second attempt to open the device fails with
- * the EBUSY error code.
+ * Verifies that the /dev/liveupdate device can be opened by multiple openers
+ * concurrently without errors.
  */
-TEST_F(liveupdate_device, exclusive_open)
+TEST_F(liveupdate_device, multiple_open)
 {
 	self->fd1 = open(LIVEUPDATE_DEV, O_RDWR);
 
@@ -85,8 +84,7 @@ TEST_F(liveupdate_device, exclusive_open)
 
 	ASSERT_GE(self->fd1, 0);
 	self->fd2 = open(LIVEUPDATE_DEV, O_RDWR);
-	EXPECT_LT(self->fd2, 0);
-	EXPECT_EQ(errno, EBUSY);
+	ASSERT_GE(self->fd2, 0);
 }
 
 /* Helper function to create a LUO session via ioctl. */

base-commit: 10fd52dc7bbd6013e949ff1833be0821b7553fb0
-- 
2.55.0.141.g00534a21ce-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH] liveupdate: Allow multiple openers for /dev/liveupdate
  2026-07-14 19:03 [RFC PATCH] liveupdate: Allow multiple openers for /dev/liveupdate David Matlack
@ 2026-07-15 13:50 ` Pratyush Yadav
  2026-07-15 16:07   ` David Matlack
  2026-07-15 17:23   ` Jason Gunthorpe
  0 siblings, 2 replies; 4+ messages in thread
From: Pratyush Yadav @ 2026-07-15 13:50 UTC (permalink / raw)
  To: David Matlack
  Cc: kexec, linux-kernel, linux-kselftest, Jason Gunthorpe,
	Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Samiullah Khawaja,
	Shuah Khan

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH] liveupdate: Allow multiple openers for /dev/liveupdate
  2026-07-15 13:50 ` Pratyush Yadav
@ 2026-07-15 16:07   ` David Matlack
  2026-07-15 17:23   ` Jason Gunthorpe
  1 sibling, 0 replies; 4+ messages in thread
From: David Matlack @ 2026-07-15 16:07 UTC (permalink / raw)
  To: Pratyush Yadav
  Cc: kexec, linux-kernel, linux-kselftest, Jason Gunthorpe,
	Mike Rapoport, Pasha Tatashin, Samiullah Khawaja, Shuah Khan

On Wed, Jul 15, 2026 at 6:50 AM Pratyush Yadav <pratyush@kernel.org> wrote:
>
> 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.

The lock here is just to protect the local static variables
(is_deserialized and saved_err) so that's why I went with a local
static lock. But re-using sh->rwsem should work as well and would not
block luo_session_retrieve() any more than the current static lock
would (opening /dev/liveupdate must always preceded retrieving a
session).

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH] liveupdate: Allow multiple openers for /dev/liveupdate
  2026-07-15 13:50 ` Pratyush Yadav
  2026-07-15 16:07   ` David Matlack
@ 2026-07-15 17:23   ` Jason Gunthorpe
  1 sibling, 0 replies; 4+ messages in thread
From: Jason Gunthorpe @ 2026-07-15 17:23 UTC (permalink / raw)
  To: Pratyush Yadav
  Cc: David Matlack, kexec, linux-kernel, linux-kselftest,
	Mike Rapoport, Pasha Tatashin, Samiullah Khawaja, Shuah Khan

On Wed, Jul 15, 2026 at 03:50:33PM +0200, Pratyush Yadav wrote:
> 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.

Shouldn't systemd open and own /dev/liveupdate? That was at least what
I originally expected here, you'd talk to it and get a session FD
through dbus.

Moving to multi-opening /dev/liveupdate and removing visibility of
what sessions are open from systemd is a different model

Not saying this patch is wrong or anything, but that I don't really
understand what kind of model you are going for now.

Jason

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-15 17:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 19:03 [RFC PATCH] liveupdate: Allow multiple openers for /dev/liveupdate David Matlack
2026-07-15 13:50 ` Pratyush Yadav
2026-07-15 16:07   ` David Matlack
2026-07-15 17:23   ` Jason Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox