From patchwork Sat Nov 9 07:19:11 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: David Turner X-Patchwork-Id: 289955 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 458E42C009E for ; Sat, 9 Nov 2013 18:19:29 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932967Ab3KIHTW (ORCPT ); Sat, 9 Nov 2013 02:19:22 -0500 Received: from mailbigip.dreamhost.com ([208.97.132.5]:37670 "EHLO homiemail-a39.g.dreamhost.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932616Ab3KIHTO convert rfc822-to-8bit (ORCPT ); Sat, 9 Nov 2013 02:19:14 -0500 Received: from homiemail-a39.g.dreamhost.com (localhost [127.0.0.1]) by homiemail-a39.g.dreamhost.com (Postfix) with ESMTP id 7F444150074; Fri, 8 Nov 2013 23:19:13 -0800 (PST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=novalis.org; h=message-id:subject :from:to:cc:date:in-reply-to:references:content-type :mime-version:content-transfer-encoding; q=dns; s=novalis.org; b=G9c353emhApK72CSaqfGMfiCmIqHpQBnInm4Ud8Ctz1l2t/LWdkCCzWIsJB1M B/fGuupXsZ6f7Xby5tqcwxiB3VVT9binMu1Fs8XKaAdTz9D9fjHKpuTUmGDDk7c+ emqJ2f7A+F9iXUcb5a1EAz6TmJ6HP8iHxON1M/t1F/dw/U= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=novalis.org; h=message-id :subject:from:to:cc:date:in-reply-to:references:content-type :mime-version:content-transfer-encoding; s=novalis.org; bh=i2cpG EsraKLGg4zARpJr0B2ASC0=; b=HmG9diYQUTgHT4odluUs3zRFI0SeK6oaRdIK+ XMJAZyYav1FgGhr6aQaQhcX+M8YkaQPfIthE5X2gnrV+6iKRqCUcLsOKvLMHSMLl Xtlz/b3nGooipbd5gdAhp9949WegJ+5egBpcmoFvAufN2OvjQY4qYZwXt7w9j2zi 8bh/6c= Received: from [10.0.1.4] (ool-4579628d.dyn.optonline.net [69.121.98.141]) (using SSLv3 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: novalis@novalis.org) by homiemail-a39.g.dreamhost.com (Postfix) with ESMTPSA id DDF3315006D; Fri, 8 Nov 2013 23:19:12 -0800 (PST) Message-ID: <1383981551.8994.27.camel@chiang> Subject: [PATCH] ext4: explain encoding of 34-bit a,c,mtime values From: David Turner To: Andreas Dilger Cc: Jan Kara , Ext4 Developers List , Linux Kernel Mailing List , Theodore Ts'o Date: Sat, 09 Nov 2013 02:19:11 -0500 In-Reply-To: References: <1383808590.23882.13.camel@chiang> <20131107160341.GA3850@quack.suse.cz> <1383864864.23882.33.camel@chiang> <20131107231445.GG2054@quack.suse.cz> <1383866807.23882.41.camel@chiang> X-Mailer: Evolution 3.6.4-0ubuntu1 Mime-Version: 1.0 Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Fri, 2013-11-08 at 14:37 -0700, Andreas Dilger wrote: > On Nov 7, 2013, at 4:26 PM, David Turner 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 > > > > 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 --- 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) {