From: Deepa Dinamani <deepa.kernel@gmail.com> To: Al Viro <viro@zeniv.linux.org.uk> Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>, Arnd Bergmann <arnd@arndb.de>, Miklos Szeredi <miklos@szeredi.hu>, y2038 Mailman List <y2038@lists.linaro.org>, Amir Goldstein <amir73il@gmail.com>, Jeff Layton <jlayton@kernel.org>, overlayfs <linux-unionfs@vger.kernel.org>, "J . Bruce Fields" <bfields@fieldses.org>, Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org> Subject: Re: [PATCH] utimes: Clamp the timestamps in notify_change() Date: Fri, 29 Nov 2019 21:34:35 -0800 [thread overview] Message-ID: <CABeXuvouvniNnxPwZbejPgZPUVgShvmPnRiGFPF8-0Z6AmmvQA@mail.gmail.com> (raw) In-Reply-To: <20191124213415.GD4203@ZenIV.linux.org.uk> On Sun, Nov 24, 2019 at 1:34 PM Al Viro <viro@zeniv.linux.org.uk> wrote: > > On Sun, Nov 24, 2019 at 01:13:50PM -0800, Deepa Dinamani wrote: > > > We also want to replace all uses of timespec64_trunc() with > > timestamp_truncate() for all fs cases. > > > > In that case we have a few more: > > > > fs/ceph/mds_client.c: req->r_stamp = timespec64_trunc(ts, > > mdsc->fsc->sb->s_time_gran); > > Umm... That comes from ktime_get_coarse_real_ts64(&ts); > > > fs/cifs/inode.c: fattr->cf_mtime = > > timespec64_trunc(fattr->cf_mtime, sb->s_time_gran); > ktime_get_real_ts64(&fattr->cf_mtime) here > > > fs/cifs/inode.c: fattr->cf_atime = > > timespec64_trunc(fattr->cf_atime, sb->s_time_gran); > ditto > > > fs/fat/misc.c: inode->i_ctime = > > timespec64_trunc(*now, 10000000); > > I wonder... some are from setattr, some (with NULL passed to fat_truncate_time()) > from current_time()... Wouldn't it make more sense to move the truncation into > the few callers that really need it (if any)? Quite a few of those are *also* > getting the value from current_time(), after all. fat_fill_inode() looks like > the only case that doesn't fall into these classes; does it need truncation? I've posted a series at https://lore.kernel.org/lkml/20191130053030.7868-1-deepa.kernel@gmail.com/ I was able to get rid of all instances but it seemed like it would be easier for cifs to use timestamp_truncate() directly. If you really don't want it exported, I could find some other way of doing it. > BTW, could we *please* do something about fs/inode.c:update_time()? I mean, > sure, local variable shadows file-scope function, so it's legitimate C, but > this is not IOCCC and having a function called 'update_time' end with > return update_time(inode, time, flags); > is actively hostile towards casual readers... Added this to the series as well. -Deepa _______________________________________________ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
WARNING: multiple messages have this Message-ID (diff)
From: Deepa Dinamani <deepa.kernel@gmail.com> To: Al Viro <viro@zeniv.linux.org.uk> Cc: Amir Goldstein <amir73il@gmail.com>, Arnd Bergmann <arnd@arndb.de>, Jeff Layton <jlayton@kernel.org>, "J . Bruce Fields" <bfields@fieldses.org>, Miklos Szeredi <miklos@szeredi.hu>, Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org>, overlayfs <linux-unionfs@vger.kernel.org>, Linux NFS Mailing List <linux-nfs@vger.kernel.org>, y2038 Mailman List <y2038@lists.linaro.org> Subject: Re: [PATCH] utimes: Clamp the timestamps in notify_change() Date: Fri, 29 Nov 2019 21:34:35 -0800 [thread overview] Message-ID: <CABeXuvouvniNnxPwZbejPgZPUVgShvmPnRiGFPF8-0Z6AmmvQA@mail.gmail.com> (raw) In-Reply-To: <20191124213415.GD4203@ZenIV.linux.org.uk> On Sun, Nov 24, 2019 at 1:34 PM Al Viro <viro@zeniv.linux.org.uk> wrote: > > On Sun, Nov 24, 2019 at 01:13:50PM -0800, Deepa Dinamani wrote: > > > We also want to replace all uses of timespec64_trunc() with > > timestamp_truncate() for all fs cases. > > > > In that case we have a few more: > > > > fs/ceph/mds_client.c: req->r_stamp = timespec64_trunc(ts, > > mdsc->fsc->sb->s_time_gran); > > Umm... That comes from ktime_get_coarse_real_ts64(&ts); > > > fs/cifs/inode.c: fattr->cf_mtime = > > timespec64_trunc(fattr->cf_mtime, sb->s_time_gran); > ktime_get_real_ts64(&fattr->cf_mtime) here > > > fs/cifs/inode.c: fattr->cf_atime = > > timespec64_trunc(fattr->cf_atime, sb->s_time_gran); > ditto > > > fs/fat/misc.c: inode->i_ctime = > > timespec64_trunc(*now, 10000000); > > I wonder... some are from setattr, some (with NULL passed to fat_truncate_time()) > from current_time()... Wouldn't it make more sense to move the truncation into > the few callers that really need it (if any)? Quite a few of those are *also* > getting the value from current_time(), after all. fat_fill_inode() looks like > the only case that doesn't fall into these classes; does it need truncation? I've posted a series at https://lore.kernel.org/lkml/20191130053030.7868-1-deepa.kernel@gmail.com/ I was able to get rid of all instances but it seemed like it would be easier for cifs to use timestamp_truncate() directly. If you really don't want it exported, I could find some other way of doing it. > BTW, could we *please* do something about fs/inode.c:update_time()? I mean, > sure, local variable shadows file-scope function, so it's legitimate C, but > this is not IOCCC and having a function called 'update_time' end with > return update_time(inode, time, flags); > is actively hostile towards casual readers... Added this to the series as well. -Deepa
next prev parent reply other threads:[~2019-11-30 5:34 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-24 19:31 [PATCH] utimes: Clamp the timestamps in notify_change() Amir Goldstein 2019-11-24 19:31 ` Amir Goldstein 2019-11-24 19:49 ` Al Viro 2019-11-24 19:49 ` Al Viro 2019-11-24 20:50 ` Amir Goldstein 2019-11-24 20:50 ` Amir Goldstein 2019-11-24 21:14 ` Deepa Dinamani 2019-11-24 21:14 ` Deepa Dinamani 2019-11-24 21:13 ` Deepa Dinamani 2019-11-24 21:13 ` Deepa Dinamani 2019-11-24 21:34 ` Al Viro 2019-11-24 21:34 ` Al Viro 2019-11-30 5:34 ` Deepa Dinamani [this message] 2019-11-30 5:34 ` Deepa Dinamani 2019-11-25 16:46 ` J . Bruce Fields 2019-11-25 16:46 ` J . Bruce Fields 2019-11-25 17:35 ` Amir Goldstein 2019-11-25 17:35 ` Amir Goldstein 2019-11-25 18:16 ` Deepa Dinamani 2019-11-25 18:16 ` Deepa Dinamani
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CABeXuvouvniNnxPwZbejPgZPUVgShvmPnRiGFPF8-0Z6AmmvQA@mail.gmail.com \ --to=deepa.kernel@gmail.com \ --cc=amir73il@gmail.com \ --cc=arnd@arndb.de \ --cc=bfields@fieldses.org \ --cc=jlayton@kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-nfs@vger.kernel.org \ --cc=linux-unionfs@vger.kernel.org \ --cc=miklos@szeredi.hu \ --cc=viro@zeniv.linux.org.uk \ --cc=y2038@lists.linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.