From patchwork Wed Feb 14 22:17:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olga Kornievskaia X-Patchwork-Id: 10220041 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 82834601C2 for ; Wed, 14 Feb 2018 22:17:32 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6236228459 for ; Wed, 14 Feb 2018 22:17:32 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 537E5289D1; Wed, 14 Feb 2018 22:17:32 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B235128459 for ; Wed, 14 Feb 2018 22:17:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031554AbeBNWRa (ORCPT ); Wed, 14 Feb 2018 17:17:30 -0500 Received: from mail-ua0-f169.google.com ([209.85.217.169]:35609 "EHLO mail-ua0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1031527AbeBNWR3 (ORCPT ); Wed, 14 Feb 2018 17:17:29 -0500 Received: by mail-ua0-f169.google.com with SMTP id n1so14684827uaa.2 for ; Wed, 14 Feb 2018 14:17:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=MoEf2JqwpVeCeU0UrgV7dtT8GFlfO1ebEMl3uG9is5Q=; b=dNvXlLEcA4xT9Y0VBIqGtH81H+8MILRPaQ2NKezlVEHy4ZsEGmGYwXqThDof5QwDd1 /dbSej+n3MaFZxh4QTnPlYj2X65OCgJpnraW520A/Lcx7sf3jTODsfeVyAIGPMNunLwt P2ZzEoGH0F6lmIaw/MLRQOnYB1abPYQT21nIqSIN5hMOKYIEhoMmlUK7gsLvgutbLND/ fVp/cLrjI4BgkD6AyRm/YXl7cmq3tIUz4rQZ588hti5g6sJvJsCXYzhweKV33SyQqwJH cZcp4vnXQ+vuUQZMiugIVMY9kih7gPGAwqDzqOPJ6COoIsNmjtqoBNu/8PwlsyCGVC2e ndmQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=MoEf2JqwpVeCeU0UrgV7dtT8GFlfO1ebEMl3uG9is5Q=; b=c/fgF+0uiuL2gJzHY3IqvUTHBeGn6KYw2vOHJgnLK1P/VTcj8Lg7PMj9Cn+1K3v4pk zoolvjc+AWz11i+3T/ooguco4/Hcu+jM0UA5W17KYiezcWbBQiaKy7749NzjoBJSClgg jZfLGlRcortvqDoOVe/kOqPcfy9B+ajtE6S4Qn9h18hi1kwf5mY0F5WQo1DD1S4vgC+q sm9+Qkyoy6a76NCTdrkM38UFrQxh65TBGLIgSY6BEJY3+ifH5fnXMgDqq2jznmiXSHKi qBz4drQUw86t4HbCnjzlEcP/dSsoBT73oIhZGHSFCq5O9C9yafCl+OAhJgfrss38QS2D fMdg== X-Gm-Message-State: APf1xPCaUUPBWwciiEP7TVXQfyrUM5XQF2Gxu6c3abzoyyDzWERF+547 kilEFoEAXWek+jyCC4TwB0mgLDCY3wphElcDLuo= X-Google-Smtp-Source: AH8x225UjyqaL2UqKpYES6NrSqLuDV8Ji03IdbMCChEYTjw8S/wzOAGU9j+Ymj0KKCDmy5KWQr61iscM6g+u34pXhZM= X-Received: by 10.176.94.174 with SMTP id y46mr699930uag.8.1518646648974; Wed, 14 Feb 2018 14:17:28 -0800 (PST) MIME-Version: 1.0 Received: by 10.103.91.71 with HTTP; Wed, 14 Feb 2018 14:17:28 -0800 (PST) In-Reply-To: <1518627560.7484.4.camel@primarydata.com> References: <1479140396-17779-1-git-send-email-trond.myklebust@primarydata.com> <1479140396-17779-2-git-send-email-trond.myklebust@primarydata.com> <698A2B0C-1E38-4B65-ABA3-60C396393E30@redhat.com> <1518621683.13566.5.camel@redhat.com> <1518622194.4737.1.camel@primarydata.com> <1518627560.7484.4.camel@primarydata.com> From: Olga Kornievskaia Date: Wed, 14 Feb 2018 17:17:28 -0500 X-Google-Sender-Auth: h91GEBSHQzCLTT37GBXc47QYEOA Message-ID: Subject: Re: [PATCH 1/2] NFSv4: Fix CLOSE races with OPEN To: Trond Myklebust Cc: bcodding redhat , "jlayton@redhat.com" , "linux-nfs@vger.kernel.org" Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Feb 14, 2018 at 11:59 AM, Trond Myklebust wrote: > On Wed, 2018-02-14 at 11:06 -0500, Olga Kornievskaia wrote: >> On Wed, Feb 14, 2018 at 10:42 AM, Benjamin Coddington >> wrote: >> > On 14 Feb 2018, at 10:29, Trond Myklebust wrote: >> > > On Wed, 2018-02-14 at 10:21 -0500, Jeff Layton wrote: >> > > > On Wed, 2018-02-14 at 10:05 -0500, Benjamin Coddington wrote: >> > > > > Hi Olga, >> > > > > >> > > > > On 13 Feb 2018, at 15:08, Olga Kornievskaia wrote: >> > > > > >> > > > > > On Mon, Nov 14, 2016 at 11:19 AM, Trond Myklebust >> > > > > > wrote: >> > > > > > ... >> > > > > >> > > > > Ah, good question there.. Even though the stateid is not >> > > > > useful >> > > > > for >> > > > > operations that follow, I think the sequenceid should be >> > > > > incremented because >> > > > > the CLOSE is an operation that modifies the set of locks or >> > > > > state: >> > > > > >> > > > >> > > > It doesn't matter. >> > >> > Yes, good condensed point. It doesn't matter. >> > >> > > > > ... >> > > >> > > Is there a proposal to change the current client behaviour here? >> > > As far >> > > as I can tell, the answer is "no". Am I missing something? >> > >> > Not that I can see. I think we're just comparing linux and netapp >> > server >> > behaviors.. >> > >> > Ben >> >> I just found very surprising that in nfs4_close_done() which calls >> eventually calls nfs_clear_open_stateid_locked() we change the state >> based on the stateid received from the CLOSE. >> nfs_clear_open_stateid_locked() is only called from nfs4_close_done() >> and no other function. >> >> I guess I'm not wondering if we had this problem described in this >> patch of the delayed CLOSE, if we didn't update the state after >> receiving the close... (so yes this is a weak proposal). >> > > nfs4_close_done() doesn't only deal with CLOSE. It also has to handle > OPEN_DOWNGRADE. What about doing an update for OPEN_DOWNGRADE but not for the CLOSE (untested): goto out; > > -- > Trond Myklebust > Linux NFS client maintainer, PrimaryData > trond.myklebust@primarydata.com --- 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 f8a2b226..5868a6a 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1472,7 +1472,7 @@ static void nfs_clear_open_stateid_locked(struct nfs4_state *state, if (stateid == NULL) return; /* Handle OPEN+OPEN_DOWNGRADE races */ - if (nfs4_stateid_match_other(stateid, &state->open_stateid) && + if (fmode && nfs4_stateid_match_other(stateid, &state->open_stateid) && !nfs4_stateid_is_newer(stateid, &state->open_stateid)) { nfs_resync_open_stateid_locked(state);