diff mbox series

[v19,68/70] ceph: fix updating the i_truncate_pagecache_size for fscrypt

Message ID 20230417032654.32352-69-xiubli@redhat.com (mailing list archive)
State New, archived
Headers show
Series ceph+fscrypt: full support | expand

Commit Message

Xiubo Li April 17, 2023, 3:26 a.m. UTC 
From: Xiubo Li <xiubli@redhat.com>

When fscrypt is enabled we will align the truncate size up to the
CEPH_FSCRYPT_BLOCK_SIZE always, so if we truncate the size in the
same block more than once, the latter ones will be skipped being
invalidated from the page caches.

This will force invalidating the page caches by using the smaller
size than the real file size.

At the same time add more debug log and fix the debug log for
truncate code.

URL: https://tracker.ceph.com/issues/58834
Tested-by: Luís Henriques <lhenriques@suse.de>
Tested-by: Venky Shankar <vshankar@redhat.com>
Reviewed-by: Luís Henriques <lhenriques@suse.de>
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/caps.c  |  4 ++--
 fs/ceph/inode.c | 35 ++++++++++++++++++++++++-----------
 2 files changed, 26 insertions(+), 13 deletions(-)

Comments

Milind Changire June 6, 2023, 7:04 a.m. UTC | #1 
Looks good to me.

Reviewed-by: Milind Changire <mchangir@redhat.com>

On Mon, Apr 17, 2023 at 9:03 AM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> When fscrypt is enabled we will align the truncate size up to the
> CEPH_FSCRYPT_BLOCK_SIZE always, so if we truncate the size in the
> same block more than once, the latter ones will be skipped being
> invalidated from the page caches.
>
> This will force invalidating the page caches by using the smaller
> size than the real file size.
>
> At the same time add more debug log and fix the debug log for
> truncate code.
>
> URL: https://tracker.ceph.com/issues/58834
> Tested-by: Luís Henriques <lhenriques@suse.de>
> Tested-by: Venky Shankar <vshankar@redhat.com>
> Reviewed-by: Luís Henriques <lhenriques@suse.de>
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  fs/ceph/caps.c  |  4 ++--
>  fs/ceph/inode.c | 35 ++++++++++++++++++++++++-----------
>  2 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index e1bb6d9c16f8..3c5450f9d825 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -3929,8 +3929,8 @@ static bool handle_cap_trunc(struct inode *inode,
>         if (IS_ENCRYPTED(inode) && size)
>                 size = extra_info->fscrypt_file_size;
>
> -       dout("handle_cap_trunc inode %p mds%d seq %d to %lld seq %d\n",
> -            inode, mds, seq, truncate_size, truncate_seq);
> +       dout("%s inode %p mds%d seq %d to %lld truncate seq %d\n",
> +            __func__, inode, mds, seq, truncate_size, truncate_seq);
>         queue_trunc = ceph_fill_file_size(inode, issued,
>                                           truncate_seq, truncate_size, size);
>         return queue_trunc;
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 1cfcbc39f7c6..059ebe42367c 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -763,7 +763,7 @@ int ceph_fill_file_size(struct inode *inode, int issued,
>                         ceph_fscache_update(inode);
>                 ci->i_reported_size = size;
>                 if (truncate_seq != ci->i_truncate_seq) {
> -                       dout("truncate_seq %u -> %u\n",
> +                       dout("%s truncate_seq %u -> %u\n", __func__,
>                              ci->i_truncate_seq, truncate_seq);
>                         ci->i_truncate_seq = truncate_seq;
>
> @@ -787,15 +787,26 @@ int ceph_fill_file_size(struct inode *inode, int issued,
>                         }
>                 }
>         }
> -       if (ceph_seq_cmp(truncate_seq, ci->i_truncate_seq) >= 0 &&
> -           ci->i_truncate_size != truncate_size) {
> -               dout("truncate_size %lld -> %llu\n", ci->i_truncate_size,
> -                    truncate_size);
> +
> +       /*
> +        * It's possible that the new sizes of the two consecutive
> +        * size truncations will be in the same fscrypt last block,
> +        * and we need to truncate the corresponding page caches
> +        * anyway.
> +        */
> +       if (ceph_seq_cmp(truncate_seq, ci->i_truncate_seq) >= 0) {
> +               dout("%s truncate_size %lld -> %llu, encrypted %d\n", __func__,
> +                    ci->i_truncate_size, truncate_size, !!IS_ENCRYPTED(inode));
> +
>                 ci->i_truncate_size = truncate_size;
> -               if (IS_ENCRYPTED(inode))
> +
> +               if (IS_ENCRYPTED(inode)) {
> +                       dout("%s truncate_pagecache_size %lld -> %llu\n",
> +                            __func__, ci->i_truncate_pagecache_size, size);
>                         ci->i_truncate_pagecache_size = size;
> -               else
> +               } else {
>                         ci->i_truncate_pagecache_size = truncate_size;
> +               }
>         }
>         return queue_trunc;
>  }
> @@ -2150,7 +2161,7 @@ void __ceph_do_pending_vmtruncate(struct inode *inode)
>  retry:
>         spin_lock(&ci->i_ceph_lock);
>         if (ci->i_truncate_pending == 0) {
> -               dout("__do_pending_vmtruncate %p none pending\n", inode);
> +               dout("%s %p none pending\n", __func__, inode);
>                 spin_unlock(&ci->i_ceph_lock);
>                 mutex_unlock(&ci->i_truncate_mutex);
>                 return;
> @@ -2162,8 +2173,7 @@ void __ceph_do_pending_vmtruncate(struct inode *inode)
>          */
>         if (ci->i_wrbuffer_ref_head < ci->i_wrbuffer_ref) {
>                 spin_unlock(&ci->i_ceph_lock);
> -               dout("__do_pending_vmtruncate %p flushing snaps first\n",
> -                    inode);
> +               dout("%s %p flushing snaps first\n", __func__, inode);
>                 filemap_write_and_wait_range(&inode->i_data, 0,
>                                              inode->i_sb->s_maxbytes);
>                 goto retry;
> @@ -2174,7 +2184,7 @@ void __ceph_do_pending_vmtruncate(struct inode *inode)
>
>         to = ci->i_truncate_pagecache_size;
>         wrbuffer_refs = ci->i_wrbuffer_ref;
> -       dout("__do_pending_vmtruncate %p (%d) to %lld\n", inode,
> +       dout("%s %p (%d) to %lld\n", __func__, inode,
>              ci->i_truncate_pending, to);
>         spin_unlock(&ci->i_ceph_lock);
>
> @@ -2361,6 +2371,9 @@ static int fill_fscrypt_truncate(struct inode *inode,
>                 header.data_len = cpu_to_le32(8 + 8 + 4 + CEPH_FSCRYPT_BLOCK_SIZE);
>                 header.file_offset = cpu_to_le64(orig_pos);
>
> +               dout("%s encrypt block boff/bsize %d/%lu\n", __func__,
> +                    boff, CEPH_FSCRYPT_BLOCK_SIZE);
> +
>                 /* truncate and zero out the extra contents for the last block */
>                 memset(iov.iov_base + boff, 0, PAGE_SIZE - boff);
>
> --
> 2.39.1
>
diff mbox series

Patch

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index e1bb6d9c16f8..3c5450f9d825 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -3929,8 +3929,8 @@  static bool handle_cap_trunc(struct inode *inode,
 	if (IS_ENCRYPTED(inode) && size)
 		size = extra_info->fscrypt_file_size;
 
-	dout("handle_cap_trunc inode %p mds%d seq %d to %lld seq %d\n",
-	     inode, mds, seq, truncate_size, truncate_seq);
+	dout("%s inode %p mds%d seq %d to %lld truncate seq %d\n",
+	     __func__, inode, mds, seq, truncate_size, truncate_seq);
 	queue_trunc = ceph_fill_file_size(inode, issued,
 					  truncate_seq, truncate_size, size);
 	return queue_trunc;
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 1cfcbc39f7c6..059ebe42367c 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -763,7 +763,7 @@  int ceph_fill_file_size(struct inode *inode, int issued,
 			ceph_fscache_update(inode);
 		ci->i_reported_size = size;
 		if (truncate_seq != ci->i_truncate_seq) {
-			dout("truncate_seq %u -> %u\n",
+			dout("%s truncate_seq %u -> %u\n", __func__,
 			     ci->i_truncate_seq, truncate_seq);
 			ci->i_truncate_seq = truncate_seq;
 
@@ -787,15 +787,26 @@  int ceph_fill_file_size(struct inode *inode, int issued,
 			}
 		}
 	}
-	if (ceph_seq_cmp(truncate_seq, ci->i_truncate_seq) >= 0 &&
-	    ci->i_truncate_size != truncate_size) {
-		dout("truncate_size %lld -> %llu\n", ci->i_truncate_size,
-		     truncate_size);
+
+	/*
+	 * It's possible that the new sizes of the two consecutive
+	 * size truncations will be in the same fscrypt last block,
+	 * and we need to truncate the corresponding page caches
+	 * anyway.
+	 */
+	if (ceph_seq_cmp(truncate_seq, ci->i_truncate_seq) >= 0) {
+		dout("%s truncate_size %lld -> %llu, encrypted %d\n", __func__,
+		     ci->i_truncate_size, truncate_size, !!IS_ENCRYPTED(inode));
+
 		ci->i_truncate_size = truncate_size;
-		if (IS_ENCRYPTED(inode))
+
+		if (IS_ENCRYPTED(inode)) {
+			dout("%s truncate_pagecache_size %lld -> %llu\n",
+			     __func__, ci->i_truncate_pagecache_size, size);
 			ci->i_truncate_pagecache_size = size;
-		else
+		} else {
 			ci->i_truncate_pagecache_size = truncate_size;
+		}
 	}
 	return queue_trunc;
 }
@@ -2150,7 +2161,7 @@  void __ceph_do_pending_vmtruncate(struct inode *inode)
 retry:
 	spin_lock(&ci->i_ceph_lock);
 	if (ci->i_truncate_pending == 0) {
-		dout("__do_pending_vmtruncate %p none pending\n", inode);
+		dout("%s %p none pending\n", __func__, inode);
 		spin_unlock(&ci->i_ceph_lock);
 		mutex_unlock(&ci->i_truncate_mutex);
 		return;
@@ -2162,8 +2173,7 @@  void __ceph_do_pending_vmtruncate(struct inode *inode)
 	 */
 	if (ci->i_wrbuffer_ref_head < ci->i_wrbuffer_ref) {
 		spin_unlock(&ci->i_ceph_lock);
-		dout("__do_pending_vmtruncate %p flushing snaps first\n",
-		     inode);
+		dout("%s %p flushing snaps first\n", __func__, inode);
 		filemap_write_and_wait_range(&inode->i_data, 0,
 					     inode->i_sb->s_maxbytes);
 		goto retry;
@@ -2174,7 +2184,7 @@  void __ceph_do_pending_vmtruncate(struct inode *inode)
 
 	to = ci->i_truncate_pagecache_size;
 	wrbuffer_refs = ci->i_wrbuffer_ref;
-	dout("__do_pending_vmtruncate %p (%d) to %lld\n", inode,
+	dout("%s %p (%d) to %lld\n", __func__, inode,
 	     ci->i_truncate_pending, to);
 	spin_unlock(&ci->i_ceph_lock);
 
@@ -2361,6 +2371,9 @@  static int fill_fscrypt_truncate(struct inode *inode,
 		header.data_len = cpu_to_le32(8 + 8 + 4 + CEPH_FSCRYPT_BLOCK_SIZE);
 		header.file_offset = cpu_to_le64(orig_pos);
 
+		dout("%s encrypt block boff/bsize %d/%lu\n", __func__,
+		     boff, CEPH_FSCRYPT_BLOCK_SIZE);
+
 		/* truncate and zero out the extra contents for the last block */
 		memset(iov.iov_base + boff, 0, PAGE_SIZE - boff);