From: NeilBrown <neilb@suse.com>
To: Jeff Layton <jlayton@redhat.com>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: wrong stateid used after flock lock taken
Date: Fri, 30 Sep 2016 12:16:48 +1000 [thread overview]
Message-ID: <87twcy5dyn.fsf@notabene.neil.brown.name> (raw)
[-- Attachment #1: Type: text/plain, Size: 1456 bytes --]
Hi Jeff et al.
I think your patch
Commit: 8003d3c4aaa5 ("nfs4: treat lock owners as opaque values")
introduced a regression ... or maybe exposed a latent problem.
The particular symptom that I can demonstrate is that if I open a file
with NFSv4, take a flock() exclusive lock, and then write to the file,
then the WRITE request uses the stateid returned by OPEN, not the one
returned by LOCK.
The Linux NFS server doesn't have a problem with that, but some NFS
servers do (one returns NFS4ERR_LOCKED, which seems to imply it imposes
mandatory locking!).
In any case, this is the wrong stateid to use.
The patch changed nfs4_copy_lock_stateid() so it was more restrictive in
the stateids it allowed.
I must admit that I find the code that you removed incredibly confusing.
I defined a union field
- pid_t flock_owner;
and I cannot understand how a pid_t would be relevant for a flock_owner,
as the flock is tied to the 'struct file', not the pid.
Anyway, a write request includes an 'nfs_lock_context' and from that we
need to somehow find the correct stateid.
I'm wondering if nfs4_set_rw_stateid() should call
nfs4_select_rw_stateid() twice, once to look for a flock stated, and
once to look for a posix-lock stateid .... or something like that.
I'll take a fresh look at the code next week and maybe it will be easier
to understand then, but meanwhile if you have any suggestions I'd be
very happy to hear them.
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]
next reply other threads:[~2016-09-30 2:16 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-30 2:16 NeilBrown [this message]
2016-09-30 11:22 ` wrong stateid used after flock lock taken Jeff Layton
2016-09-30 18:30 ` Benjamin Coddington
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=87twcy5dyn.fsf@notabene.neil.brown.name \
--to=neilb@suse.com \
--cc=jlayton@redhat.com \
--cc=linux-nfs@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).