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

Reply via email to