[PATCH 0/3 v6] Fixups for l_pid

[PATCH 0/3 v6] Fixups for l_pid

[Benjamin Coddington]

LTP fcntl tests (fcntl11 fcntl14 fcntl17 fcntl19 fcntl20 fcntl21) have been
failing for NFSv4 mounts due to an unexpected l_pid.  What follows are some
fixups:

v2: - Rebase onto linux-next
    - Revert back to using the stack in locks_mandatory_area(), and fixup
	patch description for 1/3

v3: - The lkp-robot found some serious per_thread_ops performance
	regressions for v1 and v2, so this version changes things around to not
	acquire a reference to struct pid in fl_nspid for every lock.  Instead,
	it drops fl_nspid altogether, and defers the lookup of the
	namespace-translated pid until it actually needed.

v4: - Instead of looking up the virtual pid by way of referencing the struct
	task of the that pid, instead use find_pid_ns() and pid_nr_ns(), which
	avoids a the problem where we race to get a reference to the struct task
	while it may be freed.

v5: - Squash previous 2/3 and 3/3 to avoid regression where F_GETLK would
	return the init_ns pid instead of a translated pid.

v6: - State clearly how the differing cases of l_pid translation should be
	handled, specifically regarding remote locks on remote files: that
	fl_pid ought to be returned from the filesystem as <= 0 to indicate that
	it makes no sense to translate this l_pid.
	- Follow up with a patch to have filesystems negate fl_pid for remote
	locks on remote files.

Benjamin Coddington (3):
  fs/locks: Use allocation rather than the stack in fcntl_getlk()
  fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks
  staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

 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 +-
 fs/fuse/file.c                                  |   6 +-
 fs/locks.c                                      | 108 ++++++++++++++----------
 include/linux/fs.h                              |   2 +-
 8 files changed, 72 insertions(+), 54 deletions(-)

-- 
2.9.3

[PATCH 0/3 v6] Fixups for l_pid

[Benjamin Coddington]

LTP fcntl tests (fcntl11 fcntl14 fcntl17 fcntl19 fcntl20 fcntl21) have been
failing for NFSv4 mounts due to an unexpected l_pid.  What follows are some
fixups:

v2: - Rebase onto linux-next
    - Revert back to using the stack in locks_mandatory_area(), and fixup
	patch description for 1/3

v3: - The lkp-robot found some serious per_thread_ops performance
	regressions for v1 and v2, so this version changes things around to not
	acquire a reference to struct pid in fl_nspid for every lock.  Instead,
	it drops fl_nspid altogether, and defers the lookup of the
	namespace-translated pid until it actually needed.

v4: - Instead of looking up the virtual pid by way of referencing the struct
	task of the that pid, instead use find_pid_ns() and pid_nr_ns(), which
	avoids a the problem where we race to get a reference to the struct task
	while it may be freed.

v5: - Squash previous 2/3 and 3/3 to avoid regression where F_GETLK would
	return the init_ns pid instead of a translated pid.

v6: - State clearly how the differing cases of l_pid translation should be
	handled, specifically regarding remote locks on remote files: that
	fl_pid ought to be returned from the filesystem as <= 0 to indicate that
	it makes no sense to translate this l_pid.
	- Follow up with a patch to have filesystems negate fl_pid for remote
	locks on remote files.

Benjamin Coddington (3):
  fs/locks: Use allocation rather than the stack in fcntl_getlk()
  fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks
  staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

 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 +-
 fs/fuse/file.c                                  |   6 +-
 fs/locks.c                                      | 108 ++++++++++++++----------
 include/linux/fs.h                              |   2 +-
 8 files changed, 72 insertions(+), 54 deletions(-)

-- 
2.9.3

[lustre-devel] [PATCH 0/3 v6] Fixups for l_pid

[Benjamin Coddington]

LTP fcntl tests (fcntl11 fcntl14 fcntl17 fcntl19 fcntl20 fcntl21) have been
failing for NFSv4 mounts due to an unexpected l_pid.  What follows are some
fixups:

v2: - Rebase onto linux-next
    - Revert back to using the stack in locks_mandatory_area(), and fixup
	patch description for 1/3

v3: - The lkp-robot found some serious per_thread_ops performance
	regressions for v1 and v2, so this version changes things around to not
	acquire a reference to struct pid in fl_nspid for every lock.  Instead,
	it drops fl_nspid altogether, and defers the lookup of the
	namespace-translated pid until it actually needed.

v4: - Instead of looking up the virtual pid by way of referencing the struct
	task of the that pid, instead use find_pid_ns() and pid_nr_ns(), which
	avoids a the problem where we race to get a reference to the struct task
	while it may be freed.

v5: - Squash previous 2/3 and 3/3 to avoid regression where F_GETLK would
	return the init_ns pid instead of a translated pid.

v6: - State clearly how the differing cases of l_pid translation should be
	handled, specifically regarding remote locks on remote files: that
	fl_pid ought to be returned from the filesystem as <= 0 to indicate that
	it makes no sense to translate this l_pid.
	- Follow up with a patch to have filesystems negate fl_pid for remote
	locks on remote files.

Benjamin Coddington (3):
  fs/locks: Use allocation rather than the stack in fcntl_getlk()
  fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks
  staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

 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 +-
 fs/fuse/file.c                                  |   6 +-
 fs/locks.c                                      | 108 ++++++++++++++----------
 include/linux/fs.h                              |   2 +-
 8 files changed, 72 insertions(+), 54 deletions(-)

-- 
2.9.3

[Cluster-devel] [PATCH 0/3 v6] Fixups for l_pid

[Benjamin Coddington]

LTP fcntl tests (fcntl11 fcntl14 fcntl17 fcntl19 fcntl20 fcntl21) have been
failing for NFSv4 mounts due to an unexpected l_pid.  What follows are some
fixups:

v2: - Rebase onto linux-next
    - Revert back to using the stack in locks_mandatory_area(), and fixup
	patch description for 1/3

v3: - The lkp-robot found some serious per_thread_ops performance
	regressions for v1 and v2, so this version changes things around to not
	acquire a reference to struct pid in fl_nspid for every lock.  Instead,
	it drops fl_nspid altogether, and defers the lookup of the
	namespace-translated pid until it actually needed.

