From 5c89fa57cb2dfff667ddd75eaeb13f942f5b155d Mon Sep 17 00:00:00 2001 From: Neil Brown Date: Mon, 1 Mar 2021 10:13:56 -0500 Subject: [PATCH] LU-6142 lustre: ptlrpc: don't use list_for_each_entry_safe unnecessarily. list_for_each_entry_safe() is only needed if the body of the loop might change the list, or if it might drop a lock that would otherwise prevent the list from being changed. When the body does neither of these, list_for_each_entry() should be preferred as it makes the behaviour of the loop more clear to readers. In each of the cases changed there, the list cannot change while the loop proceeds. Change-Id: Ib0f08c5d4d7959b80a7a1490fb606e40e1cf5f85 Signed-off-by: NeilBrown Reviewed-on: https://review.whamcloud.com/41939 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Arshad Hussain Reviewed-by: Oleg Drokin --- lustre/ptlrpc/client.c | 10 +++++----- lustre/ptlrpc/import.c | 16 +++++++--------- lustre/ptlrpc/ptlrpcd.c | 5 ++--- lustre/ptlrpc/recover.c | 14 +++++++------- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/lustre/ptlrpc/client.c b/lustre/ptlrpc/client.c index 6f9bfbf..0577f6a 100644 --- a/lustre/ptlrpc/client.c +++ b/lustre/ptlrpc/client.c @@ -3276,7 +3276,7 @@ int ptlrpc_replay_req(struct ptlrpc_request *req) */ void ptlrpc_abort_inflight(struct obd_import *imp) { - struct ptlrpc_request *req, *n; + struct ptlrpc_request *req; ENTRY; /* @@ -3291,7 +3291,7 @@ void ptlrpc_abort_inflight(struct obd_import *imp) * locked? Also, how do we know if the requests on the list are * being freed at this time? */ - list_for_each_entry_safe(req, n, &imp->imp_sending_list, rq_list) { + list_for_each_entry(req, &imp->imp_sending_list, rq_list) { DEBUG_REQ(D_RPCTRACE, req, "inflight"); spin_lock(&req->rq_lock); @@ -3303,7 +3303,7 @@ void ptlrpc_abort_inflight(struct obd_import *imp) spin_unlock(&req->rq_lock); } - list_for_each_entry_safe(req, n, &imp->imp_delayed_list, rq_list) { + list_for_each_entry(req, &imp->imp_delayed_list, rq_list) { DEBUG_REQ(D_RPCTRACE, req, "aborting waiting req"); spin_lock(&req->rq_lock); @@ -3330,11 +3330,11 @@ void ptlrpc_abort_inflight(struct obd_import *imp) */ void ptlrpc_abort_set(struct ptlrpc_request_set *set) { - struct ptlrpc_request *req, *tmp; + struct ptlrpc_request *req; LASSERT(set != NULL); - list_for_each_entry_safe(req, tmp, &set->set_requests, rq_set_chain) { + list_for_each_entry(req, &set->set_requests, rq_set_chain) { spin_lock(&req->rq_lock); if (req->rq_phase != RQ_PHASE_RPC) { spin_unlock(&req->rq_lock); diff --git a/lustre/ptlrpc/import.c b/lustre/ptlrpc/import.c index b7c07e2..beb67b3 100644 --- a/lustre/ptlrpc/import.c +++ b/lustre/ptlrpc/import.c @@ -278,11 +278,11 @@ static time64_t ptlrpc_inflight_deadline(struct ptlrpc_request *req, static time64_t ptlrpc_inflight_timeout(struct obd_import *imp) { time64_t now = ktime_get_real_seconds(); - struct ptlrpc_request *req, *n; + struct ptlrpc_request *req; time64_t timeout = 0; spin_lock(&imp->imp_lock); - list_for_each_entry_safe(req, n, &imp->imp_sending_list, rq_list) + list_for_each_entry(req, &imp->imp_sending_list, rq_list) timeout = max(ptlrpc_inflight_deadline(req, now), timeout); spin_unlock(&imp->imp_lock); return timeout; @@ -296,7 +296,7 @@ static time64_t ptlrpc_inflight_timeout(struct obd_import *imp) */ void ptlrpc_invalidate_import(struct obd_import *imp) { - struct ptlrpc_request *req, *n; + struct ptlrpc_request *req; time64_t timeout; int rc; @@ -372,15 +372,13 @@ void ptlrpc_invalidate_import(struct obd_import *imp) * this point. */ rc = 1; } else { - list_for_each_entry_safe(req, n, - &imp->imp_sending_list, - rq_list) { + list_for_each_entry(req, &imp->imp_sending_list, + rq_list) { DEBUG_REQ(D_ERROR, req, "still on sending list"); } - list_for_each_entry_safe(req, n, - &imp->imp_delayed_list, - rq_list) { + list_for_each_entry(req, &imp->imp_delayed_list, + rq_list) { DEBUG_REQ(D_ERROR, req, "still on delayed list"); } diff --git a/lustre/ptlrpc/ptlrpcd.c b/lustre/ptlrpc/ptlrpcd.c index 11803a8..d520447 100644 --- a/lustre/ptlrpc/ptlrpcd.c +++ b/lustre/ptlrpc/ptlrpcd.c @@ -240,13 +240,12 @@ void ptlrpcd_add_rqset(struct ptlrpc_request_set *set) static int ptlrpcd_steal_rqset(struct ptlrpc_request_set *des, struct ptlrpc_request_set *src) { - struct ptlrpc_request *req, *tmp; + struct ptlrpc_request *req; int rc = 0; spin_lock(&src->set_new_req_lock); if (likely(!list_empty(&src->set_new_requests))) { - list_for_each_entry_safe(req, tmp, &src->set_new_requests, - rq_set_chain) + list_for_each_entry(req, &src->set_new_requests, rq_set_chain) req->rq_set = des; list_splice_init(&src->set_new_requests, diff --git a/lustre/ptlrpc/recover.c b/lustre/ptlrpc/recover.c index e5fae61..3cbc423 100644 --- a/lustre/ptlrpc/recover.c +++ b/lustre/ptlrpc/recover.c @@ -68,7 +68,7 @@ void ptlrpc_initiate_recovery(struct obd_import *imp) int ptlrpc_replay_next(struct obd_import *imp, int *inflight) { int rc = 0; - struct ptlrpc_request *req = NULL, *pos; + struct ptlrpc_request *req = NULL; __u64 last_transno; ENTRY; @@ -123,8 +123,8 @@ int ptlrpc_replay_next(struct obd_import *imp, int *inflight) if (req == NULL) { struct ptlrpc_request *tmp; - list_for_each_entry_safe(tmp, pos, &imp->imp_replay_list, - rq_replay_list) { + list_for_each_entry(tmp, &imp->imp_replay_list, + rq_replay_list) { if (tmp->rq_transno > last_transno) { req = tmp; break; @@ -173,7 +173,7 @@ int ptlrpc_replay_next(struct obd_import *imp, int *inflight) */ int ptlrpc_resend(struct obd_import *imp) { - struct ptlrpc_request *req, *next; + struct ptlrpc_request *req; ENTRY; @@ -189,7 +189,7 @@ int ptlrpc_resend(struct obd_import *imp) RETURN(-1); } - list_for_each_entry_safe(req, next, &imp->imp_sending_list, rq_list) { + list_for_each_entry(req, &imp->imp_sending_list, rq_list) { LASSERTF((long)req > PAGE_SIZE && req != LP_POISON, "req %p bad\n", req); LASSERTF(req->rq_type != LI_POISON, "req %p freed\n", req); @@ -212,10 +212,10 @@ int ptlrpc_resend(struct obd_import *imp) */ void ptlrpc_wake_delayed(struct obd_import *imp) { - struct ptlrpc_request *req, *pos; + struct ptlrpc_request *req; spin_lock(&imp->imp_lock); - list_for_each_entry_safe(req, pos, &imp->imp_delayed_list, rq_list) { + list_for_each_entry(req, &imp->imp_delayed_list, rq_list) { DEBUG_REQ(D_HA, req, "waking (set %p):", req->rq_set); ptlrpc_client_wake_req(req); } -- 1.8.3.1