[PATCH] update ext4-nanosecond-patch comments

[PATCH] update ext4-nanosecond-patch comments

[Aneesh Kumar K.V]

Also can we have a description of why s_{min, want}_extra_isize
fields are added in the commit message ?



diff --git a/ext4-nanosecond-patch b/ext4-nanosecond-patch
index ceaf339..02d00b7 100644
--- a/ext4-nanosecond-patch
+++ b/ext4-nanosecond-patch
@@ -1,8 +1,8 @@
 This patch is a spinoff of the old nanosecond patches.
 
 It includes some cleanups and addition of a creation timestamp. The
-EXT3_FEATURE_RO_COMPAT_EXTRA_ISIZE flag has also been added along with
-s_{min, want}_extra_isize fields in struct ext3_super_block.
+EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE flag has also been added along with
+s_{min, want}_extra_isize fields in struct ext4_super_block.
 
 Signed-off-by: Andreas Dilger <adilger@clusterfs.com>
 Signed-off-by: Kalpak Shah <kalpak@clusterfs.com>

Re: [PATCH] update ext4-nanosecond-patch comments

[Kalpak Shah]

On Tue, 2007-05-29 at 11:18 +0530, Aneesh Kumar K.V wrote:
> Also can we have a description of why s_{min, want}_extra_isize
> fields are added in the commit message ?

The i_extra_isize for each inode should ideally be s_want_extra_isize
after inode expansion. If expansion by s_want_extra_isize is not
possible, then i_extra_isize must be atleast s_min_extra_isize.

Thanks,
Kalpak.

> diff --git a/ext4-nanosecond-patch b/ext4-nanosecond-patch
> index ceaf339..02d00b7 100644
> --- a/ext4-nanosecond-patch
> +++ b/ext4-nanosecond-patch
> @@ -1,8 +1,8 @@
>  This patch is a spinoff of the old nanosecond patches.
>  
>  It includes some cleanups and addition of a creation timestamp. The
> -EXT3_FEATURE_RO_COMPAT_EXTRA_ISIZE flag has also been added along with
> -s_{min, want}_extra_isize fields in struct ext3_super_block.
> +EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE flag has also been added along with
> +s_{min, want}_extra_isize fields in struct ext4_super_block.
>  
>  Signed-off-by: Andreas Dilger <adilger@clusterfs.com>
>  Signed-off-by: Kalpak Shah <kalpak@clusterfs.com>
> -
> 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

Re: [PATCH] update ext4-nanosecond-patch comments

[Aneesh Kumar K.V]

Kalpak Shah wrote:
> On Tue, 2007-05-29 at 11:18 +0530, Aneesh Kumar K.V wrote:
>> Also can we have a description of why s_{min, want}_extra_isize
>> fields are added in the commit message ?
> 
> The i_extra_isize for each inode should ideally be s_want_extra_isize
> after inode expansion. If expansion by s_want_extra_isize is not
> possible, then i_extra_isize must be atleast s_min_extra_isize.
> 
>

My point was that the commit message of ext4-nanosecond-patch should 
explain the usage scenario for these variables. We may want to move the
commit message of ext4_expand_inode_extra_isize.patch reworded to this.


BTW i also think that adding s_{min, want}_extra_isize can be a separate 
patch altogether.


-aneesh

Re: [PATCH] update ext4-nanosecond-patch comments

[Andreas Dilger]

On May 29, 2007  13:48 +0530, Aneesh Kumar K.V wrote:
> Kalpak Shah wrote:
> >On Tue, 2007-05-29 at 11:18 +0530, Aneesh Kumar K.V wrote:
> >>Also can we have a description of why s_{min, want}_extra_isize
> >>fields are added in the commit message ?
> >
> >The i_extra_isize for each inode should ideally be s_want_extra_isize
> >after inode expansion. If expansion by s_want_extra_isize is not
> >possible, then i_extra_isize must be atleast s_min_extra_isize.
> 
> My point was that the commit message of ext4-nanosecond-patch should 
> explain the usage scenario for these variables. We may want to move the
> commit message of ext4_expand_inode_extra_isize.patch reworded to this.
> 
> BTW i also think that adding s_{min, want}_extra_isize can be a separate 
> patch altogether.

When the nanosecond timestamp extension was first proposed, the requirement
from Ted and Stephen were that s_min_extra_isize was a requirement.  Otherwise
it would be possible to have a filesystem where the timestamps are going
backward on some files due to MOST of the files supporting ns timestamps,
but some with full EAs having to truncate the ns part away.

Now, this might not be critical for some users, but for others it can be.
Since this functionality is all here there isn't any reason to move it to
a separate patch.  The same fields will be important for the inode version
also.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

Re: [PATCH] update ext4-nanosecond-patch comments

[Aneesh Kumar K.V]

Andreas Dilger wrote:
> On May 29, 2007  13:48 +0530, Aneesh Kumar K.V wrote:
>>
> 
> When the nanosecond timestamp extension was first proposed, the requirement
> from Ted and Stephen were that s_min_extra_isize was a requirement.  Otherwise
> it would be possible to have a filesystem where the timestamps are going
> backward on some files due to MOST of the files supporting ns timestamps,
> but some with full EAs having to truncate the ns part away.
> 
> Now, this might not be critical for some users, but for others it can be.
> Since this functionality is all here there isn't any reason to move it to
> a separate patch.  The same fields will be important for the inode version
> also.
>

That is why i was thinking it should not be buried in the nanosecond 
patch. Since there are multiple features depending on this, a nice patch 
list would be

Add extra fields to superblock to take care of enabling feature after 
file system creation

Add nano second feature

Add inode version feature

etc.

If wanted, i can attempt to split the patch as above. Let me know. If we 
don't think the above is important I would say we should at least move 
some of the commit message found in expand_inode_extra_isize.patch that 
explains the usage to the patch that introduce these fields (nano second 
patch ).

-aneesh