From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757750AbcAJXFE (ORCPT ); Sun, 10 Jan 2016 18:05:04 -0500 Received: from ipmail04.adl6.internode.on.net ([150.101.137.141]:11690 "EHLO ipmail04.adl6.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757563AbcAJXFB (ORCPT ); Sun, 10 Jan 2016 18:05:01 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2A1EwAw45JWPGu7LXleKAECgw+BP4Jig36BeaAWAQEBAQEBBoteiUSGCQQCAoESTQEBAQEBAQcBAQEBQT9BEgGDYQEBBCcTHCMQCAMYCSUPBSUDBxoTiC2+cQEBAQEGAgEgGYV1hUeIIYEbAQSXE4g2hRqBZ4dthTKOUYJyH4FxKjSDfT4kgScBAQE Date: Mon, 11 Jan 2016 10:03:39 +1100 From: Dave Chinner To: Deepa Dinamani Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, y2038@lists.linaro.org Subject: Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Message-ID: <20160110230339.GD10456@dastard> References: <1452144972-15802-1-git-send-email-deepa.kernel@gmail.com> <1452144972-15802-3-git-send-email-deepa.kernel@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1452144972-15802-3-git-send-email-deepa.kernel@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jan 06, 2016 at 09:35:59PM -0800, Deepa Dinamani wrote: > The current representation of inode times in struct inode, struct iattr, > and struct kstat use struct timespec. timespec is not y2038 safe. > > Use scalar data types (seconds and nanoseconds stored separately) to > represent timestamps in struct inode in order to maintain same size for > times across 32 bit and 64 bit architectures. > In addition, lay them out such that they are packed on a naturally > aligned boundary on 64 bit arch as 4 bytes are used to store nsec. > This makes each tuple(sec, nscec) use 12 bytes instead of 16 bytes. > This will help save RAM space as inode structure is cached in memory. > The other structures are transient and do not benefit from these > changes. IMO, this decisions sends the patch series immediately down the wrong path. TO me, this is a severe case of premature optimisation because everything gets way more complex just to save those 8 bytes, especially as those holes can be filled simply by changing the variable declaration order in the structure and adding a simple comment. And, really, I don't like those VFS_INODE_[GS]ET_XTIME macros at all; you've got to touch lots of code(*), making it all shouty and harder to read. They seem only to exist because of the above structural change requires an abstract timestamp accessor while CONFIG_FS_USES_64BIT_TIME exists. Given that goes away at the end o the series, so should the macro - if we use a struct timespec64 in the first place, it isn't even necessary as a temporary construct. (*) I note you haven't touched XFS, which means you've probably broken lots of other filesystem code. e.g. in XFS, functions like xfs_vn_getattr() and xfs_vn_update_time() access inode->i_[acm]time directly and hence are not going to compile after this patch series. Cheers, Dave. -- Dave Chinner david@fromorbit.com