v4: - Instead of looking up the virtual pid by way of referencing the struct
	task of the that pid, instead use find_pid_ns() and pid_nr_ns(), which
	avoids a the problem where we race to get a reference to the struct task
	while it may be freed.

v5: - Squash previous 2/3 and 3/3 to avoid regression where F_GETLK would
	return the init_ns pid instead of a translated pid.

v6: - State clearly how the differing cases of l_pid translation should be
	handled, specifically regarding remote locks on remote files: that
	fl_pid ought to be returned from the filesystem as <= 0 to indicate that
	it makes no sense to translate this l_pid.
	- Follow up with a patch to have filesystems negate fl_pid for remote
	locks on remote files.

Benjamin Coddington (3):
  fs/locks: Use allocation rather than the stack in fcntl_getlk()
  fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks
  staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

 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 +-
 fs/fuse/file.c                                  |   6 +-
 fs/locks.c                                      | 108 ++++++++++++++----------
 include/linux/fs.h                              |   2 +-
 8 files changed, 72 insertions(+), 54 deletions(-)

-- 
2.9.3

[PATCH 1/3] fs/locks: Use allocation rather than the stack in fcntl_getlk()

[Benjamin Coddington]

Struct file_lock is fairly large, so let's save some space on the stack by
using an allocation for struct file_lock in fcntl_getlk(), just as we do
for fcntl_setlk().

Signed-off-by: Benjamin Coddington <bcodding-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/locks.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index afefeb4ad6de..d7daa6c8932f 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2086,14 +2086,17 @@ static void posix_lock_to_flock64(struct flock64 *flock, struct file_lock *fl)
  */
 int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock_to_posix_lock(filp, &file_lock, flock);
+	error = flock_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2103,23 +2106,22 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 			goto out;
 
 		cmd = F_GETLK;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
  
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK) {
-		error = posix_lock_to_flock(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK) {
+		error = posix_lock_to_flock(flock, fl);
 		if (error)
-			goto rel_priv;
+			goto out;
 	}
-rel_priv:
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
@@ -2298,14 +2300,18 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
  */
 int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
+
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock64_to_posix_lock(filp, &file_lock, flock);
+	error = flock64_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2315,20 +2321,20 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 			goto out;
 
 		cmd = F_GETLK64;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
 
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK)
-		posix_lock_to_flock64(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK)
+		posix_lock_to_flock64(flock, fl);
 
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
-- 
2.9.3

[PATCH 1/3] fs/locks: Use allocation rather than the stack in fcntl_getlk()

[Benjamin Coddington]

Struct file_lock is fairly large, so let's save some space on the stack by
using an allocation for struct file_lock in fcntl_getlk(), just as we do
for fcntl_setlk().

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/locks.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index afefeb4ad6de..d7daa6c8932f 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2086,14 +2086,17 @@ static void posix_lock_to_flock64(struct flock64 *flock, struct file_lock *fl)
  */
 int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock_to_posix_lock(filp, &file_lock, flock);
+	error = flock_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2103,23 +2106,22 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 			goto out;
 
 		cmd = F_GETLK;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
  
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK) {
-		error = posix_lock_to_flock(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK) {
+		error = posix_lock_to_flock(flock, fl);
 		if (error)
-			goto rel_priv;
+			goto out;
 	}
-rel_priv:
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
@@ -2298,14 +2300,18 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
  */
 int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
+
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock64_to_posix_lock(filp, &file_lock, flock);
+	error = flock64_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2315,20 +2321,20 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 			goto out;
 
 		cmd = F_GETLK64;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
 
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK)
-		posix_lock_to_flock64(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK)
+		posix_lock_to_flock64(flock, fl);
 
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
-- 
2.9.3

[lustre-devel] [PATCH 1/3] fs/locks: Use allocation rather than the stack in fcntl_getlk()

[Benjamin Coddington]

Struct file_lock is fairly large, so let's save some space on the stack by
using an allocation for struct file_lock in fcntl_getlk(), just as we do
for fcntl_setlk().

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/locks.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index afefeb4ad6de..d7daa6c8932f 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2086,14 +2086,17 @@ static void posix_lock_to_flock64(struct flock64 *flock, struct file_lock *fl)
  */
 int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock_to_posix_lock(filp, &file_lock, flock);
+	error = flock_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2103,23 +2106,22 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 			goto out;
 
 		cmd = F_GETLK;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
  
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK) {
-		error = posix_lock_to_flock(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK) {
+		error = posix_lock_to_flock(flock, fl);
 		if (error)
-			goto rel_priv;
+			goto out;
 	}
-rel_priv:
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
@@ -2298,14 +2300,18 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
  */
 int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
+
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock64_to_posix_lock(filp, &file_lock, flock);
+	error = flock64_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2315,20 +2321,20 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 			goto out;
 
 		cmd = F_GETLK64;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
 
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK)
-		posix_lock_to_flock64(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK)
+		posix_lock_to_flock64(flock, fl);
 
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
-- 
2.9.3

[Cluster-devel] [PATCH 1/3] fs/locks: Use allocation rather than the stack in fcntl_getlk()

[Benjamin Coddington]

Struct file_lock is fairly large, so let's save some space on the stack by
using an allocation for struct file_lock in fcntl_getlk(), just as we do
for fcntl_setlk().

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/locks.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index afefeb4ad6de..d7daa6c8932f 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2086,14 +2086,17 @@ static void posix_lock_to_flock64(struct flock64 *flock, struct file_lock *fl)
  */
 int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock_to_posix_lock(filp, &file_lock, flock);
+	error = flock_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2103,23 +2106,22 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 			goto out;
 
 		cmd = F_GETLK;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
  
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK) {
-		error = posix_lock_to_flock(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK) {
+		error = posix_lock_to_flock(flock, fl);
 		if (error)
-			goto rel_priv;
+			goto out;
 	}
-rel_priv:
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
@@ -2298,14 +2300,18 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
  */
 int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
+
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock64_to_posix_lock(filp, &file_lock, flock);
+	error = flock64_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2315,20 +2321,20 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 			goto out;
 
 		cmd = F_GETLK64;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
 
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK)
-		posix_lock_to_flock64(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK)
+		posix_lock_to_flock64(flock, fl);
 
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
-- 
2.9.3

