From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932398AbcARVqQ (ORCPT ); Mon, 18 Jan 2016 16:46:16 -0500 Received: from mout.kundenserver.de ([212.227.126.133]:58246 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756558AbcARVqN (ORCPT ); Mon, 18 Jan 2016 16:46:13 -0500 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: Mon, 18 Jan 2016 22:46:07 +0100 Message-ID: <4006633.5eZd1aWvky@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20160118211459.GA16611@dastard> References: <1452144972-15802-1-git-send-email-deepa.kernel@gmail.com> <16314879.ZTNSN2flN7@wuerfel> <20160118211459.GA16611@dastard> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:epEbK8r43KJQxmrMvfRpce4B8gV/Tz7UfyL3i8OWHq/G+Av1u5X aFi7VUpkudkyLQgSvvdSHIjr2yTcHz4Q4wc2yBwbXWfBaMjqXu0p10YnJbZrENBLBL8Asfm dKBwHagcbEjJ92Ar7jmqPLosXgNHXNV/7TS/SNB6bltkLzxGnAWIXI5m3TkU7sZMOwfk3Dj /KV/ATmVFiinUrLK7Aq3Q== X-UI-Out-Filterresults: notjunk:1;V01:K0:SPrcGk0RfIc=:jdV5PYUIOAfmzSPuwmimV1 zFAzfaHHtWVEs8CWkAwQOhkjdA4VjM3kEK1h1KPm4pyV/hiaWrtwIWR5C8j/hFZnvn1Xs9vcf RYzj9uyE/E6h0ZOwly0gpINZz8Cr17DBoefvUMAE2QRtYne9eeoDxzwZmblA8sVy+oXbfBrH6 tUe4AMbXp8L85uaxFT9oqDALCArbXY8cH/udJGCZvs7ti9O5MOSUFYPCe6VKiFdLonbKuY7q5 q+9D6PAK3DtGZI2SoDNurmPHECMUNebr9l1LonCmVp05uzFd504LNeSn5TmcX28HIVtRPrmCQ EWKpzMS/4UKWnrXJkGwx3KgvQjqc37LcQVAIjvEL8mN9n47UPexmljEodSRNG/MKsgoqPrHRz tZ3RmtbnnOPGTPniJccV/LGlOK75X7Y/zd+718NGv5Xafte1iq7f9YqtuIdW+T6fn9P7HA8V2 hlerOHGoHT3+xNVz+tmjJQp7JkfOA1wQ1Ganjtvd6EVJkBr0JasarZ+ELWH042FwjumHck0QC /HSeICdEr7/Qgza4PmJhfbuR2ixsDXjPMLoJDYfk4aw8P+lMitD2fZO937aBSf1SeauR5xZpn xO0sGj60GNzP42QQVdns80YnMaQtA5XwB4Gv3VHz6SE9r9UnVujJ5RhmbWAOV1IRcxRgpys5X Vk6I2T1rYyyPxLCK6r7pCgMofzNVtTUrkmN01DCoSbLdmYXWfBnWYWDss4ks6JwuJ7ZH/PbFl o/Qvz7Bn6VmXz6wm Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 19 January 2016 08:14:59 Dave Chinner wrote: > On Mon, Jan 18, 2016 at 08:53:22PM +0100, Arnd Bergmann wrote: > > On Monday 18 January 2016 09:40:12 Deepa Dinamani wrote: > > > On Mon, Jan 18, 2016 at 2:56 AM, Arnd Bergmann wrote: > > > > I agree it's ugly and fragile to have one huge patch, > > Nobody is suggesting one huge patch here. This can all be done with > small steps. > > > but I think the > > best way to illustrate it is to make it as small as possible and > > then talk about whether that makes it acceptable or how we can > > work around the problems. > > > > Do you have an estimate what portion of the file systems need any > > changes at all before we can flip over VFS to the new types? > > All filesystems will, at least, need auditing. A large number of > them will need changes, no matter how we "abstract" the VFS type > change, even if it is just for 32->64 bit sign extension bugs. > > Filesystems that have intermediate timestamp formats such as Lustre, > NFS, CIFS, etc will need conversion at the vfs/filesytem entry > points, and their internals will remain unchanged. Fixing the > internals is outside the scope fo the VFS change - the 64 bit VFS > inode support stops at the VFS inode/filesystem boundary. What I meant with "one huge patch" is simply just the change that is needed to modify the type, if we don't use conversion helper functions. > > If it's less than half, we you can try yet another variation (nothing > > new really, we are always dealing with the same few tricks): > > > > 1. add timestamp range checking and clamping > > 2. kill off CURRENT_TIME > > Other way around. First make everything use the existing current > time functions, then ensure that incoming timestamps are truncated > correctly, then add range checking and clamping to the existing > time modification functions. Makes sense. > > 3. for each file system that uses struct timespec internally to pass > > around inode timestamps, do one patch that adds a > > timespec_to_inode_time() and vice versa, which gets defined like > > > > static inline struct timespec timespec_to_inode(struct timespec t) > > { > > return t; > > } > > This works, and is much cleaner than propagating the macro nastiness > everywhere. IMO vfs_time_to_timespec()/timespec_to_vfs_time would be > better named as it describes the conversion exactly. I don't think > this is a huge patch, though - it's mainly the setattr/kstat > operations that need changing here. Good idea for the name. If you are ok with adding those helpers, then it can be done in small steps indeed. I was under the assumption that you didn't like any kind of abstraction of the type in struct inode at all. > > 4. change the internal representation in one patch that changes those > > helpers along with the struct members. > > If you are talking about converting internal filesystem > representations to (e.g. CIFS fattr, NFS fattr, etc) then this is > wrong. Those filesystems are isolated and able to use timespecs > internally by step 3, and without protocol/format changes can't > support y2038k compliant dates. Hence fixing such problems is a > problem for the filesystem developers and is not an issue for the > VFS timestamp conversion. No, once we have the timespec_to_vfs_time helpers in all file systems, that change is just for VFS, and should not touch any file system specific code. It is the equivalent of patch 8/15 in the current version of the series, except that it changes one version of the code to another rather than changing a CONFIG_* symbol that alternates between the two versions coexisting in source. When I first attempted the conversion, I ended up with a very similar trick that Deepa has now, and it's very helpful to find what the code locations are that need to be touched, without doing them all at the same time, as you can simply flip that option to try out another file system. However, I agree that this is better not reflected in how the patches get applied in the end, and there is no need to clutter the git history with having both options in the code at the same time, and we should try to avoid touching a lot of code more than once wherever possible. > > 5. change the file systems to use timespec64 internally instead of > > timespec. > > I think that will work and leave use with a relatively clean code > base, as well as be able to address y2038k support each individual > filesystem in our own time. Ok, thanks. Arnd