From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752675AbcAPTOn (ORCPT ); Sat, 16 Jan 2016 14:14:43 -0500 Received: from mail-pa0-f54.google.com ([209.85.220.54]:34117 "EHLO mail-pa0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752614AbcAPTO2 (ORCPT ); Sat, 16 Jan 2016 14:14:28 -0500 Subject: Re: [Y2038] [RFC 02/15] vfs: Change all structures to support 64 bit time Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\)) Content-Type: multipart/signed; boundary="Apple-Mail=_1323BEE1-F3E3-4838-BD71-9474334F9AD0"; protocol="application/pgp-signature"; micalg=pgp-sha256 X-Pgp-Agent: GPGMail 2.5.2 From: Andreas Dilger In-Reply-To: <4756197.dta9JQvfZs@wuerfel> Date: Sat, 16 Jan 2016 12:14:22 -0700 Cc: y2038@lists.linaro.org, Dave Chinner , linux-fsdevel@vger.kernel.org, Deepa Dinamani , linux-kernel@vger.kernel.org Message-Id: References: <1452144972-15802-1-git-send-email-deepa.kernel@gmail.com> <4096827.pGyVFmGqbD@wuerfel> <20160115024937.GM6033@dastard> <4756197.dta9JQvfZs@wuerfel> To: Arnd Bergmann X-Mailer: Apple Mail (2.2104) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --Apple-Mail=_1323BEE1-F3E3-4838-BD71-9474334F9AD0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii > On Jan 15, 2016, at 9:50 AM, Arnd Bergmann wrote: >=20 > On Friday 15 January 2016 13:49:37 Dave Chinner wrote: >> On Thu, Jan 14, 2016 at 11:46:16PM +0100, Arnd Bergmann wrote: >>> On Friday 15 January 2016 08:00:01 Dave Chinner wrote: >>>> On Thu, Jan 14, 2016 at 05:53:21PM +0100, Arnd Bergmann wrote: >>>>> On Thursday 14 January 2016 08:04:36 Dave Chinner wrote: >>>=20 >>> Yes, that is the obvious case, and I guess works for at least half = the >>> file systems when they always assign righthand side and lefthand >>> side of the time stamps using the external types or helpers like >>> CURRENT_TIME and current_fs_time(). >>>=20 >>> However, there are a couple of file systems that need a bit more = refactoring >>> before we can do this, e.g. in ntfs_truncate: >>=20 >> Sure, and nfs is a pain because of all it's internal use of >> timespecs, too. >=20 > lustre is probably the worst. Lustre currently only has one-second granularity in a 64-bit field, so it doesn't really care about the difference between timespec or timespec64 at all. The only other uses are for measuring relative times, so the 64-bitness shouldn't really matter. Could you please point out what issues exist so they can be fixed. Cheers, Andreas >> But we have these timespec_to_timespec64 helper >> functions, and that's what we should use in these cases where the >> filesystem cannot support full 64 bit timestamps internally. In >> those cases, they'll be telling the superblock this at mount time >> things like current_fs_time() won't be returning then a timestamp >> that is out of range for a 32 bit timestamp.... >=20 > I'm not worried about the runtime problems here, just how to > get a series of patches that are each doing one reasonable thing > at a time. >=20 >>> if (!IS_NOCMTIME(VFS_I(base_ni)) && = !IS_RDONLY(VFS_I(base_ni))) { >>> struct timespec now =3D = current_fs_time(VFS_I(base_ni)->i_sb); >>> int sync_it =3D 0; >>>=20 >>> if (!timespec_equal(&VFS_I(base_ni)->i_mtime, &now) = || >>> !timespec_equal(&VFS_I(base_ni)->i_ctime, &now)) >>> sync_it =3D 1; >>> VFS_I(base_ni)->i_mtime =3D now; >>> VFS_I(base_ni)->i_ctime =3D now; >>> } >>>=20 >>> The type of the local variable must match the return code of >>> current_fs_time(), so if we change over i_mtime and current_fs_time >>> globally, this either has to be rewritten first to avoid the use of >>> local variables, or it needs temporary conversion helpers, or >>> it has to be changed in the same patch. None of those is = particularly >>> appealing. There are a few dozen such things in various file = systems. >>=20 >> it gets rewritten to: >>=20 >> struct timespec now; >>=20 >> now =3D = timespec64_to_timespec(current_fs_time(VFS_I(base_ni)->i_sb)); >> .... >>=20 >> and the valid timestamp range for ntfs is set to 32bit timestamps. >> This then leaves it up to the filesystem developers to make >> the ntfs filesystem code 64 bit timestamp clean if the on disk >> format is ever changed to support 64 bit times. >>=20 >> Same goes for NFS, and any of the other filesystems that use struct >> timespec internally for time representation. >=20 > That is what I meant in my previous mail about approach c) being ugly > because it requires sprinkling lots of timespec64_to_timespec() and > timespec_to_timespec64() in the initial patch in order to atomically > change the types in inode/iattr/kstat/... without introducing build > regressions. >=20 > It's a rather horrible patch, and quite likely to cause conflicts with > other patches that introduce another use of those structures in the > merge window. >=20 >>> Having a global sysctl knob or >>> a compile-time option is better than having each file system >>> implementor take a guess at what users might prefer, if we can't >>> come up with a behavior (e.g. clamp all the time, or error out >>> all the time) that everybody agrees is always correct. >>=20 >> filesystem implementors will use the helper funtions that are >> provided for this. If they don't (like all the current use of >> CURRENT_TIME), then that's a bug that needs fixing. >=20 > Ok, then we are in total agreement here: the policy remains > to be decided by common code, but the implementation can differ > per file system. >=20 >> i.e. we need >> a timespec_clamp() function, similar to timespec_trunc(), and y2038k >> compliant filesystems and syscalls need to use them.... >=20 > I was thinking we end up with a single function that does both > clamp() and trunk(), but that's an implementation detail. >>>>> Let me clarify what my idea is here: I want a global kernel option >>>>> that disables all code that has known y2038 issues. If anyone = tries >>>>> to build an embedded system with support beyond 2038, that should >>>>> disable all of those things, including file systems, drivers and >>>>> system calls, so we can reasonably assume that everything that = works >>>>> today with that kernel build will keep working in the future and >>>>> not break in random ways. >>>>=20 >>>> It's not that black and white when it comes to filesystems. y2038k >>>> support is determined by the on-disk structure of the filesystem >>>> being mounted, and that is determined at mount time. When the >>>> filesystem mounts and sets it's valid timestamp ranges the VFS >>>> will need to decide as to whether the filesystem is allowed to >>>> continue mounting or not. >>>=20 >>> Some file systems are always broken around 2038 (e.g. HFS in 2040), >>> so if we can't fix them, I want to be able to turn them off >>> in Kconfig along with the 32-bit time_t syscalls. >>=20 >> That can be done with kconfig depends rules - it has nothing to do >> with this patch set. >=20 > kconfig dependencies is what I meant for the simple cases where a > file system is known to always be broken, we just need a small > modification for the cases you mentioned below. >=20 >>> ext2/3/4, xfs and ocfs2 (maybe one or two more, I'd have to check) >>> currently behave in a consistent manner across 32-bit and 64-bit >>> architectures by allowing a range between 1902 and 2037, and we >>> obviously don't have a choice there but to keep that current >>> behavior, and extend the time format in one way or another to >>> store additional bits for the epoch. >>=20 >> That's a filesystem implementation problem, not a generic inode >> timestamp problem. i.e. this is handled when the filesystem converts >> the inode timestamp from a timespec64 in the struct inode to >> whatever format it stores the timestamp on disk. That conversion >> does not change just because the VFS inode moves from a timespec to >> a timespec64. Again, those on-disk format changes to support beyond >> the current epoch are outside the scope of this patchset, because >> they are not affected by the timestamp format the VFS choses to use. >=20 > Fine with me, we can have another series to add the Kconfig = dependencies > and modify the file systems that need this. >=20 > 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 Cheers, Andreas --Apple-Mail=_1323BEE1-F3E3-4838-BD71-9474334F9AD0 Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=signature.asc Content-Type: application/pgp-signature; name=signature.asc Content-Description: Message signed with OpenPGP using GPGMail -----BEGIN PGP SIGNATURE----- Comment: GPGTools - http://gpgtools.org iQIVAwUBVpqWj3Kl2rkXzB/gAQi1wRAAq00bF3BojUYmTzsUdih6tMrAtEL9n904 HXXdBWEFtDFS3uOW6nyiJtiW5+XlTx/HasUY5DMQQkjBQ5zuwMR387tF/4fRIfRu fsZ7KCmyXC0uPNgD5sKNa35DZLP7yIbJ0g+zgpqaLcR8Q5WwFMZkfC7XtOX5EM3O RFMImehUqIoNxtSY0Yer5P53+q7gcykrNh7FDnSPXWCg5pY5hEevQv41N4FIYjRY 5INx3LkVPjL+SldvXE9GviQe6lD1nAPUnak8GOxaIrJ1ro3GogI+0C3et81wu5So NrKvD3HsLIzrxk7C+YWRc+zDYhk2L6vf9qQcnr1LAu+DKjYkdp4nEu8owDk7QhSC R+eGh8ms1wuFTEtpNAYKqzYUh0bhwlbHEFPhnoYuA+Jq//yHnZoOl7rFJn4Pp6+J YZZX8DpSiQgG8hPRJPETVR759hf0UuT+VA+apZhvIbWPvCyuJLFxodfa4k6pqZGq mAg9igmUKGLltAOkk0zE+lQDHRmoBiPlDeHAMCE0lPnmyiu0iA2+ea13qaCUABRo nv3h91YWWZttHJxgIozhKuO6p1A5tzv7RlHcwQg7S81R7SZrKUyqkfpXAuSbG40G tswD2Jujfb1olR2veXkn20fSIk0JKKuJzPMqud5PPAxPMN7FLxJ970+GM/aDIgL0 B52PLb4Q50g= =VG/M -----END PGP SIGNATURE----- --Apple-Mail=_1323BEE1-F3E3-4838-BD71-9474334F9AD0--