From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: linux-nfs-owner@vger.kernel.org Received: from fieldses.org ([174.143.236.118]:55895 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030934Ab2CFUOj (ORCPT ); Tue, 6 Mar 2012 15:14:39 -0500 Date: Tue, 6 Mar 2012 15:14:38 -0500 From: "J. Bruce Fields" To: Jeff Layton Cc: linux-nfs@vger.kernel.org Subject: Re: [PATCH 1/2] nfsd: convert nfs4_client->cl_cb_flags to a generic flags field Message-ID: <20120306201438.GB24741@fieldses.org> References: <1330965756-24180-1-git-send-email-jlayton@redhat.com> <1330965756-24180-2-git-send-email-jlayton@redhat.com> <20120306201119.GA24741@fieldses.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20120306201119.GA24741@fieldses.org> Sender: linux-nfs-owner@vger.kernel.org List-ID: On Tue, Mar 06, 2012 at 03:11:19PM -0500, J. Bruce Fields wrote: > On Mon, Mar 05, 2012 at 11:42:35AM -0500, Jeff Layton wrote: > > We'll need a way to flag the nfs4_client as already being recorded on > > stable storage so that we don't continually upcall. Currently, that's > > recorded in the cl_firststate field of the client struct. Using an > > entire u32 to store a flag is rather wasteful though. > > > > The cl_cb_flags field is only using 2 bits right now, so repurpose that > > to a generic flags field. Rename NFSD4_CLIENT_KILL to > > NFSD4_CLIENT_CB_KILL to make it evident that it's part of the callback > > flags. Add a mask that we can use for existing checks that look to see > > whether any flags are set, so that the new flags don't interfere. > > > > Convert all references to cl_firstate to the NFSD4_CLIENT_STABLE flag, > > and add a new NFSD4_CLIENT_RECLAIM_COMPLETE flag. I believe there's an > > existing bug here too that this should fix: > > > > nfs4_set_claim_prev sets cl_firststate on the first CLAIM_PREV open. > > nfsd4_reclaim_complete looks for that flag though, and returns > > NFS4ERR_COMPLETE_ALREADY if it's set. The upshot here is that once a > > client does a CLAIM_PREV open, the RECLAIM_COMPLETE call will fail. > > Let's fix this by adding a new RECLAIM_COMPLETE flag on the client to > > indicate that that's already been done. > > I think this is right, and agree that flags make more sense. But as a > quick bugfix only, how about this? Also I think there's another preexisting bug here. Yuch. commit e02c696c47958adb0a2f5499aa6032288c5b475b Author: J. Bruce Fields Date: Tue Mar 6 14:43:36 2012 -0500 nfsd4: purge stable client records with insufficient state To escape having your stable storage record purged at the end of the grace period, it's not sufficient to simply have performed a setclientid_confirm; you also need to meet the same requirements as someone creating a new record: either you should have done an open or open reclaim (in the 4.0 case) or a reclaim_complete (in the 4.1 case). Signed-off-by: J. Bruce Fields diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 207c3bd..c9c446d 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -4415,7 +4415,9 @@ nfs4_has_reclaimed_state(const char *name, bool use_exchange_id) struct nfs4_client *clp; clp = find_confirmed_client_by_str(name, strhashval); - return clp ? 1 : 0; + if (!clp) + return 0; + return clp->cl_firststate; } /*