From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751542AbdEBH7R (ORCPT ); Tue, 2 May 2017 03:59:17 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:36731 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751039AbdEBH7O (ORCPT ); Tue, 2 May 2017 03:59:14 -0400 Date: Tue, 2 May 2017 09:59:10 +0200 From: Ingo Molnar To: "Paul E. McKenney" Cc: Linus Torvalds , Linux Kernel Mailing List , Peter Zijlstra , Thomas Gleixner Subject: Re: [GIT PULL] RCU changes for v4.12 Message-ID: <20170502075910.d7dl762muvuc5uoh@gmail.com> References: <20170501095915.rm4zi5z4ta44il5p@gmail.com> <20170502040202.GR3956@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170502040202.GR3956@linux.vnet.ibm.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Paul E. McKenney wrote: > On Mon, May 01, 2017 at 06:19:44PM -0700, Linus Torvalds wrote: > > On Mon, May 1, 2017 at 2:59 AM, Ingo Molnar wrote: > > > Linus, > > > > > > Please pull the latest core-rcu-for-linus git tree from: > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core-rcu-for-linus > > > > I pulled this, and then after looking at it, ended up un-pulling it again. > > > > I refuse to take that nasty header file from hell. > > > > I see absolutely no point in taking a header file of several hundred > > lines of code. > > > > We have traditionally done too much inline code anyway, but we've > > learnt our lesson - and even back when we did too much of it, we > > didn't put random code that nobody uses and by definition cannot be > > performance-critical in big inline functions in header files. > > > > If it was some one-liner helper function, that would be one thing. But > > there are functions that don't even fit on the screen, and that have > > multiple loops and memory barriers in them. > > > > The one function I decided to grep for was used EXACTLY NOWHERE. Yet > > it was apparently SO INCREDIBLY important that it needed to be inlined > > in a huge header file despite being huge and complicated. > > > > So no. This is too ugly to live, and certainly too ugly to be pulled. > > > > The RCU code needs to start showing some good taste. > > > > There are valid reasons to inline even large functions, if they have > > constant arguments that make us expect them to generate a single > > instruction of code in the end. But that was very much not the case > > here. > > > > Not pulling. Try again next merge window when the code has been > > cleaned up and isn't too ugly to live. > > Please accept my apologies! > > I was patterning this code too much after the various *list*.h header > files, and failed to notice that the functions were getting large. I too should have noticed the large inline functions when pulling it. :-/ Header file bloat is a creeping problem that has gotten (much) worse over the last 10 years, so the pushback from Linus against adding more bloat to include/linux/ is fully justified. > I will get rid of the unused rcu_segcblist_extract_all() function and create a > kernel/rcu/segcblist.c for the functions that are either non-trivial or > performance-insensitive. > > Does that cover it, or am I missing something? I'd also suggest moving as much of the RCU internal data types into kernel/rcu/ as possible. It's not clear to me which part of it is supposed to be a public API and which bits are internal. It might make sense to keep it internal for the time being, and only export things once there are users. I.e. a pretty good solution would be to move all of include/linux/rcu_segcblist.h to kernel/rcu/rcu_segcblist.c or so - and do a kernel/rcu/rcu_segcblist.h with the data types and function prototypes. There's also appears to be inline functions wrappery that I think obfuscates the code: for example why is there rcu_cblist_n_cbs()? Users could directly dereference ->len. Once these are eliminated there's very few inline functions remaining that should truly be inline. Thanks, Ingo