From: Jeff Layton <jlayton@poochiereds.net>
To: Benjamin Coddington <bcodding@redhat.com>
Cc: bfields@fieldses.org, Alexander Viro <viro@zeniv.linux.org.uk>,
linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks
Date: Tue, 20 Jun 2017 15:32:31 -0400 [thread overview]
Message-ID: <1497987151.5054.1.camel@poochiereds.net> (raw)
In-Reply-To: <9AA05B58-8D17-4A03-88F8-0F44F346AE0F@redhat.com>
On Tue, 2017-06-20 at 15:17 -0400, Benjamin Coddington wrote:
> On 20 Jun 2017, at 13:06, Jeff Layton wrote:
> >
> > Now that I think about it a bit more, I don't think we really need a
> > flag here.
> >
> > Just have the ->lock operation set the fl_pid to a negative value. That
> > will never be a valid pid anyway. Then flock_translate_pid could just
> > return any negative value directly instead of trying to translate it.
> >
> > In practice we would always just set it to -1. Maybe even add something
> > like this that the lock-> operation could set it to?
> >
> > #define FILE_LOCK_OWNER_UNDEFINED -1
>
> So for filesystems that set a remote pid, they should negate the pid to mean
> that the pid should not be translated? Then when we return that pid, we
> flip it back again, or display a negative number, or turn it into -1?
>
> The flag, having a readable name, would make things a bit clearer as to what
> the filesystems expect to happen to that pid value.
>
I now think that we really only ought to be filling out the pid when it
refers to a process on the local host. It seems sketchy to me to return
a pid here that is really the pid on another host, but happens to have
the same pid as something else on this host. It's misleading at best,
and if anyone tries to act on that info it could be dangerous. So I'm
thinking that we should just set it to -1 when the lock is held by
another host entirely.
But, since pid values must be positive, we can code the basic
infrastructure to return any negative value as-is instead of trying to
translate it.
--
Jeff Layton <jlayton@poochiereds.net>
next prev parent reply other threads:[~2017-06-20 19:32 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-19 13:24 [PATCH 0/2 v5] Fixups for l_pid Benjamin Coddington
2017-06-19 13:24 ` [PATCH 1/2] fs/locks: Use allocation rather than the stack in fcntl_getlk() Benjamin Coddington
2017-06-19 13:24 ` [PATCH 2/2] fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks Benjamin Coddington
2017-06-19 17:32 ` Jeff Layton
2017-06-20 14:03 ` Benjamin Coddington
2017-06-20 16:09 ` Benjamin Coddington
2017-06-20 17:06 ` Jeff Layton
2017-06-20 19:17 ` Benjamin Coddington
2017-06-20 19:32 ` Jeff Layton [this message]
2017-06-20 19:39 ` Benjamin Coddington
2017-06-20 20:13 ` Jeff Layton
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=1497987151.5054.1.camel@poochiereds.net \
--to=jlayton@poochiereds.net \
--cc=bcodding@redhat.com \
--cc=bfields@fieldses.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/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).