Re: [PATCH v3 02/10] liveupdate: Synchronize lazy initialization of FLB private state

[Pasha Tatashin]

On Tue, Mar 31, 2026 at 6:38 AM Pratyush Yadav <pratyush@xxxxxxxxxx> wrote:
>
> On Fri, Mar 27 2026, Pasha Tatashin wrote:
>
> > The luo_flb_get_private() function, which is responsible for lazily
> > initializing the private state of FLB objects, can be called
> > concurrently from multiple threads. This creates a data
> > race on the 'initialized' flag and can lead to multiple executions of
> > mutex_init() and INIT_LIST_HEAD() on the same memory.
> >
> > Introduce a static spinlock (luo_flb_init_lock) local to the function
> > to synchronize the initialization path. Use smp_load_acquire() and
> > smp_store_release() for memory ordering between the fast path and the
> > slow path.
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@xxxxxxxxxx>
>
> Reviewed-by: Pratyush Yadav <pratyush@xxxxxxxxxx>

Thank you.

>
> But... wouldn't it be a whole lot simpler if we introduce a
> DEFINE_LUO_FLB() and get rid of luo_flb_get_private() entirely:

We are planning some updates to this code in the near future:

1. David Matlack will send a patch in the next version of VFIO LUO
preservation series to add liveupdate_flb_put_incoming() so that
caller can protect from race with release of the last FD release that
will also release FLB.
This change will increment FLB 'count' not only when FH uses it, but
also when the object is being accessed.

2. After I plan to convert 'count' to use kref, and also decouble init
and liveupdate_flb_get_incoming(), this will allow to access FLB
object with interrupts disabled, as requested by Sami for the IOMMU
work. I think, that while working on this 2nd change, we can also do
some clean-ups if necessary.

Pasha

>
> #define DEFINE_LUO_FLB(_name, _ops, _compatible)                        \
>         struct liveupdate_flb _name = {                                 \
>                 .ops = _ops,                                            \
>                 .compatible = _compatible,                              \
>                 .private = {                                            \
>                         .list = LIST_HEAD_INIT(_name.private.list),     \
>                         .list = LIST_HEAD_INIT(_name.private.list),     \
>                         / ...
>                 },                                                      \
>         }
>
> I can't get sparse to work so not sure if I need some special syntax to
> initialize stuff in .private, but I reckon we can get something working.
>
> [...]
>
> --
> Regards,
> Pratyush Yadav
>