[Cluster-devel] [PATCH/RFC dlm/next 6/6] fs: dlm: use a non-static queue for callbacks

[Alexander Aring]

Hi,

On Tue, Sep 20, 2022 at 2:36 PM Alexander Aring <aahringo at redhat.com> wrote:
>
> Hi,
>
> On Fri, Sep 16, 2022 at 2:43 PM Alexander Aring <aahringo at redhat.com> wrote:
> >
> > This patch will introducde a queue implementation for callbacks by using
> > the Linux lists. The current callback queue handling is implemented by a
> > static limit of 6 entries, see DLM_CALLBACKS_SIZE. The sequence number
> > inside the callback structure was used to see if the entries inside the
> > static entry is valid or not. We don't need any sequence numbers anymore
> > with a dynamic datastructure with grows and shrinks during runtime to
> > offer such functionality.
> >
> > We assume that every callback will be delivered to the DLM user if once
> > queued. Therefore the callback flag DLM_CB_SKIP was dropped and the
> > check for skipping bast was moved before worker handling and not skip
> > while the callback worker executes. This will reduce unnecessary queues
> > of the callback worker.
> >
> > All last callback saves are pointers now and don't need to copied over.
> > There is a reference counter for callback structures which will care
> > about to free the callback structures at the right time if they are not
> > referenced anymore.
> >
> > Signed-off-by: Alexander Aring <aahringo at redhat.com>
> ...
> >
> > -       for (i = 1; i < DLM_CALLBACKS_SIZE; i++) {
> > -               if (!lkb->lkb_callbacks[i].seq)
> > -                       break;
> > -               memcpy(&lkb->lkb_callbacks[i-1], &lkb->lkb_callbacks[i],
> > -                      sizeof(struct dlm_callback));
> > -               memset(&lkb->lkb_callbacks[i], 0, sizeof(struct dlm_callback));
> > -               (*resid)++;
> > -       }
> > -
> > -       /* if cb is a bast, it should be skipped if the blocking mode is
> > -          compatible with the last granted mode */
> > -
> > -       if ((cb->flags & DLM_CB_BAST) && lkb->lkb_last_cast.seq) {
> > -               if (dlm_modes_compat(cb->mode, lkb->lkb_last_cast.mode)) {
> > -                       cb->flags |= DLM_CB_SKIP;
> > -
> > -                       log_debug(ls, "skip %x bast %llu mode %d "
> > -                                 "for cast %llu mode %d",
> > -                                 lkb->lkb_id,
> > -                                 (unsigned long long)cb->seq,
> > -                                 cb->mode,
> > -                                 (unsigned long long)lkb->lkb_last_cast.seq,
> > -                                 lkb->lkb_last_cast.mode);
> > -                       rv = 0;
> > -                       goto out;
> > -               }
> > -       }
> > +       if (flags & DLM_CB_BAST)
> > +               dlm_callback_set_last_ptr(&lkb->lkb_last_bast, cb);
>
> I will change this to have an int lkb->lkb_last_bast_mode, this is
> only used for debugfs and we need to hold a lock to get this
> information. However we only need the mode and this only for debugging
> information... so we copy just the mode value.

since this is debug only, then we move this directly right to the bast
callback call and not when it's enqueued. The others however are for
optimizations and we should not queue callbacks when we know there can
be already skipped and not after dequeuing them.

- Alex