All of lore.kernel.org
 help / color / mirror / Atom feed
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()
Date: Wed, 25 Oct 2023 20:10:49 +0200	[thread overview]
Message-ID: <20231025181049.GD29779@redhat.com> (raw)
In-Reply-To: <ZTlXD/hQAVQMKfaE@tissot.1015granger.net>

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.


  reply	other threads:[~2023-10-25 18:12 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-25 16:30 nfsd_copy_write_verifier: wrong usage of read_seqbegin_or_lock() Oleg Nesterov
2023-10-25 17:00 ` Chuck Lever
2023-10-25 17:39   ` Oleg Nesterov
2023-10-25 17:47     ` Oleg Nesterov
2023-10-25 17:57     ` Chuck Lever
2023-10-25 18:10       ` Oleg Nesterov [this message]
2023-10-25 17:54   ` Oleg Nesterov
2023-10-25 18:07     ` Chuck Lever
2023-10-25 18:19       ` Oleg Nesterov
2023-10-26 14:50 ` [PATCH] nfsd_copy_write_verifier: use read_seqbegin() rather than read_seqbegin_or_lock() Oleg Nesterov
     [not found]   ` <ZTvc0Z6DJEYXI/TL@tissot.1015granger.net>
2023-10-27 19:34     ` Oleg Nesterov
2023-10-27 19:40       ` Chuck Lever III
2023-10-27 20:28   ` Jeff Layton
2023-10-27 22:52   ` NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231025181049.GD29779@redhat.com \
    --to=oleg@redhat.com \
    --cc=Dai.Ngo@oracle.com \
    --cc=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=kolga@netapp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=neilb@suse.de \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.