[PATCH v2 0/6] Delete timespec64_trunc()

[PATCH v2 0/6] Delete timespec64_trunc()

[Deepa Dinamani]

This series aims at deleting timespec64_trunc().
There is a new api: timestamp_truncate() that is the
replacement api. The api additionally does a limits
check on the filesystem timestamps.

The suggestion to open code some of the truncate logic
came from Al Viro. And, this does make the code in some
filesystems easy to follow.

The series also does some update_time() cleanup as
suggested by Al Viro.

Deepa Dinamani (6):
  fs: fat: Eliminate timespec64_trunc() usage
  fs: cifs: Delete usage of timespec64_trunc
  fs: ceph: Delete timespec64_trunc() usage
  fs: ubifs: Eliminate timespec64_trunc() usage
  fs: Delete timespec64_trunc()
  fs: Do not overload update_time

 fs/ceph/mds_client.c |  4 +---
 fs/cifs/inode.c      | 13 +++++++------
 fs/fat/misc.c        | 10 +++++++++-
 fs/inode.c           | 33 +++------------------------------
 fs/ubifs/sb.c        | 11 ++++-------
 include/linux/fs.h   |  1 -
 6 files changed, 24 insertions(+), 48 deletions(-)

-- 
Changes since v1:
* Dropped the atime comparison (patch 2/7) taken through cifs tree.
* Refactored update_time according to review comments.
2.17.1

Cc: ceph-devel@vger.kernel.org
Cc: hirofumi@mail.parknet.co.jp
Cc: jlayton@kernel.org
Cc: linux-cifs@vger.kernel.org
Cc: linux-mtd@lists.infradead.org
Cc: richard@nod.at
Cc: stfrench@microsoft.com

[PATCH v2 3/6] fs: ceph: Delete timespec64_trunc() usage

[Deepa Dinamani]

