From patchwork Thu Jul 3 22:31:15 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 4477591 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 E7F5CBEEAA for ; Thu, 3 Jul 2014 22:31:22 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 37C7B20225 for ; Thu, 3 Jul 2014 22:31:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 50B8520221 for ; Thu, 3 Jul 2014 22:31:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752673AbaGCWbT (ORCPT ); Thu, 3 Jul 2014 18:31:19 -0400 Received: from mail-qc0-f181.google.com ([209.85.216.181]:58160 "EHLO mail-qc0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752275AbaGCWbS (ORCPT ); Thu, 3 Jul 2014 18:31:18 -0400 Received: by mail-qc0-f181.google.com with SMTP id x13so849117qcv.12 for ; Thu, 03 Jul 2014 15:31:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:date:to:cc:subject:message-id:in-reply-to :references:mime-version:content-type; bh=GslUsPt6nLcwpHVD2rYVVZd6BJu+va8FIJBb/1SoLN8=; b=SuWlMaCds4EUvWxQ6Bnt9LBYRwjRYrWCuhK1u4bo+eeeyy/3OyDslugCU/DWAK8QMx +9+6MPdSwCdChUQQulxqnNYGmEypRGmkVOZwvGE9VuvOEiNRKULUxVpl33z7cDIzfy2t 7vgEp2eWph/UtN7c7OIHsDJK4WiPbAo984p6M9OJM+TILJeovPui/9H2G6uB0mlsAqmc Qv0CzVbptjWq1ZTKqcZPQFruQUU5ajLlDSJ85n5Dw6sVAFMoDMKy+B+nlCtotB7thugE NKfvZ/3Nc2PB//3jAUL8tjV3ZAIbB2qDHG/lLfFlAxjbwtplaa7iACZd4k/2X2xTkYhz eVXw== X-Gm-Message-State: ALoCoQmAL3YVXA/qPAp6fZXo5cE16tdkKZKx6eZzpY3hWG7mSMrA5FiwM+xc7yPJsNUsnOKvuWHU X-Received: by 10.140.35.232 with SMTP id n95mr11246173qgn.82.1404426677458; Thu, 03 Jul 2014 15:31:17 -0700 (PDT) Received: from tlielax.poochiereds.net ([2001:470:8:d63:3a60:77ff:fe93:a95d]) by mx.google.com with ESMTPSA id d5sm15935033qar.38.2014.07.03.15.31.17 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 03 Jul 2014 15:31:17 -0700 (PDT) From: Jeff Layton X-Google-Original-From: Jeff Layton Date: Thu, 3 Jul 2014 18:31:15 -0400 To: "J. Bruce Fields" Cc: linux-nfs@vger.kernel.org Subject: Re: [PATCH v3 015/114] nfsd: Allow struct nfsd4_compound_state to cache the nfs4_client Message-ID: <20140703183115.397af08e@tlielax.poochiereds.net> In-Reply-To: <20140703213526.GG24322@fieldses.org> References: <1404143423-24381-1-git-send-email-jlayton@primarydata.com> <1404143423-24381-16-git-send-email-jlayton@primarydata.com> <20140703203259.GF24322@fieldses.org> <20140703213526.GG24322@fieldses.org> X-Mailer: Claws Mail 3.10.0 (GTK+ 2.24.22; x86_64-redhat-linux-gnu) MIME-Version: 1.0 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, T_TVD_MIME_EPI, 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 On Thu, 3 Jul 2014 17:35:26 -0400 "J. Bruce Fields" wrote: > On Thu, Jul 03, 2014 at 04:32:59PM -0400, J. Bruce Fields wrote: > > On Mon, Jun 30, 2014 at 11:48:44AM -0400, Jeff Layton wrote: > > > We want to use the nfsd4_compound_state to cache the nfs4_client in > > > order to optimise away extra lookups of the clid. > > > > > > In the v4.0 case, we use this to ensure that we only have to look up the > > > client at most once per compound for each call into lookup_clientid. For > > > v4.1+ we set the pointer in the cstate during SEQUENCE processing so we > > > should never need to do a search for it. > > > > The connectathon locking test is failing for me in the nfsv4/krb5i case > > as of this commit. > > > > Which makes no sense to me whatsoever, so it's entirely possible this is > > some unrelated problem on my side. I'll let you know when I've figured > > out anything more. > > It's intermittent. > > I've reproduced it on the previous commit so I know at least that this > one isn't at fault. > > I doubt it's really dependent on krb5i, at most that's probably just > making it more likely to reproduce. > > --b. Bruce, Does this patch help? I suspect this is where the bug crept in, but I'm unclear on why it would be intermittent... FWIW, this all gets cleaned up in a later patch that changes how the refcounting on lock and openowners works. From 84884320b983eaeb68c652de8fe4b48aad4052c3 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Thu, 3 Jul 2014 18:20:31 -0400 Subject: [PATCH] nfsd: fix lock stateid cleanup on error in nfsd4_lock commit 43a1b041e3b changed nfsd4_lock to call release_lockowner_if_empty if the the lock stateid was new and the function was going to return an error. This is wrong. The lockowner will never be empty in that situation since it still has the new lock stateid attached to it. Change it to call release_lock_stateid instead, which will destroy the lock stateid and then release the lockowner if it's empty at that point. Cc: Trond Myklebust Reported-by: J. Bruce Fields Signed-off-by: Jeff Layton --- fs/nfsd/nfs4state.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index a22c73f14a17..b2ea0a06fbf2 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -4600,7 +4600,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, } out: if (status && new_state) - release_lockowner_if_empty(lock_sop); + release_lock_stateid(lock_stp); nfsd4_bump_seqid(cstate, status); if (!cstate->replay_owner) nfs4_unlock_state(); -- 1.9.3