Re: [PATCH] ceph: do not truncate pagecache if truncate size doesn't change

From: Xiubo Li <xiubli@redhat.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: idryomov@gmail.com, vshankar@redhat.com, ceph-devel@vger.kernel.org
Subject: Re: [PATCH] ceph: do not truncate pagecache if truncate size doesn't change
Date: Fri, 19 Nov 2021 10:20:11 +0800	[thread overview]
Message-ID: <3133b10b-677f-cdde-5839-61a0e7bcbc39@redhat.com> (raw)
In-Reply-To: <b7d901c7f1c660f2f1ec634e18d17988f0e348eb.camel@kernel.org>

On 11/18/21 8:19 PM, Jeff Layton wrote:
> On Thu, 2021-11-18 at 10:38 +0800, Xiubo Li wrote:
>> On 11/17/21 11:06 PM, Jeff Layton wrote:
>>> On Wed, 2021-11-17 at 09:21 +0800, Xiubo Li wrote:
[...]
>> Hi Jeff,
>>
>> I tested these two cases many times again today and both worked well for me.
>>
>> [root@lxbceph1 xfstests]# ./check generic/075
>> FSTYP         -- ceph
>> PLATFORM      -- Linux/x86_64 lxbceph1 5.15.0+
>> MKFS_OPTIONS  -- 10.72.7.17:40543:/testB
>> MOUNT_OPTIONS -- -o
>> test_dummy_encryption,name=admin,secret=AQDS3IFhEtxvORAAxn1d4FVN2bRUsc/TZMpQvQ==
>> -o context=system_u:object_r:root_t:s0 10.72.7.17:40543:/testB
>> /mnt/kcephfs/testD
>>
>> generic/075 106s ... 356s
>> Ran: generic/075
>> Passed all 1 tests
>>
>> [root@lxbceph1 xfstests]# ./check generic/029
>> FSTYP         -- ceph
>> PLATFORM      -- Linux/x86_64 lxbceph1 5.15.0+
>> MKFS_OPTIONS  -- 10.72.7.17:40543:/testB
>> MOUNT_OPTIONS -- -o
>> test_dummy_encryption,name=admin,secret=AQDS3IFhEtxvORAAxn1d4FVN2bRUsc/TZMpQvQ==
>> -o context=system_u:object_r:root_t:s0 10.72.7.17:40543:/testB
>> /mnt/kcephfs/testD
>>
>> generic/029 4s ... 3s
>> Ran: generic/029
>> Passed all 1 tests
>>
>>
>>> On the cluster-side, I'm using a cephadm cluster built using an image
>>> based on your fsize_support branch, rebased onto master (the Oct 7 base
>>> you're using is not good for cephadm).
>> I have updated this branch last night by rebasing it onto the latest
>> upstream master.
>>
>> And at the same time I have removed the commit:
>>
>>           bf39d32d936 mds: bump truncate seq when fscrypt_file changes
>>
>>> On the client side, I'm using the ceph-client/wip-fscrypt-size branch,
>>> along with this patch on top.
>> This I am also using the same branch from ceph-client repo. Nothing
>> changed in my side.
>>
>> To be safe I just deleted my local branches and synced from ceph-client
>> repo today and test them again, still the same and worked for me.
>>
> Ok, I think I see the problem. You sent this patch with just a [PATCH]
> prefix and there is no mention in the description that it's only
> intended to work on top of the other fscrypt changes, so I interpreted
> that to mean that this was a pre-existing bug and that this needed to go
> in ahead of those patches.
>
> That's not the case here. This patch does seem to work OK on top of the
> regular fscrypt series (but I did still see some failures in certain
> tests even with that). I'll look more closely at those today.
>
> It would probably have been clearer to send this patch along with the
> rest of the series to make that clear. Alternately, when you post a
> series and then find that it needs a follow-on patch, you can send it as
> if you're continuing the series:
>
>      [PATCH v7 10/9] ceph: do not truncate pagecache if truncate size doesn't change
>
> Ideally though, this change would be rolled into one of the other
> patches in your series.

Sure Jeff.

Sorry for confusing, since I am mainly working and testing the fscrypt 
related patches and I didn't make it clear where should this patch go.

In future, I will mention if we need to patch it on top of the fscrypt 
related branches and will do it as you mentioned above.

Thanks.

BRs

-- Xiubo

> Thanks,