From ec701ff594be91c0a08c14b62f3be9b8a5602bd6 Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 10 Jul 2026 15:17:55 +0800 Subject: ocfs2: cluster: use GFP_NOFS for heartbeat bio allocation o2hb_setup_one_bio() allocates the heartbeat bio with GFP_ATOMIC. The disk heartbeat runs in the o2hb kernel thread (o2hb_do_disk_heartbeat), which is process context and can sleep, so there is no atomicity requirement here. GFP_ATOMIC lacks __GFP_DIRECT_RECLAIM, so the allocation is not served from the fs_bio_set mempool reserve and can return NULL under memory pressure. A failed heartbeat allocation aborts the heartbeat and can lead to the local node being fenced, which is exactly what the old comment worried about. Use GFP_NOFS instead. It keeps __GFP_DIRECT_RECLAIM so the allocation is backed by the fs_bio_set mempool and cannot fail, while avoiding recursion back into the filesystem during heartbeat I/O. As the allocation can no longer fail, drop the dead ERR_PTR(-ENOMEM) path in o2hb_setup_one_bio() and the now-redundant IS_ERR() handling in its callers. Link: https://lore.kernel.org/20260710071756.3586797-1-joseph.qi@linux.alibaba.com Signed-off-by: Joseph Qi Cc: Mark Fasheh Cc: Joel Becker Cc: Junxiao Bi Cc: Changwei Ge Cc: Jun Piao Cc: Heming Zhao Signed-off-by: Andrew Morton --- fs/ocfs2/cluster/heartbeat.c | 41 ++++++++--------------------------------- 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c index 6da96a374fcd..ef6a11fdef5b 100644 --- a/fs/ocfs2/cluster/heartbeat.c +++ b/fs/ocfs2/cluster/heartbeat.c @@ -522,16 +522,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, struct bio *bio; struct page *page; - /* Testing has shown this allocation to take long enough under - * GFP_KERNEL that the local node can get fenced. It would be - * nicest if we could pre-allocate these bios and avoid this - * all together. */ - bio = bio_alloc(reg_bdev(reg), 16, opf, GFP_ATOMIC); - if (!bio) { - mlog(ML_ERROR, "Could not alloc slots BIO!\n"); - bio = ERR_PTR(-ENOMEM); - goto bail; - } + /* + * The heartbeat runs in process context and can sleep, so use + * GFP_NOFS. It is backed by the fs_bio_set mempool and thus cannot + * fail, while avoiding recursion back into the filesystem. + */ + bio = bio_alloc(reg_bdev(reg), 16, opf, GFP_NOFS); /* Must put everything in 512 byte sectors for the bio... */ bio->bi_iter.bi_sector = (reg->hr_start_block + cs) << (bits - 9); @@ -556,7 +552,6 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, vec_start = 0; } -bail: *current_slot = cs; return bio; } @@ -566,7 +561,6 @@ static int o2hb_read_slots(struct o2hb_region *reg, unsigned int max_slots) { unsigned int current_slot = begin_slot; - int status; struct o2hb_bio_wait_ctxt wc; struct bio *bio; @@ -575,30 +569,18 @@ static int o2hb_read_slots(struct o2hb_region *reg, while(current_slot < max_slots) { bio = o2hb_setup_one_bio(reg, &wc, ¤t_slot, max_slots, REQ_OP_READ); - if (IS_ERR(bio)) { - status = PTR_ERR(bio); - mlog_errno(status); - goto bail_and_wait; - } - atomic_inc(&wc.wc_num_reqs); submit_bio(bio); } - status = 0; - -bail_and_wait: o2hb_wait_on_io(&wc); - if (wc.wc_error && !status) - status = wc.wc_error; - return status; + return wc.wc_error; } static int o2hb_issue_node_write(struct o2hb_region *reg, struct o2hb_bio_wait_ctxt *write_wc) { - int status; unsigned int slot; struct bio *bio; @@ -610,18 +592,11 @@ static int o2hb_issue_node_write(struct o2hb_region *reg, bio = o2hb_setup_one_bio(reg, write_wc, &slot, slot+1, REQ_OP_WRITE | REQ_SYNC); - if (IS_ERR(bio)) { - status = PTR_ERR(bio); - mlog_errno(status); - goto bail; - } atomic_inc(&write_wc->wc_num_reqs); submit_bio(bio); - status = 0; -bail: - return status; + return 0; } static u32 o2hb_compute_block_crc_le(struct o2hb_region *reg, -- cgit v1.2.3