Linux-CIFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/7] Delete timespec64_trunc()
@ 2019-11-30  5:30 Deepa Dinamani
  2019-11-30  5:30 ` [PATCH 2/7] fs: cifs: Fix atime update check vs mtime Deepa Dinamani
  2019-11-30  5:30 ` [PATCH 3/7] fs: cifs: Delete usage of timespec64_trunc Deepa Dinamani
  0 siblings, 2 replies; 6+ messages in thread
From: Deepa Dinamani @ 2019-11-30  5:30 UTC (permalink / raw)
  To: viro, linux-kernel
  Cc: linux-fsdevel, arnd, hirofumi, jlayton, richard, stfrench,
	ceph-devel, linux-cifs, linux-mtd

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 (7):
  fs: fat: Eliminate timespec64_trunc() usage
  fs: cifs: Fix atime update check vs mtime
  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 |  3 +--
 fs/cifs/inode.c      | 15 ++++++++-------
 fs/fat/misc.c        | 10 +++++++++-
 fs/inode.c           | 30 +++---------------------------
 fs/ubifs/sb.c        | 11 ++++-------
 include/linux/fs.h   |  1 -
 6 files changed, 25 insertions(+), 45 deletions(-)

-- 
2.17.1

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/7] fs: cifs: Fix atime update check vs mtime
  2019-11-30  5:30 [PATCH 0/7] Delete timespec64_trunc() Deepa Dinamani
@ 2019-11-30  5:30 ` Deepa Dinamani
  2019-12-01 18:23   ` Steve French
  2019-11-30  5:30 ` [PATCH 3/7] fs: cifs: Delete usage of timespec64_trunc Deepa Dinamani
  1 sibling, 1 reply; 6+ messages in thread
From: Deepa Dinamani @ 2019-11-30  5:30 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, stfrench, linux-cifs

According to the comment in the code and commit log, some apps
expect atime >= mtime; but the introduced code results in
atime==mtime.  Fix the comparison to guard against atime<mtime.

Fixes: 9b9c5bea0b96 ("cifs: do not return atime less than mtime")
Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: stfrench@microsoft.com
Cc: linux-cifs@vger.kernel.org
---
 fs/cifs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 8a76195e8a69..ca76a9287456 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -163,7 +163,7 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
 
 	spin_lock(&inode->i_lock);
 	/* we do not want atime to be less than mtime, it broke some apps */
-	if (timespec64_compare(&fattr->cf_atime, &fattr->cf_mtime))
+	if (timespec64_compare(&fattr->cf_atime, &fattr->cf_mtime) < 0)
 		inode->i_atime = fattr->cf_mtime;
 	else
 		inode->i_atime = fattr->cf_atime;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/7] fs: cifs: Delete usage of timespec64_trunc
  2019-11-30  5:30 [PATCH 0/7] Delete timespec64_trunc() Deepa Dinamani
  2019-11-30  5:30 ` [PATCH 2/7] fs: cifs: Fix atime update check vs mtime Deepa Dinamani
@ 2019-11-30  5:30 ` Deepa Dinamani
  2019-11-30 21:25   ` Steve French
  1 sibling, 1 reply; 6+ messages in thread
From: Deepa Dinamani @ 2019-11-30  5:30 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, stfrench, linux-cifs

timestamp_truncate() is the replacement api for
timespec64_trunc. timestamp_truncate() additionally clamps
timestamps to make sure the timestamps lie within the
permitted range for the filesystem.

Truncate the timestamps in the struct cifs_attr at the
site of assignment to inode times. This
helps us use the right fs api timestamp_trucate() to
perform the truncation.

Also update the ktime_get_* api to match the one used in
current_time(). This allows for timestamps to be updated
the same way always.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: stfrench@microsoft.com
Cc: linux-cifs@vger.kernel.org
---
 fs/cifs/inode.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ca76a9287456..026ed49e8aa4 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -113,6 +113,7 @@ cifs_revalidate_cache(struct inode *inode, struct cifs_fattr *fattr)
 	}
 
 	 /* revalidate if mtime or size have changed */
