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:19:14 +0200	[thread overview]
Message-ID: <20231025181913.GE29779@redhat.com> (raw)
In-Reply-To: <ZTlZan240vG8HG/B@tissot.1015granger.net>

On 10/25, Chuck Lever wrote:
>
> On Wed, Oct 25, 2023 at 07:54:36PM +0200, Oleg Nesterov wrote:
> > On 10/25, Chuck Lever wrote:
> > >
> > > > Another question is why we can't simply turn nn->writeverf into seqcount_t.
> > > > I guess we can't because nfsd_reset_write_verifier() needs spin_lock() to
> > > > serialise with itself, right?
> > >
> > > "reset" is supposed to be very rare operation. Using a lock in that
> > > case is probably quite acceptable, as long as reading the verifier
> > > is wait-free and guaranteed to be untorn.
> > >
> > > But a seqcount_t is only 32 bits.
> >
> > Again, I don't understand you.
> >
> > Once again, we can turn writeverf into seqcount_t, see the patch below.
>
> The patch below does not turn "writeverf" into a seqcount_t, it
> turns "writeverf_lock" into a seqcount_t.

Yes, typo. Of course I meant writeverf_lock. A bit strange it was not clear.

> Your original proposal made no sense.

Which one??? I thought that you agree that the current nfsd_copy_write_verifier()
code makes no send, at least that is how I interpreted your previous email. Confused.

> But I see now what you
> would like to change.

OK,

> I'm not familiar enough with these primitives to have a strong
> opinion. What do you think would be the benefit?

See above. And just in case let me repeat. No, I don't think we can/should turn
writeverf_lock (double check I didn't say "writeverf") into seqcount_t.

Oleg.


  reply	other threads:[~2023-10-25 18:21 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
2023-10-25 17:54   ` Oleg Nesterov
2023-10-25 18:07     ` Chuck Lever
2023-10-25 18:19       ` Oleg Nesterov [this message]
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=20231025181913.GE29779@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.