Re: [PATCH v2] ocfs2: drain quorum work before removing the cluster

[Joseph Qi <joseph.qi@linux.alibaba.com>]

On 10/20/26 11:45 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.
> 
> Add o2quo_flush_work() to drain pending or running quorum decisions
> 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 worker can finish while the cluster
> and its configured fencing method remain available. It does not acquire
> the configfs mutex held by the removal path.
> 
> Keep o2quo_exit() as the counterpart of o2quo_init() for module teardown
> and initialization failure, and have it call the same drain helper.
> Cluster removal uses only the explicitly named drain operation.
> 
> 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>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> Assisted-by: LLM
> ---
> Changes in v2:
> - Add a dedicated o2quo_flush_work() helper and use it before clearing the
>   cluster singleton, as requested by Joseph Qi.
> - Preserve o2quo_exit() as the counterpart of o2quo_init() and make it call
>   the shared drain helper.
> 
> Link to v1: https://lore.kernel.org/r/pm-ocfs2-objects-candidate-0064-v4-51daf51db746eedf7533@gmail.com
> 
>  fs/ocfs2/cluster/nodemanager.c | 3 +++
>  fs/ocfs2/cluster/quorum.c      | 7 ++++++-
>  fs/ocfs2/cluster/quorum.h      | 1 +
>  3 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/cluster/nodemanager.c b/fs/ocfs2/cluster/nodemanager.c
> index e08850a5d736..ced4fe6d3b59 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_flush_work();
>  	o2nm_single_cluster = NULL;
>  
>  	configfs_remove_default_groups(&cluster->cl_group);
> diff --git a/fs/ocfs2/cluster/quorum.c b/fs/ocfs2/cluster/quorum.c
> index bfb8b456876c..ab5b3d4fd0e8 100644
> --- a/fs/ocfs2/cluster/quorum.c
> +++ b/fs/ocfs2/cluster/quorum.c
> @@ -318,9 +318,14 @@ void o2quo_init(void)
>  	INIT_WORK(&qs->qs_work, o2quo_make_decision);
>  }
>  
> -void o2quo_exit(void)
> +void o2quo_flush_work(void)
>  {
>  	struct o2quo_state *qs = &o2quo_state;
>  
>  	flush_work(&qs->qs_work);
>  }
> +
> +void o2quo_exit(void)
> +{
> +	o2quo_flush_work();
> +}
> diff --git a/fs/ocfs2/cluster/quorum.h b/fs/ocfs2/cluster/quorum.h
> index d64bf4482a4a..1ce5fc2a8ca9 100644
> --- a/fs/ocfs2/cluster/quorum.h
> +++ b/fs/ocfs2/cluster/quorum.h
> @@ -8,6 +8,7 @@
>  
>  void o2quo_init(void);
>  void o2quo_exit(void);
> +void o2quo_flush_work(void);
>  
>  void o2quo_hb_up(u8 node);
>  void o2quo_hb_down(u8 node);