From a32c87805ffb04dad416705e8058775835df0899 Mon Sep 17 00:00:00 2001 From: Timothy Day Date: Wed, 21 Aug 2024 21:20:45 -0400 Subject: [PATCH] LU-18155 misc: use LASSERT/F instead of if () LBUG() We should use a proper LASSERT statement rather than more verbose if/LBUG blocks. The patch has been generated with the coccinelle script below. I manually inverted the logic in the asserts. @@ expression L; expression list F; @@ - if (L) { ( - CDEBUG(F); | - CWARN(F); | - CERROR(F); | - CEMERG(F); | - CNETERR(F); | - LCONSOLE(F); | - LCONSOLE_INFO(F); | - LCONSOLE_WARN(F); | - LCONSOLE_ERROR(F); | - LCONSOLE_EMERG(F); ) - LBUG(); - } + LASSERTF(L, F); @@ expression L; @@ -if (L) LBUG(); +LASSERT(L); Test-Parameters: trivial Signed-off-by: Timothy Day Change-Id: Ie0b5b8872004f446253ac7c613c41e07af6b168a Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/56115 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Arshad Hussain Reviewed-by: Andreas Dilger Reviewed-by: James Simmons Reviewed-by: Oleg Drokin --- lustre/mdd/mdd_object.c | 27 +++++++++++---------------- lustre/obdclass/genops.c | 8 +++----- lustre/obdclass/local_storage.c | 8 ++++---- lustre/obdclass/lprocfs_status.c | 8 +++----- lustre/target/tgt_lastrcvd.c | 24 +++++++++--------------- 5 files changed, 30 insertions(+), 45 deletions(-) diff --git a/lustre/mdd/mdd_object.c b/lustre/mdd/mdd_object.c index c497833..b753198 100644 --- a/lustre/mdd/mdd_object.c +++ b/lustre/mdd/mdd_object.c @@ -2756,8 +2756,9 @@ static int mdd_swap_layouts(const struct lu_env *env, int retried = 0; __u32 fst_gen; __u32 snd_gen; + int steps = 0; int fst_fl; - int rc2; + int rc2 = 0; int rc; ENTRY; @@ -3007,8 +3008,6 @@ retry: out_restore: if (rc != 0) { - int steps = 0; - /* failure on second file, but first was done, so we have * to roll back first. */ @@ -3043,19 +3042,15 @@ out_restore_hsm_fst: } do_lbug: - if (rc2 < 0) { - /* very bad day */ - CERROR("%s: unable to roll back layout swap of "DFID" and "DFID", steps: %d: rc = %d/%d\n", - mdd_obj_dev_name(fst_o), - PFID(mdd_object_fid(snd_o)), - PFID(mdd_object_fid(fst_o)), - rc, rc2, steps); - /* a solution to avoid journal commit is to panic, - * but it has strong consequences so we use LBUG to - * allow sysdamin to choose to panic or not - */ - LBUG(); - } + /* very bad day - a solution to avoid journal commit + * is to panic, but it has strong consequences so we + * use LASSERT to allow sysdamin to choose to panic + * or not + */ + LASSERTF(rc2 >= 0, + "%s: unable to roll back layout swap of " DFID " and " DFID ", steps: %d: rc = %d/%d\n", + mdd_obj_dev_name(fst_o), PFID(mdd_object_fid(snd_o)), + PFID(mdd_object_fid(fst_o)), rc, rc2, steps); } unlock: diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c index 5ee8fb2..f58cc2b 100644 --- a/lustre/obdclass/genops.c +++ b/lustre/obdclass/genops.c @@ -59,11 +59,9 @@ static void obd_device_free(struct obd_device *obd) LASSERTF(obd->obd_magic == OBD_DEVICE_MAGIC, "obd %px obd_magic %08x != %08x\n", obd, obd->obd_magic, OBD_DEVICE_MAGIC); - if (obd->obd_namespace != NULL) { - CERROR("obd %px: namespace %px was not properly cleaned up (obd_force=%d)!\n", - obd, obd->obd_namespace, obd->obd_force); - LBUG(); - } + LASSERTF(obd->obd_namespace == NULL, + "obd %px: namespace %px was not properly cleaned up (obd_force=%d)!\n", + obd, obd->obd_namespace, obd->obd_force); OBD_SLAB_FREE_PTR(obd, obd_device_cachep); } diff --git a/lustre/obdclass/local_storage.c b/lustre/obdclass/local_storage.c index fac4b30..23e02cb 100644 --- a/lustre/obdclass/local_storage.c +++ b/lustre/obdclass/local_storage.c @@ -711,10 +711,10 @@ struct local_oid_storage *dt_los_find(struct ls_device *ls, __u64 seq) void dt_los_put(struct local_oid_storage *los) { - if (atomic_dec_and_test(&los->los_refcount)) - /* should never happen, only local_oid_storage_fini should - * drop refcount to zero */ - LBUG(); + /* should never happen, only local_oid_storage_fini should + * drop refcount to zero + */ + LASSERT(!atomic_dec_and_test(&los->los_refcount)); } /* after Lustre 2.3 release there may be old file to store last generated FID diff --git a/lustre/obdclass/lprocfs_status.c b/lustre/obdclass/lprocfs_status.c index d250947..97dab8a 100644 --- a/lustre/obdclass/lprocfs_status.c +++ b/lustre/obdclass/lprocfs_status.c @@ -1789,11 +1789,9 @@ int lprocfs_alloc_md_stats(struct obd_device *obd, for (i = 0; i < ARRAY_SIZE(mps_stats); i++) { lprocfs_counter_init(stats, i, LPROCFS_TYPE_REQS, mps_stats[i]); - if (!stats->ls_cnt_header[i].lc_name) { - CERROR("Missing md_stat initializer md_op operation at offset %d. Aborting.\n", - i); - LBUG(); - } + LASSERTF(stats->ls_cnt_header[i].lc_name, + "Missing md_stat initializer md_op operation at offset %d. Aborting.\n", + i); } rc = lprocfs_stats_register(obd->obd_proc_entry, "md_stats", stats); diff --git a/lustre/target/tgt_lastrcvd.c b/lustre/target/tgt_lastrcvd.c index 7d56d11..5e3821b 100644 --- a/lustre/target/tgt_lastrcvd.c +++ b/lustre/target/tgt_lastrcvd.c @@ -448,11 +448,9 @@ void tgt_client_free(struct obd_export *exp) /* Clear bit when lcd is freed */ LASSERT(lut && lut->lut_client_bitmap); - if (!test_and_clear_bit(ted->ted_lr_idx, lut->lut_client_bitmap)) { - CERROR("%s: client %u bit already clear in bitmap\n", - exp->exp_obd->obd_name, ted->ted_lr_idx); - LBUG(); - } + LASSERTF(test_and_clear_bit(ted->ted_lr_idx, lut->lut_client_bitmap), + "%s: client %u bit already clear in bitmap\n", + exp->exp_obd->obd_name, ted->ted_lr_idx); } EXPORT_SYMBOL(tgt_client_free); @@ -1132,11 +1130,9 @@ int tgt_client_add(const struct lu_env *env, struct obd_export *exp, int idx) exp_connect_flags(exp) & OBD_CONNECT_LIGHTWEIGHT) RETURN(0); - if (test_and_set_bit(idx, tgt->lut_client_bitmap)) { - CERROR("%s: client %d: bit already set in bitmap!!\n", - tgt->lut_obd->obd_name, idx); - LBUG(); - } + LASSERTF(!test_and_set_bit(idx, tgt->lut_client_bitmap), + "%s: client %d: bit already set in bitmap!!\n", + tgt->lut_obd->obd_name, idx); CDEBUG(D_INFO, "%s: client at idx %d with UUID '%s' added, " "generation %d\n", @@ -1191,11 +1187,9 @@ int tgt_client_del(const struct lu_env *env, struct obd_export *exp) /* Clear the bit _after_ zeroing out the client so we don't race with filter_client_add and zero out new clients.*/ - if (!test_bit(ted->ted_lr_idx, tgt->lut_client_bitmap)) { - CERROR("%s: client %u: bit already clear in bitmap!!\n", - tgt->lut_obd->obd_name, ted->ted_lr_idx); - LBUG(); - } + LASSERTF(test_bit(ted->ted_lr_idx, tgt->lut_client_bitmap), + "%s: client %u: bit already clear in bitmap!!\n", + tgt->lut_obd->obd_name, ted->ted_lr_idx); /* Do not erase record for recoverable client. */ if (exp->exp_flags & OBD_OPT_FAILOVER) -- 1.8.3.1