From patchwork Fri Jul 18 15:13:31 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 4585621 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 26819C0514 for ; Fri, 18 Jul 2014 15:13:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EABCC2010F for ; Fri, 18 Jul 2014 15:13:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id EDAA120120 for ; Fri, 18 Jul 2014 15:13:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761993AbaGRPNw (ORCPT ); Fri, 18 Jul 2014 11:13:52 -0400 Received: from mail-qa0-f44.google.com ([209.85.216.44]:43792 "EHLO mail-qa0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761887AbaGRPNv (ORCPT ); Fri, 18 Jul 2014 11:13:51 -0400 Received: by mail-qa0-f44.google.com with SMTP id f12so3033423qad.17 for ; Fri, 18 Jul 2014 08:13:50 -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=iJzZZJKS/IiasjrrGuuAun5Z6Finfx6Yjp8wQ04Rfr4=; b=jonceFwtMHjqpLPklh92cmZo/3p+JtKHpAIirgzk+B/NwL7EdQOtegXz0UPfZQlVYq itLMzBdtkSBxypJhJpazAmaHcdO8G0xj/u+gHBgtCB8dtiJM+Te7yr6wLRZCIOqGKbMj ID5Qa0KqPJzCX155rBlTapzqTWOrwgcSsiyH/+4SJQBzyanxXh3Z9HlZYme7eGu/GtXS 6SgaJx1v8q6gmn0yfoz1Xe6oA1gBVzrs7cpSFQ90iJC36X3ifgNcfhqL53nFRAdIVpxK EZ1IkZ3oz4fTr4Nh2ffRLsr2E2AkkgK5lfYdhRL+FeGpxqB8qobXgz09rLhQqaomAsSd 2ZMg== X-Gm-Message-State: ALoCoQmO41buv72D5/q5LugdTM8W9et1nbaE/FbK5HD4P+OINyflzLcXLU4FtxZVP61D1ws6AmSM X-Received: by 10.229.114.4 with SMTP id c4mr9456425qcq.16.1405696430375; Fri, 18 Jul 2014 08:13:50 -0700 (PDT) Received: from tlielax.poochiereds.net ([2001:470:8:d63:3a60:77ff:fe93:a95d]) by mx.google.com with ESMTPSA id b104sm6479580qga.24.2014.07.18.08.13.49 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Jul 2014 08:13:49 -0700 (PDT) From: Jeff Layton To: bfields@fieldses.org Cc: hch@infradead.org, linux-nfs@vger.kernel.org Subject: [PATCH v4 05/10] nfsd: ensure that clp->cl_revoked list is protected by clp->cl_lock Date: Fri, 18 Jul 2014 11:13:31 -0400 Message-Id: <1405696416-32585-6-git-send-email-jlayton@primarydata.com> X-Mailer: git-send-email 1.9.3 In-Reply-To: <1405696416-32585-1-git-send-email-jlayton@primarydata.com> References: <1405696416-32585-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, 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, 15 insertions(+), 6 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 7c5233427c9b..d11b298e625e 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -669,7 +669,7 @@ unhash_delegation_locked(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); } @@ -685,11 +685,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); } } @@ -1458,9 +1462,9 @@ destroy_client(struct nfs4_client *clp) list_del_init(&dp->dl_recall_lru); nfs4_put_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)) { @@ -3899,8 +3903,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); } list_for_each_safe(pos, next, &nn->close_lru) { @@ -4244,6 +4250,9 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, break; case NFS4_REVOKED_DELEG_STID: dp = delegstateid(s); + spin_lock(&cl->cl_lock); + list_del_init(&dp->dl_recall_lru); + spin_unlock(&cl->cl_lock); destroy_revoked_delegation(dp); ret = nfs_ok; break;