On Wed, Jul 15, 2026 at 6:50 AM Pratyush Yadav <[email protected]> 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) <[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.
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).
