task->tgid conversion in fs/locks.c

PATCH sukadev-r/[email protected] 2 participants 2 messages Sep 8, 2007 – Sep 10, 2007 1 message not in the archive

Subsystems (2)

VFS File locking (flock() and fcntl()/lockf())

lore

Share & tools

lore

sukadev-r/[email protected]

author

2007-09-08 01:00 UTC #0 lore raw

Pavel,

I noticed that fcntl tests in LTP fail when LTP is run a child pid namespace. I just hacked up this quick patch and seems to fix the failures.

Are you already working on this or do you want me to test and send this out for review.

I have one concern that I could use some review/confirmation :-)

Even if the main thread of a process exits before the child threads, the main thread will not be reaped until all threads exit. So, the 'task->group_leader' remains valid for the child threads until the last non-leader thread exits.

IOW the call to task_tgid_vnr() is safe in locks_remove_posix() and locks_remove_flock().

---
 fs/locks.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

fs/locks.c +7 -7

Index: 2.6.23-rc4-mm1/fs/locks.c
===================================================================
@@ -278,7 +278,7 @@ static int flock_make_lock(struct file *
 		return -ENOMEM;
 
 	fl->fl_file = filp;
-	fl->fl_pid = current->tgid;
+	fl->fl_pid = task_tgid_vnr(current);
 	fl->fl_flags = FL_FLOCK;
 	fl->fl_type = type;
 	fl->fl_end = OFFSET_MAX;

@@ -344,7 +344,7 @@ static int flock_to_posix_lock(struct fi
 		return -EOVERFLOW;
 	
 	fl->fl_owner = current->files;
-	fl->fl_pid = current->tgid;
+	fl->fl_pid = task_tgid_vnr(current);
 	fl->fl_file = filp;
 	fl->fl_flags = FL_POSIX;
 	fl->fl_ops = NULL;

@@ -390,7 +390,7 @@ static int flock64_to_posix_lock(struct 
 		return -EOVERFLOW;
 	
 	fl->fl_owner = current->files;
-	fl->fl_pid = current->tgid;
+	fl->fl_pid = task_tgid_vnr(current);
 	fl->fl_file = filp;
 	fl->fl_flags = FL_POSIX;
 	fl->fl_ops = NULL;

@@ -446,7 +446,7 @@ static int lease_init(struct file *filp,
 		return -EINVAL;
 
 	fl->fl_owner = current->files;
-	fl->fl_pid = current->tgid;
+	fl->fl_pid = task_tgid_vnr(current);
 
 	fl->fl_file = filp;
 	fl->fl_flags = FL_LEASE;

@@ -1091,7 +1091,7 @@ int locks_mandatory_area(int read_write,
 
 	locks_init_lock(&fl);
 	fl.fl_owner = current->files;
-	fl.fl_pid = current->tgid;
+	fl.fl_pid = task_tgid_vnr(current);
 	fl.fl_file = filp;
 	fl.fl_flags = FL_POSIX | FL_ACCESS;
 	if (filp && !(filp->f_flags & O_NONBLOCK))

@@ -1963,7 +1963,7 @@ void locks_remove_posix(struct file *fil
 	lock.fl_start = 0;
 	lock.fl_end = OFFSET_MAX;
 	lock.fl_owner = owner;
-	lock.fl_pid = current->tgid;
+	lock.fl_pid = task_tgid_vnr(current);
 	lock.fl_file = filp;
 	lock.fl_ops = NULL;
 	lock.fl_lmops = NULL;

@@ -1990,7 +1990,7 @@ void locks_remove_flock(struct file *fil
 
 	if (filp->f_op && filp->f_op->flock) {
 		struct file_lock fl = {
-			.fl_pid = current->tgid,
+			.fl_pid = task_tgid_vnr(current),
 			.fl_file = filp,
 			.fl_flags = FL_FLOCK,
 			.fl_type = F_UNLCK,

A message that is not in the archive (sent off-list or not yet received): 20070908010016.GA19565-r/[email protected]

Pavel Emelyanov

2007-09-10 07:24 UTC #2 lore raw

Quoted 11 lines · sukadev-r/[email protected] wrote:

Pavel,
I noticed that fcntl tests in LTP fail when LTP is run a child pid namespace. I just hacked up this quick patch and seems to fix the failures.
Are you already working on this or do you want me to test and send this out for review.
I have one concern that I could use some review/confirmation :-)

Quoted 7 lines

Even if the main thread of a process exits before the child threads, the main thread will not be reaped until all threads exit. So, the 'task->group_leader' remains valid for the child threads until the last non-leader thread exits.
IOW the call to task_tgid_vnr() is safe in locks_remove_posix() and locks_remove_flock().

Well, I missed this place deliberately. This should be converted into struct pid * usage. I will do the appropriate patch soon.

Quoted 72 lines

---
 fs/locks.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Index: 2.6.23-rc4-mm1/fs/locks.c
===================================================================

fs/locks.c +7 -7

@@ -278,7 +278,7 @@ static int flock_make_lock(struct file *
 		return -ENOMEM;
 
 	fl->fl_file = filp;
-	fl->fl_pid = current->tgid;
+	fl->fl_pid = task_tgid_vnr(current);
 	fl->fl_flags = FL_FLOCK;
 	fl->fl_type = type;
 	fl->fl_end = OFFSET_MAX;

@@ -344,7 +344,7 @@ static int flock_to_posix_lock(struct fi
 		return -EOVERFLOW;
 
 	fl->fl_owner = current->files;
-	fl->fl_pid = current->tgid;
+	fl->fl_pid = task_tgid_vnr(current);
 	fl->fl_file = filp;
 	fl->fl_flags = FL_POSIX;
 	fl->fl_ops = NULL;

@@ -390,7 +390,7 @@ static int flock64_to_posix_lock(struct 
 		return -EOVERFLOW;
 
 	fl->fl_owner = current->files;
-	fl->fl_pid = current->tgid;
+	fl->fl_pid = task_tgid_vnr(current);
 	fl->fl_file = filp;
 	fl->fl_flags = FL_POSIX;
 	fl->fl_ops = NULL;

@@ -446,7 +446,7 @@ static int lease_init(struct file *filp,
 		return -EINVAL;
 
 	fl->fl_owner = current->files;
-	fl->fl_pid = current->tgid;
+	fl->fl_pid = task_tgid_vnr(current);
 
 	fl->fl_file = filp;
 	fl->fl_flags = FL_LEASE;

@@ -1091,7 +1091,7 @@ int locks_mandatory_area(int read_write,
 
 	locks_init_lock(&fl);
 	fl.fl_owner = current->files;
-	fl.fl_pid = current->tgid;
+	fl.fl_pid = task_tgid_vnr(current);
 	fl.fl_file = filp;
 	fl.fl_flags = FL_POSIX | FL_ACCESS;
 	if (filp && !(filp->f_flags & O_NONBLOCK))

@@ -1963,7 +1963,7 @@ void locks_remove_posix(struct file *fil
 	lock.fl_start = 0;
 	lock.fl_end = OFFSET_MAX;
 	lock.fl_owner = owner;
-	lock.fl_pid = current->tgid;
+	lock.fl_pid = task_tgid_vnr(current);
 	lock.fl_file = filp;
 	lock.fl_ops = NULL;
 	lock.fl_lmops = NULL;

@@ -1990,7 +1990,7 @@ void locks_remove_flock(struct file *fil
 
 	if (filp->f_op && filp->f_op->flock) {
 		struct file_lock fl = {
-			.fl_pid = current->tgid,
+			.fl_pid = task_tgid_vnr(current),
 			.fl_file = filp,
 			.fl_flags = FL_FLOCK,
 			.fl_type = F_UNLCK,