Date:	Sat, 09 Nov 2013 02:19:11 -0500
From:	David Turner <novalis@...alis.org>
To:	Andreas Dilger <adilger@...ger.ca>
Cc:	Jan Kara <jack@...e.cz>,
	Ext4 Developers List <linux-ext4@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Theodore Ts'o <tytso@....edu>
Subject: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values

On Fri, 2013-11-08 at 14:37 -0700, Andreas Dilger wrote:
> On Nov 7, 2013, at 4:26 PM, David Turner <novalis@...alis.org> wrote:
> > On Fri, 2013-11-08 at 00:14 +0100, Jan Kara wrote:
> >> Still unnecessary type cast here (but that's a cosmetic issue).
> > ...
> >> Otherwise the patch looks good. You can add:
> >> Reviewed-by: Jan Kara <jack@...e.cz>
> > 
> > Thanks.  A version with this correction and the reviewed-by follows.
> 
> Thanks for working on this.  It was (seriously) in my list of things to
> get done, but I figured I still had a few years to work on it...
> 
> My (unfinished) version of this patch had a nice comment at ext4_encode_time()
> that explained the encoding that is being used very clearly:
> 
> /*
>  * We need is an encoding that preserves the times for extra epoch "00":
>  *
>  * extra  msb of                         adjust for signed
>  * epoch  32-bit                         32-bit tv_sec to
>  * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
>  * 0 0    1    -0x80000000..-0x00000001  0x000000000     1901-12-13..1969-12-31
>  * 0 0    0    0x000000000..0x07fffffff  0x000000000     1970-01-01..2038-01-19
>  * 0 1    1    0x080000000..0x0ffffffff  0x100000000     2038-01-19..2106-02-07
>  * 0 1    0    0x100000000..0x17fffffff  0x100000000     2106-02-07..2174-02-25
>  * 1 0    1    0x180000000..0x1ffffffff  0x200000000     2174-02-25..2242-03-16
>  * 1 0    0    0x200000000..0x27fffffff  0x200000000     2242-03-16..2310-04-04
>  * 1 1    1    0x280000000..0x2ffffffff  0x300000000     2310-04-04..2378-04-22
>  * 1 1    0    0x300000000..0x37fffffff  0x300000000     2378-04-22..2446-05-10
>  */
> 
> It seems that your version of the patch seems to use a different encoding.  Not
> that this is a problem, per-se, since my patch wasn’t in use anywhere, but it
> would be nice to have a similarly clear explanation of what the mapping is so
> that it can be clearly understood.

I have included a patch with an explanation (the patch is against
tytso/dev -- I hope that's the correct place).  

> My ext4_{encode,decode}_extra_time() used add/subtract instead of mask/OR ops,
> which changed the on-disk format for times beyond 2038, but those were already
> broken, and presumably not in use by anyone yet. 

They were actually correct according to my patch's encoding (that is, my
patch used the existing encoding).  The existing encoding was written
correctly, but read wrongly.  As you say, this should not matter, since
nobody should be writing these timestamps, but I assumed that the
existing encoding had happened for a reason, and wanted to make the
minimal change.

If you believe it is important, I would be happy to change it.

>  However, it seemed to me this
> was easier to produce the correct results.  Have you tested your patch with
> a variety of timestamps to verify its correctness? 

I tested by using touch -d to create one file in each year between 1902
and 2446.  Then I unmounted and remounted the FS, and did ls -l and
manually verified that each file's date matched its name.

> It looks to me like you
> have mapped the 1901-1969 range onto 0x3 for the epoch bits, instead 
> of the (IMHO) natural 0x0 bits. The critical time ranges are listed 
> above.

I think the idea of this is that it is the bottom 34 bits of the 64-bit
signed time.  However, it occurs to me that this relies on a two's
complement machine.  Even though the C standard does not guarantee this,
I believe the kernel requires it, so that's probably OK.

Patch follows:

--
Add a comment explaining the encoding of ext4's extra {a,c,m}time
bits.

Signed-off-by: David Turner <novalis@...alis.org>
---
 fs/ext4/ext4.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 121da383..ab69f14 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -713,6 +713,24 @@ struct move_extent {
 	  sizeof((ext4_inode)->field))			\
 	<= (EXT4_GOOD_OLD_INODE_SIZE +			\
 	    (einode)->i_extra_isize))			\
+/*
+ * We use the bottom 34 bits of the signed 64-bit time value, with
+ * the top two of these bits in the bottom of extra.  This leads
+ * to a slightly odd encoding, which works like this:
+ *
+ * extra  msb of
+ * epoch  32-bit
+ * bits   time    decoded 64-bit tv_sec   valid time range
+ * 0 0    0    0x000000000..0x07fffffff  1970-01-01..2038-01-19
+ * 0 0    1    0x080000000..0x0ffffffff  2038-01-19..2106-02-07
+ * 0 1    0    0x100000000..0x17fffffff  2106-02-07..2174-02-25
+ * 0 1    1    0x180000000..0x1ffffffff  2174-02-25..2242-03-16
+ * 1 0    0    0x200000000..0x27fffffff  2242-03-16..2310-04-04
+ * 1 0    1    0x280000000..0x2ffffffff  2310-04-04..2378-04-22
+ * 1 1    0    0x300000000..0x37fffffff  2378-04-22..2446-05-10
+
+ * 1 1    1    -0x80000000..-0x00000001  1901-12-13..1969-12-31
+ */
 
 static inline __le32 ext4_encode_extra_time(struct timespec *time)
 {
-- 
1.8.1.2



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists