Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935961AbcJXMiT (ORCPT ); Mon, 24 Oct 2016 08:38:19 -0400 Received: from merlin.infradead.org ([205.233.59.134]:55780 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932372AbcJXMiQ (ORCPT ); Mon, 24 Oct 2016 08:38:16 -0400 Date: Mon, 24 Oct 2016 14:38:14 +0200 From: Peter Zijlstra To: Oleg Nesterov Cc: "Ni, BaoleX" , "mingo@redhat.com" , "acme@kernel.org" , "linux-kernel@vger.kernel.org" , "alexander.shishkin@linux.intel.com" , "Liu, Chuansheng" Subject: Re: hit a KASan bug related to Perf during stress test Message-ID: <20161024123814.GP3102@twins.programming.kicks-ass.net> References: <318B87A793BE164187D8851D6CE09D64371C8811@shsmsx102.ccr.corp.intel.com> <20161024095341.GF3102@twins.programming.kicks-ass.net> <20161024111526.GA13509@redhat.com> <20161024112402.GI3102@twins.programming.kicks-ass.net> <20161024120231.GA16554@redhat.com> <20161024121030.GA17007@redhat.com> <20161024122210.GM3102@twins.programming.kicks-ass.net> <20161024122942.GC17007@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161024122942.GC17007@redhat.com> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1605 Lines: 45 On Mon, Oct 24, 2016 at 02:29:42PM +0200, Oleg Nesterov wrote: > On 10/24, Peter Zijlstra wrote: > > > > On Mon, Oct 24, 2016 at 02:10:31PM +0200, Oleg Nesterov wrote: > > > --- x/kernel/pid.c > > > +++ x/kernel/pid.c > > > @@ -526,8 +526,11 @@ pid_t __task_pid_nr_ns(struct task_struc > > > if (!ns) > > > ns = task_active_pid_ns(current); > > > if (likely(pid_alive(task))) { > > > - if (type != PIDTYPE_PID) > > > + if (type != PIDTYPE_PID) { > > > + if (type == PIDTYPE_TGID) > > > + type = PIDTYPE_PID; > > > task = task->group_leader; > > > + } > > > > Aah, that makes much more sense ;-) > > > > > nr = pid_nr_ns(rcu_dereference(task->pids[type].pid), ns); > > > } > > > rcu_read_unlock(); > > > > > > Still, I wonder if returning 0 is the right thing. 0 is a 'valid' PID > > for the init/idle task. > > Yes, now I think that -1 would make more sense. Unfortunately we can't > just change __task_pid_nr_ns(), it already has the users which assume > it returns zero... attach_to_pi_state() for example. Indeed. And I have a patch that assumes task_pid_vnr(&init_task) == 0, is that true because of this !alive case or true in general? No worries though, we can revert to your earlier explicit test and return -1 while adding a comment to explain details? I'll go write one up in a bit, but I need to run an errand first. > > And we still have the re-use issue for the TID, because when we get here > > TID is already unhashed too afaict, > > Yes, so perf_event_tid() will report zero. Ah, ok. So whould we change that to match pid and return (explicit) -1 there too?