From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754558AbcA0J6i (ORCPT ); Wed, 27 Jan 2016 04:58:38 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:45282 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752287AbcA0J6f (ORCPT ); Wed, 27 Jan 2016 04:58:35 -0500 Date: Wed, 27 Jan 2016 10:58:22 +0100 From: Peter Zijlstra To: Alexei Starovoitov Cc: Alexander Shishkin , Ingo Molnar , linux-kernel@vger.kernel.org, vince@deater.net, eranian@google.com, Arnaldo Carvalho de Melo , Jiri Olsa , Daniel Borkmann , Wang Nan Subject: Re: [PATCH v2] perf: Synchronously cleanup child events Message-ID: <20160127095822.GN6357@twins.programming.kicks-ass.net> References: <87lh7hhmnn.fsf@ashishki-desk.ger.corp.intel.com> <20160122123847.GS6357@twins.programming.kicks-ass.net> <20160122194403.GC11338@ast-mbp.thefacebook.com> <20160125114846.GW6357@twins.programming.kicks-ass.net> <20160125145414.GG6375@twins.programming.kicks-ass.net> <20160125210410.GH6375@twins.programming.kicks-ass.net> <20160126045947.GA40151@ast-mbp.thefacebook.com> <20160126161637.GF6357@twins.programming.kicks-ass.net> <20160126172425.GJ6375@twins.programming.kicks-ass.net> <20160126233158.GA45142@ast-mbp.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160126233158.GA45142@ast-mbp.thefacebook.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 26, 2016 at 03:31:59PM -0800, Alexei Starovoitov wrote: > This patch will conflict with kernel/bpf/arraymap.c and > kernel/trace/bpf_trace.c that are planned for net-next, > but the conflicts in kernel/events/core.c are probably harder > to resolve, so yes please take it into tip/perf. Thanks. > I think your scm_right fixes depend on this patch and together > it's an important bug fix, so probably makes sense to send > them right now without waiting for the next merge window? I'll leave that up to Ingo, but likely yes. > As soon as you get the whole thing into tip, I'll test it > to make sure bpf side is ok and I hope Wang will test it too. > > I'm still a bit concerned about taking file reference for this, > since bpf prorgams that use perf_events won't be able to be > 'detached'. I was not aware BPF could be detached like that. > Meaning there gotta be always a user space process > that will be holding perf_event FDs. By using fget() the BPF array thing will hold the FDs, right? I mean once you do a full fget() userspace can go and kill itself, the struct file will persists. > On networking side we > don't have this limitation. Like we can attach bpf to TC, > iproute2 will exit and reattach some time later. So it > kinda sux, but sounds like you want to get rid of > perf_event->refcnt completely, We cannot actually get rid of it, we need it for some existence stuff. But yes, we need stricter cleanup. > so I don't see any other way. > We can fix it later if it really becomes an issue. One possible avenue would be to allow BPF to create its own (kernel) events and manage its lifetime along with the BPF object. But so far I don't see a problem with the file thing.