From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757674AbZHGMKv (ORCPT ); Fri, 7 Aug 2009 08:10:51 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1757361AbZHGMKv (ORCPT ); Fri, 7 Aug 2009 08:10:51 -0400 Received: from mail-fx0-f228.google.com ([209.85.220.228]:61539 "EHLO mail-fx0-f228.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753737AbZHGMKt convert rfc822-to-8bit (ORCPT ); Fri, 7 Aug 2009 08:10:49 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=googlemail.com; s=gamma; h=mime-version:reply-to:in-reply-to:references:date:message-id :subject:from:to:cc:content-type:content-transfer-encoding; b=hCoWwpgwpozuoJSNxjL/1X9mAWvfZtC8e4sOT9JdNUN3iRnolPh3TueUCcOEffoml/ soL3ahEEJ+8Z9cLMwynBqyX4NEobOjZ+4PklTL1RUv+xULTtRmSN1uaoaa3EhGFa7jw0 fWQRk/7qzd9VRB/34Ak8plINQAa3yMNB/P7F0= MIME-Version: 1.0 Reply-To: eranian@gmail.com In-Reply-To: <20090806190557.GA5244@redhat.com> References: <20090803171619.GA17876@redhat.com> <20090803180602.GA19719@redhat.com> <1249324619.4842.1.camel@laptop> <20090803190231.GA22313@redhat.com> <1249385966.7924.204.camel@twins> <20090804162033.GB5211@redhat.com> <1249404763.4762.26.camel@laptop> <20090804171905.GA9080@redhat.com> <1249564477.32113.383.camel@twins> <20090806190557.GA5244@redhat.com> Date: Fri, 7 Aug 2009 14:10:49 +0200 Message-ID: <7c86c4470908070510recfc3f5u2a4a07ee843978ac@mail.gmail.com> Subject: Re: [PATCH 3/2 -v4] fcntl: F_[SG]ETOWN_EX From: stephane eranian To: Oleg Nesterov Cc: Peter Zijlstra , Andrew Morton , mingo@elte.hu, linux-kernel@vger.kernel.org, tglx@linutronix.de, robert.richter@amd.com, paulus@samba.org, andi@firstfloor.org, mpjohn@us.ibm.com, cel@us.ibm.com, cjashfor@us.ibm.com, mucci@eecs.utk.edu, terpstra@eecs.utk.edu, perfmon2-devel@lists.sourceforge.net, mtk.manpages@googlemail.com, roland@redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org HI, On Thu, Aug 6, 2009 at 9:05 PM, Oleg Nesterov wrote: > On 08/06, Peter Zijlstra wrote: >> >> Subject: fcntl: F_[SG]ETOWN_EX >> From: Peter Zijlstra >> Date: Fri, 31 Jul 2009 10:35:30 +0200 >> >> In order to direct the SIGIO signal to a particular thread of a >> multi-threaded application we cannot, like suggested by the manpage, put >> a TID into the regular fcntl(F_SETOWN) call. It will still be send to >> the whole process of which that thread is part. >> >> Since people do want to properly direct SIGIO we introduce F_SETOWN_EX. >> >> The need to direct SIGIO comes from self-monitoring profiling such as >> with perf-counters. Perf-counters uses SIGIO to notify that new sample >> data is available. If the signal is delivered to the same task that >> generated the new sample it can augment that data by inspecting the >> task's user-space state right after it returns from the kernel. This >> is esp. convenient for interpreted or virtual machine driven >> environments. >> >> Both F_SETOWN_EX and F_GETOWN_EX take a pointer to a struct f_owner_ex >> as argument: >> >> struct f_owner_ex { >>       int   type; >>       pid_t pid; >> }; >> >> Where type is one of F_OWNER_TID, F_OWNER_PID or F_OWNER_GID. > > I think the patch is right. > > Reviewed-by: Oleg Nesterov > I have tested the patch in 2.6.30 (backport) + perfmon and it seems to work in my test case. Have not tried with perfcounters + 2.6.31. I am glad there is finally a solution to this problem. Thanks. >> Signed-off-by: Peter Zijlstra >> --- >>  arch/alpha/include/asm/fcntl.h  |    2 >>  arch/parisc/include/asm/fcntl.h |    2 >>  fs/fcntl.c                      |  108 +++++++++++++++++++++++++++++++++++++--- >>  include/asm-generic/fcntl.h     |   13 ++++ >>  4 files changed, 117 insertions(+), 8 deletions(-) >> >> Index: linux-2.6/arch/parisc/include/asm/fcntl.h >> =================================================================== >> --- linux-2.6.orig/arch/parisc/include/asm/fcntl.h >> +++ linux-2.6/arch/parisc/include/asm/fcntl.h >> @@ -28,6 +28,8 @@ >>  #define F_SETOWN     12      /*  for sockets. */ >>  #define F_SETSIG     13      /*  for sockets. */ >>  #define F_GETSIG     14      /*  for sockets. */ >> +#define F_GETOWN_EX  15 >> +#define F_SETOWN_EX  16 >> >>  /* for posix fcntl() and lockf() */ >>  #define F_RDLCK              01 >> Index: linux-2.6/fs/fcntl.c >> =================================================================== >> --- linux-2.6.orig/fs/fcntl.c >> +++ linux-2.6/fs/fcntl.c >> @@ -263,6 +263,79 @@ pid_t f_getown(struct file *filp) >>       return pid; >>  } >> >> +static int f_setown_ex(struct file *filp, unsigned long arg) >> +{ >> +     struct f_owner_ex * __user owner_p = (void * __user)arg; >> +     struct f_owner_ex owner; >> +     struct pid *pid; >> +     int type; >> +     int ret; >> + >> +     ret = copy_from_user(&owner, owner_p, sizeof(owner)); >> +     if (ret) >> +             return ret; >> + >> +     switch (owner.type) { >> +     case F_OWNER_TID: >> +             type = PIDTYPE_MAX; >> +             break; >> + >> +     case F_OWNER_PID: >> +             type = PIDTYPE_PID; >> +             break; >> + >> +     case F_OWNER_GID: >> +             type = PIDTYPE_PGID; >> +             break; >> + >> +     default: >> +             return -EINVAL; >> +     } >> + >> +     rcu_read_lock(); >> +     pid = find_vpid(owner.pid); >> +     if (owner.pid && !pid) >> +             ret = -ESRCH; >> +     else >> +             ret = __f_setown(filp, pid, type, 1); >> +     rcu_read_unlock(); >> + >> +     return ret; >> +} >> + >> +static int f_getown_ex(struct file *filp, unsigned long arg) >> +{ >> +     struct f_owner_ex * __user owner_p = (void * __user)arg; >> +     struct f_owner_ex owner; >> +     int ret = 0; >> + >> +     read_lock(&filp->f_owner.lock); >> +     owner.pid = pid_vnr(filp->f_owner.pid); >> +     switch (filp->f_owner.pid_type) { >> +     case PIDTYPE_MAX: >> +             owner.type = F_OWNER_TID; >> +             break; >> + >> +     case PIDTYPE_PID: >> +             owner.type = F_OWNER_PID; >> +             break; >> + >> +     case PIDTYPE_PGID: >> +             owner.type = F_OWNER_GID; >> +             break; >> + >> +     default: >> +             WARN_ON(1); >> +             ret = -EINVAL; >> +             break; >> +     } >> +     read_unlock(&filp->f_owner.lock); >> + >> +     if (!ret) >> +             ret = copy_to_user(owner_p, &owner, sizeof(owner)); >> +     return ret; >> +} >> + >>  static long do_fcntl(int fd, unsigned int cmd, unsigned long arg, >>               struct file *filp) >>  { >> @@ -313,6 +386,12 @@ static long do_fcntl(int fd, unsigned in >>       case F_SETOWN: >>               err = f_setown(filp, arg, 1); >>               break; >> +     case F_GETOWN_EX: >> +             err = f_getown_ex(filp, arg); >> +             break; >> +     case F_SETOWN_EX: >> +             err = f_setown_ex(filp, arg); >> +             break; >>       case F_GETSIG: >>               err = filp->f_owner.signum; >>               break; >> @@ -428,8 +507,7 @@ static inline int sigio_perm(struct task >> >>  static void send_sigio_to_task(struct task_struct *p, >>                              struct fown_struct *fown, >> -                            int fd, >> -                            int reason) >> +                            int fd, int reason, int group) >>  { >>       /* >>        * F_SETSIG can change ->signum lockless in parallel, make >> @@ -461,11 +539,11 @@ static void send_sigio_to_task(struct ta >>                       else >>                               si.si_band = band_table[reason - POLL_IN]; >>                       si.si_fd    = fd; >> -                     if (!do_send_sig_info(signum, &si, p, true)) >> +                     if (!do_send_sig_info(signum, &si, p, group)) >>                               break; >>               /* fall-through: fall back on the old plain SIGIO signal */ >>               case 0: >> -                     do_send_sig_info(SIGIO, SEND_SIG_PRIV, p, true); >> +                     do_send_sig_info(SIGIO, SEND_SIG_PRIV, p, group); >>       } >>  } >> >> @@ -474,16 +552,23 @@ void send_sigio(struct fown_struct *fown >>       struct task_struct *p; >>       enum pid_type type; >>       struct pid *pid; >> +     int group = 1; >> >>       read_lock(&fown->lock); >> + >>       type = fown->pid_type; >> +     if (type == PIDTYPE_MAX) { >> +             group = 0; >> +             type = PIDTYPE_PID; >> +     } >> + >>       pid = fown->pid; >>       if (!pid) >>               goto out_unlock_fown; >> >>       read_lock(&tasklist_lock); >>       do_each_pid_task(pid, type, p) { >> -             send_sigio_to_task(p, fown, fd, band); >> +             send_sigio_to_task(p, fown, fd, band, group); >>       } while_each_pid_task(pid, type, p); >>       read_unlock(&tasklist_lock); >>   out_unlock_fown: >> @@ -491,10 +576,10 @@ void send_sigio(struct fown_struct *fown >>  } >> >>  static void send_sigurg_to_task(struct task_struct *p, >> -                                struct fown_struct *fown) >> +                             struct fown_struct *fown, int group) >>  { >>       if (sigio_perm(p, fown, SIGURG)) >> -             group_send_sig_info(SIGURG, SEND_SIG_PRIV, p); >> +             do_send_sig_info(SIGURG, SEND_SIG_PRIV, p, group); >>  } >> >>  int send_sigurg(struct fown_struct *fown) >> @@ -502,10 +587,17 @@ int send_sigurg(struct fown_struct *fown >>       struct task_struct *p; >>       enum pid_type type; >>       struct pid *pid; >> +     int group = 1; >>       int ret = 0; >> >>       read_lock(&fown->lock); >> + >>       type = fown->pid_type; >> +     if (type == PIDTYPE_MAX) { >> +             group = 0; >> +             type = PIDTYPE_PID; >> +     } >> + >>       pid = fown->pid; >>       if (!pid) >>               goto out_unlock_fown; >> @@ -514,7 +606,7 @@ int send_sigurg(struct fown_struct *fown >> >>       read_lock(&tasklist_lock); >>       do_each_pid_task(pid, type, p) { >> -             send_sigurg_to_task(p, fown); >> +             send_sigurg_to_task(p, fown, group); >>       } while_each_pid_task(pid, type, p); >>       read_unlock(&tasklist_lock); >>   out_unlock_fown: >> Index: linux-2.6/include/asm-generic/fcntl.h >> =================================================================== >> --- linux-2.6.orig/include/asm-generic/fcntl.h >> +++ linux-2.6/include/asm-generic/fcntl.h >> @@ -73,6 +73,19 @@ >>  #define F_SETSIG     10      /* for sockets. */ >>  #define F_GETSIG     11      /* for sockets. */ >>  #endif >> +#ifndef F_SETOWN_EX >> +#define F_SETOWN_EX  12 >> +#define F_GETOWN_EX  13 >> +#endif >> + >> +#define F_OWNER_TID  0 >> +#define F_OWNER_PID  1 >> +#define F_OWNER_GID  2 >> + >> +struct f_owner_ex { >> +     int     type; >> +     pid_t   pid; >> +}; >> >>  /* for F_[GET|SET]FL */ >>  #define FD_CLOEXEC   1       /* actually anything with low bit set goes */ >> Index: linux-2.6/arch/alpha/include/asm/fcntl.h >> =================================================================== >> --- linux-2.6.orig/arch/alpha/include/asm/fcntl.h >> +++ linux-2.6/arch/alpha/include/asm/fcntl.h >> @@ -26,6 +26,8 @@ >>  #define F_GETOWN     6       /*  for sockets. */ >>  #define F_SETSIG     10      /*  for sockets. */ >>  #define F_GETSIG     11      /*  for sockets. */ >> +#define F_SETOWN_EX  12 >> +#define F_GETOWN_EX  13 >> >>  /* for posix fcntl() and lockf() */ >>  #define F_RDLCK              1 >> >> > >