Hi,
On Wed, Feb 16, 2022 at 11:08 AM Andreas Gruenbacher
<agrue...@redhat.com> wrote:
>
> On Wed, Feb 16, 2022 at 4:53 PM Alexander Aring <aahri...@redhat.com> wrote:
> > There are several sanity checks and recover handling if they occur in
> > the dlm plock handling. They should never occur otherwise we have a bug
> > in the code. To make such bugs more visible we remove the recover
> > handling and add a WARN_ON() on those sanity checks.
> >
> > Signed-off-by: Alexander Aring <aahri...@redhat.com>
> > ---
> > fs/dlm/plock.c | 32 ++++----------------------------
> > 1 file changed, 4 insertions(+), 28 deletions(-)
> >
> > diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
> > index a10d2bcfe75a..55fba2f0234f 100644
> > --- a/fs/dlm/plock.c
> > +++ b/fs/dlm/plock.c
> > @@ -157,13 +157,7 @@ int dlm_posix_lock(dlm_lockspace_t *lockspace, u64
> > number, struct file *file,
> > goto out;
> > }
> >
> > - spin_lock(&ops_lock);
> > - if (!list_empty(&op->list)) {
> > - log_error(ls, "dlm_posix_lock: op on list %llx",
> > - (unsigned long long)number);
> > - list_del(&op->list);
> > - }
> > - spin_unlock(&ops_lock);
> > + WARN_ON(!list_empty(&op->list));
>
> Why don't those checks need the ops_lock spin lock anymore?
> Why does it make sense to get rid of the list_del calls?
My understanding is that those list_del() calls try to recover
something if a sanity check hits. The list_emptry() check should
always be true at this point no matter if lock is held or not.
Therefore no lock is required here to do some sanity checking.
If those checks hit there is a bug and trying to recover from it makes
no sense from my point of view.
- Alex
