Skip to contentSkip to conversation

proc: use #pragma once

PATCH In review Alexey Dobriyan 6 participants 12 messages Apr 23, 2018 – May 4, 2018 Proc filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 proc: use #pragma once

Bring /proc into 21st century.

---

 fs/proc/fd.h            |    6 +-----
 fs/proc/internal.h      |    1 +
 include/linux/proc_fs.h |    6 +-----
 3 files changed, 3 insertions(+), 10 deletions(-)
fs/proc/fd.h +1 -5
@@ -1,7 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __PROCFS_FD_H__
-#define __PROCFS_FD_H__
-
+#pragma once
 #include <linux/fs.h>
 
 extern const struct file_operations proc_fd_operations;
@@ -16,5 +14,3 @@ static inline unsigned int proc_fd(struct inode *inode)
 {
 	return PROC_I(inode)->fd;
 }
-
-#endif /* __PROCFS_FD_H__ */
fs/proc/internal.h +1 -0
@@ -1,3 +1,4 @@
+#pragma once
 /* Internal procfs definitions
  *
  * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
include/linux/proc_fs.h +1 -5
@@ -1,10 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+#pragma once
 /*
  * The proc filesystem constants/structures
  */
-#ifndef _LINUX_PROC_FS_H
-#define _LINUX_PROC_FS_H
-
 #include <linux/types.h>
 #include <linux/fs.h>
 
