From: "Serge E. Hallyn" <serue@us.ibm.com>
To: Vitaliy Gusev <vgusev@openvz.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
"Serge E. Hallyn" <serue@us.ibm.com>,
Pavel Emelyanov <xemul@openvz.org>,
"J. Bruce Fields" <bfields@fieldses.org>,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] Pid namespaces vs locks interaction
Date: Fri, 21 Dec 2007 08:42:30 -0600 [thread overview]
Message-ID: <20071221144230.GB27197@sergelap.austin.ibm.com> (raw)
In-Reply-To: <200712211522.31279.vgusev@openvz.org>
Quoting Vitaliy Gusev (vgusev@openvz.org):
> fcntl(F_GETLK,..) can return pid of process for not current pid namespace (if
> process is belonged to the several namespaces). It is true also for pids
> in /proc/locks. So correct behavior is saving pointer to the struct pid of
> the process lock owner.
>
> Assigned-off-by: Vitaliy Gusev <vgusev@openvz.org>
> Acked-by: Serge Hallyn <serue@us.ibm.com>
Thanks, Vitaliy.
-serge
> fs/locks.c | 26 +++++++++++++++++++++-----
> include/linux/fs.h | 3 ++-
> 2 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/fs/locks.c b/fs/locks.c
> index 8b8388e..14989fa 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -125,6 +125,7 @@
> #include <linux/syscalls.h>
> #include <linux/time.h>
> #include <linux/rcupdate.h>
> +#include <linux/pid_namespace.h>
>
> #include <asm/semaphore.h>
> #include <asm/uaccess.h>
> @@ -185,6 +186,7 @@ void locks_init_lock(struct file_lock *fl)
> fl->fl_fasync = NULL;
> fl->fl_owner = NULL;
> fl->fl_pid = 0;
> + fl->fl_nspid = NULL;
> fl->fl_file = NULL;
> fl->fl_flags = 0;
> fl->fl_type = 0;
> @@ -553,6 +555,8 @@ static void locks_insert_lock(struct file_lock **pos, struct file_lock *fl)
> {
> list_add(&fl->fl_link, &file_lock_list);
>
> + fl->fl_nspid = get_pid(task_tgid(current));
> +
> /* insert into file's list */
> fl->fl_next = *pos;
> *pos = fl;
> @@ -584,6 +588,9 @@ static void locks_delete_lock(struct file_lock **thisfl_p)
> if (fl->fl_ops && fl->fl_ops->fl_remove)
> fl->fl_ops->fl_remove(fl);
>
> + put_pid(fl->fl_nspid);
> + fl->fl_nspid = NULL;
> +
> locks_wake_up_blocks(fl);
> locks_free_lock(fl);
> }
> @@ -673,9 +680,12 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
> if (posix_locks_conflict(fl, cfl))
> break;
> }
> - if (cfl)
> + if (cfl) {
> __locks_copy_lock(fl, cfl);
> - else
> + if (cfl->fl_nspid)
> + fl->fl_pid = pid_nr_ns(cfl->fl_nspid,
> + task_active_pid_ns(current));
> + } else
> fl->fl_type = F_UNLCK;
> unlock_kernel();
> return;
> @@ -2084,6 +2094,12 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
> int id, char *pfx)
> {
> struct inode *inode = NULL;
> + unsigned int fl_pid;
> +
> + if (fl->fl_nspid)
> + fl_pid = pid_nr_ns(fl->fl_nspid, task_active_pid_ns(current));
> + else
> + fl_pid = fl->fl_pid;
>
> if (fl->fl_file != NULL)
> inode = fl->fl_file->f_path.dentry->d_inode;
> @@ -2124,16 +2140,16 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
> }
> if (inode) {
> #ifdef WE_CAN_BREAK_LSLK_NOW
> - seq_printf(f, "%d %s:%ld ", fl->fl_pid,
> + seq_printf(f, "%d %s:%ld ", fl_pid,
> inode->i_sb->s_id, inode->i_ino);
> #else
> /* userspace relies on this representation of dev_t ;-( */
> - seq_printf(f, "%d %02x:%02x:%ld ", fl->fl_pid,
> + seq_printf(f, "%d %02x:%02x:%ld ", fl_pid,
> MAJOR(inode->i_sb->s_dev),
> MINOR(inode->i_sb->s_dev), inode->i_ino);
> #endif
> } else {
> - seq_printf(f, "%d <none>:0 ", fl->fl_pid);
> + seq_printf(f, "%d <none>:0 ", fl_pid);
> }
> if (IS_POSIX(fl)) {
> if (fl->fl_end == OFFSET_MAX)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index b3ec4a4..1fb952f 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -869,7 +869,8 @@ struct file_lock {
> struct list_head fl_link; /* doubly linked list of all locks */
> struct list_head fl_block; /* circular list of blocked processes */
> fl_owner_t fl_owner;
> - unsigned int fl_pid;
> + unsigned int fl_pid; /* unique id and sometimes global pid */
> + struct pid *fl_nspid; /* to calculate owner pid_nr for userspace */
> wait_queue_head_t fl_wait;
> struct file *fl_file;
> unsigned char fl_flags;
>
> diff --git a/fs/locks.c b/fs/locks.c
> index 8b8388e..d2d3d75 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -125,6 +125,7 @@
> #include <linux/syscalls.h>
> #include <linux/time.h>
> #include <linux/rcupdate.h>
> +#include <linux/pid_namespace.h>
>
> #include <asm/semaphore.h>
> #include <asm/uaccess.h>
> @@ -185,6 +186,7 @@ void locks_init_lock(struct file_lock *fl)
> fl->fl_fasync = NULL;
> fl->fl_owner = NULL;
> fl->fl_pid = 0;
> + fl->fl_nspid = NULL;
> fl->fl_file = NULL;
> fl->fl_flags = 0;
> fl->fl_type = 0;
> @@ -553,6 +555,8 @@ static void locks_insert_lock(struct file_lock **pos, struct file_lock *fl)
> {
> list_add(&fl->fl_link, &file_lock_list);
>
> + fl->fl_nspid = get_pid(task_tgid(current));
> +
> /* insert into file's list */
> fl->fl_next = *pos;
> *pos = fl;
> @@ -584,6 +588,11 @@ static void locks_delete_lock(struct file_lock **thisfl_p)
> if (fl->fl_ops && fl->fl_ops->fl_remove)
> fl->fl_ops->fl_remove(fl);
>
> + if (fl->fl_nspid) {
> + put_pid(fl->fl_nspid);
> + fl->fl_nspid = NULL;
> + }
> +
> locks_wake_up_blocks(fl);
> locks_free_lock(fl);
> }
> @@ -673,14 +682,16 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
> if (posix_locks_conflict(fl, cfl))
> break;
> }
> - if (cfl)
> + if (cfl) {
> __locks_copy_lock(fl, cfl);
> - else
> + if (cfl->fl_nspid)
> + fl->fl_pid = pid_nr_ns(cfl->fl_nspid,
> + task_active_pid_ns(current));
> + } else
> fl->fl_type = F_UNLCK;
> unlock_kernel();
> return;
> }
> -
> EXPORT_SYMBOL(posix_test_lock);
>
> /* This function tests for deadlock condition before putting a process to
> @@ -2084,6 +2095,12 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
> int id, char *pfx)
> {
> struct inode *inode = NULL;
> + unsigned int fl_pid;
> +
> + if (fl->fl_nspid)
> + fl_pid = pid_nr_ns(fl->fl_nspid, task_active_pid_ns(current));
> + else
> + fl_pid = fl->fl_pid;
>
> if (fl->fl_file != NULL)
> inode = fl->fl_file->f_path.dentry->d_inode;
> @@ -2124,16 +2141,16 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
> }
> if (inode) {
> #ifdef WE_CAN_BREAK_LSLK_NOW
> - seq_printf(f, "%d %s:%ld ", fl->fl_pid,
> + seq_printf(f, "%d %s:%ld ", fl_pid,
> inode->i_sb->s_id, inode->i_ino);
> #else
> /* userspace relies on this representation of dev_t ;-( */
> - seq_printf(f, "%d %02x:%02x:%ld ", fl->fl_pid,
> + seq_printf(f, "%d %02x:%02x:%ld ", fl_pid,
> MAJOR(inode->i_sb->s_dev),
> MINOR(inode->i_sb->s_dev), inode->i_ino);
> #endif
> } else {
> - seq_printf(f, "%d <none>:0 ", fl->fl_pid);
> + seq_printf(f, "%d <none>:0 ", fl_pid);
> }
> if (IS_POSIX(fl)) {
> if (fl->fl_end == OFFSET_MAX)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index b3ec4a4..5876f68 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -870,6 +870,7 @@ struct file_lock {
> struct list_head fl_block; /* circular list of blocked processes */
> fl_owner_t fl_owner;
> unsigned int fl_pid;
> + struct pid *fl_nspid;
> wait_queue_head_t fl_wait;
> struct file *fl_file;
> unsigned char fl_flags;
next prev parent reply other threads:[~2007-12-21 14:42 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <200712061411.32159.vgusev@openvz.org>
2007-12-06 12:57 ` [RFC][PATCH] Pid namespaces vs locks interaction Vitaliy Gusev
2007-12-06 14:53 ` Serge E. Hallyn
2007-12-06 15:19 ` Vitaliy Gusev
2007-12-06 15:51 ` Serge E. Hallyn
2007-12-08 22:21 ` Brad Boyer
[not found] ` <20071206155130.GA12463-6s5zFf/epYLPQpwDFJZrxKsjOiXwFzmk@public.gmane.org>
2007-12-12 16:07 ` Vitaliy Gusev
2007-12-12 17:31 ` Serge E. Hallyn
[not found] ` <20071212173115.GA21956-6s5zFf/epYLPQpwDFJZrxKsjOiXwFzmk@public.gmane.org>
2007-12-12 17:42 ` Vitaliy Gusev
2007-12-12 18:42 ` Serge E. Hallyn
[not found] ` <20071212184225.GA23504-6s5zFf/epYLPQpwDFJZrxKsjOiXwFzmk@public.gmane.org>
2007-12-13 14:13 ` Vitaliy Gusev
2007-12-13 16:40 ` Serge E. Hallyn
2007-12-06 22:15 ` J. Bruce Fields
2007-12-07 14:51 ` Serge E. Hallyn
2007-12-21 12:22 ` [PATCH] " Vitaliy Gusev
2007-12-21 14:42 ` Serge E. Hallyn [this message]
2007-12-22 0:50 ` Andrew Morton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20071221144230.GB27197@sergelap.austin.ibm.com \
--to=serue@us.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=bfields@fieldses.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=vgusev@openvz.org \
--cc=xemul@openvz.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).