From patchwork Tue Jan 19 22:25:02 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 8065131 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 24B95BEEE5 for ; Tue, 19 Jan 2016 22:25:34 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2FB8720452 for ; Tue, 19 Jan 2016 22:25:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1C9F720450 for ; Tue, 19 Jan 2016 22:25:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933355AbcASWZQ (ORCPT ); Tue, 19 Jan 2016 17:25:16 -0500 Received: from mout.kundenserver.de ([212.227.126.187]:64358 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933317AbcASWZL (ORCPT ); Tue, 19 Jan 2016 17:25:11 -0500 Received: from wuerfel.localnet ([78.42.132.4]) by mrelayeu.kundenserver.de (mreue005) with ESMTPSA (Nemesis) id 0MVqHM-1afTpK2Lg8-00X2DB; Tue, 19 Jan 2016 23:25:06 +0100 From: Arnd Bergmann To: Dave Chinner Cc: Deepa Dinamani , Andreas Dilger , "y2038@lists.linaro.org" , "linux-fsdevel@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Date: Tue, 19 Jan 2016 23:25:02 +0100 Message-ID: <14988333.dbUrT9DQnz@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20160119204946.GA20456@dastard> References: <1452144972-15802-1-git-send-email-deepa.kernel@gmail.com> <20160119052713.GA20081@myubuntu.deepaubuntu.g7.internal.cloudapp.net> <20160119204946.GA20456@dastard> MIME-Version: 1.0 X-Provags-ID: V03:K0:cYwRdncncLsKrzJSi5w8TVfRjL0/QBSRlsoh2/kwYow3XAeJ5gx tvVE6QzNyq9IT4Z2eCN+xe0QQHCPkiUpzz+dBCRLZ2QqFthJRp2pyGck8mvoWE9va17mSWt a6eE6Sz0UozyFwgqYeW3yZ8NhPqoDBT3vfpZbLd5jygWn3LJfZ+sD+6RowPH6CvAtmRhSId RhTVgpP42IdtemNsGlbPw== X-UI-Out-Filterresults: notjunk:1; V01:K0:DAugGXQUBJc=:DWm91FQni8wsYeiVXUInJt A/vSWUzjyrlDmBv/mQWzxOggMJ+Muy4WrC+IAP+KIyadpwG8eu8iXeSeKuffRj5Dvg5KFWOPQ jfSeb6F/PFs5BcjIjg3XI82v3wtYjsQYDxDc2b9K8iC/DG+Qye9e2RDHJgR2FtlWYs2MuMXxk jUCyqQvCD1YPzmiXx9cHoHPIpYom8jW4LtyxaRcjvz7R7XnWTtvsB7cHlIxtO4w24DIODBXBc ZOTSk6plG+Tj3u1l10w6qifN8fpVr0kPBHEwUFHqXmBOa1MohZL09VMvJ3C0mKMiIgZNGUJYA SH406lY3EdaLNqIrvt8Z+tYkgrLyVrAF7EMRBP2atj/7JT8dcFvAv5IBDQhh7npR66nIs3+L+ 9EVRls23EPCa8NVoHhKzOHBp9lWndEEZHHkK6FTNnhwwLnZ31rjevEFz+Dqyt9G7mUQ/jZocd lOQtS4Tm5MA6CgcmGTfmCZjqpawRadMSEyh+fVDv5+RN4EPd7X24jmxLR4px+YTBG015NiKc2 t4IOWaryPvpoRypPt4ZKveXnZgoTDn33tivWdMWMAp1ybUjQmGJCL6KRlqoREFY0eemqyeu2v knOpiK+dkZvz7UXl5ezSQsKOvOu24fZxUPJoOHFE+hK+87zFohHfiDkpLDCG5Dz8K75jHNMFK 2rpTd6xAaQ1phQ5M2Dss79dMhwqTETulPyvEETZDkR48uf9aGId94vC08t2Nr0nuafiCjn5mz 3ANQEHonD/UtHmHg Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wednesday 20 January 2016 07:49:46 Dave Chinner wrote: > On Mon, Jan 18, 2016 at 09:27:13PM -0800, Deepa Dinamani wrote: > > On Mon, Jan 18, 2016 at 5:38 PM, Dave Chinner wrote: > > > On Mon, Jan 18, 2016 at 10:46:07PM +0100, Arnd Bergmann wrote: > > >> On Tuesday 19 January 2016 08:14:59 Dave Chinner wrote: > > >> > On Mon, Jan 18, 2016 at 08:53:22PM +0100, Arnd Bergmann wrote: > > > > Let's back out a bit and consider a few changes with the suggested "abstraction": > > > > original code: > > > > extern void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec *ts, > > __le16 __time, __le16 __date, u8 time_cs); > > > > fat_time_fat2unix(sbi, &inode->i_mtime, de->time, de->date, 0); > > > > becomes ugly > > > > extern void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec64 *ts, > > __le16 __time, __le16 __date, u8 time_cs); > > > > struct timespec64 mtime = vfs_time_to_timespec64(i_mtime, inode); > > fat_time_fat2unix(sbi, &mtime, de->time, de->date, 0); > > You're doing it wrong. fat_time_fat2unix() still gets passed > &inode->i_mtime, and the function prototype is changed to a > timespec64. *Nothing else needs to change*, because > fat_time_fat2unix() does it own calculations and then stores the > time directly into the timespec structure members.... That puts us back at the 'one big patch' problem: We can't change fat_time_fat2unix() to pass a timespec64 until we also change struct inode. The change may be small, but I see roughly 30 file systems that assign i_?time into or from a local variable or pass it into by reference into a function that is not from VFS. see http://pastebin.com/BSnwJa1N for a list (certainly some false positives and some false negatives in there) Roughly two thirds of the instances can be handled easily using vfs_time_to_timespec(), the others could be done much nicer with additional helpers such as inode_timespec_compare() > I think you're making a mountain out of a molehill. Most filesystems > will be unchanged except for s/timespec/timespec64/ as they store > values directly into timespec members when encoding/decoding. There > is no need for timestamp conversion in places like this - you're > simply not looking deep enough and applying the conversion at the > wrong layer. Any idea how to improve this somewhat lacking patch? which I would much prefer here. Arnd --- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c index b97f1df910ab..7fbb07dcad36 100644 --- a/fs/xfs/xfs_trans_inode.c +++ b/fs/xfs/xfs_trans_inode.c @@ -68,22 +68,24 @@ xfs_trans_ichgtime( int flags) { struct inode *inode = VFS_I(ip); - struct timespec tv; + struct timespec tv, mtime, ctime; ASSERT(tp); ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); - tv = current_fs_time(inode->i_sb); + tv = vfs_time_to_timespec(current_fs_time(inode->i_sb)); + mtime = vfs_time_to_timespec(inode->i_mtime); + ctime = vfs_time_to_timespec(inode->i_ctime); if ((flags & XFS_ICHGTIME_MOD) && - !timespec_equal(&inode->i_mtime, &tv)) { - inode->i_mtime = tv; + !timespec_equal(&mtime, &tv)) { + inode->i_mtime = timespec_to_vfs_time(tv); ip->i_d.di_mtime.t_sec = tv.tv_sec; ip->i_d.di_mtime.t_nsec = tv.tv_nsec; } if ((flags & XFS_ICHGTIME_CHG) && - !timespec_equal(&inode->i_ctime, &tv)) { - inode->i_ctime = tv; + !timespec_equal(&ctime, &tv)) { + inode->i_ctime = timespec_to_vfs_time(tv); ip->i_d.di_ctime.t_sec = tv.tv_sec; ip->i_d.di_ctime.t_nsec = tv.tv_nsec; } The way that Deepa suggests I think would turn out as: diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c index b97f1df910ab..54fc3c41047a 100644 --- a/fs/xfs/xfs_trans_inode.c +++ b/fs/xfs/xfs_trans_inode.c @@ -68,7 +68,7 @@ xfs_trans_ichgtime( int flags) { struct inode *inode = VFS_I(ip); - struct timespec tv; + struct vfs_time tv; ASSERT(tp); ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); @@ -76,13 +76,13 @@ xfs_trans_ichgtime( tv = current_fs_time(inode->i_sb); if ((flags & XFS_ICHGTIME_MOD) && - !timespec_equal(&inode->i_mtime, &tv)) { + !vfs_time_equal(&inode->i_mtime, &tv)) { inode->i_mtime = tv; ip->i_d.di_mtime.t_sec = tv.tv_sec; ip->i_d.di_mtime.t_nsec = tv.tv_nsec; } if ((flags & XFS_ICHGTIME_CHG) && - !timespec_equal(&inode->i_ctime, &tv)) { + !vfs_time_equal(&inode->i_ctime, &tv)) { inode->i_ctime = tv; ip->i_d.di_ctime.t_sec = tv.tv_sec; ip->i_d.di_ctime.t_nsec = tv.tv_nsec;