| Message ID | 3154a78290017da7bbbcb920456b860dbfe9ba26.1498572504.git.bcodding@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@redhat.com> > --- > 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!
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;
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@redhat.com> --- 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(-)