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) <[email protected]> That said, a comment on the code below. > > Signed-off-by: David Matlack <[email protected]> > --- [...] > 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
