Re: [PATCH v3 07/10] liveupdate: Auto unregister FLBs on file handler unregistration

[Pasha Tatashin]

On Fri, Apr 3, 2026 at 6:17 AM Pratyush Yadav <pratyush@xxxxxxxxxx> wrote:
>
> On Fri, Mar 27 2026, Pasha Tatashin wrote:
>
> > To ensure that unregistration is always successful and doesn't leave
> > dangling resources, introduce auto-unregistration of FLBs: when a file
> > handler is unregistered, all FLBs associated with it are automatically
> > unregistered.
> >
> > Introduce a new helper luo_flb_unregister_all() which unregisters all
> > FLBs linked to the given file handler.
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@xxxxxxxxxx>
> > ---
> >  kernel/liveupdate/luo_file.c     | 14 +-----
> >  kernel/liveupdate/luo_flb.c      | 84 ++++++++++++++++++++++----------
> >  kernel/liveupdate/luo_internal.h |  1 +
> >  3 files changed, 60 insertions(+), 39 deletions(-)
> >
> > diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c
> > index 439177c10723..80f4e3b796a6 100644
> > --- a/kernel/liveupdate/luo_file.c
> > +++ b/kernel/liveupdate/luo_file.c
> > @@ -895,26 +895,16 @@ int liveupdate_register_file_handler(struct liveupdate_file_handler *fh)
> >   */
> >  int liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh)
> >  {
> > -     int err = -EBUSY;
> > -
> >       if (!liveupdate_enabled())
> >               return -EOPNOTSUPP;
> >
> >       liveupdate_test_unregister(fh);
> >
> > -     down_write(&luo_register_rwlock);
> > -     if (!list_empty(&ACCESS_PRIVATE(fh, flb_list)))
> > -             goto err_unlock;
> > -
> > +     guard(rwsem_write)(&luo_register_rwlock);
> > +     luo_flb_unregister_all(fh);
> >       list_del(&ACCESS_PRIVATE(fh, list));
> > -     up_write(&luo_register_rwlock);
> >
> >       module_put(fh->ops->owner);
> >
> >       return 0;
> > -
> > -err_unlock:
> > -     up_write(&luo_register_rwlock);
> > -     liveupdate_test_register(fh);
> > -     return err;
> >  }
> > diff --git a/kernel/liveupdate/luo_flb.c b/kernel/liveupdate/luo_flb.c
> > index ac6fa5b34b43..f8348138de70 100644
> > --- a/kernel/liveupdate/luo_flb.c
> > +++ b/kernel/liveupdate/luo_flb.c
> > @@ -335,6 +335,62 @@ void luo_flb_file_finish(struct liveupdate_file_handler *fh)
> >               luo_flb_file_finish_one(iter->flb);
> >  }
> >
> > +static void luo_flb_unregister_one(struct liveupdate_file_handler *fh,
> > +                                struct liveupdate_flb *flb)
> > +{
> > +     struct luo_flb_private *private = luo_flb_get_private(flb);
> > +     struct list_head *flb_list = &ACCESS_PRIVATE(fh, flb_list);
> > +     struct luo_flb_link *iter;
> > +     bool found = false;
> > +
> > +     /* Find and remove the link from the file handler's list */
>
> Nit: it would be nice to have a lockdep_assert_held_write() here to

Done

> clearly show that the luo_register_rwlock must be held when modifying
> the list. I know that luo_flb_unregister_all() already has one, but in
> case we refactor later or add another caller.
>
> Other than this,
>
> Reviewed-by: Pratyush Yadav (Google) <pratyush@xxxxxxxxxx>

Thank you.

Pasha

>
> > +     list_for_each_entry(iter, flb_list, list) {
> > +             if (iter->flb == flb) {
> > +                     list_del(&iter->list);
> > +                     kfree(iter);
> > +                     found = true;
> > +                     break;
> > +             }
> > +     }
> > +
> [...]
>
> --
> Regards,
> Pratyush Yadav