mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Joseph Qi <joseph.qi@linux.alibaba.com>
To: Cen Zhang <zzzccc427@gmail.com>
Cc: mark@fasheh.com, jlbec@evilplan.org, heming.zhao@suse.com,
	akpm@linux-foundation.org, christophe.jaillet@wanadoo.fr,
	blum@kernel.org, kees@kernel.org, sunil.mushran@oracle.com,
	ocfs2-devel@lists.linux.dev, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org, baijiaju1990@gmail.com,
	jjzuming@gmail.com
Subject: Re: [PATCH] ocfs2: drain quorum work before removing the cluster
Date: Tue, 20 Oct 2026 09:04:19 +0800	[thread overview]
Message-ID: <1beb38aa-e068-456c-a4d8-6590f2bd1266@linux.alibaba.com> (raw)
In-Reply-To: <pm-ocfs2-objects-candidate-0064-v4-51daf51db746eedf7533@gmail.com>



On 10/16/26 12:57 AM, Cen Zhang wrote:
> The configured cluster must remain available until an in-flight quorum
> decision has finished using its fencing method. o2quo_make_decision()
> drops qs_lock before calling o2quo_fence_self(), which marks heartbeat
> regions for unclean stop and then reads
> o2nm_single_cluster->cl_fence_method. Cluster removal clears the
> singleton without waiting for this independent system-workqueue item.
> 
> When a failed-quorum decision overlaps removal of the heartbeat regions,
> nodes and cluster through configfs, the worker can be preempted after
> marking the regions and resume after the singleton has been cleared:
> 
>   Quorum worker                     Configfs removal
>   select fence; drop qs_lock
>   o2hb_stop_all_regions()
>                                     remove regions; join heartbeat threads
>                                     remove nodes; stop o2net
>                                     clear singleton; release cluster
>   read cl_fence_method
> 
> The resumed read dereferences NULL instead of applying the configured
> fencing method. The heartbeat thread's dependency protects the local
> node only until the thread exits, and draining o2net does not wait for
> qs_work.
> 
> Call the existing o2quo_exit() flush before clearing the singleton in
> o2nm_cluster_group_drop_item(). By this point the heartbeat regions and
> nodes have been removed, so their work publishers are stopped. The
> flush lets any pending or running decision finish while the cluster is
> still available. It introduces no new state or locking; the worker does
> not acquire the configfs mutex held by the removal path.
> 
> KASAN report as below:
> 
>     Oops: general protection fault, probably for non-canonical address 0xdffffc000000011b: 0000 [#1] SMP KASAN NOPTI
>     KASAN: null-ptr-deref in range [0x00000000000008d8-0x00000000000008df]
>     CPU: 0 UID: 0 PID: 72 Comm: kworker/0:2 Not tainted 7.3.0-rc4-next-20260921-pmb-ocfs2-functional-v1+ #1 PREEMPT(lazy)
>     Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
>     Workqueue: events o2quo_make_decision
>     RIP: 0010:o2quo_fence_self+0x65/0x200
> 
> Register dump omitted.
> 
>     Call Trace:
>      <TASK>
>      o2quo_make_decision+0x345/0x3a0
>      ? __pfx_o2quo_make_decision+0x10/0x10
>      ? process_one_work+0x834/0x1b40
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? lock_release+0xc8/0x290
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      process_one_work+0x9a8/0x1b40
>      ? __pfx_process_one_work+0x10/0x10
>      ? lock_acquire+0x190/0x300
>      ? lock_is_held_type+0x8f/0x100
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      worker_thread+0x65c/0xe40
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __kthread_parkme+0x177/0x220
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __pfx_worker_thread+0x10/0x10
>      kthread+0x351/0x460
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __pfx_kthread+0x10/0x10
>      ret_from_fork+0x659/0x940
>      ? __pfx_ret_from_fork+0x10/0x10
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __switch_to+0x74f/0xf70
>      ? __pfx_kthread+0x10/0x10
>      ret_from_fork_asm+0x1a/0x30
>      </TASK>
>     Modules linked in:
>     ---[ end trace 0000000000000000 ]---
> 
> Fixes: f6656d26d17b ("ocfs2/cluster: Make fence method configurable - v2")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> Assisted-by: LLM
> 
> ---
> diff --git a/fs/ocfs2/cluster/nodemanager.c b/fs/ocfs2/cluster/nodemanager.c
> index e08850a5d736..13d7f579047f 100644
> --- a/fs/ocfs2/cluster/nodemanager.c
> +++ b/fs/ocfs2/cluster/nodemanager.c
> @@ -12,6 +12,7 @@
>  #include "tcp.h"
>  #include "nodemanager.h"
>  #include "heartbeat.h"
> +#include "quorum.h"
>  #include "masklog.h"
>  #include "sys.h"
>  
> @@ -740,6 +741,8 @@ static void o2nm_cluster_group_drop_item(struct config_group *group, struct conf
>  	struct o2nm_cluster *cluster = to_o2nm_cluster(item);
>  
>  	BUG_ON(o2nm_single_cluster != cluster);
> +	/* Drain quorum work before it loses access to the cluster. */
> +	o2quo_exit();
>  	o2nm_single_cluster = NULL;
>  
>  	configfs_remove_default_groups(&cluster->cl_group);
------------------------------------------------------------------------
o2quo_exit() is the counterpart of o2quo_init(), and it is called from
o2net_exit() when the module goes away.

I'd like a separate drain helper, say o2quo_flush_work(), to describe
this better than reusing the exit function.

Thanks,
Joseph

      reply	other threads:[~2026-10-20  1:04 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-15 16:57 Cen Zhang
2026-10-20  1:04 ` Joseph Qi [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=1beb38aa-e068-456c-a4d8-6590f2bd1266@linux.alibaba.com \
    --to=joseph.qi@linux.alibaba.com \
    --cc=akpm@linux-foundation.org \
    --cc=baijiaju1990@gmail.com \
    --cc=blum@kernel.org \
    --cc=christophe.jaillet@wanadoo.fr \
    --cc=heming.zhao@suse.com \
    --cc=jjzuming@gmail.com \
    --cc=jlbec@evilplan.org \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark@fasheh.com \
    --cc=ocfs2-devel@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=sunil.mushran@oracle.com \
    --cc=zzzccc427@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®