From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751178AbWDVUt4 (ORCPT ); Sat, 22 Apr 2006 16:49:56 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751168AbWDVUt4 (ORCPT ); Sat, 22 Apr 2006 16:49:56 -0400 Received: from zeus1.kernel.org ([204.152.191.4]:18900 "EHLO zeus1.kernel.org") by vger.kernel.org with ESMTP id S1751184AbWDVUt4 (ORCPT ); Sat, 22 Apr 2006 16:49:56 -0400 Organization: Dipartimento di Informatica From: Claudio Scordino To: "Ulrich Drepper" Subject: Re: [PATCH] Extending getrusage Date: Sat, 22 Apr 2006 12:25:30 +0200 User-Agent: KMail/1.8 Cc: "Andrew Morton" , linux-kernel@vger.kernel.org, luto@myrealbox.com, alan@lxorguk.ukuu.org.uk, torvalds@osdl.org, kernel-janitors@lists.osdl.org, bert.hubert@netherlabs.nl References: In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200604221225.31851.cloud.of.andor@gmail.com> X-MailScanner-Information: Please contact the ISP for more information X-MailScanner: Found to be clean X-MailScanner-SpamCheck: non spam, SpamAssassin (punteggio=-1.44, necessario 5, autolearn=disabled, ALL_TRUSTED -1.44) X-MailScanner-From: cloud.of.andor@gmail.com Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On Friday 21 April 2006 16:32, Ulrich Drepper wrote: > On 4/21/06, Claudio Scordino wrote: > > Recently, while writing some code at user level, I needed a fast way > > to have such information about another process. > > That's not very specific. And one program isn't really a compelling > reason. You should specify with some level of detail why you need > that information and why you cannot depend on collaboration of the > process you try to get the information for. I'm just adding a new feature "for free": - Without adding any new syscall - Without changing the prototype of the current syscall (sys_getrusage) - Ensuring backward compatibility (all old programs will continue to work properly) This new feature can be used by any process that needs to profile another process for whatever reason. Since it just adds a new feature without any drawback, I don't understand why it would be wrong... > > > - return -EINVAL; > > - return getrusage(current, who, ru); > > + struct rusage r; > > + struct task_struct *tsk = current; > > + read_lock(&tasklist_lock); > > You are introducing scalability problems where there were none before. > Even if there is some justification revealed in the end IMO the patch > shouldn't be accepted in this form. You should not slow down the > normal case of operation. If the current thread is observed the lock > isn't needed. I think you're wrong: I'm not introducing any *new* scalability issue. Currently the code is: asmlinkage long sys_getrusage(int who, struct rusage __user *ru) { if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN) return -EINVAL; return getrusage(current, who, ru); } which in turn calls int getrusage(struct task_struct *p, int who, struct rusage __user *ru) { struct rusage r; read_lock(&tasklist_lock); k_getrusage(p, who, &r); read_unlock(&tasklist_lock); return copy_to_user(ru, &r, sizeof(r)) ? -EFAULT : 0; } As you can see, the lock is acquired also when not needed. That's how it is currently implemented. With my patch, k_getrusage is called directly from sys_getrusage without going through getrusage. Claudio