From patchwork Mon Jul 9 15:43:49 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chuck Lever X-Patchwork-Id: 1173891 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 40C3D3FC2A for ; Mon, 9 Jul 2012 15:46:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751934Ab2GIPny (ORCPT ); Mon, 9 Jul 2012 11:43:54 -0400 Received: from mail-gh0-f174.google.com ([209.85.160.174]:38374 "EHLO mail-gh0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751694Ab2GIPny (ORCPT ); Mon, 9 Jul 2012 11:43:54 -0400 Received: by ghrr11 with SMTP id r11so10207806ghr.19 for ; Mon, 09 Jul 2012 08:43:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:subject:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-type:content-transfer-encoding; bh=PS3NlRyz259FrzUqmq/Vn80i0E0ADwwuDGuA7m+Ne+M=; b=xkgRDZqMU6GKwOz06qOlSef4fUeph1srcZ5KSRVwN9HiZaHY/6haV9LhBtqUiyuIT7 OgZWadCWvEivXr8I76x1asnjMjmEfRQ6KEYxmEbDQ2SjACLro7Oo/jUGSgB8w6wQQY8C UvrZ4fxyAvylBHVvHShpTkFIxrEVPUqTVgSSUkUHcYVwK000CV0BzXOQBWbiSZRH1QHV RwwYVfOPq/16c4StqQ90mp1K0HTBEn4mMuQL5O5CoCjr3zExPLXWG+XVYaJFN7+TI4sd MHu+6AIFOvpxfu2C23wGinAinr8MHlVzAYwxCFiC+BLzpuCy3LOjicQMoEA60O9QOyap ejfQ== Received: by 10.50.242.73 with SMTP id wo9mr7287005igc.1.1341848633162; Mon, 09 Jul 2012 08:43:53 -0700 (PDT) Received: from degas.1015granger.net (adsl-99-26-161-222.dsl.sfldmi.sbcglobal.net. [99.26.161.222]) by mx.google.com with ESMTPS id ga6sm9271818igc.2.2012.07.09.08.43.49 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 09 Jul 2012 08:43:50 -0700 (PDT) From: Chuck Lever Subject: [PATCH 01/14] NFS: Clean up TEST_STATEID and FREE_STATEID proc error reporting To: trond.myklebust@netapp.com Cc: linux-nfs@vger.kernel.org Date: Mon, 09 Jul 2012 11:43:49 -0400 Message-ID: <20120709154349.1604.58109.stgit@degas.1015granger.net> In-Reply-To: <20120709153355.1604.14102.stgit@degas.1015granger.net> References: <20120709153355.1604.14102.stgit@degas.1015granger.net> User-Agent: StGIT/0.14.3 MIME-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org The TEST_STATEID and FREE_STATEID operations can return NFS4ERR_BAD_STATEID, NFS4ERR_OLD_STATEID, or NFS4ERR_DEADSESSION. These error values should not be passed to nfs4_handle_exception(), since that will recursively kick off state recovery, resulting in a deadlock. The reason to continue using nfs4_handle_exception() is to deal with NFS4ERR_DELAY. Moreover, specifically when the TEST_STATEID operation returns NFS4_OK, res.status can contain one of these errors. _nfs41_test_stateid() replaces NFS4_OK with the value in res.status, which is then returned to callers. But res.status is not passed through nfs4_stat_to_errno(), and thus is a positive NFS4ERR value. Currently callers are only interested in !NFS4_OK, and nfs4_handle_exception() ignores positive values, so this oversight hasn't bitten us so far. Bryan agrees the original intent was to return res.status as a negative NFS4ERR value to callers of nfs41_test_stateid(), as long as no state ID recovery is attempted. As a finishing touch, add appropriate documenting comments and some debugging printk's. Signed-off-by: Chuck Lever --- fs/nfs/nfs4proc.c | 51 ++++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 40 insertions(+), 11 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 15fc7e4..971ec8c 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -6587,22 +6587,36 @@ static int _nfs41_test_stateid(struct nfs_server *server, nfs4_stateid *stateid) .rpc_resp = &res, }; + dprintk("NFS call test_stateid %p\n", stateid); nfs41_init_sequence(&args.seq_args, &res.seq_res, 0); status = nfs4_call_sync_sequence(server->client, server, &msg, &args.seq_args, &res.seq_res, 1); - - if (status == NFS_OK) - return res.status; - return status; + if (status != NFS_OK) { + dprintk("NFS reply test_stateid: failed, %d\n", status); + return status; + } + dprintk("NFS reply test_stateid: succeeded, %d\n", -res.status); + return -res.status; } +/** + * nfs41_test_stateid - perform a TEST_STATEID operation + * + * @server: NFS server that may know about "stateid" + * @stateid: state ID to test + * + * Returns NFS_OK if the server recognizes the state ID as valid. + * Otherwise a negative NFS4ERR value is returned if the operation + * failed or the state ID is not currently valid. + */ static int nfs41_test_stateid(struct nfs_server *server, nfs4_stateid *stateid) { struct nfs4_exception exception = { }; int err; do { - err = nfs4_handle_exception(server, - _nfs41_test_stateid(server, stateid), - &exception); + err = _nfs41_test_stateid(server, stateid); + if (err != -NFS4ERR_DELAY) + break; + nfs4_handle_exception(server, err, &exception); } while (exception.retry); return err; } @@ -6618,19 +6632,34 @@ static int _nfs4_free_stateid(struct nfs_server *server, nfs4_stateid *stateid) .rpc_argp = &args, .rpc_resp = &res, }; + int status; + dprintk("NFS call free_stateid %p\n", stateid); nfs41_init_sequence(&args.seq_args, &res.seq_res, 0); - return nfs4_call_sync_sequence(server->client, server, &msg, &args.seq_args, &res.seq_res, 1); + status = nfs4_call_sync_sequence(server->client, server, &msg, + &args.seq_args, &res.seq_res, 1); + dprintk("NFS reply free_stateid: %d\n", status); + return status; } +/** + * nfs41_free_stateid - perform a FREE_STATEID operation + * + * @server: NFS server that may know about "stateid" + * @stateid: state ID to release + * + * Returns NFS_OK if the server freed the state ID. Otherwise a + * negative NFS4ERR value is returned if the operation failed. + */ static int nfs41_free_stateid(struct nfs_server *server, nfs4_stateid *stateid) { struct nfs4_exception exception = { }; int err; do { - err = nfs4_handle_exception(server, - _nfs4_free_stateid(server, stateid), - &exception); + err = _nfs4_free_stateid(server, stateid); + if (err != -NFS4ERR_DELAY) + break; + nfs4_handle_exception(server, err, &exception); } while (exception.retry); return err; }