From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755335Ab1KXJub (ORCPT ); Thu, 24 Nov 2011 04:50:31 -0500 Received: from merlin.infradead.org ([205.233.59.134]:37868 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751321Ab1KXJu3 convert rfc822-to-8bit (ORCPT ); Thu, 24 Nov 2011 04:50:29 -0500 Message-ID: <1322128199.2921.3.camel@twins> Subject: Re: Fwd: uprobes: register/unregister probes. From: Peter Zijlstra To: Srikar Dronamraju Cc: Linus Torvalds , Oleg Nesterov , Andrew Morton , LKML , Linux-mm , Ingo Molnar , Andi Kleen , Christoph Hellwig , Steven Rostedt , Roland McGrath , Thomas Gleixner , Masami Hiramatsu , Arnaldo Carvalho de Melo , Anton Arapov , Ananth N Mavinakayanahalli , Jim Keniston , tulasidhard@gmail.com Date: Thu, 24 Nov 2011 10:49:59 +0100 In-Reply-To: <20111124070303.GB28065@linux.vnet.ibm.com> References: <603b0079-5f54-4299-9a9a-a5e237ccca73@l23g2000pro.googlegroups.com> <20111124070303.GB28065@linux.vnet.ibm.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT X-Mailer: Evolution 3.2.1- Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2011-11-24 at 12:33 +0530, Srikar Dronamraju wrote: > > On Fri, 2011-11-18 at 16:37 +0530, Srikar Dronamraju wrote: > > > +int register_uprobe(struct inode *inode, loff_t offset, > > > + struct uprobe_consumer *consumer) > > > +{ > > > + struct uprobe *uprobe; > > > + int ret = -EINVAL; > > > + > > > + if (!consumer || consumer->next) > > > + return ret; > > > + > > > + inode = igrab(inode); > > > > So why are you dealing with !consumer but not with !inode? and why > > does > > it make sense to allow !consumer at all? > > > > > I am not sure if I got your comment correctly. > > I do check for inode just after the igrab. No you don't, you check the return value of igrab(), but you crash hard when someone calls register_uprobe(.inode=NULL). > I am actually not dealing with !consumer. > If the consumer is NULL, then we dont have any handler to run so why > would we want to register such a probe? Why allow someone calling register_uprobe(.consumer=NULL) to begin with? That doesn't make any sense. > Also if consumer->next is Non-NULL, that means that this consumer was > already used. Reusing the consumer, can result in consumers list getting > broken into two. Yeah, although at that point why be nice about it? Just but a WARN_ON() in or so.