On Tue, 2017-06-27 at 11:18 -0400, Benjamin Coddington wrote:
> In the previous patch, the locks API will expect that if a filesystem
> returns a remote pid as opposed to a local pid for F_GETLK, that remote pid
> will be <= 0. This signifies that the pid is remote, and the locks API
> will forego translating that pid into the pid namespace of the local
> calling process. Since local pids will never be larger than PID_MAX_LIMIT
> (which is currently defined as <= 4 million), but pid_t is an unsigned int,
> we should have plenty of room to represent remote pids with negative
> numbers if we assume that remote pid numbers are similarly limited. If
> this is not the case, then we run the risk of having a remote pid returned
> for which there is also a corresponding local pid. This is a problem we
> have now, but this patch should reduce the chances of that occurring, while
> also returning those remote pid numbers, for whatever that may be worth.
>
> This patch updates lustre, 9p, ceph, cifs, and dlm to negate the remote pid
> returned for F_GETLK lock requests.
>
> Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
> ---
> drivers/staging/lustre/lustre/ldlm/ldlm_flock.c | 2 +-
> fs/9p/vfs_file.c | 2 +-
> fs/ceph/locks.c | 2 +-
> fs/cifs/cifssmb.c | 2 +-
> fs/dlm/plock.c | 2 +-
> 5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c
> index b7f28b39c7b3..abcbf075acc0 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c
> @@ -596,7 +596,7 @@ ldlm_flock_completion_ast(struct ldlm_lock *lock, __u64 flags, void *data)
> default:
> getlk->fl_type = F_UNLCK;
> }
> - getlk->fl_pid = (pid_t)lock->l_policy_data.l_flock.pid;
> + getlk->fl_pid = -(pid_t)lock->l_policy_data.l_flock.pid;
> getlk->fl_start = (loff_t)lock->l_policy_data.l_flock.start;
> getlk->fl_end = (loff_t)lock->l_policy_data.l_flock.end;
> } else {
> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
> index 3de3b4a89d89..43c242e17132 100644
> --- a/fs/9p/vfs_file.c
> +++ b/fs/9p/vfs_file.c
> @@ -288,7 +288,7 @@ static int v9fs_file_getlock(struct file *filp, struct file_lock *fl)
> fl->fl_end = OFFSET_MAX;
> else
> fl->fl_end = glock.start + glock.length - 1;
> - fl->fl_pid = glock.proc_id;
> + fl->fl_pid = -glock.proc_id;
> }
> kfree(glock.client_id);
> return res;
> diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
> index 6806dbeaee19..0fd5c288ce4e 100644
> --- a/fs/ceph/locks.c
> +++ b/fs/ceph/locks.c
> @@ -79,7 +79,7 @@ static int ceph_lock_message(u8 lock_type, u16 operation, struct file *file,
> err = ceph_mdsc_do_request(mdsc, inode, req);
>
> if (operation == CEPH_MDS_OP_GETFILELOCK) {
> - fl->fl_pid = le64_to_cpu(req->r_reply_info.filelock_reply->pid);
> + fl->fl_pid = -le64_to_cpu(req->r_reply_info.filelock_reply->pid);
> if (CEPH_LOCK_SHARED == req->r_reply_info.filelock_reply->type)
> fl->fl_type = F_RDLCK;
> else if (CEPH_LOCK_EXCL == req->r_reply_info.filelock_reply->type)
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index fbb0d4cbda41..cb367050f972 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -2515,7 +2515,7 @@ CIFSSMBPosixLock(const unsigned int xid, struct cifs_tcon *tcon,
> pLockData->fl_start = le64_to_cpu(parm_data->start);
> pLockData->fl_end = pLockData->fl_start +
> le64_to_cpu(parm_data->length) - 1;
> - pLockData->fl_pid = le32_to_cpu(parm_data->pid);
> + pLockData->fl_pid = -le32_to_cpu(parm_data->pid);
> }
> }
>
> diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
> index d401425f602a..e631b1689228 100644
> --- a/fs/dlm/plock.c
> +++ b/fs/dlm/plock.c
> @@ -367,7 +367,7 @@ int dlm_posix_get(dlm_lockspace_t *lockspace, u64 number, struct file *file,
> locks_init_lock(fl);
> fl->fl_type = (op->info.ex) ? F_WRLCK : F_RDLCK;
> fl->fl_flags = FL_POSIX;
> - fl->fl_pid = op->info.pid;
> + fl->fl_pid = -op->info.pid;
> fl->fl_start = op->info.start;
> fl->fl_end = op->info.end;
> rv = 0;
I think this is probably a reasonable thing to do, given that we also
report OFD locks today with an l_pid of -1. The pid on any sort of
distributed fs is pretty meaningless anyway.
I think this all looks good. I'll plan to merge it for -next in a bit
and do some testing with it.
Thanks!
--
Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
