Re: [PATCH v7 21/22] liveupdate: luo_flb: Introduce File-Lifecycle-Bound global state

On Mon, Nov 24, 2025 at 6:45 PM David Matlack [off-list ref] wrote:

On Sat, Nov 22, 2025 at 2:24 PM Pasha Tatashin
[off-list ref] wrote:

quoted

+int liveupdate_flb_incoming_locked(struct liveupdate_flb *flb, void **objp);
+void liveupdate_flb_incoming_unlock(struct liveupdate_flb *flb, void *obj);
+int liveupdate_flb_outgoing_locked(struct liveupdate_flb *flb, void **objp);
+void liveupdate_flb_outgoing_unlock(struct liveupdate_flb *flb, void *obj);

nit: "locked" should be "lock". "locked" is used for situations where
the lock must already be held by the caller.

I am going to clean-up this API, and remove locked/unlocked; just
return the object directly.

quoted

@@ -633,6 +639,7 @@ static void luo_file_finish_one(struct luo_file_set *file_set,
        args.file = luo_file->file;
        args.serialized_data = luo_file->serialized_data;
        args.retrieved = luo_file->retrieved;
+       luo_flb_file_finish(luo_file->fh);

        luo_file->fh->ops->finish(&args);

I think luo_flb_file_finish() should be called after the file's
finish() callback. Otherwise the FLB data will be cleaned just before
the last file's finish() callback.

i.e. The order should be

  file1->finish()
  file2->finish()
  file3->finish() // last file
  flb->finish()

rather than

  file1->finish()
  file2->finish()
  flb->finish()
  file3->finish() // last file

Yes, I will make this change in the next version of FLB patch
(currently FLB has been dropped from LUO and will be sent separately
since there currently no in-kernel users beside the self-test)

quoted

+static void luo_flb_unlock(struct liveupdate_flb *flb, bool incoming,
+                          void *obj)
+{
+       struct luo_flb_private *private = luo_flb_get_private(flb);
+       struct luo_flb_private_state *state;
+
+       state = incoming ? &private->incoming : &private->outgoing;
+
+       lockdep_assert_held(&state->lock);
+       state->obj = obj;

I tripped over this when developing the PCI FLB state. The following
compiles fine and looks innocent enough:

  liveupdate_flb_incoming_locked(&pci_liveupdate_flb, &ser);
  ...
  liveupdate_flb_incoming_unlock(&pci_liveupdate_flb, &ser);

But this ends up corrupting state->obj.

Do we have a use-case for replacing obj on unlock? If not I'd suggest
dropping it.

I can remove internal obj updates.