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

[David Matlack]

On Sat, Nov 22, 2025 at 2:24 PM Pasha Tatashin
<pasha.tatashin@xxxxxxxxxx> wrote:

> +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.

> @@ -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

> +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.