Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Date:   Wed, 25 Oct 2023 20:10:49 +0200
From:   Oleg Nesterov <oleg@redhat.com>
To:     Chuck Lever <chuck.lever@oracle.com>
Cc:     Jeff Layton <jlayton@kernel.org>, Neil Brown <neilb@suse.de>,
        Olga Kornievskaia <kolga@netapp.com>,
        Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
        Ingo Molnar <mingo@redhat.com>, linux-nfs@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: nfsd_copy_write_verifier: wrong usage of read_seqbegin_or_lock()
Message-ID: <20231025181049.GD29779@redhat.com>
References: <20231025163006.GA8279@redhat.com>
 <ZTlJmuDpGE+U3pEF@tissot.1015granger.net>
 <20231025173931.GA29779@redhat.com>
 <ZTlXD/hQAVQMKfaE@tissot.1015granger.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZTlXD/hQAVQMKfaE@tissot.1015granger.net>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

On 10/25, Chuck Lever wrote:
>
> On Wed, Oct 25, 2023 at 07:39:31PM +0200, Oleg Nesterov wrote:
> > Hi Chuck,
> >
> > Thanks for your reply. But I am already sleeping and I can't understand it.
>
> I was responding to "I can not understand the intent." But also I
> was hoping that explanation would help you provide a correct
> replacement for the existing code.

In case I was not clear, I have already provided the replacement for the
existing code which looks "correct" for me ;) Nevermind, please forget.

> > 1. Do you agree that the current nfsd_copy_write_verifier() code makes no sense?
>
> Probably.
>
>
> >    I mean, the usage of read_seqbegin_or_lock() suggests that if the lockless
> >    pass fails it should take writeverf_lock for writing. But this can't happen,
> >    and thus this code doesn't look right no matter what. None of the
> >    read_seqbegin_or_lock/need_seqretry/done_seqretry helpers make any sense
> >    because "seq" is alway even.
>
> > 2. If yes, which change do you prefer? I'd prefer the patch at the end.
>
> Based on my limited understanding of read_seqbegin(), the patch at
> the end seems cleanest and is on-point. Please post an official
> version of that to linux-nfs@ with a full patch description, and
> I'll see that it gets into v6.8-rc with proper tags, review, and
> testing.

Ok, will do tomorrow.

Thanks,

Oleg.