From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754727AbbLKWlY (ORCPT ); Fri, 11 Dec 2015 17:41:24 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:50380 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750724AbbLKWlW (ORCPT ); Fri, 11 Dec 2015 17:41:22 -0500 Date: Fri, 11 Dec 2015 23:41:17 +0100 From: Peter Zijlstra To: Mathieu Poirier Cc: Alexander Shishkin , Ingo Molnar , "linux-kernel@vger.kernel.org" , vince@deater.net, Stephane Eranian , Arnaldo Carvalho de Melo Subject: Re: [PATCH v0 3/5] perf: Introduce instruction trace filtering Message-ID: <20151211224117.GF6356@twins.programming.kicks-ass.net> References: <1449840998-29902-1-git-send-email-alexander.shishkin@linux.intel.com> <1449840998-29902-4-git-send-email-alexander.shishkin@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 Fri, Dec 11, 2015 at 11:11:48AM -0700, Mathieu Poirier wrote: > On 11 December 2015 at 06:36, Alexander Shishkin > > /** > > + * Instruction trace (ITRACE) filter > > + */ > > +struct perf_itrace_filter { > > + struct list_head entry; > > + struct rcu_head rcu_head; > > + struct inode *inode; > > + struct task_struct *task; > > + unsigned long offset; > > + unsigned long size; > > + unsigned long start; > > + unsigned long end; > > + unsigned int range : 1, /* 1: range, 0: addr */ > > + filter : 1, /* 1: filter/start, 0: stop */ > > + kernel : 1; /* 1: kernel, 0: object file*/ > > I've seen a rant on the list before about bit fields like these... Do > we gain anything with having them? I personally avoid them to favour > bool types but that will be for Peter to decide. Given its > importance, I also think this structure could you more documentation. Bool doesn't have a well defined storage type (although the x86_64 ABI defines one, the language itself does not), so I tend to shy away from using it in structures since its very hard to tell what the layout will be. Agreed on the comments though.