linux-kernel - Re: [git-pull -tip] x86: include inverse Xmas tree patches

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090329102548.GA3014@x200.localdomain>
Date:	Sun, 29 Mar 2009 14:25:48 +0400
From:	Alexey Dobriyan <adobriyan@...il.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...nel.org>, Sam Ravnborg <sam@...nborg.org>,
	Jaswinder Singh Rajput <jaswinder@...nel.org>,
	x86 maintainers <x86@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [git-pull -tip] x86: include inverse Xmas tree patches

On Sun, Mar 29, 2009 at 01:00:26AM +0100, Ingo Molnar wrote:
> 
> * Thomas Gleixner <tglx@...utronix.de> wrote:
> 
> > On Sun, 29 Mar 2009, Ingo Molnar wrote:
> > > * Thomas Gleixner <tglx@...utronix.de> wrote:
> > > 
> > > > > Personally I'd prefer alphabetic order, sorting based on length 
> > > > > isn't a complete ordering.  Nearly all editors can sort 
> > > > > alphabetically at the push of a key.
> > > > 
> > > > I'd prefer if somebody would sit down and write a tool to analyse 
> > > > the include hell instead of manually shuffling crap around to 
> > > > avoid trivial merge conflicts. I have cleaned up enough stuff in 
> > > > the x86 merger myself where I was able to cut the number of 
> > > > includes at least in half just by staring at the gcc intermediate 
> > > > files. We could do better and automate the analysis so we get down 
> > > > to a handful of includes instead of including the world and more.
> > > 
> > > I do not disagree with include file cleanups (we've done many of 
> > > them in this cycle and in previous cycles), but note that the 
> > > reduction in include files at the top of .c files actually increases 
> > > the chance of patch conflicts: when a new include file is added by 
> > > two patches to the same .c file.
> > 
> > Those conflicts are trivial and if we have a mechanism to anlyse 
> > include dependencies then we can avoid such conflicts often at 
> > all.
> 
> Yeah, i agree that least 50% of the existing #include's are not 
> needed.
> 
> That's not the point though. When two new files both append to the
> #include file section, they'll clash. Those _new_ includes tend to 
> be justified - people dont add #includes needlessly - it's years 
> long bitrot that causes #includes to bloat up slowly.
> 
> > Go through some of the include madness and watch the compiler 
> > reading the same header file ten times for a single source file 
> > compile.

Care to read strace(1) output? Include file is not even opened second
time.

[reads strace(1) output himself]

Correction!

Except, e. g., <asm/atomic.h> because i386/x86_64 unification dropped
ifndef guards in the right place and added them in the wrong one.

> Yes, we need both efforts. That section needs to look professional 
> (unordered lines look amateurishly random, arbitrary and confusing), 
> and it needs to be minimalized as well.

These are #include directives, how else should they look like.
As for confusing, as if you're looking for the first time at C code.

> And the xmas-tree helps the minimizing effort - as it trivially 
> eliminates duplicate #includes (which do happen frequently as well), 

yes, 5-10-20 for the whole tree per release and people periodically
removing them regardless of -tip, Xmas and so on.

> and they eliminate related includes as well because they get moved 
> together and get noticed.

I'll answer on this later.

> Most importantly, such code needs to look like as if someone 
> actually cared about it.

Reminder, this is include directives, not real code.

> If code looks tidy, people assume that it's 
> supposed to be perfect, and will notice and fix more subtle 
> imperfections in it. If code looks totally messy on the other hand, 
> _nobody_ will care because it's visibly broken. So these changes are 
> a small step forward.

People probably forgot the real problem with includes and start
inventing new non-problems with all the consequences.



So here is my countersuggestion for small and useful improvements:

1. Plug ifndef guargs back on x86.
   Check with strace(1) for past behaviour re non-opened includes.
   Remove current ifdef guards (need much more thinking for consequences)

2. Do #1 for other arches which exercised similar transformation
   (sparc/sparc64?)

3. For headers with small and let's say bounded area like <asm/unistd.h>,
   go check for every file including them and remove it if header genuinely
   unneeded.

  In case of <asm/unistd.h> this would check for copy_from_user/copy_to_user,
  put_user/get_user, clear_user and not much more.

This will reduce compile time _and_ size of DEBUG_INFO=y kernels.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/