From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1757557AbbIXRhl (ORCPT <rfc822;w@1wt.eu>);
	Thu, 24 Sep 2015 13:37:41 -0400
Received: from mx1.redhat.com ([209.132.183.28]:51009 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1757491AbbIXRhe (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Thu, 24 Sep 2015 13:37:34 -0400
Date: Thu, 24 Sep 2015 19:34:30 +0200
From: Oleg Nesterov <oleg@redhat.com>
To: Konstantin Khlebnikov <koct9i@gmail.com>
Cc: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>,
        Linux API <linux-api@vger.kernel.org>,
        Linux Containers <containers@lists.linux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Serge Hallyn <serge.hallyn@ubuntu.com>,
        "Eric W. Biederman" <ebiederm@xmission.com>,
        Chen Fan <chen.fan.fnst@cn.fujitsu.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        =?iso-8859-1?Q?St=E9phane?= Graber <stgraber@ubuntu.com>
Subject: Re: [PATCH RFC v2] pidns: introduce syscall getvpid
Message-ID: <20150924173430.GA31073@redhat.com>
References: <20150924135332.27813.21640.stgit@buzz> <20150924145647.GA24151@redhat.com> <CALYGNiPCVFNLi=1iL1gOf411iU6tT+DbVx1JuGG+f73Rpvx3dw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CALYGNiPCVFNLi=1iL1gOf411iU6tT+DbVx1JuGG+f73Rpvx3dw@mail.gmail.com>
User-Agent: Mutt/1.5.18 (2008-05-17)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 09/24, Konstantin Khlebnikov wrote:
>
> On Thu, Sep 24, 2015 at 5:56 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> > On 09/24, Konstantin Khlebnikov wrote:
> >>
> >> +SYSCALL_DEFINE3(getvpid, pid_t, pid, int, source, int, target)
> >> +{
> >> +     struct file *source_file = NULL, *target_file = NULL;
> >> +     struct pid_namespace *source_ns, *target_ns;
> >> +     struct pid *struct_pid;
> >> +     struct ns_common *ns;
> >> +     pid_t result;
> >> +
> >> +     if (source >= 0) {
> >> +             source_file = proc_ns_fget(source);
> >> +             result = PTR_ERR(source_file);
> >> +             if (IS_ERR(source_file))
> >> +                     goto out;
> >> +             ns = get_proc_ns(file_inode(source_file));
> >> +             result = -EINVAL;
> >> +             if (ns->ops->type != CLONE_NEWPID)
> >> +                     goto out;
> >> +             source_ns = container_of(ns, struct pid_namespace, ns);
> >> +     } else
> >> +             source_ns = task_active_pid_ns(current);
> >> +
> >> +     if (target >= 0) {
> >> +             target_file = proc_ns_fget(target);
> >> +             result = PTR_ERR(target_file);
> >> +             if (IS_ERR(target_file))
> >> +                     goto out;
> >> +             ns = get_proc_ns(file_inode(target_file));
> >> +             result = -EINVAL;
> >> +             if (ns->ops->type != CLONE_NEWPID)
> >> +                     goto out;
> >> +             target_ns = container_of(ns, struct pid_namespace, ns);
> >> +     } else
> >> +             target_ns = task_active_pid_ns(current);
> >> +
> >
> > Hmm. Eric, Konstantin, how about (uncompiled/untested) patch below
> > in a preparation? The code above doesn't look very readable.
>
> I've tried to do something like that but that comes too far so send patch as is.

OK, I won't insist.

The code above asks for cleanup/factorization, but we can do this
later.

> Actually we can go deeper and replace struct file* with struct fd: this saves
> couple atomic ops for singlethreaded task.

it's not about performance...

But yes, we can do more and create get_pid_ns_by_fd() similar to
get_net_ns_by_fd(). We do not need file at all, we need pid_ns.

Oleg.