Re: [PATCH 0/2] ceph: misc fixes for the fscrypt truncate size handling

Xiubo Li <xiubli@xxxxxxxxxx> · Tue, 16 Nov 2021 12:56:41 +0800

On 11/9/21 8:33 PM, Jeff Layton wrote:
On Mon, 2021-11-08 at 20:57 -0500, Jeff Layton wrote:
On Tue, 2021-11-09 at 08:21 +0800, Xiubo Li wrote:
On 11/9/21 2:36 AM, Jeff Layton wrote:
On Mon, 2021-11-08 at 21:50 +0800, xiubli@xxxxxxxxxx wrote:
From: Xiubo Li <xiubli@xxxxxxxxxx>

Hi Jeff,

The #1 could be squashed to the previous "ceph: add truncate size handling support for fscrypt" commit.
The #2 could be squashed to the previous "ceph: fscrypt_file field handling in MClientRequest messages" commit.

Thanks.

Xiubo Li (2):
    ceph: fix possible crash and data corrupt bugs
    ceph: there is no need to round up the sizes when new size is 0

   fs/ceph/inode.c | 8 +++++---
   1 file changed, 5 insertions(+), 3 deletions(-)

I'm running xfstests today with the current state of wip-fscrypt-size
(including the two patches above) with test_dummy_encryption enabled.
generic/030 failed. The expected output of this part of the test was:

wrote 5136912/5136912 bytes at offset 0
XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
==== Pre-Remount ===
00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58
XXXXXXXXXXXXXXXX|
*
004e6210  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59
YYYYYYYYYYYYYYYY|
*
004e6d00  59 59 59 59 59 59 59 59  00 00 00 00 00 00 00 00
YYYYYYYY........|
004e6d10  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
................|
*
004e7000
==== Post-Remount ==
00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58
XXXXXXXXXXXXXXXX|
*
004e6210  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59
YYYYYYYYYYYYYYYY|
*
004e6d00  59 59 59 59 59 59 59 59  00 00 00 00 00 00 00 00
YYYYYYYY........|
004e6d10  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
................|
*
004e7000

...but I got this instead:

wrote 5136912/5136912 bytes at offset 0
XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
==== Pre-Remount ===
00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58
XXXXXXXXXXXXXXXX|
*
00400000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
................|
*
004e7000
==== Post-Remount ==
00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58
XXXXXXXXXXXXXXXX|
*
00400000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
................|
*
004e7000

...I suspect this is related to the 075 failures, but I don't see it on
every attempt, which makes me think "race condition". I'll keep hunting.
Yeah, seems the same issue.

I wonder if we're hitting the same problem that this patch was intended
to fix:

     057ba5b24532 ceph: Fix race between hole punch and page fault

It seems like the problem is very similar. It may be worth testing a
patch that takes the filemap_invalidate_lock across the on the wire
truncate and the vmtruncate.

Nope. I tried a draft patch that make setattr hold this lock over the
MDS call and the vmtruncate and it didn't help.

It's buggy by using the `filer.write_trunc()` caller in MDS side, it 
works well in case the truncate_from - truncate_size < 4MB, or it will 
keep the stale file contents without trimming or zeroing them.

I will push the MDS side PR to fix it.

Thanks

-- Xiubo

Cheers,