From patchwork Thu Mar 14 00:11:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 10852005 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 513C31515 for ; Thu, 14 Mar 2019 00:15:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3B9072884E for ; Thu, 14 Mar 2019 00:15:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2FE952A06B; Thu, 14 Mar 2019 00:15:42 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from pdx1-mailman02.dreamhost.com (pdx1-mailman02.dreamhost.com [64.90.62.194]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A3BDD2884E for ; Thu, 14 Mar 2019 00:15:41 +0000 (UTC) Received: from pdx1-mailman02.dreamhost.com (localhost [IPv6:::1]) by pdx1-mailman02.dreamhost.com (Postfix) with ESMTP id 62A35720167; Wed, 13 Mar 2019 17:15:41 -0700 (PDT) X-Original-To: lustre-devel@lists.lustre.org Delivered-To: lustre-devel-lustre.org@pdx1-mailman02.dreamhost.com Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by pdx1-mailman02.dreamhost.com (Postfix) with ESMTP id A0C3B720154 for ; Wed, 13 Mar 2019 17:15:39 -0700 (PDT) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id D4DFCAF5F; Thu, 14 Mar 2019 00:15:38 +0000 (UTC) From: NeilBrown To: Andreas Dilger , James Simmons , Oleg Drokin Date: Thu, 14 Mar 2019 11:11:51 +1100 Message-ID: <155252231140.26912.8140429396611661381.stgit@noble.brown> In-Reply-To: <155252182126.26912.1842463462595601611.stgit@noble.brown> References: <155252182126.26912.1842463462595601611.stgit@noble.brown> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Subject: [lustre-devel] [PATCH 26/32] lustre: ptlrpc: don't use list_for_each_entry_safe unnecessarily. X-BeenThere: lustre-devel@lists.lustre.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "For discussing Lustre software development." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lustre Development List Errors-To: lustre-devel-bounces@lists.lustre.org Sender: "lustre-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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. Signed-off-by: NeilBrown --- drivers/staging/lustre/lustre/ptlrpc/client.c | 10 +++++----- drivers/staging/lustre/lustre/ptlrpc/import.c | 14 +++++++------- drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c | 4 ++-- drivers/staging/lustre/lustre/ptlrpc/recover.c | 14 +++++++------- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c index 476435633694..71bb563765cc 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/client.c +++ b/drivers/staging/lustre/lustre/ptlrpc/client.c @@ -2916,7 +2916,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; /* * Make sure that no new requests get processed for this import. @@ -2930,7 +2930,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); @@ -2942,7 +2942,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); @@ -2969,9 +2969,9 @@ 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; - 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/drivers/staging/lustre/lustre/ptlrpc/import.c b/drivers/staging/lustre/lustre/ptlrpc/import.c index db4ed6dbf362..26a976865fbd 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/import.c +++ b/drivers/staging/lustre/lustre/ptlrpc/import.c @@ -244,11 +244,11 @@ ptlrpc_inflight_deadline(struct ptlrpc_request *req, time64_t now) static unsigned int ptlrpc_inflight_timeout(struct obd_import *imp) { time64_t now = ktime_get_real_seconds(); - struct ptlrpc_request *req, *n; + struct ptlrpc_request *req; unsigned int 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); @@ -263,7 +263,7 @@ static unsigned int ptlrpc_inflight_timeout(struct obd_import *imp) */ void ptlrpc_invalidate_import(struct obd_import *imp) { - struct ptlrpc_request *req, *n; + struct ptlrpc_request *req; unsigned int timeout; int rc; @@ -335,13 +335,13 @@ void ptlrpc_invalidate_import(struct obd_import *imp) */ rc = 0; } 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/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c index f0ac2962d64a..c295e9943bf7 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c +++ b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c @@ -200,12 +200,12 @@ ptlrpcd_select_pc(struct ptlrpc_request *req) 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, &des->set_requests); diff --git a/drivers/staging/lustre/lustre/ptlrpc/recover.c b/drivers/staging/lustre/lustre/ptlrpc/recover.c index af672ab5b212..f33e4c541a17 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/recover.c +++ b/drivers/staging/lustre/lustre/ptlrpc/recover.c @@ -66,7 +66,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; *inflight = 0; @@ -120,8 +120,8 @@ int ptlrpc_replay_next(struct obd_import *imp, int *inflight) if (!req) { 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; @@ -172,7 +172,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; /* As long as we're in recovery, nothing should be added to the sending * list, so we don't need to hold the lock during this iteration and @@ -186,7 +186,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); @@ -211,10 +211,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); }