From patchwork Thu Jun 26 19:20:58 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 4431021 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 947C59F2C8 for ; Thu, 26 Jun 2014 19:21:48 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BC41920154 for ; Thu, 26 Jun 2014 19:21:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B0C4E20306 for ; Thu, 26 Jun 2014 19:21:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751665AbaFZTVm (ORCPT ); Thu, 26 Jun 2014 15:21:42 -0400 Received: from mail-qg0-f53.google.com ([209.85.192.53]:61978 "EHLO mail-qg0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751570AbaFZTVj (ORCPT ); Thu, 26 Jun 2014 15:21:39 -0400 Received: by mail-qg0-f53.google.com with SMTP id i50so3423547qgf.26 for ; Thu, 26 Jun 2014 12:21:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=CpG7DOslktqpa9Rqg9NOkb3co5hXqq3VjF5N0uWoppk=; b=UgW2IrdVSV9vCJ2E18a+v54xzKYuNBiubvYc8IRs+xoqqOIxNGIWwKbYhIUzwOvnZc mckJqgNeh7JGQfR4XmJHV6Ng+3dBvksVCSL4nfta21gTfdJ5rPC21LG1hq80MRxe6lwU 3yVIwu7S8X/r0cF4UiCj15BQam90hXbDm9QSrL1pPOvrt63q+i4YAO0+N92VpIPVrHGU m0VcOXhsVvgfsQRWRXvjyImIl/6q20II2K61a4CKIfiEXVBHBGFElEn6K9wJoLq/Odod LScsNEHNR4MkQq2oJZI6SNnBfHLRcAtSXKJf23OnsLs0f8We/Oshj01Kp3ONbJInhmAC VNyw== X-Gm-Message-State: ALoCoQmepvpXYsEpkkDE8ssh+XPXZeIwiojn7DXzYDwWdARPlKi3cBs+3kDl9SMj8ZMmYyu7TWZE X-Received: by 10.224.87.130 with SMTP id w2mr25658417qal.5.1403810499246; Thu, 26 Jun 2014 12:21:39 -0700 (PDT) Received: from tlielax.poochiereds.net ([2001:470:8:d63:3a60:77ff:fe93:a95d]) by mx.google.com with ESMTPSA id k76sm4765555qgd.38.2014.06.26.12.21.37 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 26 Jun 2014 12:21:38 -0700 (PDT) From: Jeff Layton To: bfields@fieldses.org Cc: linux-nfs@vger.kernel.org Subject: [PATCH v2 090/117] nfsd: ensure that clp->cl_revoked list is protected by clp->cl_lock Date: Thu, 26 Jun 2014 15:20:58 -0400 Message-Id: <1403810485-16497-7-git-send-email-jlayton@primarydata.com> X-Mailer: git-send-email 1.9.3 In-Reply-To: <1403810017-16062-1-git-send-email-jlayton@primarydata.com> References: <1403810017-16062-1-git-send-email-jlayton@primarydata.com> Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Currently, both destroy_revoked_delegation and revoke_delegation manipulate the cl_revoked list without any locking aside from the client_mutex. Ensure that the clp->cl_lock is held when manipulating it, except for the list walking in destroy_client. At that point, the client should no longer be in use, and so it should be safe to walk the list without any locking. That also means that we don't need to do the list_splice_init there either. Also, the fact that destroy_revoked_delegation and revoke_delegation delete dl_recall_lru without any locking makes it difficult to know whether they're doing so safely in all cases. Move the list_del_init calls into the callers, and add WARN_ONs in the event that these calls are passed a delegation that has a non-empty list_head. Signed-off-by: Jeff Layton --- fs/nfsd/nfs4state.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 73eb34901bf2..e9daa2649275 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -764,7 +764,7 @@ static void unhash_and_destroy_delegation(struct nfs4_delegation *dp) static void destroy_revoked_delegation(struct nfs4_delegation *dp) { - list_del_init(&dp->dl_recall_lru); + WARN_ON(!list_empty(&dp->dl_recall_lru)); nfs4_put_delegation(dp); } @@ -772,11 +772,15 @@ static void revoke_delegation(struct nfs4_delegation *dp) { struct nfs4_client *clp = dp->dl_stid.sc_client; + WARN_ON(!list_empty(&dp->dl_recall_lru)); + if (clp->cl_minorversion == 0) destroy_revoked_delegation(dp); else { dp->dl_stid.sc_type = NFS4_REVOKED_DELEG_STID; - list_move(&dp->dl_recall_lru, &clp->cl_revoked); + spin_lock(&clp->cl_lock); + list_add(&dp->dl_recall_lru, &clp->cl_revoked); + spin_unlock(&clp->cl_lock); } } @@ -1596,9 +1600,9 @@ __destroy_client(struct nfs4_client *clp) list_del_init(&dp->dl_recall_lru); destroy_delegation(dp); } - list_splice_init(&clp->cl_revoked, &reaplist); - while (!list_empty(&reaplist)) { + while (!list_empty(&clp->cl_revoked)) { dp = list_entry(reaplist.next, struct nfs4_delegation, dl_recall_lru); + list_del_init(&dp->dl_recall_lru); destroy_revoked_delegation(dp); } while (!list_empty(&clp->cl_openowners)) { @@ -4165,8 +4169,10 @@ nfs4_laundromat(struct nfsd_net *nn) list_add(&dp->dl_recall_lru, &reaplist); } spin_unlock(&state_lock); - list_for_each_safe(pos, next, &reaplist) { - dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru); + while (!list_empty(&reaplist)) { + dp = list_first_entry(&reaplist, struct nfs4_delegation, + dl_recall_lru); + list_del_init(&dp->dl_recall_lru); revoke_delegation(dp); } @@ -4529,8 +4535,9 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, ret = nfs_ok; goto out; case NFS4_REVOKED_DELEG_STID: - spin_unlock(&cl->cl_lock); dp = delegstateid(s); + list_del_init(&dp->dl_recall_lru); + spin_unlock(&cl->cl_lock); destroy_revoked_delegation(dp); ret = nfs_ok; goto out;