From: Deepa Dinamani Subject: Re: [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros Date: Mon, 20 Jun 2016 11:58:31 -0700 Message-ID: References: <1466382443-11063-1-git-send-email-deepa.kernel@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Dave Kleikamp , jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, Trond Myklebust , Adrian Hunter , Chris Mason , "adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org" , Brian Uchino , Thomas Gleixner , "Yan, Zheng" , "James E.J. Bottomley" , Paul Moore , Linux SCSI List , y2038 Mailman List , Ilya Dryomov , "linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Changman Lee , Arnd Bergmann , Mark Fasheh , Suma Ramars , John Stultz , Al Viro , David Sterba , Jaegeuk Kim Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: lustre-devel-bounces-aLEFhgZF4x6X6Mz3xDxJMA@public.gmane.org Sender: "lustre-devel" List-Id: linux-ext4.vger.kernel.org > This version now looks ok to me. > > I do have a comment (or maybe just a RFD) for future work. > > It does strike me that once we actually change over the inode times to > use timespec64, the calling conventions are going to be fairly > horrendous on most 32-bit architectures. > > Gcc handles 8-byte structure returns (on most architectures) by > returning them as two 32-bit registers (%edx:%eax on x86). But once it > is timespec64, that will no longer be the case, and the calling > convention will end up using a pointer to the local stack instead. > > So for 32-bit code generation, we *may* want to introduce a new model of doing > > set_inode_time(inode, ATTR_ATIME | ATTR_MTIME); > > which basically just does > > inode->i_atime = inode->i_mtime = current_time(inode); > > but with a much easier calling convention on 32-bit architectures. Arnd and I had discussed something like this before. But, for entirely different reasons: Having the set_inode_time() like you suggest will also help switching of vfs inode times to timespec64. We were suggesting all the accesses to inode time be abstracted through something like inode_set_time(). Arnd also had suggested a split representation of fields in the struct inode as well which led to space savings as well. And, having the split representation also meant no more direct assignments: https://lkml.org/lkml/2016/1/7/20 This in general will be similar to setattr_copy(), but only sets times rather than other attributes as well. If this is what is preferred, then the patches to change vfs to use timespec64 could make use of this and will need to be refactored. So maybe it would be good to discuss before I post those patches. -Deepa