[PATCH] ceph: truncate page cache when doing DIO in encrypted inodes

[PATCH] ceph: truncate page cache when doing DIO in encrypted inodes

[Luís Henriques]

When doing DIO on an encrypted node, we need to truncate the page cache in
the range being written to, otherwise the cache will include invalid data.

Signed-off-by: Luís Henriques <lhenriques@suse.de>
---
 fs/ceph/file.c | 5 +++++
 1 file changed, 5 insertions(+)

This patch should fix generic/647 fstest when run with test_dummy_encryption.

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 5072570c2203..0f31c4d352a4 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -1895,6 +1895,11 @@ ceph_sync_write(struct kiocb *iocb, struct iov_iter *from, loff_t pos,
 		req->r_inode = inode;
 		req->r_mtime = mtime;
 
+		if (IS_ENCRYPTED(inode) && (iocb->ki_flags & IOCB_DIRECT))
+			truncate_inode_pages_range(
+				inode->i_mapping, write_pos,
+				PAGE_ALIGN(write_pos + write_len) - 1);
+
 		/* Set up the assertion */
 		if (rmw) {
 			/*

Re: [PATCH] ceph: truncate page cache when doing DIO in encrypted inodes

[Jeff Layton]

On Fri, 2022-04-01 at 12:38 +0100, Luís Henriques wrote:
> When doing DIO on an encrypted node, we need to truncate the page cache in
> the range being written to, otherwise the cache will include invalid data.
> 
> Signed-off-by: Luís Henriques <lhenriques@suse.de>
> ---
>  fs/ceph/file.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> This patch should fix generic/647 fstest when run with test_dummy_encryption.
> 
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 5072570c2203..0f31c4d352a4 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -1895,6 +1895,11 @@ ceph_sync_write(struct kiocb *iocb, struct iov_iter *from, loff_t pos,
>  		req->r_inode = inode;
>  		req->r_mtime = mtime;
>  
> +		if (IS_ENCRYPTED(inode) && (iocb->ki_flags & IOCB_DIRECT))
> +			truncate_inode_pages_range(
> +				inode->i_mapping, write_pos,
> +				PAGE_ALIGN(write_pos + write_len) - 1);
> +
>  		/* Set up the assertion */
>  		if (rmw) {
>  			/*

Truncating the pagecache like this could cause dirty data to be
discarded. I know we're planning to overwrite this range, but you are
having to invalidate more than the written range here. We could
potentially lose a write to that region.

Have you tried using something like invalidate_inode_pages2_range ?
That's more of what we'd want here, as it's a bit more cautious about
tossing out dirty pages. I see too that that is what
ceph_direct_read_write calls in the write case as well.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] ceph: truncate page cache when doing DIO in encrypted inodes

[Luís Henriques]

Jeff Layton <jlayton@kernel.org> writes:

> On Fri, 2022-04-01 at 12:38 +0100, Luís Henriques wrote:
>> When doing DIO on an encrypted node, we need to truncate the page cache in
>> the range being written to, otherwise the cache will include invalid data.
>> 
>> Signed-off-by: Luís Henriques <lhenriques@suse.de>
>> ---
>>  fs/ceph/file.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> This patch should fix generic/647 fstest when run with test_dummy_encryption.
>> 
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index 5072570c2203..0f31c4d352a4 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -1895,6 +1895,11 @@ ceph_sync_write(struct kiocb *iocb, struct iov_iter *from, loff_t pos,
>>  		req->r_inode = inode;
>>  		req->r_mtime = mtime;
>>  
>> +		if (IS_ENCRYPTED(inode) && (iocb->ki_flags & IOCB_DIRECT))
>> +			truncate_inode_pages_range(
>> +				inode->i_mapping, write_pos,
>> +				PAGE_ALIGN(write_pos + write_len) - 1);
>> +
>>  		/* Set up the assertion */
>>  		if (rmw) {
>>  			/*
>
> Truncating the pagecache like this could cause dirty data to be
> discarded. I know we're planning to overwrite this range, but you are
> having to invalidate more than the written range here. We could
> potentially lose a write to that region.
>
> Have you tried using something like invalidate_inode_pages2_range ?
> That's more of what we'd want here, as it's a bit more cautious about
> tossing out dirty pages. I see too that that is what
> ceph_direct_read_write calls in the write case as well.

OK, let me try that instead.  Yeah, I've used what the usual direct path
was using (the truncate_inode_pages_range()), but if
invalidate_inode_pages2_range works here I guess that's better.  I'll test
that and send out v2.

Cheers,
-- 
Luís