Patchwork [1/1] ext4: Remove duplicate inclusion of ext4_extents.h in super.c

login
register
mail settings
Submitter Sachin Kamat
Date Nov. 19, 2012, 11:17 a.m.
Message ID <1353323842-32283-1-git-send-email-sachin.kamat@linaro.org>
Download mbox | patch
Permalink /patch/199988/
State Superseded
Headers show

Comments

Sachin Kamat - Nov. 19, 2012, 11:17 a.m.
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(-)
Zheng Liu - Nov. 19, 2012, 1:39 p.m.
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
--
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
Theodore Ts'o - Nov. 19, 2012, 3 p.m.
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
--
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
Sachin Kamat - Nov. 19, 2012, 3:06 p.m.
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
Zheng Liu - Nov. 19, 2012, 4:22 p.m.
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
--
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
Zheng Liu - Nov. 19, 2012, 4:23 p.m.
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
--
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
Sachin Kamat - Nov. 22, 2012, 5:13 a.m.
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.
Sachin Kamat - Nov. 27, 2012, 3:37 a.m.
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

Patch

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>