ext4_extents.h was included twice.
Signed-off-by: Sachin Kamat <[email protected]>
---
fs/ext4/super.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ad6cd8a..c8a6138 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -50,7 +50,6 @@
#include "xattr.h"
#include "acl.h"
#include "mballoc.h"
-#include "ext4_extents.h"
#define CREATE_TRACE_POINTS
#include <trace/events/ext4.h>
--
1.7.4.1
On Mon, Nov 19, 2012 at 04:47:22PM +0530, Sachin Kamat wrote:
> ext4_extents.h was included twice.
>
> Signed-off-by: Sachin Kamat <[email protected]>
> ---
> fs/ext4/super.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index ad6cd8a..c8a6138 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -50,7 +50,6 @@
> #include "xattr.h"
> #include "acl.h"
> #include "mballoc.h"
> -#include "ext4_extents.h"
Hi Sachin,
Sorry, I don't find this duplicated code in mainline kernel 3.7-rc6.
Regards,
- Zheng
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/ext4.h>
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 19, 2012 at 09:39:45PM +0800, Zheng Liu wrote:
> Hi Sachin,
>
> Sorry, I don't find this duplicated code in mainline kernel 3.7-rc6.
It's there because ext4.h includes ext4_extents.h -- at the end of the
header file, where it's not quite as obvious.
What we should probably do is move the function declarations into
ext4.h, and then see if we can isolate the number of fs/ext4/*.c files
that are aware of the on-disk extents encoding, such that it doesn't
make sense to #include ext4_extenst.h from the ext4.h header file.
It's mainly a cleanup thing, but it would probably also help if we
ever want to support alternate extents encodings (for example to
support a full 64-bit physical block numbers, or more likely, more
than 32 bits worth of logical block nunbers --- so we can test large
file systems natively using ext4, instead of using xfs, which is what
I currently do). That's a low priority thing in my book, but if
someone is interesting in taking on the project, they should let me
know.
- Ted
Hi Zheng,
On 19 November 2012 19:09, Zheng Liu <[email protected]> wrote:
> On Mon, Nov 19, 2012 at 04:47:22PM +0530, Sachin Kamat wrote:
>> ext4_extents.h was included twice.
>>
>> Signed-off-by: Sachin Kamat <[email protected]>
>> ---
>> fs/ext4/super.c | 1 -
>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index ad6cd8a..c8a6138 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -50,7 +50,6 @@
>> #include "xattr.h"
>> #include "acl.h"
>> #include "mballoc.h"
>> -#include "ext4_extents.h"
>
> Hi Sachin,
>
> Sorry, I don't find this duplicated code in mainline kernel 3.7-rc6.
The duplicated code is available on the linux-next tree (20121115).
The second instance was added by the commit
"ext4: let ext4 maintain extent status tree" (commit id: 51865fda).
>
> Regards,
> - Zheng
>
>>
>> #define CREATE_TRACE_POINTS
>> #include <trace/events/ext4.h>
>> --
>> 1.7.4.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
With warm regards,
Sachin
On Mon, Nov 19, 2012 at 10:00:00AM -0500, Theodore Ts'o wrote:
> On Mon, Nov 19, 2012 at 09:39:45PM +0800, Zheng Liu wrote:
> > Hi Sachin,
> >
> > Sorry, I don't find this duplicated code in mainline kernel 3.7-rc6.
>
> It's there because ext4.h includes ext4_extents.h -- at the end of the
> header file, where it's not quite as obvious.
Ah, I see. Thanks for pointing out.
>
> What we should probably do is move the function declarations into
> ext4.h, and then see if we can isolate the number of fs/ext4/*.c files
> that are aware of the on-disk extents encoding, such that it doesn't
> make sense to #include ext4_extenst.h from the ext4.h header file.
>
> It's mainly a cleanup thing, but it would probably also help if we
> ever want to support alternate extents encodings (for example to
> support a full 64-bit physical block numbers, or more likely, more
> than 32 bits worth of logical block nunbers --- so we can test large
> file systems natively using ext4, instead of using xfs, which is what
> I currently do). That's a low priority thing in my book, but if
> someone is interesting in taking on the project, they should let me
> know.
Cool! Thanks for sharing this information with us.
Regards,
- Zheng
On Mon, Nov 19, 2012 at 08:36:06PM +0530, Sachin Kamat wrote:
> Hi Zheng,
>
> On 19 November 2012 19:09, Zheng Liu <[email protected]> wrote:
> > On Mon, Nov 19, 2012 at 04:47:22PM +0530, Sachin Kamat wrote:
> >> ext4_extents.h was included twice.
> >>
> >> Signed-off-by: Sachin Kamat <[email protected]>
> >> ---
> >> fs/ext4/super.c | 1 -
> >> 1 files changed, 0 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> >> index ad6cd8a..c8a6138 100644
> >> --- a/fs/ext4/super.c
> >> +++ b/fs/ext4/super.c
> >> @@ -50,7 +50,6 @@
> >> #include "xattr.h"
> >> #include "acl.h"
> >> #include "mballoc.h"
> >> -#include "ext4_extents.h"
> >
> > Hi Sachin,
> >
> > Sorry, I don't find this duplicated code in mainline kernel 3.7-rc6.
>
> The duplicated code is available on the linux-next tree (20121115).
> The second instance was added by the commit
> "ext4: let ext4 maintain extent status tree" (commit id: 51865fda).
Good catch. Sorry for my fault that introduces this duplicated code.
The patch looks good to me.
Regards,
- Zheng
Hi Theodore,
On 19 November 2012 21:53, Zheng Liu <[email protected]> wrote:
> On Mon, Nov 19, 2012 at 08:36:06PM +0530, Sachin Kamat wrote:
>> Hi Zheng,
>>
>> On 19 November 2012 19:09, Zheng Liu <[email protected]> wrote:
>> > On Mon, Nov 19, 2012 at 04:47:22PM +0530, Sachin Kamat wrote:
>> >> ext4_extents.h was included twice.
>> >>
>> >> Signed-off-by: Sachin Kamat <[email protected]>
>> >> ---
>> >> fs/ext4/super.c | 1 -
>> >> 1 files changed, 0 insertions(+), 1 deletions(-)
>> >>
>> >> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> >> index ad6cd8a..c8a6138 100644
>> >> --- a/fs/ext4/super.c
>> >> +++ b/fs/ext4/super.c
>> >> @@ -50,7 +50,6 @@
>> >> #include "xattr.h"
>> >> #include "acl.h"
>> >> #include "mballoc.h"
>> >> -#include "ext4_extents.h"
>> >
>> > Hi Sachin,
>> >
>> > Sorry, I don't find this duplicated code in mainline kernel 3.7-rc6.
>>
>> The duplicated code is available on the linux-next tree (20121115).
>> The second instance was added by the commit
>> "ext4: let ext4 maintain extent status tree" (commit id: 51865fda).
>
> Good catch. Sorry for my fault that introduces this duplicated code.
> The patch looks good to me.
>
> Regards,
> - Zheng
This is just a simple patch to remove the include statement coded
twice in the file.
Please pick this up.
--
With warm regards,
Sachin
ping
On 22 November 2012 10:43, Sachin Kamat <[email protected]> wrote:
> Hi Theodore,
>
> On 19 November 2012 21:53, Zheng Liu <[email protected]> wrote:
>> On Mon, Nov 19, 2012 at 08:36:06PM +0530, Sachin Kamat wrote:
>>> Hi Zheng,
>>>
>>> On 19 November 2012 19:09, Zheng Liu <[email protected]> wrote:
>>> > On Mon, Nov 19, 2012 at 04:47:22PM +0530, Sachin Kamat wrote:
>>> >> ext4_extents.h was included twice.
>>> >>
>>> >> Signed-off-by: Sachin Kamat <[email protected]>
>>> >> ---
>>> >> fs/ext4/super.c | 1 -
>>> >> 1 files changed, 0 insertions(+), 1 deletions(-)
>>> >>
>>> >> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>>> >> index ad6cd8a..c8a6138 100644
>>> >> --- a/fs/ext4/super.c
>>> >> +++ b/fs/ext4/super.c
>>> >> @@ -50,7 +50,6 @@
>>> >> #include "xattr.h"
>>> >> #include "acl.h"
>>> >> #include "mballoc.h"
>>> >> -#include "ext4_extents.h"
>>> >
>>> > Hi Sachin,
>>> >
>>> > Sorry, I don't find this duplicated code in mainline kernel 3.7-rc6.
>>>
>>> The duplicated code is available on the linux-next tree (20121115).
>>> The second instance was added by the commit
>>> "ext4: let ext4 maintain extent status tree" (commit id: 51865fda).
>>
>> Good catch. Sorry for my fault that introduces this duplicated code.
>> The patch looks good to me.
>>
>> Regards,
>> - Zheng
>
> This is just a simple patch to remove the include statement coded
> twice in the file.
> Please pick this up.
>
> --
> With warm regards,
> Sachin
--
With warm regards,
Sachin
This is the patch which I plan to be using instead. There is more
cleanup that could be done, but I assume the reason the reason why you
were interested in this was to reduce ext4's compile time (we have
protection against double inclusion, so it otherwise wouldn't have
made much difference). So this approach (which I had suggested and
had been hoping you would do :-) is better in that regard...
- Ted
commit 4a092d737955301da22b9d5e07f5036da821a932
Author: Theodore Ts'o <[email protected]>
Date: Wed Nov 28 13:03:30 2012 -0500
ext4: rationalize ext4_extents.h inclusion
Previously, ext4_extents.h was being included at the end of ext4.h,
which was bad for a number of reasons: (a) it was not being included
in the expected place, and (b) it caused the header to be included
multiple times. There were #ifdef's to prevent this from causing any
problems, but it still was unnecessary.
By moving the function declarations that were in ext4_extents.h to
ext4.h, which is standard practice for where the function declarations
for the rest of ext4.h can be found, we can remove ext4_extents.h from
being included in ext4.h at all, and then we can only include
ext4_extents.h where it is needed in ext4's source files.
It should be possible to move a few more things into ext4.h, and
further reduce the number of source files that need to #include
ext4_extents.h, but that's a cleanup for another day.
Reported-by: Sachin Kamat <[email protected]>
Reported-by: Wei Yongjun <[email protected]>
Signed-off-by: "Theodore Ts'o" <[email protected]>
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 246e38f..2e9ffa9 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -57,6 +57,16 @@
#define ext4_debug(fmt, ...) no_printk(fmt, ##__VA_ARGS__)
#endif
+/*
+ * Turn on EXT_DEBUG to get lots of info about extents operations.
+ */
+#define EXT_DEBUG__
+#ifdef EXT_DEBUG
+#define ext_debug(fmt, ...) printk(fmt, ##__VA_ARGS__)
+#else
+#define ext_debug(fmt, ...) no_printk(fmt, ##__VA_ARGS__)
+#endif
+
#define EXT4_ERROR_INODE(inode, fmt, a...) \
ext4_error_inode((inode), __func__, __LINE__, 0, (fmt), ## a)
@@ -2399,6 +2409,9 @@ extern int ext4_check_blockref(const char *, unsigned int,
struct inode *, __le32 *, unsigned int);
/* extents.c */
+struct ext4_ext_path;
+struct ext4_extent;
+
extern int ext4_ext_tree_init(handle_t *handle, struct inode *);
extern int ext4_ext_writepage_trans_blocks(struct inode *, int);
extern int ext4_ext_index_trans_blocks(struct inode *inode, int nrblocks,
@@ -2416,8 +2429,27 @@ extern int ext4_convert_unwritten_extents(struct inode *inode, loff_t offset,
ssize_t len);
extern int ext4_map_blocks(handle_t *handle, struct inode *inode,
struct ext4_map_blocks *map, int flags);
+extern int ext4_ext_calc_metadata_amount(struct inode *inode,
+ ext4_lblk_t lblocks);
+extern int ext4_extent_tree_init(handle_t *, struct inode *);
+extern int ext4_ext_calc_credits_for_single_extent(struct inode *inode,
+ int num,
+ struct ext4_ext_path *path);
+extern int ext4_can_extents_be_merged(struct inode *inode,
+ struct ext4_extent *ex1,
+ struct ext4_extent *ex2);
+extern int ext4_ext_insert_extent(handle_t *, struct inode *,
+ struct ext4_ext_path *,
+ struct ext4_extent *, int);
+extern struct ext4_ext_path *ext4_ext_find_extent(struct inode *, ext4_lblk_t,
+ struct ext4_ext_path *);
+extern void ext4_ext_drop_refs(struct ext4_ext_path *);
+extern int ext4_ext_check_inode(struct inode *inode);
+extern int ext4_find_delalloc_cluster(struct inode *inode, ext4_lblk_t lblk);
extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
__u64 start, __u64 len);
+
+
/* move_extent.c */
extern int ext4_move_extents(struct file *o_filp, struct file *d_filp,
__u64 start_orig, __u64 start_donor,
@@ -2505,6 +2537,4 @@ extern void ext4_resize_end(struct super_block *sb);
#endif /* __KERNEL__ */
-#include "ext4_extents.h"
-
#endif /* _EXT4_H */
diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
index 173b6c5..487fda1 100644
--- a/fs/ext4/ext4_extents.h
+++ b/fs/ext4/ext4_extents.h
@@ -43,16 +43,6 @@
#define CHECK_BINSEARCH__
/*
- * Turn on EXT_DEBUG to get lots of info about extents operations.
- */
-#define EXT_DEBUG__
-#ifdef EXT_DEBUG
-#define ext_debug(fmt, ...) printk(fmt, ##__VA_ARGS__)
-#else
-#define ext_debug(fmt, ...) no_printk(fmt, ##__VA_ARGS__)
-#endif
-
-/*
* If EXT_STATS is defined then stats numbers are collected.
* These number will be displayed at umount time.
*/
@@ -286,20 +276,5 @@ static inline void ext4_idx_store_pblock(struct ext4_extent_idx *ix,
0xffff);
}
-extern int ext4_ext_calc_metadata_amount(struct inode *inode,
- ext4_lblk_t lblocks);
-extern int ext4_extent_tree_init(handle_t *, struct inode *);
-extern int ext4_ext_calc_credits_for_single_extent(struct inode *inode,
- int num,
- struct ext4_ext_path *path);
-extern int ext4_can_extents_be_merged(struct inode *inode,
- struct ext4_extent *ex1,
- struct ext4_extent *ex2);
-extern int ext4_ext_insert_extent(handle_t *, struct inode *, struct ext4_ext_path *, struct ext4_extent *, int);
-extern struct ext4_ext_path *ext4_ext_find_extent(struct inode *, ext4_lblk_t,
- struct ext4_ext_path *);
-extern void ext4_ext_drop_refs(struct ext4_ext_path *);
-extern int ext4_ext_check_inode(struct inode *inode);
-extern int ext4_find_delalloc_cluster(struct inode *inode, ext4_lblk_t lblk);
#endif /* _EXT4_EXTENTS */
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 5625146..1dc19a7 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -41,6 +41,7 @@
#include <asm/uaccess.h>
#include <linux/fiemap.h>
#include "ext4_jbd2.h"
+#include "ext4_extents.h"
#include <trace/events/ext4.h>
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index f6663c3..20862f9 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -22,6 +22,7 @@
#include "ext4_jbd2.h"
#include "truncate.h"
+#include "ext4_extents.h" /* Needed for EXT_MAX_BLOCKS */
#include <trace/events/ext4.h>
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index f1bb32e..db8226d 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -14,6 +14,7 @@
#include <linux/slab.h>
#include "ext4_jbd2.h"
+#include "ext4_extents.h"
/*
* The contiguous blocks details which can be
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 292daee..d9cc5ee 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -18,6 +18,7 @@
#include <linux/slab.h>
#include "ext4_jbd2.h"
#include "ext4.h"
+#include "ext4_extents.h"
/**
* get_ext_path - Find an extent path for designated logical block number.
diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 0fd16e6..0016fbc 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -27,7 +27,6 @@
#include "ext4_jbd2.h"
#include "xattr.h"
#include "acl.h"
-#include "ext4_extents.h"
static struct kmem_cache *io_page_cachep, *io_end_cachep;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 66a4e20..856206f 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -45,12 +45,11 @@
#include <linux/freezer.h>
#include "ext4.h"
-#include "ext4_extents.h"
+#include "ext4_extents.h" /* Needed for trace points definition */
#include "ext4_jbd2.h"
#include "xattr.h"
#include "acl.h"
#include "mballoc.h"
-#include "ext4_extents.h"
#define CREATE_TRACE_POINTS
#include <trace/events/ext4.h>