* [PATCH 1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c @ 2012-11-19 11:17 Sachin Kamat 2012-11-19 13:39 ` Zheng Liu 0 siblings, 1 reply; 9+ messages in thread From: Sachin Kamat @ 2012-11-19 11:17 UTC (permalink / raw) To: linux-ext4; +Cc: tytso, adilger.kernel, sachin.kamat, patches ext4_extents.h was included twice. Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c 2012-11-19 11:17 [PATCH 1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c Sachin Kamat @ 2012-11-19 13:39 ` Zheng Liu 2012-11-19 15:00 ` Theodore Ts'o 2012-11-19 15:06 ` Sachin Kamat 0 siblings, 2 replies; 9+ messages in thread From: Zheng Liu @ 2012-11-19 13:39 UTC (permalink / raw) To: Sachin Kamat; +Cc: linux-ext4, tytso, adilger.kernel, patches On Mon, Nov 19, 2012 at 04:47:22PM +0530, Sachin Kamat wrote: > ext4_extents.h was included twice. > > Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> > --- > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c 2012-11-19 13:39 ` Zheng Liu @ 2012-11-19 15:00 ` Theodore Ts'o 2012-11-19 16:22 ` Zheng Liu 2012-11-19 15:06 ` Sachin Kamat 1 sibling, 1 reply; 9+ messages in thread From: Theodore Ts'o @ 2012-11-19 15:00 UTC (permalink / raw) To: Sachin Kamat, linux-ext4, adilger.kernel, patches 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c 2012-11-19 15:00 ` Theodore Ts'o @ 2012-11-19 16:22 ` Zheng Liu 0 siblings, 0 replies; 9+ messages in thread From: Zheng Liu @ 2012-11-19 16:22 UTC (permalink / raw) To: Theodore Ts'o; +Cc: Sachin Kamat, linux-ext4, adilger.kernel, patches 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c 2012-11-19 13:39 ` Zheng Liu 2012-11-19 15:00 ` Theodore Ts'o @ 2012-11-19 15:06 ` Sachin Kamat 2012-11-19 16:23 ` Zheng Liu 1 sibling, 1 reply; 9+ messages in thread From: Sachin Kamat @ 2012-11-19 15:06 UTC (permalink / raw) To: gnehzuil.liu, linux-ext4, tytso, adilger.kernel, patches Hi Zheng, On 19 November 2012 19:09, Zheng Liu <gnehzuil.liu@gmail.com> 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 <sachin.kamat@linaro.org> >> --- >> 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 majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- With warm regards, Sachin ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c 2012-11-19 15:06 ` Sachin Kamat @ 2012-11-19 16:23 ` Zheng Liu 2012-11-22 5:13 ` Sachin Kamat 0 siblings, 1 reply; 9+ messages in thread From: Zheng Liu @ 2012-11-19 16:23 UTC (permalink / raw) To: Sachin Kamat; +Cc: linux-ext4, tytso, adilger.kernel, patches On Mon, Nov 19, 2012 at 08:36:06PM +0530, Sachin Kamat wrote: > Hi Zheng, > > On 19 November 2012 19:09, Zheng Liu <gnehzuil.liu@gmail.com> 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 <sachin.kamat@linaro.org> > >> --- > >> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c 2012-11-19 16:23 ` Zheng Liu @ 2012-11-22 5:13 ` Sachin Kamat 2012-11-27 3:37 ` Sachin Kamat 0 siblings, 1 reply; 9+ messages in thread From: Sachin Kamat @ 2012-11-22 5:13 UTC (permalink / raw) To: Sachin Kamat, linux-ext4, tytso, adilger.kernel, patches, Zheng Liu Hi Theodore, On 19 November 2012 21:53, Zheng Liu <gnehzuil.liu@gmail.com> wrote: > On Mon, Nov 19, 2012 at 08:36:06PM +0530, Sachin Kamat wrote: >> Hi Zheng, >> >> On 19 November 2012 19:09, Zheng Liu <gnehzuil.liu@gmail.com> 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 <sachin.kamat@linaro.org> >> >> --- >> >> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c 2012-11-22 5:13 ` Sachin Kamat @ 2012-11-27 3:37 ` Sachin Kamat 2012-11-28 18:08 ` Theodore Ts'o 0 siblings, 1 reply; 9+ messages in thread From: Sachin Kamat @ 2012-11-27 3:37 UTC (permalink / raw) To: Sachin Kamat, linux-ext4, tytso, adilger.kernel, patches, Zheng Liu ping On 22 November 2012 10:43, Sachin Kamat <sachin.kamat@linaro.org> wrote: > Hi Theodore, > > On 19 November 2012 21:53, Zheng Liu <gnehzuil.liu@gmail.com> wrote: >> On Mon, Nov 19, 2012 at 08:36:06PM +0530, Sachin Kamat wrote: >>> Hi Zheng, >>> >>> On 19 November 2012 19:09, Zheng Liu <gnehzuil.liu@gmail.com> 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 <sachin.kamat@linaro.org> >>> >> --- >>> >> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c 2012-11-27 3:37 ` Sachin Kamat @ 2012-11-28 18:08 ` Theodore Ts'o 0 siblings, 0 replies; 9+ messages in thread From: Theodore Ts'o @ 2012-11-28 18:08 UTC (permalink / raw) To: Sachin Kamat; +Cc: linux-ext4, adilger.kernel, patches, Zheng Liu 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 <tytso@mit.edu> 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 <sachin.kamat@linaro.org> Reported-by: Wei Yongjun <weiyj.lk@gmail.com> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> 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> ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-11-28 18:08 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-11-19 11:17 [PATCH 1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c Sachin Kamat 2012-11-19 13:39 ` Zheng Liu 2012-11-19 15:00 ` Theodore Ts'o 2012-11-19 16:22 ` Zheng Liu 2012-11-19 15:06 ` Sachin Kamat 2012-11-19 16:23 ` Zheng Liu 2012-11-22 5:13 ` Sachin Kamat 2012-11-27 3:37 ` Sachin Kamat 2012-11-28 18:08 ` Theodore Ts'o
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).