From patchwork Mon Jul 21 15:02:45 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 4596181 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 A37469F2B8 for ; Mon, 21 Jul 2014 15:05:01 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8323020149 for ; Mon, 21 Jul 2014 15:04:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5C57A2013D for ; Mon, 21 Jul 2014 15:04:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932481AbaGUPDr (ORCPT ); Mon, 21 Jul 2014 11:03:47 -0400 Received: from mail-qg0-f52.google.com ([209.85.192.52]:57890 "EHLO mail-qg0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755217AbaGUPDr (ORCPT ); Mon, 21 Jul 2014 11:03:47 -0400 Received: by mail-qg0-f52.google.com with SMTP id f51so5508530qge.11 for ; Mon, 21 Jul 2014 08:03:46 -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=DAKnWnP1L7W/GwDxBwhNzzrCiVHzNR15K/xnYuKOwig=; b=JUF6h7RQtK347WtxbP3j8gfBNpV04kP+KCwKRxONE7c9L9nRt3uFAARm3VoMEHwBtv CpqSQdt+dyOgpOvbkV3pVsO4znAu3sW5ANJ+7aE9G3Bb7UNkeHVdQC+K3sFA0QcvyQXx SjipRFDVNmi2vHBw8rgc9Bm89HJfaZReZT+igJyHyTV9Pz9BL5vTzbXtCmX4seC6kXmU e9L5PmUJHjIvhPL0qGS1YD38pCzD/SSSJXNUsmeZgywrPkSkaSA7B0fIpsoxe9/eD1HY 4Bh8ZC9CBAgdlt2Nq/J8aHqIBW6+HIEfSiyKDDFGb9UxwUtXTh/+0CxJg526Gg2S77kS KP7A== X-Gm-Message-State: ALoCoQnO0jo0/X+nK36H39dBxbBDk4d4lHtJp9F2Dh06h5scuTnHJUUA4iEuB5WS5Rkbw9UaEoa1 X-Received: by 10.229.39.73 with SMTP id f9mr42139605qce.27.1405955026338; Mon, 21 Jul 2014 08:03:46 -0700 (PDT) Received: from tlielax.poochiereds.net ([2001:470:8:d63:3a60:77ff:fe93:a95d]) by mx.google.com with ESMTPSA id l76sm16375384qga.8.2014.07.21.08.03.44 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 21 Jul 2014 08:03:45 -0700 (PDT) From: Jeff Layton To: bfields@fieldses.org Cc: linux-nfs@vger.kernel.org, hch@infradead.org, Trond Myklebust Subject: [PATCH 33/40] nfsd: Protect adding/removing lock owners using client_lock Date: Mon, 21 Jul 2014 11:02:45 -0400 Message-Id: <1405954972-28904-34-git-send-email-jlayton@primarydata.com> X-Mailer: git-send-email 1.9.3 In-Reply-To: <1405954972-28904-1-git-send-email-jlayton@primarydata.com> References: <1405954972-28904-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 From: Trond Myklebust Once we remove client mutex protection, we'll need to ensure that stateowner lookup and creation are atomic between concurrent compounds. Ensure that alloc_init_lock_stateowner checks the hashtable under the client_lock before adding a new element. Signed-off-by: Trond Myklebust --- fs/nfsd/nfs4state.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index a3fae742315d..2556499ed8d8 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -957,26 +957,42 @@ static void release_lock_stateid(struct nfs4_ol_stateid *stp) put_generic_stateid(stp); } -static void unhash_lockowner(struct nfs4_lockowner *lo) +static void unhash_lockowner_locked(struct nfs4_lockowner *lo) { + struct nfsd_net *nn = net_generic(lo->lo_owner.so_client->net, + nfsd_net_id); + + lockdep_assert_held(&nn->client_lock); + list_del_init(&lo->lo_owner.so_strhash); } static void release_lockowner_stateids(struct nfs4_lockowner *lo) { + struct nfsd_net *nn = net_generic(lo->lo_owner.so_client->net, + nfsd_net_id); struct nfs4_ol_stateid *stp; + lockdep_assert_held(&nn->client_lock); + while (!list_empty(&lo->lo_owner.so_stateids)) { stp = list_first_entry(&lo->lo_owner.so_stateids, struct nfs4_ol_stateid, st_perstateowner); + spin_unlock(&nn->client_lock); release_lock_stateid(stp); + spin_lock(&nn->client_lock); } } static void release_lockowner(struct nfs4_lockowner *lo) { - unhash_lockowner(lo); + struct nfsd_net *nn = net_generic(lo->lo_owner.so_client->net, + nfsd_net_id); + + spin_lock(&nn->client_lock); + unhash_lockowner_locked(lo); release_lockowner_stateids(lo); + spin_unlock(&nn->client_lock); nfs4_put_stateowner(&lo->lo_owner); } @@ -4819,7 +4835,7 @@ nevermind: } static struct nfs4_lockowner * -find_lockowner_str(clientid_t *clid, struct xdr_netobj *owner, +find_lockowner_str_locked(clientid_t *clid, struct xdr_netobj *owner, struct nfsd_net *nn) { unsigned int strhashval = ownerstr_hashval(clid->cl_id, owner); @@ -4836,9 +4852,25 @@ find_lockowner_str(clientid_t *clid, struct xdr_netobj *owner, return NULL; } +static struct nfs4_lockowner * +find_lockowner_str(clientid_t *clid, struct xdr_netobj *owner, + struct nfsd_net *nn) +{ + struct nfs4_lockowner *lo; + + spin_lock(&nn->client_lock); + lo = find_lockowner_str_locked(clid, owner, nn); + spin_unlock(&nn->client_lock); + return lo; +} + static void nfs4_unhash_lockowner(struct nfs4_stateowner *sop) { - unhash_lockowner(lockowner(sop)); + struct nfsd_net *nn = net_generic(sop->so_client->net, nfsd_net_id); + + spin_lock(&nn->client_lock); + unhash_lockowner_locked(lockowner(sop)); + spin_unlock(&nn->client_lock); } static void nfs4_free_lockowner(struct nfs4_stateowner *sop) @@ -4857,9 +4889,12 @@ static void nfs4_free_lockowner(struct nfs4_stateowner *sop) * strhashval = ownerstr_hashval */ static struct nfs4_lockowner * -alloc_init_lock_stateowner(unsigned int strhashval, struct nfs4_client *clp, struct nfs4_ol_stateid *open_stp, struct nfsd4_lock *lock) { - struct nfs4_lockowner *lo; +alloc_init_lock_stateowner(unsigned int strhashval, struct nfs4_client *clp, + struct nfs4_ol_stateid *open_stp, + struct nfsd4_lock *lock) +{ struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); + struct nfs4_lockowner *lo, *ret; lo = alloc_stateowner(lockowner_slab, &lock->lk_new_owner, clp); if (!lo) @@ -4869,7 +4904,16 @@ alloc_init_lock_stateowner(unsigned int strhashval, struct nfs4_client *clp, str lo->lo_owner.so_seqid = lock->lk_new_lock_seqid; lo->lo_owner.so_free = nfs4_free_lockowner; lo->lo_owner.so_unhash = nfs4_unhash_lockowner; - list_add(&lo->lo_owner.so_strhash, &nn->ownerstr_hashtbl[strhashval]); + spin_lock(&nn->client_lock); + ret = find_lockowner_str_locked(&clp->cl_clientid, + &lock->lk_new_owner, nn); + if (ret == NULL) { + list_add(&lo->lo_owner.so_strhash, + &nn->ownerstr_hashtbl[strhashval]); + ret = lo; + } else + nfs4_free_lockowner(&lo->lo_owner); + spin_unlock(&nn->client_lock); return lo; } @@ -5397,6 +5441,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp, unsigned int hashval = ownerstr_hashval(clid->cl_id, owner); __be32 status; struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id); + struct nfs4_client *clp; dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n", clid->cl_boot, clid->cl_id); @@ -5410,6 +5455,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp, status = nfserr_locks_held; /* Find the matching lock stateowner */ + spin_lock(&nn->client_lock); list_for_each_entry(tmp, &nn->ownerstr_hashtbl[hashval], so_strhash) { if (tmp->so_is_open_owner) continue; @@ -5419,6 +5465,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp, break; } } + spin_unlock(&nn->client_lock); /* No matching owner found, maybe a replay? Just declare victory... */ if (!sop) { @@ -5428,16 +5475,22 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp, lo = lockowner(sop); /* see if there are still any locks associated with it */ + clp = cstate->clp; + spin_lock(&clp->cl_lock); list_for_each_entry(stp, &sop->so_stateids, st_perstateowner) { if (check_for_locks(stp->st_stid.sc_file, lo)) { - nfs4_put_stateowner(sop); + spin_unlock(&clp->cl_lock); goto out; } } + spin_unlock(&clp->cl_lock); status = nfs_ok; + sop = NULL; release_lockowner(lo); out: + if (sop) + nfs4_put_stateowner(sop); nfs4_unlock_state(); return status; }