Since ceph always uses ns granularity, skip the
truncation which is a no-op.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: jlayton@kernel.org
Cc: ceph-devel@vger.kernel.org
---
 fs/ceph/mds_client.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 068b029cf073..88687ed65cff 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2069,7 +2069,6 @@ struct ceph_mds_request *
 ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
 {
 	struct ceph_mds_request *req = kzalloc(sizeof(*req), GFP_NOFS);
-	struct timespec64 ts;
 
 	if (!req)
 		return ERR_PTR(-ENOMEM);
@@ -2088,8 +2087,7 @@ ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
 	init_completion(&req->r_safe_completion);
 	INIT_LIST_HEAD(&req->r_unsafe_item);
 
-	ktime_get_coarse_real_ts64(&ts);
-	req->r_stamp = timespec64_trunc(ts, mdsc->fsc->sb->s_time_gran);
+	ktime_get_coarse_real_ts64(&req->r_stamp);
 
 	req->r_op = op;
 	req->r_direct_mode = mode;
-- 
2.17.1

Re: [PATCH v2 3/6] fs: ceph: Delete timespec64_trunc() usage

[Jeff Layton]

On Mon, 2019-12-02 at 21:19 -0800, Deepa Dinamani wrote:
> Since ceph always uses ns granularity, skip the
> truncation which is a no-op.
> 
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: jlayton@kernel.org
> Cc: ceph-devel@vger.kernel.org
> ---
>  fs/ceph/mds_client.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 068b029cf073..88687ed65cff 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -2069,7 +2069,6 @@ struct ceph_mds_request *
>  ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
>  {
>  	struct ceph_mds_request *req = kzalloc(sizeof(*req), GFP_NOFS);
> -	struct timespec64 ts;
>  
>  	if (!req)
>  		return ERR_PTR(-ENOMEM);
> @@ -2088,8 +2087,7 @@ ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
>  	init_completion(&req->r_safe_completion);
>  	INIT_LIST_HEAD(&req->r_unsafe_item);
>  
> -	ktime_get_coarse_real_ts64(&ts);
> -	req->r_stamp = timespec64_trunc(ts, mdsc->fsc->sb->s_time_gran);
> +	ktime_get_coarse_real_ts64(&req->r_stamp);
>  
>  	req->r_op = op;
>  	req->r_direct_mode = mode;

Thanks Deepa. We'll plan to take this one in via the ceph tree.

Cheers,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 3/6] fs: ceph: Delete timespec64_trunc() usage

[Deepa Dinamani]

> Thanks Deepa. We'll plan to take this one in via the ceph tree.

Actually, deletion of the timespec64_trunc() will depend on this
patch. Can we merge the series through a common tree? Otherwise,
whoever takes the [PATCH 6/7] ("fs:
Delete timespec64_trunc()") would have to depend on your tree. If you
are ok with the change, can you ack it?

Thanks,
Deepa

Re: [PATCH v2 3/6] fs: ceph: Delete timespec64_trunc() usage

[Jeff Layton]

On Tue, 2019-12-03 at 11:41 -0800, Deepa Dinamani wrote:
> > Thanks Deepa. We'll plan to take this one in via the ceph tree.
> 
> Actually, deletion of the timespec64_trunc() will depend on this
> patch. Can we merge the series through a common tree? Otherwise,
> whoever takes the [PATCH 6/7] ("fs:
> Delete timespec64_trunc()") would have to depend on your tree. If you
> are ok with the change, can you ack it?
> 
> Thanks,
> Deepa

Sure, no problem if that works better for you.

Acked-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 0/6] Delete timespec64_trunc()

[Deepa Dinamani]

On Mon, Dec 2, 2019 at 9:20 PM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> This series aims at deleting timespec64_trunc().
> There is a new api: timestamp_truncate() that is the
> replacement api. The api additionally does a limits
> check on the filesystem timestamps.

Al/Andrew, can one of you help merge these patches?

Thanks,
-Deepa

Re: [PATCH v2 0/6] Delete timespec64_trunc()

[Al Viro]

On Thu, Dec 05, 2019 at 06:43:26PM -0800, Deepa Dinamani wrote:
> On Mon, Dec 2, 2019 at 9:20 PM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> > This series aims at deleting timespec64_trunc().
> > There is a new api: timestamp_truncate() that is the
> > replacement api. The api additionally does a limits
> > check on the filesystem timestamps.
> 
> Al/Andrew, can one of you help merge these patches?

Looks sane.  Could you check if #misc.timestamp looks sane to you?

One thing that leaves me scratching head is kernfs - surely we
are _not_ limited by any external layouts there, so why do we
need to bother with truncation?

Re: [PATCH v2 0/6] Delete timespec64_trunc()

[Deepa Dinamani]

On Fri, Dec 6, 2019 at 10:02 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Thu, Dec 05, 2019 at 06:43:26PM -0800, Deepa Dinamani wrote:
> > On Mon, Dec 2, 2019 at 9:20 PM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> > > This series aims at deleting timespec64_trunc().
> > > There is a new api: timestamp_truncate() that is the
> > > replacement api. The api additionally does a limits
> > > check on the filesystem timestamps.
> >
> > Al/Andrew, can one of you help merge these patches?
>
> Looks sane.  Could you check if #misc.timestamp looks sane to you?

Yes, that looks sane to me.

> One thing that leaves me scratching head is kernfs - surely we
> are _not_ limited by any external layouts there, so why do we
> need to bother with truncation?

I think I was more pedantic then, and was explicitly truncating times
before assignment to inode timestamps. But, Arnd has since coached me
that we should not introduce things to safe guard against all
possibilities, but only what is needed currently. So this kernfs
truncate is redundant, given the limits and the granularity match vfs
timestamp representation limits.

-Deepa

Re: [PATCH v2 0/6] Delete timespec64_trunc()

[Al Viro]

On Sat, Dec 07, 2019 at 06:04:38PM -0800, Deepa Dinamani wrote:
> On Fri, Dec 6, 2019 at 10:02 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Thu, Dec 05, 2019 at 06:43:26PM -0800, Deepa Dinamani wrote:
> > > On Mon, Dec 2, 2019 at 9:20 PM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> > > > This series aims at deleting timespec64_trunc().
> > > > There is a new api: timestamp_truncate() that is the
> > > > replacement api. The api additionally does a limits
> > > > check on the filesystem timestamps.
> > >
> > > Al/Andrew, can one of you help merge these patches?
> >
> > Looks sane.  Could you check if #misc.timestamp looks sane to you?
> 
> Yes, that looks sane to me.
> 
> > One thing that leaves me scratching head is kernfs - surely we
> > are _not_ limited by any external layouts there, so why do we
> > need to bother with truncation?
> 
> I think I was more pedantic then, and was explicitly truncating times
> before assignment to inode timestamps. But, Arnd has since coached me
> that we should not introduce things to safe guard against all
> possibilities, but only what is needed currently. So this kernfs
> truncate is redundant, given the limits and the granularity match vfs
> timestamp representation limits.

OK...  I've tossed a followup removing the truncation from kernfs;
the whole series looks reasonably safe, but I don't think it's urgent
enough to even try getting it merged before -rc1.  So here's what
I'm going to do: immediately after -rc1 it gets renamed[*] to #imm.timestamp,
which will be in the never-modified mode, in #for-next from the very
begining and safe for other trees to pull.  Current shortlog:

Al Viro (1):
      kernfs: don't bother with timestamp truncation

Amir Goldstein (1):
      utimes: Clamp the timestamps in notify_change()

Deepa Dinamani (6):
      fs: fat: Eliminate timespec64_trunc() usage
      fs: cifs: Delete usage of timespec64_trunc
      fs: ceph: Delete timespec64_trunc() usage
      fs: ubifs: Eliminate timespec64_trunc() usage
      fs: Delete timespec64_trunc()
      fs: Do not overload update_time

Diffstat:
 fs/attr.c            | 23 +++++++++++------------
 fs/ceph/mds_client.c |  4 +---
 fs/cifs/inode.c      | 13 +++++++------
 fs/configfs/inode.c  |  9 +++------
 fs/f2fs/file.c       | 18 ++++++------------
 fs/fat/misc.c        | 10 +++++++++-
 fs/inode.c           | 33 +++------------------------------
 fs/kernfs/inode.c    |  6 +++---
 fs/ntfs/inode.c      | 18 ++++++------------
 fs/ubifs/file.c      | 18 ++++++------------
 fs/ubifs/sb.c        | 11 ++++-------
 fs/utimes.c          |  4 ++--
 include/linux/fs.h   |  1 -
 13 files changed, 61 insertions(+), 107 deletions(-)

[*] right now it's based on v5.4; I don't see anything that would
warrant rebasing it to -rc1 at the moment, but if anything of that
sort shows up tomorrow, s/renamed/rebased to -rc1 and renamed/.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 0/6] Delete timespec64_trunc()

[Al Viro]

On Sun, Dec 08, 2019 at 03:04:07AM +0000, Al Viro wrote:

> OK...  I've tossed a followup removing the truncation from kernfs;
> the whole series looks reasonably safe, but I don't think it's urgent
> enough to even try getting it merged before -rc1.  So here's what
> I'm going to do: immediately after -rc1 it gets renamed[*] to #imm.timestamp,
> which will be in the never-modified mode, in #for-next from the very
> begining and safe for other trees to pull.

Rebased to -rc1, pushed out as #imm.timestamp, included into #for-next.
Never-modified mode...