Re: [PATCH v2 2/8] liveupdate: Protect FLB lists with rwsem

[Pasha Tatashin <pasha.tatashin@soleen.com>]

> >@@ -355,6 +363,9 @@ int liveupdate_register_flb(struct liveupdate_file_handler *fh,
> >       if (!luo_session_quiesce())
> >               return -EBUSY;
> >
> >+      guard(rwsem_write)(&luo_flb_lock);
> >+      guard(rwsem_write)(&ACCESS_PRIVATE(fh, flb_lock));
>
> Since FLBs are linked with file handlers and the file_handler can be
> unregistered/registered while this is running, should the luo_file_lock
> write be taken here? I think maybe we don't need a separate luo_flb_lock

Hi Sami,

Yes, thank you for catching this. I thought we were safe since we are
taking fh->flb_lock, but you are correct. I am going to do what you
suggested below and use a single luo_registration_lock for both FLB
and File Handler registrations. Reads will also be shared; this will
simplify locking.

Pasha

> and the luo_file_lock should provide enough protection if we acquire it
> here, as a file_handler is supposed to be registered first and then flb
> needs to be registered against it?
>
> Maybe we can have one luo_register_lock?
> >+
> >       /* Check that this FLB is not already linked to this file handler */
> >       err = -EEXIST;
> >       list_for_each_entry(iter, flb_list, list) {
> >@@ -444,6 +455,9 @@ int liveupdate_unregister_flb(struct liveupdate_file_handler *fh,
> >       if (!luo_session_quiesce())
> >               return -EBUSY;
> >
> >+      guard(rwsem_write)(&luo_flb_lock);
> >+      guard(rwsem_write)(&ACCESS_PRIVATE(fh, flb_lock));
> >+
> >       /* Find and remove the link from the file handler's list */
> >       list_for_each_entry(iter, flb_list, list) {
> >               if (iter->flb == flb) {
> >@@ -638,6 +652,8 @@ void luo_flb_serialize(void)
> >       struct liveupdate_flb *gflb;
> >       int i = 0;
> >
> >+      guard(rwsem_read)(&luo_flb_lock);
> >+
> >       list_private_for_each_entry(gflb, &luo_flb_global.list, private.list) {
> >               struct luo_flb_private *private = luo_flb_get_private(gflb);
> >
> >--
> >2.53.0.851.ga537e3e6e9-goog
> >
> >