Re: [PATCH 2/3] fs/locks: Remove fl_nspid
From: Benjamin Coddington
Date: Tue Jun 06 2017 - 16:41:36 EST
On 6 Jun 2017, at 14:57, Benjamin Coddington wrote:
On 6 Jun 2017, at 14:25, Jeff Layton wrote:
On Tue, 2017-06-06 at 14:00 -0400, Jeff Layton wrote:
On Tue, 2017-06-06 at 13:19 -0400, Benjamin Coddington wrote:
Since commit c69899a17ca4 "NFSv4: Update of VFS byte range lock
must be
atomic with the stateid update", NFSv4 has been inserting locks in
rpciod
worker context. The result is that the file_lock's fl_nspid is the
kworker's pid instead of the original userspace pid.
The fl_nspid is only used to represent the namespaced virtual pid
number
when displaying locks or returning from F_GETLK. There's no reason
to set
it for every inserted lock, since we can usually just look it up
from
fl_pid. So, instead of looking up and holding struct pid for every
lock,
let's just look up the virtual pid number from fl_pid when it is
needed.
That means we can remove fl_nspid entirely.
With this set, I think we ought to codify that the stored pid must
be
relative
...to the init_pid_ns. Let's make that clear in the comments for
filesystem authors.
OK, but I think you mean fl_pid should always be current->tgid or the
pid as
it is in init_pid_ns. We translate that pid into the virtual pid of
the
process doing F_GETLK or reading /proc/locks.
And in that sense, we shouldn't have to add a comment, because the
expected
behavior is the same as it is today -- namely that fl_pid is
current->tgid,
or the real pid number, which is the pid number in init_pid_ns.
So, unless you feel strongly that this should be explained in a comment,
I
think I'll resend this without additional comments after making the
change
to use find_pid_ns().
Ben