[PATCH 2/3] fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks

[Benjamin Coddington]

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.

The translaton and presentation of fl_pid should handle the following four
cases:

1 - F_GETLK on a remote file with a remote lock:
    In this case, the filesystem should determine the l_pid to return here.
    Filesystems should indicate that the fl_pid represents a non-local pid
    value that should not be translated by returning an fl_pid <= 0.

2 - F_GETLK on a local file with a remote lock:
    This should be the l_pid of the lock manager process, and translated.

3 - F_GETLK on a remote file with a local lock, and
4 - F_GETLK on a local file with a local lock:
    These should be the translated l_pid of the local locking process.

Fuse was already doing the correct thing by translating the pid into the
caller's namespace.  With this change we must update fuse to translate to
init's pid namespace, so that the locks API can then translate from init's
pid namespace into the pid namespace of the caller.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/fuse/file.c     |  6 +++---
 fs/locks.c         | 62 ++++++++++++++++++++++++++++++++----------------------
 include/linux/fs.h |  2 +-
 3 files changed, 41 insertions(+), 29 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 3ee4fdc3da9e..7cd692f51d1d 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2101,11 +2101,11 @@ static int convert_fuse_file_lock(struct fuse_conn *fc,
 		fl->fl_end = ffl->end;
 
 		/*
-		 * Convert pid into the caller's pid namespace. If the pid
-		 * does not map into the namespace fl_pid will get set to 0.
+		 * Convert pid into init's pid namespace.  The locks API will
+		 * translate it into the caller's pid namespace.
 		 */
 		rcu_read_lock();
-		fl->fl_pid = pid_vnr(find_pid_ns(ffl->pid, fc->pid_ns));
+		fl->fl_pid = pid_nr_ns(find_pid_ns(ffl->pid, fc->pid_ns), &init_pid_ns);
 		rcu_read_unlock();
 		break;
 
diff --git a/fs/locks.c b/fs/locks.c
index d7daa6c8932f..6d0949880ebd 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -137,6 +137,7 @@
 #define IS_FLOCK(fl)	(fl->fl_flags & FL_FLOCK)
 #define IS_LEASE(fl)	(fl->fl_flags & (FL_LEASE|FL_DELEG|FL_LAYOUT))
 #define IS_OFDLCK(fl)	(fl->fl_flags & FL_OFDLCK)
+#define IS_REMOTELCK(fl)	(fl->fl_pid <= 0)
 
 static inline bool is_remote_lock(struct file *filp)
 {
@@ -733,7 +734,6 @@ static void locks_wake_up_blocks(struct file_lock *blocker)
 static void
 locks_insert_lock_ctx(struct file_lock *fl, struct list_head *before)
 {
-	fl->fl_nspid = get_pid(task_tgid(current));
 	list_add_tail(&fl->fl_list, before);
 	locks_insert_global_locks(fl);
 }
@@ -743,10 +743,6 @@ locks_unlink_lock_ctx(struct file_lock *fl)
 {
 	locks_delete_global_locks(fl);
 	list_del_init(&fl->fl_list);
-	if (fl->fl_nspid) {
-		put_pid(fl->fl_nspid);
-		fl->fl_nspid = NULL;
-	}
 	locks_wake_up_blocks(fl);
 }
 
@@ -823,8 +819,6 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 	list_for_each_entry(cfl, &ctx->flc_posix, fl_list) {
 		if (posix_locks_conflict(fl, cfl)) {
 			locks_copy_conflock(fl, cfl);
-			if (cfl->fl_nspid)
-				fl->fl_pid = pid_vnr(cfl->fl_nspid);
 			goto out;
 		}
 	}
@@ -2048,9 +2042,33 @@ int vfs_test_lock(struct file *filp, struct file_lock *fl)
 }
 EXPORT_SYMBOL_GPL(vfs_test_lock);
 
+/**
+ * locks_translate_pid - translate a file_lock's fl_pid number into a namespace
+ * @fl: The file_lock who's fl_pid should be translated
+ * @ns: The namespace into which the pid should be translated
+ *
+ * Used to tranlate a fl_pid into a namespace virtual pid number
+ */
+static pid_t locks_translate_pid(struct file_lock *fl, struct pid_namespace *ns)
+{
+	pid_t vnr;
+	struct pid *pid;
+
+	if (IS_OFDLCK(fl))
+		return -1;
+	if (IS_REMOTELCK(fl))
+		return fl->fl_pid;
+
+	rcu_read_lock();
+	pid = find_pid_ns(fl->fl_pid, &init_pid_ns);
+	vnr = pid_nr_ns(pid, ns);
+	rcu_read_unlock();
+	return vnr;
+}
+
 static int posix_lock_to_flock(struct flock *flock, struct file_lock *fl)
 {
-	flock->l_pid = IS_OFDLCK(fl) ? -1 : fl->fl_pid;
+	flock->l_pid = locks_translate_pid(fl, task_active_pid_ns(current));
 #if BITS_PER_LONG == 32
 	/*
 	 * Make sure we can represent the posix lock via
@@ -2072,7 +2090,7 @@ static int posix_lock_to_flock(struct flock *flock, struct file_lock *fl)
 #if BITS_PER_LONG == 32
 static void posix_lock_to_flock64(struct flock64 *flock, struct file_lock *fl)
 {
-	flock->l_pid = IS_OFDLCK(fl) ? -1 : fl->fl_pid;
+	flock->l_pid = locks_translate_pid(fl, task_active_pid_ns(current));
 	flock->l_start = fl->fl_start;
 	flock->l_len = fl->fl_end == OFFSET_MAX ? 0 :
 		fl->fl_end - fl->fl_start + 1;
@@ -2584,22 +2602,16 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 {
 	struct inode *inode = NULL;
 	unsigned int fl_pid;
+	struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
 
-	if (fl->fl_nspid) {
-		struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
-
-		/* Don't let fl_pid change based on who is reading the file */
-		fl_pid = pid_nr_ns(fl->fl_nspid, proc_pidns);
-
-		/*
-		 * If there isn't a fl_pid don't display who is waiting on
-		 * the lock if we are called from locks_show, or if we are
-		 * called from __show_fd_info - skip lock entirely
-		 */
-		if (fl_pid == 0)
-			return;
-	} else
-		fl_pid = fl->fl_pid;
+	fl_pid = locks_translate_pid(fl, proc_pidns);
+	/*
+	 * If there isn't a fl_pid don't display who is waiting on
+	 * the lock if we are called from locks_show, or if we are
+	 * called from __show_fd_info - skip lock entirely
+	 */
+	if (fl_pid == 0)
+		return;
 
 	if (fl->fl_file != NULL)
 		inode = locks_inode(fl->fl_file);
@@ -2674,7 +2686,7 @@ static int locks_show(struct seq_file *f, void *v)
 
 	fl = hlist_entry(v, struct file_lock, fl_link);
 
-	if (fl->fl_nspid && !pid_nr_ns(fl->fl_nspid, proc_pidns))
+	if (locks_translate_pid(fl, proc_pidns) == 0)
 		return 0;
 
 	lock_get_status(f, fl, iter->li_pos, "");
diff --git a/include/linux/fs.h b/include/linux/fs.h
index aa4affb38c39..179496a9719d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -908,6 +908,7 @@ static inline struct file *get_file(struct file *f)
 #define FL_UNLOCK_PENDING	512 /* Lease is being broken */
 #define FL_OFDLCK	1024	/* lock is "owned" by struct file */
 #define FL_LAYOUT	2048	/* outstanding pNFS layout */
+#define FL_PID_PRIV	4096	/* F_GETLK should report fl_pid */
 
 #define FL_CLOSE_POSIX (FL_POSIX | FL_CLOSE)
 
@@ -984,7 +985,6 @@ struct file_lock {
 	unsigned char fl_type;
 	unsigned int fl_pid;
 	int fl_link_cpu;		/* what cpu's list is this on? */
-	struct pid *fl_nspid;
 	wait_queue_head_t fl_wait;
 	struct file *fl_file;
 	loff_t fl_start;
-- 
2.9.3

[PATCH 2/3] fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks

[Benjamin Coddington]

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.

The translaton and presentation of fl_pid should handle the following four
cases:

1 - F_GETLK on a remote file with a remote lock:
    In this case, the filesystem should determine the l_pid to return here.
    Filesystems should indicate that the fl_pid represents a non-local pid
    value that should not be translated by returning an fl_pid <= 0.

2 - F_GETLK on a local file with a remote lock:
    This should be the l_pid of the lock manager process, and translated.

3 - F_GETLK on a remote file with a local lock, and
4 - F_GETLK on a local file with a local lock:
    These should be the translated l_pid of the local locking process.

Fuse was already doing the correct thing by translating the pid into the
caller's namespace.  With this change we must update fuse to translate to
init's pid namespace, so that the locks API can then translate from init's
pid namespace into the pid namespace of the caller.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/fuse/file.c     |  6 +++---
 fs/locks.c         | 62 ++++++++++++++++++++++++++++++++----------------------
 include/linux/fs.h |  2 +-
 3 files changed, 41 insertions(+), 29 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 3ee4fdc3da9e..7cd692f51d1d 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2101,11 +2101,11 @@ static int convert_fuse_file_lock(struct fuse_conn *fc,
 		fl->fl_end = ffl->end;
 
 		/*
-		 * Convert pid into the caller's pid namespace. If the pid
-		 * does not map into the namespace fl_pid will get set to 0.
+		 * Convert pid into init's pid namespace.  The locks API will
+		 * translate it into the caller's pid namespace.
 		 */
 		rcu_read_lock();
-		fl->fl_pid = pid_vnr(find_pid_ns(ffl->pid, fc->pid_ns));
+		fl->fl_pid = pid_nr_ns(find_pid_ns(ffl->pid, fc->pid_ns), &init_pid_ns);
 		rcu_read_unlock();
 		break;
 
diff --git a/fs/locks.c b/fs/locks.c
index d7daa6c8932f..6d0949880ebd 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -137,6 +137,7 @@
 #define IS_FLOCK(fl)	(fl->fl_flags & FL_FLOCK)
 #define IS_LEASE(fl)	(fl->fl_flags & (FL_LEASE|FL_DELEG|FL_LAYOUT))
 #define IS_OFDLCK(fl)	(fl->fl_flags & FL_OFDLCK)
+#define IS_REMOTELCK(fl)	(fl->fl_pid <= 0)
 
 static inline bool is_remote_lock(struct file *filp)
 {
@@ -733,7 +734,6 @@ static void locks_wake_up_blocks(struct file_lock *blocker)
 static void
 locks_insert_lock_ctx(struct file_lock *fl, struct list_head *before)
 {
-	fl->fl_nspid = get_pid(task_tgid(current));
 	list_add_tail(&fl->fl_list, before);
 	locks_insert_global_locks(fl);
 }
@@ -743,10 +743,6 @@ locks_unlink_lock_ctx(struct file_lock *fl)
 {
 	locks_delete_global_locks(fl);
 	list_del_init(&fl->fl_list);
-	if (fl->fl_nspid) {
-		put_pid(fl->fl_nspid);
-		fl->fl_nspid = NULL;
-	}
 	locks_wake_up_blocks(fl);
 }
 
@@ -823,8 +819,6 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 	list_for_each_entry(cfl, &ctx->flc_posix, fl_list) {
 		if (posix_locks_conflict(fl, cfl)) {
 			locks_copy_conflock(fl, cfl);
-			if (cfl->fl_nspid)
-				fl->fl_pid = pid_vnr(cfl->fl_nspid);
 			goto out;
 		}
 	}
@@ -2048,9 +2042,33 @@ int vfs_test_lock(struct file *filp, struct file_lock *fl)
 }
 EXPORT_SYMBOL_GPL(vfs_test_lock);
 
+/**
+ * locks_translate_pid - translate a file_lock's fl_pid number into a namespace
+ * @fl: The file_lock who's fl_pid should be translated
+ * @ns: The namespace into which the pid should be translated
+ *
+ * Used to tranlate a fl_pid into a namespace virtual pid number
+ */
+static pid_t locks_translate_pid(struct file_lock *fl, struct pid_namespace *ns)
+{
+	pid_t vnr;
+	struct pid *pid;
+
+	if (IS_OFDLCK(fl))
+		return -1;
+	if (IS_REMOTELCK(fl))
+		return fl->fl_pid;
+
+	rcu_read_lock();
+	pid = find_pid_ns(fl->fl_pid, &init_pid_ns);
+	vnr = pid_nr_ns(pid, ns);
+	rcu_read_unlock();
+	return vnr;
+}
+
 static int posix_lock_to_flock(struct flock *flock, struct file_lock *fl)
 {
-	flock->l_pid = IS_OFDLCK(fl) ? -1 : fl->fl_pid;
+	flock->l_pid = locks_translate_pid(fl, task_active_pid_ns(current));
 #if BITS_PER_LONG == 32
 	/*
 	 * Make sure we can represent the posix lock via
@@ -2072,7 +2090,7 @@ static int posix_lock_to_flock(struct flock *flock, struct file_lock *fl)
 #if BITS_PER_LONG == 32
 static void posix_lock_to_flock64(struct flock64 *flock, struct file_lock *fl)
 {
-	flock->l_pid = IS_OFDLCK(fl) ? -1 : fl->fl_pid;
+	flock->l_pid = locks_translate_pid(fl, task_active_pid_ns(current));
 	flock->l_start = fl->fl_start;
 	flock->l_len = fl->fl_end == OFFSET_MAX ? 0 :
 		fl->fl_end - fl->fl_start + 1;
@@ -2584,22 +2602,16 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 {
 	struct inode *inode = NULL;
 	unsigned int fl_pid;
+	struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
 
-	if (fl->fl_nspid) {
-		struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
-
-		/* Don't let fl_pid change based on who is reading the file */
-		fl_pid = pid_nr_ns(fl->fl_nspid, proc_pidns);
-
-		/*
-		 * If there isn't a fl_pid don't display who is waiting on
-		 * the lock if we are called from locks_show, or if we are
-		 * called from __show_fd_info - skip lock entirely
-		 */
-		if (fl_pid == 0)
-			return;
-	} else
-		fl_pid = fl->fl_pid;
+	fl_pid = locks_translate_pid(fl, proc_pidns);
+	/*
+	 * If there isn't a fl_pid don't display who is waiting on
+	 * the lock if we are called from locks_show, or if we are
+	 * called from __show_fd_info - skip lock entirely
+	 */
+	if (fl_pid == 0)
+		return;
 
 	if (fl->fl_file != NULL)
 		inode = locks_inode(fl->fl_file);
@@ -2674,7 +2686,7 @@ static int locks_show(struct seq_file *f, void *v)
 
 	fl = hlist_entry(v, struct file_lock, fl_link);
 
-	if (fl->fl_nspid && !pid_nr_ns(fl->fl_nspid, proc_pidns))
+	if (locks_translate_pid(fl, proc_pidns) == 0)
 		return 0;
 
 	lock_get_status(f, fl, iter->li_pos, "");
diff --git a/include/linux/fs.h b/include/linux/fs.h
index aa4affb38c39..179496a9719d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -908,6 +908,7 @@ static inline struct file *get_file(struct file *f)
 #define FL_UNLOCK_PENDING	512 /* Lease is being broken */
 #define FL_OFDLCK	1024	/* lock is "owned" by struct file */
 #define FL_LAYOUT	2048	/* outstanding pNFS layout */
+#define FL_PID_PRIV	4096	/* F_GETLK should report fl_pid */
 
 #define FL_CLOSE_POSIX (FL_POSIX | FL_CLOSE)
 
@@ -984,7 +985,6 @@ struct file_lock {
 	unsigned char fl_type;
 	unsigned int fl_pid;
 	int fl_link_cpu;		/* what cpu's list is this on? */
-	struct pid *fl_nspid;
 	wait_queue_head_t fl_wait;
 	struct file *fl_file;
 	loff_t fl_start;
-- 
2.9.3

[lustre-devel] [PATCH 2/3] fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks

[Benjamin Coddington]

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.

The translaton and presentation of fl_pid should handle the following four
cases:

1 - F_GETLK on a remote file with a remote lock:
    In this case, the filesystem should determine the l_pid to return here.
    Filesystems should indicate that the fl_pid represents a non-local pid
    value that should not be translated by returning an fl_pid <= 0.

2 - F_GETLK on a local file with a remote lock:
    This should be the l_pid of the lock manager process, and translated.

3 - F_GETLK on a remote file with a local lock, and
4 - F_GETLK on a local file with a local lock:
    These should be the translated l_pid of the local locking process.

Fuse was already doing the correct thing by translating the pid into the
caller's namespace.  With this change we must update fuse to translate to
init's pid namespace, so that the locks API can then translate from init's
pid namespace into the pid namespace of the caller.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/fuse/file.c     |  6 +++---
 fs/locks.c         | 62 ++++++++++++++++++++++++++++++++----------------------
 include/linux/fs.h |  2 +-
 3 files changed, 41 insertions(+), 29 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 3ee4fdc3da9e..7cd692f51d1d 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2101,11 +2101,11 @@ static int convert_fuse_file_lock(struct fuse_conn *fc,
 		fl->fl_end = ffl->end;
 
 		/*
-		 * Convert pid into the caller's pid namespace. If the pid
-		 * does not map into the namespace fl_pid will get set to 0.
+		 * Convert pid into init's pid namespace.  The locks API will
+		 * translate it into the caller's pid namespace.
 		 */
 		rcu_read_lock();
-		fl->fl_pid = pid_vnr(find_pid_ns(ffl->pid, fc->pid_ns));
+		fl->fl_pid = pid_nr_ns(find_pid_ns(ffl->pid, fc->pid_ns), &init_pid_ns);
 		rcu_read_unlock();
 		break;
 
diff --git a/fs/locks.c b/fs/locks.c
index d7daa6c8932f..6d0949880ebd 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -137,6 +137,7 @@
 #define IS_FLOCK(fl)	(fl->fl_flags & FL_FLOCK)
 #define IS_LEASE(fl)	(fl->fl_flags & (FL_LEASE|FL_DELEG|FL_LAYOUT))
 #define IS_OFDLCK(fl)	(fl->fl_flags & FL_OFDLCK)
+#define IS_REMOTELCK(fl)	(fl->fl_pid <= 0)
 
 static inline bool is_remote_lock(struct file *filp)
 {
@@ -733,7 +734,6 @@ static void locks_wake_up_blocks(struct file_lock *blocker)
 static void
 locks_insert_lock_ctx(struct file_lock *fl, struct list_head *before)
 {
-	fl->fl_nspid = get_pid(task_tgid(current));
 	list_add_tail(&fl->fl_list, before);
 	locks_insert_global_locks(fl);
 }
@@ -743,10 +743,6 @@ locks_unlink_lock_ctx(struct file_lock *fl)
 {
 	locks_delete_global_locks(fl);
 	list_del_init(&fl->fl_list);
-	if (fl->fl_nspid) {
-		put_pid(fl->fl_nspid);
-		fl->fl_nspid = NULL;
-	}
 	locks_wake_up_blocks(fl);
 }
 
@@ -823,8 +819,6 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 	list_for_each_entry(cfl, &ctx->flc_posix, fl_list) {
 		if (posix_locks_conflict(fl, cfl)) {
 			locks_copy_conflock(fl, cfl);
-			if (cfl->fl_nspid)
-				fl->fl_pid = pid_vnr(cfl->fl_nspid);
 			goto out;
 		}
 	}
@@ -2048,9 +2042,33 @@ int vfs_test_lock(struct file *filp, struct file_lock *fl)
 }
 EXPORT_SYMBOL_GPL(vfs_test_lock);
 
+/**
+ * locks_translate_pid - translate a file_lock's fl_pid number into a namespace
+ * @fl: The file_lock who's fl_pid should be translated
+ * @ns: The namespace into which the pid should be translated
+ *
+ * Used to tranlate a fl_pid into a namespace virtual pid number
+ */
+static pid_t locks_translate_pid(struct file_lock *fl, struct pid_namespace *ns)
+{
+	pid_t vnr;
+	struct pid *pid;
+
+	if (IS_OFDLCK(fl))
+		return -1;
+	if (IS_REMOTELCK(fl))
+		return fl->fl_pid;
+
+	rcu_read_lock();
+	pid = find_pid_ns(fl->fl_pid, &init_pid_ns);
+	vnr = pid_nr_ns(pid, ns);
+	rcu_read_unlock();
+	return vnr;
+}
+
 static int posix_lock_to_flock(struct flock *flock, struct file_lock *fl)
 {
-	flock->l_pid = IS_OFDLCK(fl) ? -1 : fl->fl_pid;
+	flock->l_pid = locks_translate_pid(fl, task_active_pid_ns(current));
 #if BITS_PER_LONG == 32
 	/*
 	 * Make sure we can represent the posix lock via
@@ -2072,7 +2090,7 @@ static int posix_lock_to_flock(struct flock *flock, struct file_lock *fl)
 #if BITS_PER_LONG == 32
 static void posix_lock_to_flock64(struct flock64 *flock, struct file_lock *fl)
 {
-	flock->l_pid = IS_OFDLCK(fl) ? -1 : fl->fl_pid;
+	flock->l_pid = locks_translate_pid(fl, task_active_pid_ns(current));
 	flock->l_start = fl->fl_start;
 	flock->l_len = fl->fl_end == OFFSET_MAX ? 0 :
 		fl->fl_end - fl->fl_start + 1;
@@ -2584,22 +2602,16 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 {
 	struct inode *inode = NULL;
 	unsigned int fl_pid;
+	struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
 
-	if (fl->fl_nspid) {
-		struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
-
-		/* Don't let fl_pid change based on who is reading the file */
-		fl_pid = pid_nr_ns(fl->fl_nspid, proc_pidns);
-
-		/*
-		 * If there isn't a fl_pid don't display who is waiting on
-		 * the lock if we are called from locks_show, or if we are
-		 * called from __show_fd_info - skip lock entirely
-		 */
-		if (fl_pid == 0)
-			return;
-	} else
-		fl_pid = fl->fl_pid;
+	fl_pid = locks_translate_pid(fl, proc_pidns);
+	/*
+	 * If there isn't a fl_pid don't display who is waiting on
+	 * the lock if we are called from locks_show, or if we are
+	 * called from __show_fd_info - skip lock entirely
+	 */
+	if (fl_pid == 0)
+		return;
 
 	if (fl->fl_file != NULL)
 		inode = locks_inode(fl->fl_file);
@@ -2674,7 +2686,7 @@ static int locks_show(struct seq_file *f, void *v)
 
 	fl = hlist_entry(v, struct file_lock, fl_link);
 
-	if (fl->fl_nspid && !pid_nr_ns(fl->fl_nspid, proc_pidns))
+	if (locks_translate_pid(fl, proc_pidns) == 0)
 		return 0;
 
 	lock_get_status(f, fl, iter->li_pos, "");
diff --git a/include/linux/fs.h b/include/linux/fs.h
index aa4affb38c39..179496a9719d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -908,6 +908,7 @@ static inline struct file *get_file(struct file *f)
 #define FL_UNLOCK_PENDING	512 /* Lease is being broken */
 #define FL_OFDLCK	1024	/* lock is "owned" by struct file */
 #define FL_LAYOUT	2048	/* outstanding pNFS layout */
+#define FL_PID_PRIV	4096	/* F_GETLK should report fl_pid */
 
 #define FL_CLOSE_POSIX (FL_POSIX | FL_CLOSE)
 
@@ -984,7 +985,6 @@ struct file_lock {
 	unsigned char fl_type;
 	unsigned int fl_pid;
 	int fl_link_cpu;		/* what cpu's list is this on? */
-	struct pid *fl_nspid;
 	wait_queue_head_t fl_wait;
 	struct file *fl_file;
 	loff_t fl_start;
-- 
2.9.3

[Cluster-devel] [PATCH 2/3] fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks

[Benjamin Coddington]

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.

The translaton and presentation of fl_pid should handle the following four
cases:

1 - F_GETLK on a remote file with a remote lock:
    In this case, the filesystem should determine the l_pid to return here.
    Filesystems should indicate that the fl_pid represents a non-local pid
    value that should not be translated by returning an fl_pid <= 0.

2 - F_GETLK on a local file with a remote lock:
    This should be the l_pid of the lock manager process, and translated.

3 - F_GETLK on a remote file with a local lock, and
4 - F_GETLK on a local file with a local lock:
    These should be the translated l_pid of the local locking process.

Fuse was already doing the correct thing by translating the pid into the
caller's namespace.  With this change we must update fuse to translate to
init's pid namespace, so that the locks API can then translate from init's
pid namespace into the pid namespace of the caller.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/fuse/file.c     |  6 +++---
 fs/locks.c         | 62 ++++++++++++++++++++++++++++++++----------------------
 include/linux/fs.h |  2 +-
 3 files changed, 41 insertions(+), 29 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 3ee4fdc3da9e..7cd692f51d1d 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2101,11 +2101,11 @@ static int convert_fuse_file_lock(struct fuse_conn *fc,
 		fl->fl_end = ffl->end;
 
 		/*
-		 * Convert pid into the caller's pid namespace. If the pid
-		 * does not map into the namespace fl_pid will get set to 0.
+		 * Convert pid into init's pid namespace.  The locks API will
+		 * translate it into the caller's pid namespace.
 		 */
 		rcu_read_lock();
-		fl->fl_pid = pid_vnr(find_pid_ns(ffl->pid, fc->pid_ns));
+		fl->fl_pid = pid_nr_ns(find_pid_ns(ffl->pid, fc->pid_ns), &init_pid_ns);
 		rcu_read_unlock();
 		break;
 
diff --git a/fs/locks.c b/fs/locks.c
index d7daa6c8932f..6d0949880ebd 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -137,6 +137,7 @@
 #define IS_FLOCK(fl)	(fl->fl_flags & FL_FLOCK)
 #define IS_LEASE(fl)	(fl->fl_flags & (FL_LEASE|FL_DELEG|FL_LAYOUT))
 #define IS_OFDLCK(fl)	(fl->fl_flags & FL_OFDLCK)
+#define IS_REMOTELCK(fl)	(fl->fl_pid <= 0)
 
 static inline bool is_remote_lock(struct file *filp)
 {
@@ -733,7 +734,6 @@ static void locks_wake_up_blocks(struct file_lock *blocker)
 static void
 locks_insert_lock_ctx(struct file_lock *fl, struct list_head *before)
 {
-	fl->fl_nspid = get_pid(task_tgid(current));
 	list_add_tail(&fl->fl_list, before);
 	locks_insert_global_locks(fl);
 }
@@ -743,10 +743,6 @@ locks_unlink_lock_ctx(struct file_lock *fl)
 {
 	locks_delete_global_locks(fl);
 	list_del_init(&fl->fl_list);
-	if (fl->fl_nspid) {
-		put_pid(fl->fl_nspid);
-		fl->fl_nspid = NULL;
-	}
 	locks_wake_up_blocks(fl);
 }
 
@@ -823,8 +819,6 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 	list_for_each_entry(cfl, &ctx->flc_posix, fl_list) {
 		if (posix_locks_conflict(fl, cfl)) {
 			locks_copy_conflock(fl, cfl);
-			if (cfl->fl_nspid)
-				fl->fl_pid = pid_vnr(cfl->fl_nspid);
 			goto out;
 		}
 	}
@@ -2048,9 +2042,33 @@ int vfs_test_lock(struct file *filp, struct file_lock *fl)
 }
 EXPORT_SYMBOL_GPL(vfs_test_lock);
 
+/**
+ * locks_translate_pid - translate a file_lock's fl_pid number into a namespace
+ * @fl: The file_lock who's fl_pid should be translated
+ * @ns: The namespace into which the pid should be translated
+ *
+ * Used to tranlate a fl_pid into a namespace virtual pid number
+ */
+static pid_t locks_translate_pid(struct file_lock *fl, struct pid_namespace *ns)
+{
+	pid_t vnr;
+	struct pid *pid;
+
+	if (IS_OFDLCK(fl))
+		return -1;
+	if (IS_REMOTELCK(fl))
+		return fl->fl_pid;
+
+	rcu_read_lock();
+	pid = find_pid_ns(fl->fl_pid, &init_pid_ns);
+	vnr = pid_nr_ns(pid, ns);
+	rcu_read_unlock();
+	return vnr;
+}
+
 static int posix_lock_to_flock(struct flock *flock, struct file_lock *fl)
 {
-	flock->l_pid = IS_OFDLCK(fl) ? -1 : fl->fl_pid;
+	flock->l_pid = locks_translate_pid(fl, task_active_pid_ns(current));
 #if BITS_PER_LONG == 32
 	/*
 	 * Make sure we can represent the posix lock via
@@ -2072,7 +2090,7 @@ static int posix_lock_to_flock(struct flock *flock, struct file_lock *fl)
 #if BITS_PER_LONG == 32
 static void posix_lock_to_flock64(struct flock64 *flock, struct file_lock *fl)
 {
-	flock->l_pid = IS_OFDLCK(fl) ? -1 : fl->fl_pid;
+	flock->l_pid = locks_translate_pid(fl, task_active_pid_ns(current));
 	flock->l_start = fl->fl_start;
 	flock->l_len = fl->fl_end == OFFSET_MAX ? 0 :
 		fl->fl_end - fl->fl_start + 1;
@@ -2584,22 +2602,16 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 {
 	struct inode *inode = NULL;
 	unsigned int fl_pid;
+	struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
 
-	if (fl->fl_nspid) {
-		struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info;
-
-		/* Don't let fl_pid change based on who is reading the file */
-		fl_pid = pid_nr_ns(fl->fl_nspid, proc_pidns);
-
-		/*
-		 * If there isn't a fl_pid don't display who is waiting on
-		 * the lock if we are called from locks_show, or if we are
-		 * called from __show_fd_info - skip lock entirely
-		 */
-		if (fl_pid == 0)
-			return;
-	} else
-		fl_pid = fl->fl_pid;
+	fl_pid = locks_translate_pid(fl, proc_pidns);
+	/*
+	 * If there isn't a fl_pid don't display who is waiting on
+	 * the lock if we are called from locks_show, or if we are
+	 * called from __show_fd_info - skip lock entirely
+	 */
+	if (fl_pid == 0)
+		return;
 
 	if (fl->fl_file != NULL)
 		inode = locks_inode(fl->fl_file);
@@ -2674,7 +2686,7 @@ static int locks_show(struct seq_file *f, void *v)
 
 	fl = hlist_entry(v, struct file_lock, fl_link);
 
-	if (fl->fl_nspid && !pid_nr_ns(fl->fl_nspid, proc_pidns))
+	if (locks_translate_pid(fl, proc_pidns) == 0)
 		return 0;
 
 	lock_get_status(f, fl, iter->li_pos, "");
diff --git a/include/linux/fs.h b/include/linux/fs.h
index aa4affb38c39..179496a9719d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -908,6 +908,7 @@ static inline struct file *get_file(struct file *f)
 #define FL_UNLOCK_PENDING	512 /* Lease is being broken */
 #define FL_OFDLCK	1024	/* lock is "owned" by struct file */
 #define FL_LAYOUT	2048	/* outstanding pNFS layout */
+#define FL_PID_PRIV	4096	/* F_GETLK should report fl_pid */
 
 #define FL_CLOSE_POSIX (FL_POSIX | FL_CLOSE)
 
@@ -984,7 +985,6 @@ struct file_lock {
 	unsigned char fl_type;
 	unsigned int fl_pid;
 	int fl_link_cpu;		/* what cpu's list is this on? */
-	struct pid *fl_nspid;
 	wait_queue_head_t fl_wait;
 	struct file *fl_file;
 	loff_t fl_start;
-- 
2.9.3

[PATCH 3/3] staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

[Benjamin Coddington]

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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 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;
-- 
2.9.3

[PATCH 3/3] staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

[Benjamin Coddington]

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;
-- 
2.9.3

[lustre-devel] [PATCH 3/3] staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

[Benjamin Coddington]

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;
-- 
2.9.3

[Cluster-devel] [PATCH 3/3] staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

[Benjamin Coddington]

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;
-- 
2.9.3

Re: [PATCH 3/3] staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

[Jeff Layton]

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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  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-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>

Re: [PATCH 3/3] staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

[Jeff Layton]

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!
-- 
Jeff Layton <jlayton@poochiereds.net>

Re: [PATCH 3/3] staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

[Jeff Layton]

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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  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-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>

[lustre-devel] [PATCH 3/3] staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

[Jeff Layton]

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!
-- 
Jeff Layton <jlayton@poochiereds.net>

[Cluster-devel] [PATCH 3/3] staging/lustre, 9p, ceph, cifs, dlm: negate remote pids for F_GETLK

[Jeff Layton]

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!
-- 
Jeff Layton <jlayton@poochiereds.net>

[PATCH 1/3] fs/locks: Use allocation rather than the stack in fcntl_getlk()

[Benjamin Coddington]

Struct file_lock is fairly large, so let's save some space on the stack by
using an allocation for struct file_lock in fcntl_getlk(), just as we do
for fcntl_setlk().

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/locks.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index afefeb4ad6de..d7daa6c8932f 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2086,14 +2086,17 @@ static void posix_lock_to_flock64(struct flock64 *flock, struct file_lock *fl)
  */
 int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock_to_posix_lock(filp, &file_lock, flock);
+	error = flock_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2103,23 +2106,22 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 			goto out;
 
 		cmd = F_GETLK;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
  
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK) {
-		error = posix_lock_to_flock(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK) {
+		error = posix_lock_to_flock(flock, fl);
 		if (error)
-			goto rel_priv;
+			goto out;
 	}
-rel_priv:
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
@@ -2298,14 +2300,18 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
  */
 int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
+
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock64_to_posix_lock(filp, &file_lock, flock);
+	error = flock64_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2315,20 +2321,20 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 			goto out;
 
 		cmd = F_GETLK64;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
 
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK)
-		posix_lock_to_flock64(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK)
+		posix_lock_to_flock64(flock, fl);
 
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
-- 
2.9.3

[PATCH 1/3] fs/locks: Use allocation rather than the stack in fcntl_getlk()

[Benjamin Coddington]

Struct file_lock is fairly large, so let's save some space on the stack by
using an allocation for struct file_lock in fcntl_getlk(), just as we do
for fcntl_setlk().

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/locks.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index afefeb4ad6de..d7daa6c8932f 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2086,14 +2086,17 @@ static void posix_lock_to_flock64(struct flock64 *flock, struct file_lock *fl)
  */
 int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock_to_posix_lock(filp, &file_lock, flock);
+	error = flock_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2103,23 +2106,22 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 			goto out;
 
 		cmd = F_GETLK;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
  
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK) {
-		error = posix_lock_to_flock(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK) {
+		error = posix_lock_to_flock(flock, fl);
 		if (error)
-			goto rel_priv;
+			goto out;
 	}
-rel_priv:
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
@@ -2298,14 +2300,18 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
  */
 int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 {
-	struct file_lock file_lock;
+	struct file_lock *fl;
 	int error;
 
+	fl = locks_alloc_lock();
+	if (fl == NULL)
+		return -ENOMEM;
+
 	error = -EINVAL;
 	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
 		goto out;
 
-	error = flock64_to_posix_lock(filp, &file_lock, flock);
+	error = flock64_to_posix_lock(filp, fl, flock);
 	if (error)
 		goto out;
 
@@ -2315,20 +2321,20 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 			goto out;
 
 		cmd = F_GETLK64;
-		file_lock.fl_flags |= FL_OFDLCK;
-		file_lock.fl_owner = filp;
+		fl->fl_flags |= FL_OFDLCK;
+		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, &file_lock);
+	error = vfs_test_lock(filp, fl);
 	if (error)
 		goto out;
 
-	flock->l_type = file_lock.fl_type;
-	if (file_lock.fl_type != F_UNLCK)
-		posix_lock_to_flock64(flock, &file_lock);
+	flock->l_type = fl->fl_type;
+	if (fl->fl_type != F_UNLCK)
+		posix_lock_to_flock64(flock, fl);
 
-	locks_release_private(&file_lock);
 out:
+	locks_free_lock(fl);
 	return error;
 }
 
-- 
2.9.3