@@ -82,5 +80,3 @@ static inline struct proc_dir_entry *proc_net_mkdir(
 struct ns_common;
 int open_related_ns(struct ns_common *ns,
 		   struct ns_common *(*get_ns)(struct ns_common *ns));
-
-#endif /* _LINUX_PROC_FS_H */
#1 lore raw
Quoted 1 line · Alexey Dobriyan

Bring /proc into 21st century.

Please explain what this actually buys us except for being pointlessly different from the rest of the kernel?

#2 lore raw
Quoted 5 lines · Christoph Hellwig
Quoted 1 line · Alexey Dobriyan

Bring /proc into 21st century.

Please explain what this actually buys us except for being pointlessly different from the rest of the kernel?

Negative LOC diff, less junk in preprocessor hashtables.

#3 lore raw
Quoted 8 lines · Alexey Dobriyan
Quoted 5 lines · Christoph Hellwig
Quoted 1 line · Alexey Dobriyan

Bring /proc into 21st century.

Please explain what this actually buys us except for being pointlessly different from the rest of the kernel?

Negative LOC diff, less junk in preprocessor hashtables.

There are about 20k header files, none of them has #pragma once. Updating that will bring many unnesessry git commits.

I doubt that one more define in preprocessor tables is a problem we should fix. The LOC argument also does not sound very convincing. The include protection is at the top of the file, not mixed among other code. Replacing 2-3 lines with one will be barely noticeable.

#4 lore raw
Quoted 7 lines · David Sterba

There are about 20k header files, none of them has #pragma once. Updating that will bring many unnesessry git commits.

I doubt that one more define in preprocessor tables is a problem we should fix. The LOC argument also does not sound very convincing. The include protection is at the top of the file, not mixed among other code. Replacing 2-3 lines with one will be barely noticeable.

Let's put it another way: does this make any noticable difference for compile times?

#6 lore raw
In reply to David Sterba
Quoted 5 lines · David Sterba
Quoted 4 lines · Alexey Dobriyan
Quoted 3 lines · Christoph Hellwig
Quoted 1 line · Alexey Dobriyan

Bring /proc into 21st century.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Please explain what this actually buys us except for being pointlessly different from the rest of the kernel?

Negative LOC diff, less junk in preprocessor hashtables.

There are about 20k header files, none of them has #pragma once. Updating that will bring many unnesessry git commits.

If you don't obey sometimes silly rule re splitting patches, it can be done in much less.

Quoted 2 lines

I doubt that one more define in preprocessor tables is a problem we should fix.

Those eat cycles and memory one define at a time.

Quoted 1 line

The LOC argument also does not sound very convincing.

When was the last time you did -80 kLOC patch for free?

Quoted 2 lines

The include protection is at the top of the file, not mixed among other code. Replacing 2-3 lines with one will be barely noticeable.

#5 lore raw
In reply to Christoph Hellwig
Quoted 11 lines · Christoph Hellwig
Quoted 7 lines · David Sterba

There are about 20k header files, none of them has #pragma once. Updating that will bring many unnesessry git commits.

I doubt that one more define in preprocessor tables is a problem we should fix. The LOC argument also does not sound very convincing. The include protection is at the top of the file, not mixed among other code. Replacing 2-3 lines with one will be barely noticeable.

Let's put it another way: does this make any noticable difference for compile times?

Should be the same as include guard pattern is recognized and becomes #pragma once internally.

#7 lore raw
In reply to Alexey Dobriyan
Quoted 3 lines · On Thu, 26 Apr 2018 22:24:44 +0300 Alexey Dobriyan <[email protected]> wrote:
Quoted 1 line

The LOC argument also does not sound very convincing.

When was the last time you did -80 kLOC patch for free?

That would be the way to do it - sell the idea to Linus, send him a script to do it then stand back. The piecemeal approach is ongoing pain.

#8 lore raw
Quoted 10 lines · Andrew Morton
Quoted 3 lines · On Thu, 26 Apr 2018 22:24:44 +0300 Alexey Dobriyan <[email protected]> wrote:
Quoted 1 line

The LOC argument also does not sound very convincing.

When was the last time you did -80 kLOC patch for free?

That would be the way to do it - sell the idea to Linus, send him a script to do it then stand back. The piecemeal approach is ongoing pain.

FWIW, it's not just removing some identifiers from cpp's hash tables, it also reduces I/O: Due to our header mess, we have some cyclic includes, e.g mm.h -> memremap.h -> mm.h. While parsing mm.h, cpp sees the #define _LINUX_MM_H, then goes parsing memremap.h, but since it hasn't reached the end of mm.h yet (seeing that there's nothing but comments outside the #ifndef/#endif pair), it hasn't had a chance to set the internal flag for mm.h, so it goes slurping in mm.h again. Obviously, the definedness of _LINUX_MM_H at that point means it "only" has to parse those 87K for comments and matching up #ifs, #ifdefs,#endifs etc. With #pragma once, the flag gets set for mm.h immediately, so the #include from memremap.h is entirely ignored. This can easily be verified with strace. And mm.h is not the only header getting read twice.

I had some "extract the include guard" line noise lying around, so I hacked up the below if someone wants to play some more with this. A few not-very-careful kbuild timings didn't show anything significant, but both the before and after times were way too noisy, and I only patched include/linux/*.h.

Anyway, the first order of business is to figure out which ones to leave alone. We have a bunch of #ifndef THAT_ONE #error "don't include $this_one directly". The brute-force way is to simply record all macros which are checked for definedness at least twice.

git grep -h -E '^\s*#\s*if(.*defined\s*\(|n?def)\s*[A-Za-z0-9_]+' | grep -o -E '[A-Za-z_][A-Za-z_0-9]*' | sort | uniq --repeated > multest.txt

But there's also stuff like arch/x86/boot/compressed/kaslr.c that plays games with pre-defining _EXPORT_H to avoid parsing export.h when it inevitably gets included. Oh well, just add the list of macros that have at least two definitions.

git grep -h -E '^\s*#\s*define\s+[A-Za-z0-9_]+' | grep -o -E '^\s*#\s*define\s+[A-Za-z0-9_]+' | grep -oE '[A-Za-z0-9_]+' | sort | uniq --repeated > muldef.txt

With those, one can just do

cat muldef.txt multest.txt | scripts/replace_ig.pl ...

This ends up detecting a lot of copy-pasting (e.g. __LINUX_MFD_MAX8998_H), as well as lots of headers that for no obvious reason do not have an include guard. Oh, and once.h has a redundant \.

Rasmus

wear sunglasses...

=== scripts/replace_ig.pl ===

#!/usr/bin/perl

use strict;
use warnings;
use File::Slurp;

my %preserve;

sub strip_comments {
my $txt = shift;

# Line continuations are handled before comment stripping, so # <slash> <backslash> <newline> <star> actually starts a comment, # and a // comment can swallow the following line. Let's just # assume nobody has modified the #if control flow using such dirty # tricks when we do a more naive line-by-line parsing below to # actually remove the include guard deffery.
$txt =~ s/\\\n//g;

# http://stackoverflow.com/a/911583/722859
$txt =~ s{
/\* ## Start of /* ... */ comment
[^*]*\*+ ## Non-* followed by 1-or-more *'s
(?:
[^/*][^*]*\*+
)* ## 0-or-more things which don't start with /
## but do end with '*'
/ ## End of /* ... */ comment

|
// ## Start of // comment
[^\n]* ## Anything which is not a newline
(?=\n) ## End of // comment; use look-ahead to avoid consuming the
newline

| ## OR various things which aren't comments:

(
" ## Start of " ... " string
(?:
\\. ## Escaped char
| ## OR
[^"\\] ## Non "\
)*
" ## End of " ... " string

| ## OR

' ## Start of ' ... ' string
(
\\. ## Escaped char
| ## OR
[^'\\] ## Non '\
)*
' ## End of ' ... ' string

| ## OR

. ## Anything other char
[^/"'\\]* ## Chars which doesn't start a comment, string or escape
)
}{defined $1 ? $1 : " "}gxse;

return $txt;
}

sub include_guard {
    my $txt = shift;
    my @lines = (split /^/, $txt);
    my $i = 0;
    my $level = 1;
    my $name;

# The first non-empty line must be an #ifndef or an #if !defined().

    ++$i while ($i < @lines && $lines[$i] =~ m/^\s*$/);
    goto not_found if ($i == @lines);
    goto not_found
	if (!($lines[$i] =~
m/^\s*#\s*ifndef\s+(?<name>[A-Za-z_][A-Za-z_0-9]*)\s*$/) &&
	    !($lines[$i] =~
m/^\s*#\s*if\s+!\s*defined\s*\(\s*(?<name>[A-Za-z_][A-Za-z_0-9]*)\s*\)\s*$/));
    $name = $+{name};

    # The next non-empty line must be a #define of that macro.
    1 while (++$i < @lines && $lines[$i] =~ m/^\s*$/);
    goto not_found if ($i == @lines);
    goto not_found if !($lines[$i] =~ m/^\s*#\s*define\s+\b$name\b/);

    # Now track #ifs and #endifs. #elifs and #elses don't change the level.
    while (++$i < @lines && $level > 0) {
	if ($lines[$i] =~ m/^\s*#\s*(?:if|ifdef|ifndef)\b/) {
	    $level++;
	} elsif ($lines[$i] =~ m/^\s*#\s*endif\b/) {
	    $level--;
	}
    }
    goto not_found if ($level > 0); # issue a warning?
    # Check that the rest of the file consists of empty lines.
    ++$i while ($i < @lines && $lines[$i] =~ m/^\s*$/);
    goto not_found if ($i < @lines);
    return $name;

not_found:

    return undef;
}
sub do_file {
    my $fn = shift;
    my $src = read_file($fn);
    my $ig = include_guard(strip_comments($src));
    if (not defined $ig) {
	printf STDERR "%s: no include guard\n", $fn;
	return;
    }
    if (exists $preserve{$ig}) {
	printf STDERR "%s: include guard %s exempted\n", $fn, $ig;
	return;
    }

    # OK, the entire text should match this horrible regexp.
    if ($src =~ m{
  (.*?) # arbitrary stuff before #ifndef
  (^\s*\#\s*if(?:\s*!\s*defined\s*\(\s*$ig\s*\)|ndef\s*$ig) .*? \n #
  (?:^\s*\n)*
   ^\s*\#\s*define\s*$ig .*? \n) # 2/3 of include guard
  (.*(?=^\s*\#\s*endif)) # body of file
  (^\s*\#\s*endif .*? \n) # last 1/3
  (.*) # rest of file (trailing comments)
	}smx) {
	my $pre = $1;
	my $define = $2;
	my $body = $3;
	my $endif = $4;
	my $post = $5;
	$body =~ s/\n[ \t]*\n$/\n/g;
	$src = $pre . "#pragma once\n";
	$src .= $body . $post;
    } else {
	printf STDERR "%s: has include guard %s, but I failed to replace it
with #pragma once\n",
	$fn, $ig;
	return;
    }
    write_file($fn, $src);
}
while (<STDIN>) {
    chomp;
    $preserve{$_} = 1;
}
for (@ARGV) {
    do_file($_);
}
#9 lore raw
Quoted 4 lines · Rasmus Villemoes

FWIW, it's not just removing some identifiers from cpp's hash tables, it also reduces I/O: Due to our header mess, we have some cyclic includes, e.g mm.h -> memremap.h -> mm.h. While parsing mm.h, cpp sees the #define _LINUX_MM_H, then goes parsing memremap.h, but since it hasn't reached

Quoted 8 lines

the end of mm.h yet (seeing that there's nothing but comments outside the #ifndef/#endif pair), it hasn't had a chance to set the internal flag for mm.h, so it goes slurping in mm.h again. Obviously, the definedness of _LINUX_MM_H at that point means it "only" has to parse those 87K for comments and matching up #ifs, #ifdefs,#endifs etc. With #pragma once, the flag gets set for mm.h immediately, so the #include from memremap.h is entirely ignored. This can easily be verified with strace. And mm.h is not the only header getting read twice.

Which gcc version it is? Does it *really* read anything twice? After all, you are going to read (and tokenize) the rest of mm.h anyway, so if it throws away the file it has read, only to reread it again, it's bloody dumb.

Note that sequence of preprocessor tokens does not depend upon the ifdefs; anything under #if 0 *is* tokenized all the same. So it's not even that "parsing" (tokenizing, actually) has to be repeated.

#10 lore raw
Quoted 15 lines · Al Viro
Quoted 4 lines · Rasmus Villemoes

FWIW, it's not just removing some identifiers from cpp's hash tables, it also reduces I/O: Due to our header mess, we have some cyclic includes, e.g mm.h -> memremap.h -> mm.h. While parsing mm.h, cpp sees the #define _LINUX_MM_H, then goes parsing memremap.h, but since it hasn't reached

Quoted 8 lines

the end of mm.h yet (seeing that there's nothing but comments outside the #ifndef/#endif pair), it hasn't had a chance to set the internal flag for mm.h, so it goes slurping in mm.h again. Obviously, the definedness of _LINUX_MM_H at that point means it "only" has to parse those 87K for comments and matching up #ifs, #ifdefs,#endifs etc. With #pragma once, the flag gets set for mm.h immediately, so the #include from memremap.h is entirely ignored. This can easily be verified with strace. And mm.h is not the only header getting read twice.

Quoted 1 line

Which gcc version it is?

When I stumbled on this a few years ago (I was stracing gcc for some reason), 4.something. Verifying today, 5.4.

Quoted 1 line

Does it *really* read anything twice?

Yes, just do "strace -f" and grep 'open.*= [0-9]'.

Quoted 3 lines

After all, you are going to read (and tokenize) the rest of mm.h anyway, so if it throws away the file it has read, only to reread it again, it's bloody dumb.

Hm, yes, gcc should be able to hang the buffer off of the structure it keeps around anyway for tracking the status of mm.h. But it doesn't.

Quoted 3 lines

Note that sequence of preprocessor tokens does not depend upon the ifdefs; anything under #if 0 *is* tokenized all the same. So it's not even that "parsing" (tokenizing, actually) has to be repeated.

Are you sure about that? Maybe formally it has to (if the phases are done strictly sequentially), but can't it go into some fast-forward mode under #if 0 (or #ifndef $some-defined-macro) where it only looks for string and char literals, comments and "control-flow" directives? libcpp/internal.h does have

/* True if we are skipping a failed conditional group. */ unsigned char skipping;

Rasmus

#11 lore raw
Quoted 8 lines · Rasmus Villemoes
Quoted 3 lines

Note that sequence of preprocessor tokens does not depend upon the ifdefs; anything under #if 0 *is* tokenized all the same. So it's not even that "parsing" (tokenizing, actually) has to be repeated.

Are you sure about that? Maybe formally it has to (if the phases are done strictly sequentially), but can't it go into some fast-forward mode under #if 0 (or #ifndef $some-defined-macro) where it only looks for string and char literals, comments and "control-flow" directives?

What else do you expect preprocessor tokenizer to do? It must take care of line continuations, it must recognize and skip whitespace (including comments) and if it recognizes string and character literals, what else is left to do? pp-numbers? Sure - that'd be

	[.]?[0-9]([0-9a-zA-Z_.]|[epEP][-+])*

Horribly hard to recognize, that... Identifiers are no harder, obviously (especially since keywords are _not_ something special at that level) and punctuators are also easily taken care of.

Preprocessor tokens are nowhere near as much work as C ones. And you can't do mapping to C ones before the preprocessing is done - if nothing else, quoting and token concatenation needs to happen first.