+	fattr->cf_mtime = timestamp_truncate(fattr->cf_mtime, inode);
 	if (timespec64_equal(&inode->i_mtime, &fattr->cf_mtime) &&
 	    cifs_i->server_eof == fattr->cf_eof) {
 		cifs_dbg(FYI, "%s: inode %llu is unchanged\n",
@@ -162,6 +163,9 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
 	cifs_revalidate_cache(inode, fattr);
 
 	spin_lock(&inode->i_lock);
+	fattr->cf_mtime = timestamp_truncate(fattr->cf_mtime, inode);
+	fattr->cf_atime = timestamp_truncate(fattr->cf_atime, inode);
+	fattr->cf_ctime = timestamp_truncate(fattr->cf_ctime, inode);
 	/* we do not want atime to be less than mtime, it broke some apps */
 	if (timespec64_compare(&fattr->cf_atime, &fattr->cf_mtime) < 0)
 		inode->i_atime = fattr->cf_mtime;
@@ -329,8 +333,7 @@ cifs_create_dfs_fattr(struct cifs_fattr *fattr, struct super_block *sb)
 	fattr->cf_mode = S_IFDIR | S_IXUGO | S_IRWXU;
 	fattr->cf_uid = cifs_sb->mnt_uid;
 	fattr->cf_gid = cifs_sb->mnt_gid;
-	ktime_get_real_ts64(&fattr->cf_mtime);
-	fattr->cf_mtime = timespec64_trunc(fattr->cf_mtime, sb->s_time_gran);
+	ktime_get_coarse_real_ts64(&fattr->cf_mtime);
 	fattr->cf_atime = fattr->cf_ctime = fattr->cf_mtime;
 	fattr->cf_nlink = 2;
 	fattr->cf_flags = CIFS_FATTR_DFS_REFERRAL;
@@ -609,10 +612,8 @@ cifs_all_info_to_fattr(struct cifs_fattr *fattr, FILE_ALL_INFO *info,
 
 	if (info->LastAccessTime)
 		fattr->cf_atime = cifs_NTtimeToUnix(info->LastAccessTime);
-	else {
-		ktime_get_real_ts64(&fattr->cf_atime);
-		fattr->cf_atime = timespec64_trunc(fattr->cf_atime, sb->s_time_gran);
-	}
+	else
+		ktime_get_coarse_real_ts64(&fattr->cf_atime);
 
 	fattr->cf_ctime = cifs_NTtimeToUnix(info->ChangeTime);
 	fattr->cf_mtime = cifs_NTtimeToUnix(info->LastWriteTime);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/7] fs: cifs: Delete usage of timespec64_trunc
  2019-11-30  5:30 ` [PATCH 3/7] fs: cifs: Delete usage of timespec64_trunc Deepa Dinamani
@ 2019-11-30 21:25   ` Steve French
  2019-12-01  5:35     ` Deepa Dinamani
  0 siblings, 1 reply; 6+ messages in thread
From: Steve French @ 2019-11-30 21:25 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: Al Viro, LKML, linux-fsdevel, Arnd Bergmann, Steve French, CIFS

Is this intended to merge separately or do you want it merged through
the cifs git tree?

On Fri, Nov 29, 2019 at 11:33 PM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> timestamp_truncate() is the replacement api for
> timespec64_trunc. timestamp_truncate() additionally clamps
> timestamps to make sure the timestamps lie within the
> permitted range for the filesystem.
>
> Truncate the timestamps in the struct cifs_attr at the
> site of assignment to inode times. This
> helps us use the right fs api timestamp_trucate() to
> perform the truncation.
>
> Also update the ktime_get_* api to match the one used in
> current_time(). This allows for timestamps to be updated
> the same way always.
>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: stfrench@microsoft.com
> Cc: linux-cifs@vger.kernel.org
> ---
>  fs/cifs/inode.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index ca76a9287456..026ed49e8aa4 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -113,6 +113,7 @@ cifs_revalidate_cache(struct inode *inode, struct cifs_fattr *fattr)
>         }
>
>          /* revalidate if mtime or size have changed */
> +       fattr->cf_mtime = timestamp_truncate(fattr->cf_mtime, inode);
>         if (timespec64_equal(&inode->i_mtime, &fattr->cf_mtime) &&
>             cifs_i->server_eof == fattr->cf_eof) {
>                 cifs_dbg(FYI, "%s: inode %llu is unchanged\n",
> @@ -162,6 +163,9 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
>         cifs_revalidate_cache(inode, fattr);
>
>         spin_lock(&inode->i_lock);
> +       fattr->cf_mtime = timestamp_truncate(fattr->cf_mtime, inode);
> +       fattr->cf_atime = timestamp_truncate(fattr->cf_atime, inode);
> +       fattr->cf_ctime = timestamp_truncate(fattr->cf_ctime, inode);
>         /* we do not want atime to be less than mtime, it broke some apps */
>         if (timespec64_compare(&fattr->cf_atime, &fattr->cf_mtime) < 0)
>                 inode->i_atime = fattr->cf_mtime;
> @@ -329,8 +333,7 @@ cifs_create_dfs_fattr(struct cifs_fattr *fattr, struct super_block *sb)
>         fattr->cf_mode = S_IFDIR | S_IXUGO | S_IRWXU;
>         fattr->cf_uid = cifs_sb->mnt_uid;
>         fattr->cf_gid = cifs_sb->mnt_gid;
> -       ktime_get_real_ts64(&fattr->cf_mtime);
> -       fattr->cf_mtime = timespec64_trunc(fattr->cf_mtime, sb->s_time_gran);
> +       ktime_get_coarse_real_ts64(&fattr->cf_mtime);
>         fattr->cf_atime = fattr->cf_ctime = fattr->cf_mtime;
>         fattr->cf_nlink = 2;
>         fattr->cf_flags = CIFS_FATTR_DFS_REFERRAL;
> @@ -609,10 +612,8 @@ cifs_all_info_to_fattr(struct cifs_fattr *fattr, FILE_ALL_INFO *info,
>
>         if (info->LastAccessTime)
>                 fattr->cf_atime = cifs_NTtimeToUnix(info->LastAccessTime);
> -       else {
> -               ktime_get_real_ts64(&fattr->cf_atime);
> -               fattr->cf_atime = timespec64_trunc(fattr->cf_atime, sb->s_time_gran);
> -       }
> +       else
> +               ktime_get_coarse_real_ts64(&fattr->cf_atime);
>
>         fattr->cf_ctime = cifs_NTtimeToUnix(info->ChangeTime);
>         fattr->cf_mtime = cifs_NTtimeToUnix(info->LastWriteTime);
> --
> 2.17.1
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/7] fs: cifs: Delete usage of timespec64_trunc
  2019-11-30 21:25   ` Steve French
@ 2019-12-01  5:35     ` Deepa Dinamani
  0 siblings, 0 replies; 6+ messages in thread
From: Deepa Dinamani @ 2019-12-01  5:35 UTC (permalink / raw)
  To: Steve French
  Cc: Al Viro, LKML, linux-fsdevel, Arnd Bergmann, Steve French, CIFS

On Sat, Nov 30, 2019 at 1:25 PM Steve French <smfrench@gmail.com> wrote:
>
> Is this intended to merge separately or do you want it merged through
> the cifs git tree?

It would be simplest for all uses of timespec64_trunc to be removed
through the same tree. Otherwise, whoever takes the [PATCH 6/7] ("fs:
Delete timespec64_trunc()") would have to depend on your tree.

Thanks,
-Deepa

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/7] fs: cifs: Fix atime update check vs mtime
  2019-11-30  5:30 ` [PATCH 2/7] fs: cifs: Fix atime update check vs mtime Deepa Dinamani
@ 2019-12-01 18:23   ` Steve French
  0 siblings, 0 replies; 6+ messages in thread
From: Steve French @ 2019-12-01 18:23 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: Al Viro, LKML, linux-fsdevel, Arnd Bergmann, Steve French, CIFS

merged into cifs-2.6.git for-next

On Fri, Nov 29, 2019 at 11:34 PM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> According to the comment in the code and commit log, some apps
> expect atime >= mtime; but the introduced code results in
> atime==mtime.  Fix the comparison to guard against atime<mtime.
>
> Fixes: 9b9c5bea0b96 ("cifs: do not return atime less than mtime")
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: stfrench@microsoft.com
> Cc: linux-cifs@vger.kernel.org
> ---
>  fs/cifs/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 8a76195e8a69..ca76a9287456 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -163,7 +163,7 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
>
>         spin_lock(&inode->i_lock);
>         /* we do not want atime to be less than mtime, it broke some apps */
> -       if (timespec64_compare(&fattr->cf_atime, &fattr->cf_mtime))
> +       if (timespec64_compare(&fattr->cf_atime, &fattr->cf_mtime) < 0)
>                 inode->i_atime = fattr->cf_mtime;
>         else
>                 inode->i_atime = fattr->cf_atime;
> --
> 2.17.1
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-30  5:30 [PATCH 0/7] Delete timespec64_trunc() Deepa Dinamani
2019-11-30  5:30 ` [PATCH 2/7] fs: cifs: Fix atime update check vs mtime Deepa Dinamani
2019-12-01 18:23   ` Steve French
2019-11-30  5:30 ` [PATCH 3/7] fs: cifs: Delete usage of timespec64_trunc Deepa Dinamani
2019-11-30 21:25   ` Steve French
2019-12-01  5:35     ` Deepa Dinamani

Linux-CIFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-cifs/0 linux-cifs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-cifs linux-cifs/ https://lore.kernel.org/linux-cifs \
		linux-cifs@vger.kernel.org
	public-inbox-index linux-cifs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-cifs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git