aufs3: remove include of linux/fs.h from linux/mm.h
(resending since original post was rejected by aufs-users)
This include is added by aufs3-mmap.patch but causes circular
dependencies on arm64 as seen with the Debian kernel packages in
http://buildd.debian-ports.org/status/fetch.php?pkg=linux&arch=arm64&ver=3.14.12-1&stamp=1405234443
which contains:
In file included from /«PKGBUILDDIR»/include/linux/mm.h:23:0,
from /«PKGBUILDDIR»/include/linux/pid_namespace.h:6,
from /«PKGBUILDDIR»/include/linux/ptrace.h:9,
from /«PKGBUILDDIR»/arch/arm64/include/asm/compat.h:26,
from /«PKGBUILDDIR»/arch/arm64/include/asm/stat.h:23,
from /«PKGBUILDDIR»/include/linux/stat.h:5,
from /«PKGBUILDDIR»/include/linux/module.h:10,
from /«PKGBUILDDIR»/init/main.c:15:
/«PKGBUILDDIR»/include/linux/fs.h:1575:64: warning: 'struct kstat' declared
inside parameter list [enabled by default]
int (*getattr) (struct vfsmount *mnt, struct dentry *, struct kstat *);
According to http://article.gmane.org/gmane.linux.ports.arm.kernel/342042
> The added mm.h->fs.h looks like a mistake, it should not be there, and we have
> in the past worked hard to separate mm.h, sched.h and fs.h from one another.
By turning the various additions to linux/mm.h into macros rather than static
inlines we can avoid the need for the additional includes. Convert all but the
vm?_pr_or_file functions which don't require additional includes and are a bit
trickier to turn into macros (due to having a return value).
Also take the opportunity to wrap the vmr_* macros in ifndef CONFIG_MMU
Signed-off-by: Ian Campbell
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -18,9 +18,6 @@
#include
#include
#include
-#include
-#include
-#include
struct mempolicy;
struct anon_vma;
@@ -1170,6 +1167,7 @@
#endif
}
+#ifndef CONFIG_MMU
static inline struct file *vmr_do_pr_or_file(struct vm_region *region,
const char func[], int line)
{
@@ -1178,25 +1176,29 @@
return (f && pr) ? pr : f;
}
-static inline void vmr_do_fput(struct vm_region *region,
- const char func[], int line)
-{
- struct file *f = region->vm_file, *pr = region->vm_prfile;
- aufs_trace(f, pr, func, line, __func__);
- fput(f);
- if (f && pr)
- fput(pr);
-}
+#define vmr_pr_or_file(region) vmr_do_pr_or_file(region, __func__, \
+ __LINE__)
-static inline void vma_do_file_update_time(struct vm_area_struct *vma,
- const char func[], int line)
-{
- struct file *f = vma->vm_file, *pr = vma->vm_prfile;
- aufs_trace(f, pr, func, line, __func__);
- file_update_time(f);
- if (f && pr)
- file_update_time(pr);
-}
+#define vmr_fput(region) do { \
+ struct vm_region *_region = region; \
+ struct file *f = _region->vm_file, *pr = _region->vm_prfile;\
+ aufs_trace(f, pr, __func__, __LINE__, vmr_fput);\
+ fput(f);\
+ if (f && pr)\
+ fput(pr); \
+} while(0);
+
+#endif
+
+#define vma_file_update_time(vma) {\
+ struct vm_area_struct *_vma = vma; \
+ struct file *f = _vma->vm_file, *pr = _vma->vm_prfile; \
+ aufs_trace(f, pr, __func__, __LINE__, \
+ "vma_file_update_time"); \
+ file_update_time(f);\
+ if (f && pr)\
+ file_update_time(pr); \
+} while (0)
static inline struct file *vma_do_pr_or_file(struct vm_area_struct *vma,
const char func[], int line)
@@ -1206,35 +1208,26 @@
return (f && pr) ? pr : f;
}
-static inline void vma_do_get_file(struct vm_area_struct *vma,
- const char func[], int line)
-{
- struct file *f = vma->vm_file, *pr = vma->vm_prfile;
- aufs_trace(f, pr, func, line, __func__);
- get_file(f);
- if (f && pr)
- get_file(pr);
-}
+#define vma_pr_or_file(vma)vma_do_pr_or_file(vma, __func__, \
+__LINE__)
-static inline void vma_do_fput(struct vm_area_struct *vma,
- const char func[], int line)
-{
- struct file *f = vma->vm_file, *pr = vma->vm_prfile;
- au
Re: aufs3: remove include of linux/fs.h from linux/mm.h
On Sun, 2014-07-20 at 02:38 +0900, sf...@users.sourceforge.net wrote: > Hello Ian, > > Ian Campbell: > > This include is added by aufs3-mmap.patch but causes circular > > dependencies on arm64 as seen with the Debian kernel packages in > ::: > > According to http://article.gmane.org/gmane.linux.ports.arm.kernel/342042 > > > The added mm.h->fs.h looks like a mistake, it should not be there, and we= > > have > > > in the past worked hard to separate mm.h, sched.h and fs.h from one anoth= > > er. > > Hmm, I didn't know such history... > Anyway I agree mmap approach in aufs is ugly in the terms of its logic > and looking. And I am afraid converting into macros looks still > bad-looking. Do you think can we do this? > - create a new file mm/aufs_mmap.c > - move the inline funcs to the new file. > - the calling macros are left in mm.h. > - function delarations are added in mm.h. That sounds like a plausible plan to me. > > Also take the opportunity to wrap the vmr_* macros in ifndef CONFIG_MMU > > Is it necessary? > struct vm_regin is defined regardless CONFIG_MMU, isn't it? It is defined but not used except when !CONFIG_MMU and the comment preceding the definition confirms it is !MMU specifc. The type is only used in fs/proc/nommu.c, fs/proc/task_nommu.c and mm/nommu.c. The only users of the two vmr_* macros/inlines are in mm/nommu.c. Ian. -- Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds
Re: aufs3: remove include of linux/fs.h from linux/mm.h
On Sat, 2014-07-19 at 20:40 +0100, Ian Campbell wrote:
> On Sun, 2014-07-20 at 02:38 +0900, sf...@users.sourceforge.net wrote:
> > Hello Ian,
> >
> > Ian Campbell:
> > > This include is added by aufs3-mmap.patch but causes circular
> > > dependencies on arm64 as seen with the Debian kernel packages in
> > :::
> > > According to http://article.gmane.org/gmane.linux.ports.arm.kernel/342042
> > > > The added mm.h->fs.h looks like a mistake, it should not be there, and
> > > > we=
> > > have
> > > > in the past worked hard to separate mm.h, sched.h and fs.h from one
> > > > anoth=
> > > er.
> >
> > Hmm, I didn't know such history...
> > Anyway I agree mmap approach in aufs is ugly in the terms of its logic
> > and looking. And I am afraid converting into macros looks still
> > bad-looking. Do you think can we do this?
> > - create a new file mm/aufs_mmap.c
> > - move the inline funcs to the new file.
> > - the calling macros are left in mm.h.
> > - function delarations are added in mm.h.
>
> That sounds like a plausible plan to me.
The following patch which does what I think you are suggesting works OK
for me too.
8<--
From: Ian Campbell
Subject: aufs3: remove include of linux/fs.h from linux/mm.h
This include is added by aufs3-mmap.patch but causes circular dependencies on
arm64 as seen with the Debian kernel packages in
http://buildd.debian-ports.org/status/fetch.php?pkg=linux&arch=arm64&ver=3.14.12-1&stamp=1405234443
which contains:
In file included from /«PKGBUILDDIR»/include/linux/mm.h:23:0,
from /«PKGBUILDDIR»/include/linux/pid_namespace.h:6,
from /«PKGBUILDDIR»/include/linux/ptrace.h:9,
from /«PKGBUILDDIR»/arch/arm64/include/asm/compat.h:26,
from /«PKGBUILDDIR»/arch/arm64/include/asm/stat.h:23,
from /«PKGBUILDDIR»/include/linux/stat.h:5,
from /«PKGBUILDDIR»/include/linux/module.h:10,
from /«PKGBUILDDIR»/init/main.c:15:
/«PKGBUILDDIR»/include/linux/fs.h:1575:64: warning: 'struct kstat' declared
inside parameter list [enabled by default]
int (*getattr) (struct vfsmount *mnt, struct dentry *, struct kstat *);
According to http://article.gmane.org/gmane.linux.ports.arm.kernel/342042
> The added mm.h->fs.h looks like a mistake, it should not be there, and we have
> in the past worked hard to separate mm.h, sched.h and fs.h from one another.
Move all of the static inline functions added to mm.h by
aufs3-mmap.patch into a new file, mm/aufs_mmap.c, instead to avoid the
new includes in mm.h.
Signed-off-by: Ian Campbell
Index: linux/include/linux/mm.h
===
--- linux.orig/include/linux/mm.h 2014-07-20 08:34:40.0 +0100
+++ linux/include/linux/mm.h2014-07-20 08:42:47.625617449 +0100
@@ -18,9 +18,6 @@
#include
#include
#include
-#include
-#include
-#include
struct mempolicy;
struct anon_vma;
@@ -1155,76 +1152,16 @@
}
#endif
-/*
- * Mainly for aufs which mmap(2) diffrent file and wants to print different
path
- * in /proc/PID/maps.
- */
-/* #define AUFS_DEBUG_MMAP */
-static inline void aufs_trace(struct file *f, struct file *pr,
- const char func[], int line, const char func2[])
-{
-#ifdef AUFS_DEBUG_MMAP
- if (pr)
- pr_info("%s:%d: %s, %p\n", func, line, func2,
- f ? (char *)f->f_dentry->d_name.name : "(null)");
-#endif
-}
-
-static inline struct file *vmr_do_pr_or_file(struct vm_region *region,
-const char func[], int line)
-{
- struct file *f = region->vm_file, *pr = region->vm_prfile;
- aufs_trace(f, pr, func, line, __func__);
- return (f && pr) ? pr : f;
-}
-
-static inline void vmr_do_fput(struct vm_region *region,
- const char func[], int line)
-{
- struct file *f = region->vm_file, *pr = region->vm_prfile;
- aufs_trace(f, pr, func, line, __func__);
- fput(f);
- if (f && pr)
- fput(pr);
-}
-
-static inline void vma_do_file_update_time(struct vm_area_struct *vma,
- const char func[], int line)
-{
- struct file *f = vma->vm_file, *pr = vma->vm_prfile;
- aufs_trace(f, pr, func, line, __func__);
- file_update_time(f);
- if (f && pr)
- file_update_time(pr);
-}
-
-static inline struct file *vma_do_pr_or_file(struct vm_area_struct *vma,
-const char func[], int line)
-{
- struct file *f = vma->vm_file, *pr = vma->vm_prfile;
- aufs_trace(f,
Re: aufs3: remove include of linux/fs.h from linux/mm.h
On Mon, 2014-07-21 at 04:47 +0900, sf...@users.sourceforge.net wrote: > Ian Campbell: > > The following patch which does what I think you are suggesting works OK > > for me too. > > Thanks for the patch. > #ifndef CONFIG_MMU was less important since the built kernel doesn't > contain the functions defined in the header as long as they are unused. > But by moving them into a source file, the built kernel will contain > them. So ifndef CONFIG_MMU is necessary this time. Yes, good point. > I will merge your patch with adding ifndef CONFIG_MMU for vmr_* > functions, and release next week. I hope you don't mind your > Singed-off-by is left after my minor modification. That's absolutely fine. Thanks, Ian. -- Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds
Re: aufs3: remove include of linux/fs.h from linux/mm.h
On Mon, 2014-07-21 at 16:01 +0900, sf...@users.sourceforge.net wrote: > Ian Campbell: > > In file included from /=ABPKGBUILDDIR=BB/include/linux/mm.h:23:0, > > from /=ABPKGBUILDDIR=BB/include/linux/pid_namespace.h:6, > > from /=ABPKGBUILDDIR=BB/include/linux/ptrace.h:9, > > from /=ABPKGBUILDDIR=BB/arch/arm64/include/asm/compat.h:26, > > from /=ABPKGBUILDDIR=BB/arch/arm64/include/asm/stat.h:23, > > from /=ABPKGBUILDDIR=BB/include/linux/stat.h:5, > > from /=ABPKGBUILDDIR=BB/include/linux/module.h:10, > > from /=ABPKGBUILDDIR=BB/init/main.c:15: > > /=ABPKGBUILDDIR=BB/include/linux/fs.h:1575:64: warning: 'struct kstat' decl= > > ared inside parameter list [enabled by default] > > int (*getattr) (struct vfsmount *mnt, struct dentry *, struct kstat *); > > I will definitely merge and release the patch we made, but I want you to > confirm one thing. > If the problem is the declaraion of kstat only, then declaring it in > fs.h will solve the problem too? I think it might, I'm not sure though. > Or other similar problems will happen? There were some other errors in the original buildd log at http://buildd.debian-ports.org/status/fetch.php?pkg=linux&arch=arm64&ver=3.14.12-1&stamp=1405234443 but they were resolved as part of this patch too so I didn't investigate much further. Some of them were missing prototypes for get_file and file_update_time. The one I'm least sure about is the missing S_IS??? defines. I think most of those would be hard to solve with a fwd declaration without duplicating the complete definition. Cheers, Ian. -- Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds
