Skip to contentSkip to conversation

aufs: Do not refer to AUFS_NAME in pr_fmt

PATCH In review Ben Hutchings 4 participants 25 messages Dec 30, 2011 – Jan 3, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 aufs: Do not refer to AUFS_NAME in pr_fmt

AUFS_NAME is only defined in aufs_type.h but pr_fmt may be used in headers included before aufs_type.h.

---
This should fix the build failure reported in
<http://lists.debian.org/debian-kernel/2011/12/msg00460.html>.

An alternative change was proposed in
<http://lists.debian.org/debian-kernel/2011/12/msg00482.html>.

Ben.

 fs/aufs/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
fs/aufs/Makefile +1 -1
index 58ad988..9594b8a 100644
@@ -5,7 +5,7 @@ include ${srctree}/${src}/magic.mk
 # enable pr_debug
 ccflags-y += -DDEBUG
 # sparse doesn't allow spaces
-ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'
+ccflags-y += -D'pr_fmt(fmt)="aufs\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'
 
 obj-$(CONFIG_AUFS_FS) += aufs.o
 aufs-y := module.o sbinfo.o super.o branch.o xino.o sysaufs.o opts.o \
Signature · 1 line
1.7.7.3
#1 lore raw

Hello Ben,

Ben Hutchings:

Quoted 2 lines

AUFS_NAME is only defined in aufs_type.h but pr_fmt may be used in headers included before aufs_type.h.

Thanx for bug reporting and a patch.

I don't think the "current" macro will be a problem, but I want you to test this patch since I don't have m68k environment.
It introduces a new separated file include/linux/aufs_name.h. If you (any of you) think this new file is not a good idea, please let me know.

J. R. Okajima

Documentation/filesystems/aufs/README +4 -4
index 420faeb..058f95e 100644
@@ -213,7 +213,7 @@ If you don't like aufs2-standalone.patch, then try aufs2-2.6 tree.
   path on the branch fs.
 - apply ./aufs2-standalone.patch too, if you have a plan to set
   CONFIG_AUFS_FS=m. otherwise you don't need ./aufs2-standalone.patch.
-- copy ./{Documentation,fs,include/linux/aufs_type.h} files to your
+- copy ./{Documentation,fs,include/linux/*.h} files to your
   kernel source tree. Never copy ./include/linux/Kbuild.
 - enable CONFIG_EXPERIMENTAL and CONFIG_AUFS_FS, you can select either
   =m or =y.
@@ -240,12 +240,12 @@ If you don't like aufs2-standalone.patch, then try aufs2-2.6 tree.
   + run "make install" to install the aufs module, or copy the built
     ./aufs.ko to /lib/modules/... and run depmod -a (or reboot simply).
   + run "make headers_install" to install the aufs header file (you can
-    specify DESTDIR), or copty ./usr/include/linux/aufs_type.h to
+    specify DESTDIR), or copty ./usr/include/linux/*.h to
     /usr/include/linux or wherever you like.
 - no need to apply aufs2-kbuild.patch, nor copying source files to your
   kernel source tree.
 
-Note: The haeder file aufs_type.h is necessary to build aufs2-util
+Note: The aufs haeder files under include/linux are necessary to build aufs2-util
       as well as "make headers_install" in the kernel source tree.
       headers_install is subject to be forgotten, but it is essentially
       necessary, not only for building aufs2-util.
@@ -255,7 +255,7 @@ Note: The haeder file aufs_type.h is necessary to build aufs2-util
 And then,
 - read README in aufs2-util, build and install it.
 - note that your distribution may contain an obsoleted version of
-  aufs_type.h in /usr/include/linux or something. When you build aufs
+  aufs header files in /usr/include/linux or something. When you build aufs
   utilities, make sure that your compiler refers the correct aufs header
   file which is built by "make headers_install."
 - if you want to use readdir(3) in userspace or pathconf(3) wrapper,
fs/aufs/Makefile +1 -0
index 0a757f4..b63f12a 100644
@@ -9,6 +9,7 @@ endif
 # enable pr_debug
 ccflags-y += -DDEBUG
 # sparse doesn't allow spaces
+ccflags-y += -imacros linux/aufs_name.h
 ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'
 
 obj-$(CONFIG_AUFS_FS) += aufs.o
include/linux/Kbuild +1 -0
index 4268c75..60bd38d 100644
@@ -34,6 +34,7 @@ header-y += atmppp.h
 header-y += atmsap.h
 header-y += atmsvc.h
 header-y += atm_zatm.h
+header-y += aufs_name.h
 header-y += aufs_type.h
 header-y += auto_fs4.h
 header-y += ax25.h
include/linux/aufs_name.h new file +25 -0
new file mode 100644
index 0000000..838cc43
@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2011 Junjiro R. Okajima
+ *
+ * This program, aufs is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef __AUFS_NAME_H__
+#define __AUFS_NAME_H__
+
+/* fs/aufs/Makefile refers this macro */
+#define AUFS_NAME		"aufs"
+
+#endif /* __AUFS_NAME_H__ */
include/linux/aufs_type.h +1 -1
index ca4a6be..6f23223 100644
@@ -28,6 +28,7 @@
 #include <stdint.h>
 #include <sys/types.h>
 #endif
+#include <linux/aufs_name.h>
 
 #define AUFS_VERSION	"2.2-stdalone-35-lktr-YyyyMmDd"
 
@@ -58,7 +59,6 @@ typedef int16_t aufs_bindex_t;
 
 /* ---------------------------------------------------------------------- */
 
-#define AUFS_NAME		"aufs"
 #define AUFS_FSTYPE		AUFS_NAME
 
 #define AUFS_ROOT_INO		2

------------------------------------------------------------------------------
Ridiculously easy VDI. With Citrix VDI-in-a-Box, you don't need a complex infrastructure or vast IT resources to deliver seamless, secure access to virtual desktops. With this all-in-one solution, easily deploy virtual desktops for less than the cost of PCs and save 60% on VDI infrastructure costs. Try it free! http://p.sf.net/sfu/Citrix-VDIinabox

#2 lore raw

include/linux/aufs_name.h line 23

Quoted 2 lines · On Sat, Dec 31, 2011 at 05:46, <[email protected]> wrote:
include/linux/aufs_name.h new file +2 -0
+/* fs/aufs/Makefile refers this macro */
+#define AUFS_NAME              "aufs"

If the Makefile refers to the macro, perhaps the Makefile should define it, and pass it with -D?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds

#3 lore raw

[email protected] dixit:

Quoted 1 line

test this patch since I don't have m68k environment.

I can do that, but not too many patches at a time, since it takes easily a whole day to compile it. (Also, my own hacking time is limited atm.)

Quoted 1 line

It introduces a new separated file include/linux/aufs_name.h.

Isn’t that a bit overkill?

Geert Uytterhoeven dixit:

Quoted 6 lines · On Sat, Dec 31, 2011 at 05:46, <[email protected]> wrote:

include/linux/aufs_name.h line 23

Quoted 2 lines
include/linux/aufs_name.h new file +2 -0
+/* fs/aufs/Makefile refers this macro */
+#define AUFS_NAME              "aufs"

If the Makefile refers to the macro, perhaps the Makefile should define it, and pass it with -D?

Indeed. I like Ben’s patch better. But if it must be a separate file, please move the pr_fmt definition out of the Makefile and into that file, too. Code doesn’t belong into a Makefile IMHO.

bye,
//mirabilos

Signature · 4 lines
FWIW, I'm quite impressed with mksh interactively. I thought it was much
*much* more bare bones. But it turns out it beats the living hell out of
ksh93 in that respect. I'd even consider it for my daily use if I hadn't
wasted half my life on my zsh setup. :-) -- Frank Terbeck in #!/bin/mksh
#4 lore raw

Thorsten Glaser:

Quoted 3 lines
Quoted 1 line

It introduces a new separated file include/linux/aufs_name.h.

Isn=E2=80=99t that a bit overkill?

Hmm, I may have to agree with that.
Honestly speaking, I don't like this approach. But embedding (expanding) AUFS_NAME is worse for me.

Quoted 6 lines
Quoted 2 lines

If the Makefile refers to the macro, perhaps the Makefile should define it, and pass it with -D?

Indeed. I like Ben=E2=80=99s patch better. But if it must be a separate file, please move the pr_fmt definition out of the Makefile and into that file, too. Code doesn=E2=80=99t belong into a Makefile IMHO.

You guys don't see "-imacros linux/aufs_name.h" in Makefile? Or do I totaly miunderstand -imacros?

----------------------------------------------------------------------

Quoted 8 lines

Ben Hutchings dixit:

Quoted 1 line

-ccflags-y +=3D -D'pr_fmt(fmt)=3DAUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__fun=

c__,__LINE__,current->comm,current->pid'

Quoted 1 line

+ccflags-y +=3D -D'pr_fmt(fmt)=3D"aufs\040%s:%d:%s[%d]:\040"fmt,__func__,_=

_LINE__,current->comm,current->pid'

Sadly, this doesn=E2=80=99t work either:

	:::
Quoted 2 lines

/tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/arch/m68k/inclu= de/asm/hardirq.h:23:2: error: dereferencing pointer to incomplete type

	:::
Quoted 3 lines

The difference between a static inline function and a pr=C3=A6processor macro makes the difference. I know, now, why I never used the former myself =E2=98=BA I guess =E2=80=9Ccurrent=E2=80=9D is not defined there.

That must be an error around the "current" macro which I was afraid.

----------------------------------------------------------------------

Quoted 2 lines

What do you think of this (untested)? I omitted the Documentation/** patch as that part it not present in the Debian Linux kernel.

	:::

+#define AUFS_NAME "aufs"
+#define pr_fmt(fmt) AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid

Here are the comments from me.
- aufs_name.h is used both in kernel-space and user-space, so we need "#ifdef __KERNEL__"
- the macro may already be defined, so we need "#ifndef pr_fmt" now we should decide which we force (choose), the pr_fmt macro we define in aufs or the one previously defined somewhere else.
And I chose "defined aufs aufs".
These are also reasons to put it in Makefile.

----------------------------------------------------------------------

Quoted 11 lines

well, it compiles (with warnings, see below). I=E2=80=99ll know if it links and loads tomorrow, I guess (unless the other problems are still there).

/tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/include/linux/a= ufs_name.h:27:0: warning: "pr_fmt" redefined [enabled by default] /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/include/linux/p= rintk.h:152:0: note: this is the location of the previous definition

Version 2 of this patch (attached) fixes this by #undef pr_fmt before defining it. Please decide whether you want to fix it

Yes, undefining will be necessary if we move the macro difinition into the header file. Or we may add a condition "#ifndef pr_fmt" when we define the macro in the header. In other words,
- adding a condition means giving precedence to the difinition in somewhere else.
- undefining means forcing the definition in aufs.
But this "forcing" is not enough since the macro can be defined _before_ including the header. I know you put "-include linux/aufs_name.h" in Makefile, but it is not enough either since the macro _may_ be defined in sched.h or somewhere else which will be included before aufs_name.h. Also I understand it is not a big problem. But I hope you would agree that unconditionally or blindly defined macro is effective (or less subject to bug) in a single module.

So I still think it is better to define it in Makefile.
If I remove refering the "current" macro in the definition, then the life will be easier, but it is still useful and I want to keep it. Additonally it is not a essential problem I think. Finally I'd like to add sched.h between aufs_name and pr_fmt (see the attached patch).
How do you think?

J. R. Okajima

fs/aufs/Makefile +1 -0
@@ -10,6 +10,7 @@ endif
 ccflags-y += -DDEBUG
 # sparse doesn't allow spaces
 ccflags-y += -imacros linux/aufs_name.h
+ccflags-y += -include linux/sched.h
 ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'
 
 obj-$(CONFIG_AUFS_FS) += aufs.o
#5 lore raw
Quoted 8 lines · On Mon, 2012-01-02 at 11:58 +0900, [email protected] wrote:

Thorsten Glaser:

Quoted 3 lines
Quoted 1 line

It introduces a new separated file include/linux/aufs_name.h.

Isn=E2=80=99t that a bit overkill?

Hmm, I may have to agree with that.
Honestly speaking, I don't like this approach. But embedding (expanding) AUFS_NAME is worse for me.

Why, how often do you expect to change AUFS_NAME?

[...]

Quoted 7 lines

So I still think it is better to define it in Makefile.
If I remove refering the "current" macro in the definition, then the life will be easier, but it is still useful and I want to keep it. Additonally it is not a essential problem I think. Finally I'd like to add sched.h between aufs_name and pr_fmt (see the attached patch).
How do you think?

I think it would be much better to put this in fs/aufs/aufs.h and make each of fs/aufs/*.c include that first.

fs/aufs/Makefile endif · line 16

Quoted 12 lines

J. R. Okajima

fs/aufs/Makefile +1 -0
@@ -10,6 +10,7 @@ endif
 ccflags-y += -DDEBUG
 # sparse doesn't allow spaces
 ccflags-y += -imacros linux/aufs_name.h
+ccflags-y += -include linux/sched.h
 ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'
 
 obj-$(CONFIG_AUFS_FS) += aufs.o

The comment about sparse belongs immediately before the definition of pr_fmt.

Ben.

Signature · 2 lines
Ben Hutchings
All the simple programs have been written, and all the good names taken.
#6 lore raw

Ben Hutchings:

Quoted 1 line

Why, how often do you expect to change AUFS_NAME?

I don't know.
I just don't want call myself idiot when it happens.

Quoted 2 lines

I think it would be much better to put this in fs/aufs/aufs.h and make each of fs/aufs/*.c include that first.

Yes, that is my another option I have considered. Also I am considering to move AUFS_NAME back into aufs_type.h.

In aufs_type.h:
(at the top)
#define AUFS_NAME ...
#if KERNEL
#define pr_fmt ...
#include sched.h
#endif

and include aufs_type.h at the top of source files.

Quoted 2 lines

The comment about sparse belongs immediately before the definition of pr_fmt.

Yes, I am changing like this.

fs/aufs/Makefile +3 -2
@@ -8,9 +8,10 @@ endif
 # cf. include/linux/kernel.h
 # enable pr_debug
 ccflags-y += -DDEBUG
+# sparse requires the fullpath
+ccflags-y += -imacros ${srctree}/include/linux/aufs_name.h
+ccflags-y += -include ${srctree}/include/linux/sched.h
 # sparse doesn't allow spaces
-ccflags-y += -imacros linux/aufs_name.h
-ccflags-y += -include linux/sched.h
 ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'
 
 obj-$(CONFIG_AUFS_FS) += aufs.o

J. R. Okajima

#7 lore raw
In reply to Geert Uytterhoeven

Geert Uytterhoeven:

Quoted 2 lines

If the Makefile refers to the macro, perhaps the Makefile should define it, and pass it with -D?

??
Instead of defining it in Makefile, "-imacros linux/aufs_name.h" is added. Or do you mean -imacros is useless?

J. R. Okajima

#8 lore raw
In reply to Ben Hutchings

Ben Hutchings dixit:

fs/aufs/Makefile include ${srctree}/${src}/magic.mk · line 8

Quoted 2 lines
fs/aufs/Makefile +1 -1
-ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'
+ccflags-y += -D'pr_fmt(fmt)="aufs\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'

Sadly, this doesn’t work either:

CC [M] fs/aufs/module.o
In file included from /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/include/linux/hardirq.h:7:0,

                 from /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/arch/m68k/include/asm/irqflags.h:6,
                 from /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/include/linux/irqflags.h:15,
                 from /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/include/linux/spinlock.h:53,
                 from /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/include/linux/seqlock.h:29,
                 from /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/include/linux/time.h:8,
                 from /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/include/linux/stat.h:60,
                 from /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/include/linux/module.h:10,
                 from /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/fs/aufs/module.c:23:

/tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/arch/m68k/include/asm/hardirq.h: In function 'ack_bad_irq':
/tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/arch/m68k/include/asm/hardirq.h:23:2: error: dereferencing pointer to incomplete type /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/arch/m68k/include/asm/hardirq.h:23:2: error: dereferencing pointer to incomplete type make[7]: *** [fs/aufs/module.o] Error 1

This one does work:
http://lists.debian.org/debian-kernel/2011/12/msg00474.html

The difference between a static inline function and a præprocessor macro makes the difference. I know, now, why I never used the former myself ☺ I guess “current” is not defined there.

[email protected] dixit:

Quoted 1 line

I don't think the "current" macro will be a problem, but I want you to

It does. Also, read this:
http://people.netfilter.org/rusty/unreliable-guides/kernel-hacking/routines-current.html
“This global variable (really a macro) contains a pointer to the current task structure, so is only valid in user context. For example, when a process makes a system call, this will point to the task structure of the calling process. It is not NULL in interrupt context.”

So I don’t know how safe it is to dereference 'current' in the inline function ack_bad_irq (probably, not at all). On the other hand, the ack_bad_irq function is probably not expanded in aufs3… so we could go on. I can try a patch based on yours, but probably the best fix would be to not use a static inline function in m68k.

bye,
//mirabilos

Signature · 3 lines
  “Having a smoking section in a restaurant is like having
          a peeing section in a swimming pool.”
						-- Edward Burr
#9 lore raw

Dixi quod…

Quoted 1 line

go on. I can try a patch based on yours, but probably the best fix

What do you think of this (untested)? I omitted the Documentation/** patch as that part it not present in the Debian Linux kernel.

Please do not commit this anywhere until I say I have tested it.

bye,
//mirabilos

Signature · 4 lines
I believe no one can invent an algorithm. One just happens to hit upon it
when God enlightens him. Or only God invents algorithms, we merely copy them.
If you don't believe in God, just consider God as Nature if you won't deny
existence.		-- Coywolf Qi Hunt

From 7431e1ffad6aafa36fa8dd0459c066fada25e1d6 Mon Sep 17 00:00:00 2001
From: Thorsten Glaser <[email protected]>
Date: Sat, 31 Dec 2011 22:27:15 +0000
Subject: [PATCH] =?UTF-8?q?move=20AUFS=5FNAME=20and=20aufs=E2=80=99=20pr=5Ff?=
=?UTF-8?q?mt=20into=20their=20own=20header?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

based on a patch from [email protected] in <9569.1325306817@jrobl> but changed to also move the pr_fmt definition off the Makefile and into the header, and include the definition of current and the header which details its structure so it can be dereferenced, which may or may not be safe though… see <[email protected]>

---
 fs/aufs/Makefile          |    3 +--
 include/linux/Kbuild      |    1 +
 include/linux/aufs_name.h |   29 +++++++++++++++++++++++++++++
 include/linux/aufs_type.h |    2 +-
 4 files changed, 32 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/aufs_name.h
fs/aufs/Makefile +1 -2
index 46615fe..63b8a72 100644
@@ -8,8 +8,7 @@ endif
 # cf. include/linux/kernel.h
 # enable pr_debug
 ccflags-y += -DDEBUG
-# sparse doesn't allow spaces
-ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'
+ccflags-y += -include linux/aufs_name.h
 
 obj-$(CONFIG_AUFS_FS) += aufs.o
 aufs-y := module.o sbinfo.o super.o branch.o xino.o sysaufs.o opts.o \
include/linux/Kbuild +1 -0
index 29f386b..fc6d2d2 100644
@@ -65,6 +65,7 @@ header-y += atmppp.h
 header-y += atmsap.h
 header-y += atmsvc.h
 header-y += audit.h
+header-y += aufs_name.h
 header-y += aufs_type.h
 header-y += auto_fs.h
 header-y += auto_fs4.h
include/linux/aufs_name.h new file +29 -0
new file mode 100644
index 0000000..fda7f5c
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2011 Junjiro R. Okajima
+ * Copyright (c) 2011 Thorsten Glaser
+ *
+ * This program, aufs is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef __AUFS_NAME_H__
+#define __AUFS_NAME_H__
+
+/* includes <asm/current.h> and defines struct task_struct */
+#include <linux/sched.h>
+
+#define AUFS_NAME		"aufs"
+#define pr_fmt(fmt)		AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid
+
+#endif /* __AUFS_NAME_H__ */
include/linux/aufs_type.h +1 -1
index 9443906..16b0bf8 100644
@@ -28,6 +28,7 @@
 #include <stdint.h>
 #include <sys/types.h>
 #endif
+#include <linux/aufs_name.h>
 
 #define AUFS_VERSION	"3.x-rcN-20111114"
 
@@ -58,7 +59,6 @@ typedef int16_t aufs_bindex_t;
 
 /* ---------------------------------------------------------------------- */
 
-#define AUFS_NAME		"aufs"
 #define AUFS_FSTYPE		AUFS_NAME
 
 #define AUFS_ROOT_INO		2
Signature · 1 line
1.7.7.3
#10 lore raw

Dixi quod…

Quoted 1 line

What do you think of this (untested)?

well, it compiles (with warnings, see below). I’ll know if it links and loads tomorrow, I guess (unless the other problems are still there).

/tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/include/linux/aufs_name.h:27:0: warning: "pr_fmt" redefined [enabled by default] /tmp/buildd/linux-2.6-3.2~rc7/debian/build/source_m68k_none/include/linux/printk.h:152:0: note: this is the location of the previous definition

Version 2 of this patch (attached) fixes this by #undef pr_fmt before defining it. Please decide whether you want to fix it like this. Independent of this, I’ve asked the m68k maintainer to change away from static inline to a cpp macro, to avoid this issue fully. This should not influence your decision, though.

bye,
//mirabilos

Signature · 4 lines
FWIW, I'm quite impressed with mksh interactively. I thought it was much
*much* more bare bones. But it turns out it beats the living hell out of
ksh93 in that respect. I'd even consider it for my daily use if I hadn't
wasted half my life on my zsh setup. :-) -- Frank Terbeck in #!/bin/mksh

From debdad28232a9c84f135fa1b7aa9d137b72ab4d8 Mon Sep 17 00:00:00 2001
From: Thorsten Glaser <[email protected]>
Date: Sat, 31 Dec 2011 22:27:15 +0000
Subject: [PATCH] =?UTF-8?q?move=20AUFS=5FNAME=20and=20aufs=E2=80=99=20pr=5Ff?=
=?UTF-8?q?mt=20into=20their=20own=20header?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

based on a patch from [email protected] in <9569.1325306817@jrobl> but changed to also move the pr_fmt definition off the Makefile and into the header, and include the definition of current and the header which details its structure so it can be dereferenced, which may or may not be safe though… see <[email protected]>

---
 fs/aufs/Makefile          |    3 +--
 include/linux/Kbuild      |    1 +
 include/linux/aufs_name.h |   31 +++++++++++++++++++++++++++++++
 include/linux/aufs_type.h |    2 +-
 4 files changed, 34 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/aufs_name.h
fs/aufs/Makefile +1 -2
index 46615fe..63b8a72 100644
@@ -8,8 +8,7 @@ endif
 # cf. include/linux/kernel.h
 # enable pr_debug
 ccflags-y += -DDEBUG
-# sparse doesn't allow spaces
-ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'
+ccflags-y += -include linux/aufs_name.h
 
 obj-$(CONFIG_AUFS_FS) += aufs.o
 aufs-y := module.o sbinfo.o super.o branch.o xino.o sysaufs.o opts.o \
include/linux/Kbuild +1 -0
index 29f386b..fc6d2d2 100644
@@ -65,6 +65,7 @@ header-y += atmppp.h
 header-y += atmsap.h
 header-y += atmsvc.h
 header-y += audit.h
+header-y += aufs_name.h
 header-y += aufs_type.h
 header-y += auto_fs.h
 header-y += auto_fs4.h
include/linux/aufs_name.h new file +31 -0
new file mode 100644
index 0000000..8c2c494
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2011 Junjiro R. Okajima
+ * Copyright (c) 2011 Thorsten Glaser
+ *
+ * This program, aufs is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef __AUFS_NAME_H__
+#define __AUFS_NAME_H__
+
+/* includes <asm/current.h> and defines struct task_struct */
+#include <linux/sched.h>
+
+#define AUFS_NAME		"aufs"
+
+#undef pr_fmt
+#define pr_fmt(fmt)		AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid
+
+#endif /* __AUFS_NAME_H__ */
include/linux/aufs_type.h +1 -1
index 9443906..16b0bf8 100644
@@ -28,6 +28,7 @@
 #include <stdint.h>
 #include <sys/types.h>
 #endif
+#include <linux/aufs_name.h>
 
 #define AUFS_VERSION	"3.x-rcN-20111114"
 
@@ -58,7 +59,6 @@ typedef int16_t aufs_bindex_t;
 
 /* ---------------------------------------------------------------------- */
 
-#define AUFS_NAME		"aufs"
 #define AUFS_FSTYPE		AUFS_NAME
 
 #define AUFS_ROOT_INO		2
Signature · 1 line
1.7.7.3
#11 lore raw

Thorsten Glaser:

Quoted 5 lines
Quoted 1 line

What do you think of this (untested)?

well, it compiles (with warnings, see below). I=E2=80=99ll know if it links and loads tomorrow, I guess (unless the other problems are still there).

	:::

This is the last version of my approach (documentations are omitted). Would you try on your m68k when you have time?
You need to revert my previous patch.

J. R. Okajima

fs/aufs/Makefile +2 -2
index 0a757f4..7326c22 100644
@@ -8,8 +8,8 @@ endif
 # cf. include/linux/kernel.h
 # enable pr_debug
 ccflags-y += -DDEBUG
-# sparse doesn't allow spaces
-ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'
+# sparse requires the full pathname
+ccflags-y += -include ${srctree}/include/linux/aufs_name.h
 
 obj-$(CONFIG_AUFS_FS) += aufs.o
 aufs-y := module.o sbinfo.o super.o branch.o xino.o sysaufs.o opts.o \
include/linux/Kbuild +1 -0
index 4268c75..60bd38d 100644
@@ -34,6 +34,7 @@ header-y += atmppp.h
 header-y += atmsap.h
 header-y += atmsvc.h
 header-y += atm_zatm.h
+header-y += aufs_name.h
 header-y += aufs_type.h
 header-y += auto_fs4.h
 header-y += ax25.h
include/linux/aufs_name.h new file +29 -0
new file mode 100644
index 0000000..550fbb2
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2011-2012 Junjiro R. Okajima
+ *
+ * This program, aufs is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef __AUFS_NAME_H__
+#define __AUFS_NAME_H__
+
+#define AUFS_NAME		"aufs"
+#ifdef __KERNEL__
+#define pr_fmt(fmt)	AUFS_NAME " %s:%d:%s[%d]: " fmt,\
+		__func__, __LINE__, current->comm, current->pid
+#include <linux/sched.h>
+#endif
+
+#endif /* __AUFS_NAME_H__ */
include/linux/aufs_type.h +1 -1
index ca4a6be..779f41a 100644
@@ -19,6 +19,7 @@
 #ifndef __AUFS_TYPE_H__
 #define __AUFS_TYPE_H__
 
+#include <linux/aufs_name.h>
 #include <linux/ioctl.h>
 #include <linux/kernel.h>
 #include <linux/limits.h>
@@ -58,7 +59,6 @@ typedef int16_t aufs_bindex_t;
 
 /* ---------------------------------------------------------------------- */
 
-#define AUFS_NAME		"aufs"
 #define AUFS_FSTYPE		AUFS_NAME
 
 #define AUFS_ROOT_INO		2
#12 lore raw

[email protected] dixit:

Quoted 1 line

This is the last version of my approach (documentations are omitted).

This doesn’t really differ from what I sent last,
does it?

Quoted 1 line

Would you try on your m68k when you have time?

You _are_ aware that a kernel compile takes over a day, right? Why don’t you use the patch from:
http://lists.debian.org/debian-kernel/2012/01/msg00000.html

fs/aufs/Makefile endif · line 12

Quoted 2 lines
fs/aufs/Makefile +2 -0
+# sparse requires the full pathname
+ccflags-y += -include ${srctree}/include/linux/aufs_name.h

If that is so, (whatever sparse is – the build succeeded with my patch), change this in my patch too.

Quoted 1 line

+#ifdef __KERNEL__

Hrm. Is this needed?

include/linux/aufs_name.h line 25

Quoted 2 lines
include/linux/aufs_name.h new file +2 -0
+#define pr_fmt(fmt)	AUFS_NAME " %s:%d:%s[%d]: " fmt,\
+		__func__, __LINE__, current->comm, current->pid

You need to #undef it first. It’s always defined, to just fmt.

Quoted 1 line

+#include <linux/aufs_name.h>

You include aufs_name.h twice now, once in the Makefile, once in the header. Shouldn’t one be enough?

bye,
//mirabilos

Signature · 3 lines
  “Having a smoking section in a restaurant is like having
          a peeing section in a swimming pool.”
						-- Edward Burr
#13 lore raw

Thorsten Glaser:

Quoted 2 lines

This doesn=E2=80=99t really differ from what I sent last, does it?

I am afraid you may not understand the important parts.
- the order of the definition and sched.h.
- no undef.

Quoted 3 lines
Quoted 1 line

+#ifdef __KERNEL__

Hrm. Is this needed?

Indeed necessary since aufs_name.h is exported to userspace.

Quoted 4 lines
Quoted 2 lines

+#define pr_fmt(fmt)=09AUFS_NAME " %s:%d:%s[%d]: " fmt,\
+=09=09__func__, __LINE__, current->comm, current->pid

You need to #undef it first. It=E2=80=99s always defined, to just fmt.

No, I don't think undef is a good approach. The purpose of my patch, or the difference from yours is to define pr_fmt first before all headers including sched.h.

Quoted 4 lines
Quoted 1 line

+#include <linux/aufs_name.h>

You include aufs_name.h twice now, once in the Makefile, once in the header. Shouldn=E2=80=99t one be enough?

No, because aufs_type.h is exported to userspace.

J. R. Okajima

#14 lore raw

[email protected] dixit:

Quoted 4 lines
Quoted 2 lines

You include aufs_name.h twice now, once in the Makefile, once in the header. Shouldn=E2=80=99t one be enough?

No, because aufs_type.h is exported to userspace.

Then, why include it in the Makefile at all?
(Or, why include aufs_name.h from aufs_type.h?)

bye,
//mirabilos

Signature · 3 lines
  “Having a smoking section in a restaurant is like having
          a peeing section in a swimming pool.”
						-- Edward Burr
#15 lore raw

Thorsten Glaser:

Quoted 2 lines

Then, why include it in the Makefile at all?
(Or, why include aufs_name.h from aufs_type.h?)

- AUFS_NAME is necessary for both of kernel-space and user-space.
- from userspace, users include aufs_type.h. to keep the consistency, aufs_type.h should include aufs_name.h.
- for kernelspace, to put aufs_name.h _before_ all other headers.

Do I make myself clear with my poor English?

J. R. Okajima

#16 lore raw

[email protected] dixit:

Quoted 4 lines

- AUFS_NAME is necessary for both of kernel-space and user-space.
- from userspace, users include aufs_type.h. to keep the consistency, aufs_type.h should include aufs_name.h.
- for kernelspace, to put aufs_name.h _before_ all other headers.

Hrm, okay. I’ll try your patch then, once we get that register_cpu issue solved too. I’ll get back to you with test results.

Quoted 1 line

Do I make myself clear with my poor English?

I have the same problem ;-) it’s only my third natural language.

bye,
//mirabilos

Signature · 4 lines
08:05⎜<XTaran:#grml> mika: Does grml have an tool to read Apple
     ⎜    System Log (asl) files? :)
08:08⎜<ft:#grml> yeah. /bin/rm. ;)       08:09⎜<mrud:#grml> hexdump -C
08:31⎜<XTaran:#grml> ft, mrud: *g*
#17 lore raw

Thorsten Glaser:

Quoted 2 lines

Hrm, okay. I=E2=80=99ll try your patch then, once we get that register_cpu issue solved too. I=E2=80=99ll get back to you with test results.

Hold it please.
I am going to make more changes. So it is better to git-pull and test the aufs GIT repository.
It will be released in the first in this year.

Quoted 1 line

I have the same problem ;-) it=E2=80=99s only my third natural language.

I wonder your first or second language is my mother tangue. :-)

J. R. Okajima

#18 lore raw

[email protected] dixit:

Quoted 1 line

Hold it please.

OK.

Quoted 2 lines

I am going to make more changes. So it is better to git-pull and test the aufs GIT repository.

Please send patches that _should_ apply against what’s in Debian. I don’t have time to play the merge game at the moment.

Quoted 3 lines
Quoted 1 line

I have the same problem ;-) it=E2=80=99s only my third natural language.

I wonder your first or second language is my mother tangue. :-)

Probably not (German and Latin).

bye,
//mirabilos

Signature · 4 lines
17:57 < jtsn> Der 25C3 ist lustig. Deutsche Vortragende brechen sich vor
deutschen Zuhörern auf Englisch einen ab. ;-)  18:01 < jtsn> Adolfs Werk
war sehr nachhaltig. ;-)    18:01 < jtsn> Das gab's nichtmal in der DDR,
das Deutsche mit Deutschen auf Russisch reden. ;-)          (10x cnuke@)
#19 lore raw

cover aufs: headers (Re: [PATCH] aufs: Do not refer to AUFS_NAME in pr_fmt)

From: J. R. Okajima <[email protected]>

Thorsten Glaser:

Quoted 2 lines

Please send patches that _should_ apply against what=E2=80=99s in Debian. I don=E2=80=99t have time to play the merge game at the moment.

Which version of debian?
As you might know, the aufs module in the debian stable squeeze is out of my control.
Anyway, this patch series is against the aufs2.2-35 branch in aufs2-standalone.git.

J. R. Okajima (2):
aufs: headers 1/2, bugfix, where the pr_fmt macro definition
aufs: headers 2/2, simply refined

 fs/aufs/Makefile          |    4 ++--
 fs/aufs/branch.c          |    1 -
 fs/aufs/branch.h          |    2 --
 fs/aufs/cpup.c            |    2 --
 fs/aufs/cpup.h            |    4 ----
 fs/aufs/dbgaufs.h         |    3 ---
 fs/aufs/dcsub.h           |    1 -
 fs/aufs/debug.c           |    1 -
 fs/aufs/debug.h           |    8 --------
 fs/aufs/dentry.h          |    1 -
 fs/aufs/dir.c             |    1 -
 fs/aufs/dir.h             |    1 -
 fs/aufs/dynop.h           |    4 ----
 fs/aufs/export.c          |    1 -
 fs/aufs/f_op.c            |    2 --
 fs/aufs/f_op_sp.c         |    1 -
 fs/aufs/file.c            |    3 ---
 fs/aufs/file.h            |    1 -
 fs/aufs/finfo.c           |    1 -
 fs/aufs/fstype.h          |    1 -
 fs/aufs/hfsplus.c         |    1 -
 fs/aufs/i_op.c            |    2 --
 fs/aufs/inode.h           |    2 --
 fs/aufs/ioctl.c           |    1 -
 fs/aufs/opts.c            |    2 --
 fs/aufs/opts.h            |    3 ---
 fs/aufs/rdu.c             |    2 --
 fs/aufs/rwsem.h           |    1 -
 fs/aufs/sbinfo.c          |    1 -
 fs/aufs/spl.h             |    4 ----
 fs/aufs/super.c           |    3 +--
 fs/aufs/super.h           |    1 -
 fs/aufs/sysaufs.c         |    2 --
 fs/aufs/sysaufs.h         |    2 --
 fs/aufs/sysfs.c           |    3 ---
 fs/aufs/sysrq.c           |    3 ---
 fs/aufs/vdir.c            |    1 -
 fs/aufs/vfsub.c           |    2 --
 fs/aufs/whout.c           |    1 -
 fs/aufs/whout.h           |    1 -
 fs/aufs/wkq.h             |    4 ----
 fs/aufs/xino.c            |    2 --
 include/linux/aufs_type.h |   20 ++++++++++++++------
 43 files changed, 17 insertions(+), 90 deletions(-)
Signature · 1 line
1.7.2.5
#20 lore raw

PATCH 1/2 aufs: headers 1/2, bugfix, where the pr_fmt macro definition

From: J. R. Okajima <[email protected]>

The pr_fmt macro is defined in fs/aufs/Makefile and it refers to the AUFS_NAME macro, which caused a compilation error in m68k architecture. Also it refers to the "current" macro which will be a problem too.

See-also: http://sourceforge.net/mailarchive/message.php?msg_id=28608043

---
 fs/aufs/Makefile          |    4 ++--
 include/linux/aufs_type.h |   20 ++++++++++++++------
 2 files changed, 16 insertions(+), 8 deletions(-)
fs/aufs/Makefile +2 -2
index 0aadd21..dc8fd36 100644
@@ -4,8 +4,8 @@ include ${srctree}/${src}/magic.mk
 # cf. include/linux/kernel.h
 # enable pr_debug
 ccflags-y += -DDEBUG
-# sparse doesn't allow spaces
-ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"\040%s:%d:%s[%d]:\040"fmt,__func__,__LINE__,current->comm,current->pid'
+# sparse requires the full pathname
+ccflags-y += -include ${srctree}/include/linux/aufs_type.h
 
 obj-$(CONFIG_AUFS_FS) += aufs.o
 aufs-y := module.o sbinfo.o super.o branch.o xino.o sysaufs.o opts.o \
include/linux/aufs_type.h +14 -6
index 60b52b0..25cdc06 100644
@@ -19,15 +19,24 @@
 #ifndef __AUFS_TYPE_H__
 #define __AUFS_TYPE_H__
 
-#include <linux/ioctl.h>
-#include <linux/kernel.h>
-#include <linux/limits.h>
+#define AUFS_NAME	"aufs"
+
 #ifdef __KERNEL__
-#include <linux/types.h>
+/*
+ * define it before including all other headers.
+ * sched.h may use pr_* macros before defining "current", so define the
+ * no-current version first, and re-define later.
+ */
+#define pr_fmt(fmt)	AUFS_NAME " %s:%d: " fmt, __func__, __LINE__
+#include <linux/sched.h>
+#undef pr_fmt
+#define pr_fmt(fmt)	AUFS_NAME " %s:%d:%s[%d]: " fmt, \
+		__func__, __LINE__, current->comm, current->pid
 #else
+#include <linux/limits.h>
 #include <stdint.h>
 #include <sys/types.h>
-#endif
+#endif /* __KERNEL__ */
 
 #define AUFS_VERSION	"2.2-35"
 
@@ -58,7 +67,6 @@ typedef int16_t aufs_bindex_t;
 
 /* ---------------------------------------------------------------------- */
 
-#define AUFS_NAME		"aufs"
 #define AUFS_FSTYPE		AUFS_NAME
 
 #define AUFS_ROOT_INO		2
Signature · 1 line
1.7.2.5
#21 lore raw

PATCH 2/2 aufs: headers 2/2, simply refined

In reply to [email protected]

From: J. R. Okajima <[email protected]>

By the previous commit,
8dc5387 2012-01-03 aufs: bugfix, headers 1/2, where the pr_fmt macro definition several header file inclusion in other files became unnecessary.

---
 fs/aufs/branch.c  |    1 -
 fs/aufs/branch.h  |    2 --
 fs/aufs/cpup.c    |    2 --
 fs/aufs/cpup.h    |    4 ----
 fs/aufs/dbgaufs.h |    3 ---
 fs/aufs/dcsub.h   |    1 -
 fs/aufs/debug.c   |    1 -
 fs/aufs/debug.h   |    8 --------
 fs/aufs/dentry.h  |    1 -
 fs/aufs/dir.c     |    1 -
 fs/aufs/dir.h     |    1 -
 fs/aufs/dynop.h   |    4 ----
 fs/aufs/export.c  |    1 -
 fs/aufs/f_op.c    |    2 --
 fs/aufs/f_op_sp.c |    1 -
 fs/aufs/file.c    |    3 ---
 fs/aufs/file.h    |    1 -
 fs/aufs/finfo.c   |    1 -
 fs/aufs/fstype.h  |    1 -
 fs/aufs/hfsplus.c |    1 -
 fs/aufs/i_op.c    |    2 --
 fs/aufs/inode.h   |    2 --
 fs/aufs/ioctl.c   |    1 -
 fs/aufs/opts.c    |    2 --
 fs/aufs/opts.h    |    3 ---
 fs/aufs/rdu.c     |    2 --
 fs/aufs/rwsem.h   |    1 -
 fs/aufs/sbinfo.c  |    1 -
 fs/aufs/spl.h     |    4 ----
 fs/aufs/super.c   |    3 +--
 fs/aufs/super.h   |    1 -
 fs/aufs/sysaufs.c |    2 --
 fs/aufs/sysaufs.h |    2 --
 fs/aufs/sysfs.c   |    3 ---
 fs/aufs/sysrq.c   |    3 ---
 fs/aufs/vdir.c    |    1 -
 fs/aufs/vfsub.c   |    2 --
 fs/aufs/whout.c   |    1 -
 fs/aufs/whout.h   |    1 -
 fs/aufs/wkq.h     |    4 ----
 fs/aufs/xino.c    |    2 --
 41 files changed, 1 insertions(+), 82 deletions(-)
fs/aufs/branch.c +0 -1
index 49b32de..a8bb316 100644
@@ -21,7 +21,6 @@
  */
 
 #include <linux/compat.h>
-#include <linux/file.h>
 #include <linux/statfs.h>
 #include "aufs.h"
 
fs/aufs/branch.h +0 -2
index 1f39ad6..80b5084 100644
@@ -25,9 +25,7 @@
 
 #ifdef __KERNEL__
 
-#include <linux/fs.h>
 #include <linux/mount.h>
-#include <linux/aufs_type.h>
 #include "dynop.h"
 #include "rwsem.h"
 #include "super.h"
fs/aufs/cpup.c +0 -2
index 05cb4e0..70116ba 100644
@@ -20,10 +20,8 @@
  * copy-up functions, see wbr_policy.c for copy-down
  */
 
-#include <linux/file.h>
 #include <linux/fs_stack.h>
 #include <linux/mm.h>
-#include <linux/uaccess.h>
 #include "aufs.h"
 
 void au_cpup_attr_flags(struct inode *dst, struct inode *src)
fs/aufs/cpup.h +0 -4
index 8e0aa59..8493770 100644
@@ -25,10 +25,6 @@
 
 #ifdef __KERNEL__
 
-#include <linux/path.h>
-#include <linux/time.h>
-#include <linux/aufs_type.h>
-
 struct inode;
 struct file;
 
fs/aufs/dbgaufs.h +0 -3
index 2eaa3f0..a4065d1 100644
@@ -25,9 +25,6 @@
 
 #ifdef __KERNEL__
 
-#include <linux/init.h>
-#include <linux/aufs_type.h>
-
 struct super_block;
 struct au_sbinfo;
 
fs/aufs/dcsub.h +0 -1
index 1211304..439459c 100644
@@ -27,7 +27,6 @@
 
 #include <linux/dcache.h>
 #include <linux/fs.h>
-#include <linux/types.h>
 
 struct dentry;
 
fs/aufs/debug.c +0 -1
index 9fad1c3..83e7c57 100644
@@ -20,7 +20,6 @@
  * debug print functions
  */
 
-#include <linux/module.h>
 #include <linux/vt_kern.h>
 #include "aufs.h"
 
fs/aufs/debug.h +0 -8
index 7140934..1bcc88c 100644
@@ -26,17 +26,9 @@
 #ifdef __KERNEL__
 
 #include <asm/system.h>
-#include <linux/bug.h>
-/* #include <linux/err.h> */
-#include <linux/init.h>
 #include <linux/module.h>
 #include <linux/kallsyms.h>
-/* #include <linux/kernel.h> */
-#include <linux/delay.h>
-/* #include <linux/kd.h> */
-#include <linux/vt_kern.h>
 #include <linux/sysrq.h>
-#include <linux/aufs_type.h>
 
 #ifdef CONFIG_AUFS_DEBUG
 #define AuDebugOn(a)		BUG_ON(a)
fs/aufs/dentry.h +0 -1
index 4f08275..802635f 100644
@@ -26,7 +26,6 @@
 #ifdef __KERNEL__
 
 #include <linux/dcache.h>
-#include <linux/aufs_type.h>
 #include "rwsem.h"
 
 struct au_hdentry {
fs/aufs/dir.c +0 -1
index 6c45d1a..1e0a771 100644
@@ -20,7 +20,6 @@
  * directory operations
  */
 
-#include <linux/file.h>
 #include <linux/fs_stack.h>
 #include "aufs.h"
 
fs/aufs/dir.h +0 -1
index 2d0d8d2..99e973a 100644
@@ -26,7 +26,6 @@
 #ifdef __KERNEL__
 
 #include <linux/fs.h>
-#include <linux/aufs_type.h>
 
 /* ---------------------------------------------------------------------- */
 
fs/aufs/dynop.h +0 -4
index c3217d4..3ddebe7 100644
@@ -25,10 +25,6 @@
 
 #ifdef __KERNEL__
 
-#include <linux/fs.h>
-#include <linux/mm.h>
-#include <linux/rcupdate.h>
-#include <linux/aufs_type.h>
 #include "inode.h"
 
 enum {AuDy_AOP, AuDyLast};
fs/aufs/export.c +0 -1
index 992a799..2f29e1f 100644
@@ -21,7 +21,6 @@
  */
 
 #include <linux/exportfs.h>
-#include <linux/file.h>
 #include <linux/mnt_namespace.h>
 #include <linux/namei.h>
 #include <linux/nsproxy.h>
fs/aufs/f_op.c +0 -2
index eb6b909..372308d 100644
@@ -20,10 +20,8 @@
  * file and vm operations
  */
 
-#include <linux/file.h>
 #include <linux/fs_stack.h>
 #include <linux/mman.h>
-#include <linux/mm.h>
 #include <linux/security.h>
 #include "aufs.h"
 
fs/aufs/f_op_sp.c +0 -1
index 4f5cde8..0068ff2 100644
@@ -22,7 +22,6 @@
  * their file I/O is handled out of aufs.
  */
 
-#include <linux/fs_stack.h>
 #include "aufs.h"
 
 static ssize_t aufs_aio_read_sp(struct kiocb *kio, const struct iovec *iov,
fs/aufs/file.c +0 -3
index 5dc3d0e..257cc2b 100644
@@ -20,9 +20,6 @@
  * handling file/dir, and address_space operation
  */
 
-#include <linux/file.h>
-#include <linux/fsnotify.h>
-#include <linux/namei.h>
 #include <linux/pagemap.h>
 #include "aufs.h"
 
fs/aufs/file.h +0 -1
index 831dc5d..d2468de 100644
@@ -28,7 +28,6 @@
 #include <linux/file.h>
 #include <linux/fs.h>
 #include <linux/poll.h>
-#include <linux/aufs_type.h>
 #include "rwsem.h"
 
 struct au_branch;
fs/aufs/finfo.c +0 -1
index c1d71ba..8995db9 100644
@@ -20,7 +20,6 @@
  * file private data
  */
 
-#include <linux/file.h>
 #include "aufs.h"
 
 void au_hfput(struct au_hfile *hf, struct file *file)
fs/aufs/fstype.h +0 -1
index 3bdbd14..8ed557d 100644
@@ -28,7 +28,6 @@
 #include <linux/fs.h>
 #include <linux/magic.h>
 #include <linux/romfs_fs.h>
-#include <linux/aufs_type.h>
 
 static inline int au_test_aufs(struct super_block *sb)
 {
fs/aufs/hfsplus.c +0 -1
index 678b69c..6fa5ed8 100644
@@ -26,7 +26,6 @@
  * and au_h_open_post() after releasing it.
  */
 
-#include <linux/file.h>
 #include "aufs.h"
 
 struct file *au_h_open_pre(struct dentry *dentry, aufs_bindex_t bindex)
fs/aufs/i_op.c +0 -2
index 065bb76..7141158 100644
@@ -22,10 +22,8 @@
 
 #include <linux/device_cgroup.h>
 #include <linux/fs_stack.h>
-#include <linux/mm.h>
 #include <linux/namei.h>
 #include <linux/security.h>
-#include <linux/uaccess.h>
 #include "aufs.h"
 
 static int h_permission(struct inode *h_inode, int mask,
fs/aufs/inode.h +0 -2
index d0bd9c6..dffceae 100644
@@ -25,9 +25,7 @@
 
 #ifdef __KERNEL__
 
-#include <linux/fs.h>
 #include <linux/fsnotify.h>
-#include <linux/aufs_type.h>
 #include "rwsem.h"
 
 struct vfsmount;
fs/aufs/ioctl.c +0 -1
index 93151b3..2ca49c0 100644
@@ -22,7 +22,6 @@
  * assist the pathconf(3) wrapper library.
  */
 
-#include <linux/file.h>
 #include "aufs.h"
 
 static int au_wbr_fd(struct path *path, struct aufs_wbr_fd __user *arg)
fs/aufs/opts.c +0 -2
index 82a9697..84a1bc4 100644
@@ -20,8 +20,6 @@
  * mount options/flags
  */
 
-#include <linux/file.h>
-#include <linux/jiffies.h>
 #include <linux/namei.h>
 #include <linux/types.h> /* a distribution requires */
 #include <linux/parser.h>
fs/aufs/opts.h +0 -3
index 1ef3710..76b3119 100644
@@ -25,9 +25,6 @@
 
 #ifdef __KERNEL__
 
-#include <linux/path.h>
-#include <linux/aufs_type.h>
-
 struct file;
 struct super_block;
 
fs/aufs/rdu.c +0 -2
index 4e0756d..e77b6ff 100644
@@ -23,8 +23,6 @@
 #include <linux/compat.h>
 #include <linux/fs_stack.h>
 #include <linux/security.h>
-#include <linux/uaccess.h>
-#include <linux/aufs_type.h>
 #include "aufs.h"
 
 /* bits for struct aufs_rdu.flags */
fs/aufs/rwsem.h +0 -1
index dcd1def..3c9e7c3 100644
@@ -25,7 +25,6 @@
 
 #ifdef __KERNEL__
 
-#include <linux/rwsem.h>
 #include "debug.h"
 
 struct au_rwsem {
fs/aufs/sbinfo.c +0 -1
index d9141f8..0509d05 100644
@@ -20,7 +20,6 @@
  * superblock private data
  */
 
-#include <linux/jiffies.h>
 #include "aufs.h"
 
 /*
fs/aufs/spl.h +0 -4
index 37a0329..47a28d5 100644
@@ -25,10 +25,6 @@
 
 #ifdef __KERNEL__
 
-#include <linux/spinlock.h>
-#include <linux/list.h>
-#include <linux/rculist.h>
-
 struct au_splhead {
 	spinlock_t		spin;
 	struct list_head	head;
fs/aufs/super.c +1 -2
index 0ee7ceb..735cff3 100644
@@ -20,8 +20,7 @@
  * mount and super_block operations
  */
 
-#include <linux/buffer_head.h>
-#include <linux/jiffies.h>
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/seq_file.h>
 #include <linux/statfs.h>
fs/aufs/super.h +0 -1
index f490b4a..e810699 100644
@@ -26,7 +26,6 @@
 #ifdef __KERNEL__
 
 #include <linux/fs.h>
-#include <linux/aufs_type.h>
 #include "rwsem.h"
 #include "spl.h"
 #include "wkq.h"
fs/aufs/sysaufs.c +0 -2
index ab4036e..01c6fbd 100644
@@ -21,9 +21,7 @@
  * they are necessary regardless sysfs is disabled.
  */
 
-#include <linux/fs.h>
 #include <linux/random.h>
-#include <linux/sysfs.h>
 #include "aufs.h"
 
 unsigned long sysaufs_si_mask;
fs/aufs/sysaufs.h +0 -2
index 5b6b8d0..66b0fee 100644
@@ -25,8 +25,6 @@
 
 #ifdef __KERNEL__
 
-#include <linux/sysfs.h>
-#include <linux/aufs_type.h>
 #include "module.h"
 
 struct super_block;
fs/aufs/sysfs.c +0 -3
index 8afb350..a3a1f65 100644
@@ -20,10 +20,7 @@
  * sysfs interface
  */
 
-#include <linux/fs.h>
-#include <linux/module.h>
 #include <linux/seq_file.h>
-#include <linux/sysfs.h>
 #include "aufs.h"
 
 static struct attribute *au_attr[] = {
fs/aufs/sysrq.c +0 -3
index e10f555..072e5d9 100644
@@ -20,9 +20,6 @@
  * magic sysrq hanlder
  */
 
-#include <linux/fs.h>
-#include <linux/module.h>
-#include <linux/moduleparam.h>
 /* #include <linux/sysrq.h> */
 #include <linux/writeback.h>
 #include "aufs.h"
fs/aufs/vdir.c +0 -1
index 5e42376..db8c800 100644
@@ -20,7 +20,6 @@
  * virtual or vertical directory
  */
 
-#include <linux/hash.h>
 #include "aufs.h"
 
 static unsigned int calc_size(int nlen)
fs/aufs/vfsub.c +0 -2
index dda4547..28219c9 100644
@@ -20,12 +20,10 @@
  * sub-routines for VFS
  */
 
-#include <linux/file.h>
 #include <linux/ima.h>
 #include <linux/namei.h>
 #include <linux/security.h>
 #include <linux/splice.h>
-#include <linux/uaccess.h>
 #include "aufs.h"
 
 int vfsub_update_h_iattr(struct path *h_path, int *did)
fs/aufs/whout.c +0 -1
index 61545ea..5d5ba13 100644
@@ -20,7 +20,6 @@
  * whiteout for logical deletion and opaque directory
  */
 
-#include <linux/fs.h>
 #include "aufs.h"
 
 #define WH_MASK			S_IRUGO
fs/aufs/whout.h +0 -1
index 11137f4..3a82828 100644
@@ -25,7 +25,6 @@
 
 #ifdef __KERNEL__
 
-#include <linux/aufs_type.h>
 #include "dir.h"
 
 /* whout.c */
fs/aufs/wkq.h +0 -4
index 23a6d30..8b06193 100644
@@ -26,10 +26,6 @@
 
 #ifdef __KERNEL__
 
-#include <linux/sched.h>
-#include <linux/wait.h>
-#include <linux/aufs_type.h>
-
 struct super_block;
 
 /* ---------------------------------------------------------------------- */
fs/aufs/xino.c +0 -2
index 5a44272..20fc652 100644
@@ -20,9 +20,7 @@
  * external inode number translation table and bitmap
  */
 
-#include <linux/file.h>
 #include <linux/seq_file.h>
-#include <linux/uaccess.h>
 #include "aufs.h"
 
 /* todo: unnecessary to support mmap_sem since kernel-space? */
Signature · 9 lines
1.7.2.5


------------------------------------------------------------------------------
Write once. Port to many.
Get the SDK and tools to simplify cross-platform app development. Create 
new or port existing apps to sell to consumers worldwide. Explore the 
Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
http://p.sf.net/sfu/intel-appdev
#22 lore raw
In reply to Thorsten Glaser

Dixi quod…

Quoted 2 lines

This one does work:
http://lists.debian.org/debian-kernel/2011/12/msg00474.html

Geert, would you please apply the attached patch, independent of whether we are able to find a solution in aufs, as it avoids an entire problem class?

Groeten uit Duitsland,
//mirabilos

Signature · 4 lines
[...] if maybe ext3fs wasn't a better pick, or jfs, or maybe reiserfs, oh but
what about xfs, and if only i had waited until reiser4 was ready... in the be-
ginning, there was ffs, and in the middle, there was ffs, and at the end, there
was still ffs, and the sys admins knew it was good. :)  -- Ted Unangst über *fs

From fda2529371f8cc8879877ca3f3eb9dfbaafb9755 Mon Sep 17 00:00:00 2001
From: Thorsten Glaser <[email protected]>
Date: Sat, 31 Dec 2011 22:48:52 +0000
Subject: [PATCH] replace inline ack_bad_irq with a block macro to avoid error
propagation

If any file including (directly or indirectly) hardirq.h redefines pr_fmt to something containing some value not available at hardirq inclusion time, defining a static inline function fails, whereas a macro can always be defined (and need not even be expanded once in the translation unit with pr_fmt redefined while all static inline functions will always be expanded).

---
 arch/m68k/include/asm/hardirq.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
arch/m68k/include/asm/hardirq.h +4 -4
index db30ed2..a2e00e7 100644
@@ -18,10 +18,10 @@
 
 #ifdef CONFIG_MMU
 
-static inline void ack_bad_irq(unsigned int irq)
-{
-	pr_crit("unexpected IRQ trap at vector %02x\n", irq);
-}
+#define ack_bad_irq(irq) do {					\
+	pr_crit("unexpected IRQ trap at vector %02x\n",		\
+	    (unsigned int)(irq));				\
+} while (/* CONSTCOND */ 0)
 
 /* entry.S is sensitive to the offsets of these fields */
 typedef struct {
Signature · 1 line
1.7.7.3
#23 lore raw

Hi Thorsten,

Quoted 8 lines · Thorsten Glaser

Dixi quod…

Quoted 2 lines

This one does work:
http://lists.debian.org/debian-kernel/2011/12/msg00474.html

Geert, would you please apply the attached patch, independent of whether we are able to find a solution in aufs, as it avoids an entire problem class?

Sorry, static inline functions are preferred over macros, unless there's a really good reason. Out-of-tree kernel code doing stupid things doesn't count.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds

#24 lore raw

Geert Uytterhoeven dixit:

Quoted 1 line

Sorry, static inline functions are preferred over macros, unless there's

Can I have reasons? (Also out of curiosity.)

Quoted 1 line

a really good reason. Out-of-tree kernel code doing stupid things doesn't

I see it like this:

EVERY file that, directly or indirectly, includes hardirq.h has the overhead of having GCC compile that static inline function, even if it’s never used (and thus never emitted into the object file). Also, redefining pr_fmt appears to be a desired thing, not a stupid thing; if a static inline function uses pr_fmt in the timeframe between it being redefined and the redefined one’s prerequisites being availab- le, it would necessarily fail.

In contrast to that, a cpp macro is only evaluated when it’s actual- ly needed, and by then, the prerequisites would probably be availab- le. It also has less compile-time overhead (in this case, since the hardirq.h file is probably included in more files than it is needed, whereas macros slow down when you use them more than once in all fi- les they’re defined in.

bye,
//mirabilos

Signature · 4 lines
Sometimes they [people] care too much: pretty printers [and syntax highligh-
ting, d.A.] mechanically produce pretty output that accentuates irrelevant
detail in the program, which is as sensible as putting all the prepositions
in English text in bold font.	-- Rob Pike in "Notes on Programming in C"