All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Teigland <teigland@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, gfs2@lists.linux.dev
Subject: Re: [GIT PULL] dlm fixes for 6.9
Date: Fri, 15 Mar 2024 13:42:49 -0500	[thread overview]
Message-ID: <ZfSWqXKWnalm9wE5@redhat.com> (raw)
In-Reply-To: <CAHk-=wh4qK+zHrrYehidKRp4Fi6e4qUD6Tv6Ed8USxUC+H+HrQ@mail.gmail.com>

On Fri, Mar 15, 2024 at 10:10:00AM -0700, Linus Torvalds wrote:
> Now, if the issue is that you want to clean up something that is never
> getting cleaned up by anybody else, and this is a fatal error, and
> you're just trying to fix things up (badly), and you know that this is
> all racy but the code is trying to kill a dead data structure, then
> you should
> 
>  (a) need a damn big comment (bigger than the comment is already)
> 
>  (b) should *NOT* pretend to do some stupid "atomic decrement and test" loop

Yes, that looks pretty messed up, the counter should not be an atomic_t.  I was
a bit wary of making that atomic when it wasn't necessary, but didn't push back
enough on that change:

    commit 75a7d60134ce84209f2c61ec4619ee543aa8f466
    Author: Alexander Aring <aahringo@redhat.com>
    Date:   Mon May 29 17:44:38 2023 -0400

    Currently the lkb_wait_count is locked by the rsb lock and it should be
    fine to handle lkb_wait_count as non atomic_t value. However for the
    overall process of reducing locking this patch converts it to an
    atomic_t value.

... and the result is the primitives get abused, and the code becomes crazy.
My initial plan is to go back to a non-atomic counter there.  It is indeed a
recovery situation that involves a forced reset of state, but I'll need to go
back and study that case further before I can say what it should finally look
like.  Whatever that looks like, it'll have a very good comment :)  Dropping
the pull is fine, there's a chance I may resend with the other patch and a new
fix, we'll see.

Thanks,
Dave




      reply	other threads:[~2024-03-15 18:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-14 18:43 [GIT PULL] dlm fixes for 6.9 David Teigland
2024-03-15 17:10 ` Linus Torvalds
2024-03-15 18:42   ` David Teigland [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZfSWqXKWnalm9wE5@redhat.com \
    --to=teigland@redhat.com \
    --cc=gfs2@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.