Re: aufs3: remove include of linux/fs.h from linux/mm.h
Re-reading several makefiles, - fs/proc/Makefile builds nommu.o and task_nommu.o unconditionally. - mm/Makeilfe builds nommu.o unconditionally. So I change my mind and stop adding ifndef CONFIG_MMU. Oops! I was wrong. Sorry. Please ignore my previous mail. J. R. Okajima -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/32004.1405921952@jrobl
Re: aufs3: remove include of linux/fs.h from linux/mm.h
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? Or other similar problems will happen? diff --git a/include/linux/fs.h b/include/linux/fs.h index 5c49108..37aac7b 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -39,6 +39,7 @@ struct kiocb; struct kobject; struct pipe_inode_info; struct poll_table_struct; +struct kstat; struct kstatfs; struct vm_area_struct; struct vfsmount; J. R. Okajima -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/5594.1405926069@jrobl
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=linuxarch=arm64ver=3.14.12-1stamp=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. -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/1405931005.14973.8.ca...@kazak.uk.xensource.com
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 i...@hellion.org.uk
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=linuxarch=arm64ver=3.14.12-1stamp=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 i...@hellion.org.uk
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 linux/pfn.h
#include linux/bit_spinlock.h
#include linux/shrinker.h
-#include linux/dcache.h
-#include linux/file.h
-#include linux/fs.h
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, pr, func, line, __func__);
- 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
Re: aufs3: remove include of linux/fs.h from linux/mm.h
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. 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. J. R. Okajima -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/29680.1405885673@jrobl
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. -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/1405887161.8013.2.ca...@hastur.hellion.org.uk
Re: aufs3: remove include of linux/fs.h from linux/mm.h
Ian Campbell: #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. Re-reading several makefiles, - fs/proc/Makefile builds nommu.o and task_nommu.o unconditionally. - mm/Makeilfe builds nommu.o unconditionally. So I change my mind and stop adding ifndef CONFIG_MMU. J. R. Okajima -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/31818.1405920789@jrobl
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=linuxarch=arm64ver=3.14.12-1stamp=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 i...@hellion.org.uk
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -18,9 +18,6 @@
#include linux/pfn.h
#include linux/bit_spinlock.h
#include linux/shrinker.h
-#include linux/dcache.h
-#include linux/file.h
-#include linux/fs.h
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;
- aufs_trace(f, pr, func, line, __func__);
- fput(f);
- if (f pr)
- fput(pr);
-}
-
-#define vmr_pr_or_file(region)
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=linuxarch=arm64ver=3.14.12-1stamp=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 i...@hellion.org.uk
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -18,9 +18,6 @@
#include linux/pfn.h
#include linux/bit_spinlock.h
#include linux/shrinker.h
-#include linux/dcache.h
-#include linux/file.h
-#include linux/fs.h
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;
- aufs_trace(f, pr, func, line, __func__);
- fput(f);
- if (f pr)
-
Re: aufs3: remove include of linux/fs.h from linux/mm.h
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. 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? J. R. Okajima -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/13295.1405791504@jrobl
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. -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/1405798845.27009.30.ca...@dagon.hellion.org.uk
