From patchwork Mon Jul 21 13:35:01 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 4595001 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 4AD989F295 for ; Mon, 21 Jul 2014 13:35:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4DC7920127 for ; Mon, 21 Jul 2014 13:35:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 30EFF20149 for ; Mon, 21 Jul 2014 13:35:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932114AbaGUNfu (ORCPT ); Mon, 21 Jul 2014 09:35:50 -0400 Received: from mail-qc0-f174.google.com ([209.85.216.174]:53761 "EHLO mail-qc0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932418AbaGUNfW (ORCPT ); Mon, 21 Jul 2014 09:35:22 -0400 Received: by mail-qc0-f174.google.com with SMTP id o8so5481972qcw.19 for ; Mon, 21 Jul 2014 06:35:22 -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=mxcna8YbEAuRvlQjXCYkaWApl6cNACVxjgf9ktyzHOA=; b=UD8vAKuXtyNUosKGibe56EbHZ3AAKnMUgUiTYJPTVF8BUbwlHsXmqFom9Azw4ju9Tn vmiPfQPaHVIXv0oBYMfl+8/P/viRlCJVyITWBcKT7nY6p3i8v/21wDKnuaC4/pdTv5m3 OtdRTpuTxNwXyWTeh1vmhJoA36FXIyhhWLSdOtogZUeEBFW6HaJbYgMOKblEDBbMeUhj 6JytgTKpBbtGfwxK2Z4rR9qB5YeJRv43hCaTLIWwRdF7CWRJBqnyZF+i7j239bvgUtDp 0/dSBfXv1LLoDuxEBRG7B7kLSW8vI81R2uodztQ1lNM5dAkV6OvylSEEYX8hNcskTNNG 1rLQ== X-Gm-Message-State: ALoCoQnsdas0UNPBH+SCdDNAe+T6ykScxPOKjhSj9nuHzf0K89B9kJlwjQoi/q5SxOlmORhMuZ0F X-Received: by 10.140.43.118 with SMTP id d109mr39714765qga.10.1405949722162; Mon, 21 Jul 2014 06:35:22 -0700 (PDT) Received: from tlielax.poochiereds.net ([2001:470:8:d63:3a60:77ff:fe93:a95d]) by mx.google.com with ESMTPSA id 7sm16143597qgg.25.2014.07.21.06.35.20 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 21 Jul 2014 06:35:21 -0700 (PDT) From: Jeff Layton To: bfields@fieldses.org Cc: hch@infradead.org, linux-nfs@vger.kernel.org Subject: [PATCH v5 05/10] nfsd: ensure that clp->cl_revoked list is protected by clp->cl_lock Date: Mon, 21 Jul 2014 09:35:01 -0400 Message-Id: <1405949706-27757-6-git-send-email-jlayton@primarydata.com> X-Mailer: git-send-email 1.9.3 In-Reply-To: <1405949706-27757-1-git-send-email-jlayton@primarydata.com> References: <1405949706-27757-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 Reviewed-by: Christoph Hellwig --- fs/nfsd/nfs4state.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index d8c28d5c600f..58746f0f34a1 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -667,12 +667,6 @@ unhash_delegation_locked(struct nfs4_delegation *dp) nfs4_put_deleg_lease(fp); } -static void destroy_revoked_delegation(struct nfs4_delegation *dp) -{ - list_del_init(&dp->dl_recall_lru); - nfs4_put_delegation(dp); -} - static void destroy_delegation(struct nfs4_delegation *dp) { spin_lock(&state_lock); @@ -685,11 +679,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); + nfs4_put_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,10 +1456,10 @@ 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); - destroy_revoked_delegation(dp); + list_del_init(&dp->dl_recall_lru); + nfs4_put_delegation(dp); } while (!list_empty(&clp->cl_openowners)) { oo = list_entry(clp->cl_openowners.next, struct nfs4_openowner, oo_perclient); @@ -3898,8 +3896,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) { @@ -4243,7 +4243,10 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, break; case NFS4_REVOKED_DELEG_STID: dp = delegstateid(s); - destroy_revoked_delegation(dp); + spin_lock(&cl->cl_lock); + list_del_init(&dp->dl_recall_lru); + spin_unlock(&cl->cl_lock); + nfs4_put_delegation(dp); ret = nfs_ok; break; default: