| Message ID | 20230417032654.32352-69-xiubli@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ceph+fscrypt: full support | expand |
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 --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);