Re: [Y2038] [RFC v2] vfs 64 bit time transition proposals
On Wednesday 24 February 2016 13:19:07 Thomas Gleixner wrote: > On Fri, 12 Feb 2016, Arnd Bergmann wrote: > > 2b has the main advantage of not changing behavior with the flip, so > >we can convert all file systems to use vfs_time relatively easily > >and then later make them actually use 64-bit timestamps with > >a patch that each file system developer can do for themselves. > > And that's a very clear advantage of this approch. The change is purely > mechanical and can be largely done with coccinelle. Actually Deepa pointed out one common case where the behavior actually changes: inode->i_mtime.tv_sec = le32_to_cpu(on_disk_mtime); will change behavior when tv_sec is changed to a time64_t, but it only changes in a way that is consistent with 64-bit architectures, and it will not change for user space that uses the existing syscalls to actually read the timestamps, so the risk of introducing a regression here is still really low. > >One downside is that it leads to rather ugly code as discussed > >before, examples are in "[RFC v2b 5/5] fs: xfs: change inode > >times to use vfs_time data type" and "[RFC v2b 3/5] fs: btrfs: > >Use vfs_time accessors". > > - now = current_fs_time(inode->i_sb); > - if (!timespec_equal(&inode->i_mtime, &now)) > - inode->i_mtime = now; > + now = vfs_time_to_timespec(current_fs_time(inode->i_sb)); > + ts = vfs_time_to_timespec(inode->i_mtime); > + if (!timespec_equal(&ts, &now)) > + inode->i_mtime = timespec_to_vfs_time(now); > + > + ts = vfs_time_to_timespec(inode->i_mtime); > + if (!timespec_equal(&ts, &now)) > + inode->i_ctime = timespec_to_vfs_time(now); > > You can either provide a helper function which does that m/c_time update at > the VFS level where you can hide the mess or just accept that this transition > will introduce some odd constructs like the above. I think we can live with this. There are around 10 files that do timespec_compare() or timespec_equal(), comparing an inode timestamp with a local variable or a filesystem specific timespec value, but they are all slightly different so there is no obvious helper function to address multiple file systems at once without also introducing a 'struct vfs_time'. > > 2c gets us the furthest along the way for the conversion, close > >to where we want to end up in the long run, so we could do that > >to file systems one by one. The behavior change is immediate, > >so there are fewer possible surprises than with 2a, but it > >also means the most upfront work. > > I would'nt worry about the upfront work to much, if it is the cleanest way to > do the conversion. Though, I'm not seing how that really makes the whole thing > simpler. > > So far we had good results with changing core code first and then fix up the > users one by one and at the end remove any intermediary helpers which we > introduced along the way. So I'd chose 2b as it's a very clear transition > mechanism with pretty low risk. Ok, thanks for your input, let's do that then. I'd like to get the preparation patch ([RFC v2b 1/5] vfs: Add vfs_time accessors) merged as soon as possible so we can do the rest of the series through the file system maintainer trees where possible. I guess even though the patch by itself is trivial, it's now too late to do for 4.5, but we should be able to just put that into your tip tree, or Al's vfs tree for 4.6 and then try to get all file systems moved over for 4.7, followed by the vfs patch to actually change the type. Arnd ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [RFC v2] vfs 64 bit time transition proposals
On Wed, 24 Feb 2016, Thomas Gleixner wrote: > On Fri, 12 Feb 2016, Arnd Bergmann wrote: > > Regarding the three versions, I think all of them are doable > > doable, and they all have their upsides and downsides but no > > showstoppers. > > > > Let me summarize what I see in the patches: > > > > 2a is the smallest set of changes in number of lines, as you indicated > >in the previous discussion (I was skeptical here initially, but > >you were right). The main downside is that each patch has to > >carefully consider what happens at the point when the type gets > >flipped, so that printk format strings are correct and assignments > >to local variables don't truncate the range. It also requires > >changing the types again after the VFS change, but that is > >something we can automate using coccinelle. > > Yes, you can do that final change with coccinelle, but all in all this has the > highest risk. Recent versions of Coccinelle can help a bit with format string changes. See demos/format.cocci. julia > > 2b has the main advantage of not changing behavior with the flip, so > >we can convert all file systems to use vfs_time relatively easily > >and then later make them actually use 64-bit timestamps with > >a patch that each file system developer can do for themselves. > > And that's a very clear advantage of this approch. The change is purely > mechanical and can be largely done with coccinelle. > > >One downside is that it leads to rather ugly code as discussed > >before, examples are in "[RFC v2b 5/5] fs: xfs: change inode > >times to use vfs_time data type" and "[RFC v2b 3/5] fs: btrfs: > >Use vfs_time accessors". > > - now = current_fs_time(inode->i_sb); > - if (!timespec_equal(&inode->i_mtime, &now)) > - inode->i_mtime = now; > + now = vfs_time_to_timespec(current_fs_time(inode->i_sb)); > + ts = vfs_time_to_timespec(inode->i_mtime); > + if (!timespec_equal(&ts, &now)) > + inode->i_mtime = timespec_to_vfs_time(now); > + > + ts = vfs_time_to_timespec(inode->i_mtime); > + if (!timespec_equal(&ts, &now)) > + inode->i_ctime = timespec_to_vfs_time(now); > > You can either provide a helper function which does that m/c_time update at > the VFS level where you can hide the mess or just accept that this transition > will introduce some odd constructs like the above. > > > 2c gets us the furthest along the way for the conversion, close > >to where we want to end up in the long run, so we could do that > >to file systems one by one. The behavior change is immediate, > >so there are fewer possible surprises than with 2a, but it > >also means the most upfront work. > > I would'nt worry about the upfront work to much, if it is the cleanest way to > do the conversion. Though, I'm not seing how that really makes the whole thing > simpler. > > So far we had good results with changing core code first and then fix up the > users one by one and at the end remove any intermediary helpers which we > introduced along the way. So I'd chose 2b as it's a very clear transition > mechanism with pretty low risk. > > Thanks, > > tglx > > > ___ > Y2038 mailing list > Y2038@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/y2038 > ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [RFC v2] vfs 64 bit time transition proposals
On Fri, 12 Feb 2016, Arnd Bergmann wrote: > Regarding the three versions, I think all of them are doable > doable, and they all have their upsides and downsides but no > showstoppers. > > Let me summarize what I see in the patches: > > 2a is the smallest set of changes in number of lines, as you indicated >in the previous discussion (I was skeptical here initially, but >you were right). The main downside is that each patch has to >carefully consider what happens at the point when the type gets >flipped, so that printk format strings are correct and assignments >to local variables don't truncate the range. It also requires >changing the types again after the VFS change, but that is >something we can automate using coccinelle. Yes, you can do that final change with coccinelle, but all in all this has the highest risk. > 2b has the main advantage of not changing behavior with the flip, so >we can convert all file systems to use vfs_time relatively easily >and then later make them actually use 64-bit timestamps with >a patch that each file system developer can do for themselves. And that's a very clear advantage of this approch. The change is purely mechanical and can be largely done with coccinelle. >One downside is that it leads to rather ugly code as discussed >before, examples are in "[RFC v2b 5/5] fs: xfs: change inode >times to use vfs_time data type" and "[RFC v2b 3/5] fs: btrfs: >Use vfs_time accessors". - now = current_fs_time(inode->i_sb); - if (!timespec_equal(&inode->i_mtime, &now)) - inode->i_mtime = now; + now = vfs_time_to_timespec(current_fs_time(inode->i_sb)); + ts = vfs_time_to_timespec(inode->i_mtime); + if (!timespec_equal(&ts, &now)) + inode->i_mtime = timespec_to_vfs_time(now); + + ts = vfs_time_to_timespec(inode->i_mtime); + if (!timespec_equal(&ts, &now)) + inode->i_ctime = timespec_to_vfs_time(now); You can either provide a helper function which does that m/c_time update at the VFS level where you can hide the mess or just accept that this transition will introduce some odd constructs like the above. > 2c gets us the furthest along the way for the conversion, close >to where we want to end up in the long run, so we could do that >to file systems one by one. The behavior change is immediate, >so there are fewer possible surprises than with 2a, but it >also means the most upfront work. I would'nt worry about the upfront work to much, if it is the cleanest way to do the conversion. Though, I'm not seing how that really makes the whole thing simpler. So far we had good results with changing core code first and then fix up the users one by one and at the end remove any intermediary helpers which we introduced along the way. So I'd chose 2b as it's a very clear transition mechanism with pretty low risk. Thanks, tglx ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [RFC v2] vfs 64 bit time transition proposals
> Regarding the three versions, I think all of them are doable > doable, and they all have their upsides and downsides but no > showstoppers. I agree that all the approaches are doable. > Let me summarize what I see in the patches: > > 2a is the smallest set of changes in number of lines, as you indicated >in the previous discussion (I was skeptical here initially, but >you were right). The main downside is that each patch has to >carefully consider what happens at the point when the type gets >flipped, so that printk format strings are correct and assignments >to local variables don't truncate the range. It also requires >changing the types again after the VFS change, but that is >something we can automate using coccinelle. 2c has the same downside as this. It also has to carefully consider what happens when you switch end filesystems to timespec64, be it for printks or assignments. I would say that the effort to do this was the same for 2a and 2c. And, 2c also needs to get rid of the abstraction macros when vfs is transitioned to using timespec64. > 2b has the main advantage of not changing behavior with the flip, so >we can convert all file systems to use vfs_time relatively easily >and then later make them actually use 64-bit timestamps with >a patch that each file system developer can do for themselves. >One downside is that it leads to rather ugly code as discussed >before, examples are in "[RFC v2b 5/5] fs: xfs: change inode >times to use vfs_time data type" and "[RFC v2b 3/5] fs: btrfs: >Use vfs_time accessors". Here is the breakup of the number of changes required from the table in the cover letter(https://lkml.org/lkml/2016/2/12/76): * # Changes needed in 2a = row 1 + row 7 + row 8 + row 9 + row 10 = 34 + 80 + 10 + 3 + 3 = 130 * # Changes needed in 2b = row 1 + row 4 + row 5 + row 6 + row 7 * (~3) = 34 + 80 + 141 + 74 + 85 + 240 = 654 * # Changes needed in 2c = Changes in 2b + some more It is clear to see from the above table that number of such changes will be considerably more for approaches 2b and 2c. And, 2b is not even close to what we want to achieve and will again confuse developers even more as there will be 2 sets of abstraction apis now: 1. vfs_time apis 2. timespec64 to timespec/ timespec to timespec64 apis Since there is no clean up effort here after vfs is switched over, we are just making all filesystems that use these apis harder to read. > 2c gets us the furthest along the way for the conversion, close >to where we want to end up in the long run, so we could do that >to file systems one by one. The behavior change is immediate, >so there are fewer possible surprises than with 2a, but it >also means the most upfront work. 2c abstractions can be used in more than one way. And, 2c also introduces a new timestamp data type along with timespec64 in the filesystem code. The above two factors can make it confusing for the developers until we transition vfs and remove abstractions from individual filesystems. And, this is a problem as we want to remove abstractions in a different kernel release than the one we do the transition in, as we discussed previously. 2a still seems like the right choice to me. And, will have the least number of changes. As Arnd thinks all of them are doable, if anybody else has other concerns we missed please comment. -Deepa ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [RFC v2] vfs 64 bit time transition proposals
On Friday 12 February 2016 01:21:59 Deepa Dinamani wrote: > Introduction > > This is a follow on to the series: https://lkml.org/lkml/2016/1/7/20 [1]. > This is aimed at reaching a consensus on how to transition the vfs > timestamps to use 64 bit time. This demonstrates three ways (2a, 2b and > 2c) of solving this problem. Each of the proposals has its own cover > letter that explains the individual approach. Proposals 2b and 2c also > outline variant approaches which are similar to the respective proposals. > This drives the proposal count to 5. All the changes have been discussed > with Arnd Bergmann, who posted the original series: > https://lkml.org/lkml/2014/5/30/669 [2] > > The series has been simplified to include only the 64 bit timestamp > changes as per Dave Chinner’s suggestion. > > Motivation > > The problem is how to change the vfs inode timestamps to use 64 bit > times to overcome the 2038 problem. > > Below table [3] gives an overview of the extent/ type of changes > needed of changes needed. > The series is aimed at obtaining small manageable patches for all > the cases in [3]. > Hi Deepa, Thanks a lot for posting this updated series, very nice work! Regarding the three versions, I think all of them are doable doable, and they all have their upsides and downsides but no showstoppers. Let me summarize what I see in the patches: 2a is the smallest set of changes in number of lines, as you indicated in the previous discussion (I was skeptical here initially, but you were right). The main downside is that each patch has to carefully consider what happens at the point when the type gets flipped, so that printk format strings are correct and assignments to local variables don't truncate the range. It also requires changing the types again after the VFS change, but that is something we can automate using coccinelle. 2b has the main advantage of not changing behavior with the flip, so we can convert all file systems to use vfs_time relatively easily and then later make them actually use 64-bit timestamps with a patch that each file system developer can do for themselves. One downside is that it leads to rather ugly code as discussed before, examples are in "[RFC v2b 5/5] fs: xfs: change inode times to use vfs_time data type" and "[RFC v2b 3/5] fs: btrfs: Use vfs_time accessors". 2c gets us the furthest along the way for the conversion, close to where we want to end up in the long run, so we could do that to file systems one by one. The behavior change is immediate, so there are fewer possible surprises than with 2a, but it also means the most upfront work. Arnd ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
