All of lore.kernel.org
 help / color / mirror / Atom feed
From: Xiubo Li <xiubli@redhat.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: idryomov@gmail.com, ukernel@gmail.com, pdonnell@redhat.com,
	"open list:CEPH DISTRIBUTED..." <ceph-devel@vger.kernel.org>
Subject: Re: [PATCH RFC 2/2] ceph: truncate the file contents when needed when file scrypted
Date: Wed, 22 Sep 2021 10:23:17 +0800	[thread overview]
Message-ID: <5cf666a9-3b33-ad54-3878-a975505c872e@redhat.com> (raw)
In-Reply-To: <20c3630d2ac2e2e7c4a708fdeb7409077f36d8f0.camel@kernel.org>


On 9/21/21 3:24 AM, Jeff Layton wrote:
> On Mon, 2021-09-20 at 22:32 +0800, Xiubo Li wrote:
>> On 9/18/21 1:19 AM, Jeff Layton wrote:
>>> On Thu, 2021-09-16 at 18:02 +0800, Xiubo Li wrote:
>>>> For this I am a little curious why couldn't we cache truncate operations
>>>> when attr.ia_size >= isize ?
>>>>
>>> It seems to me that if attr.ia_size == i_size, then there is nothing to
>>> change. We can just optimize that case away, assuming the client has
>>> caps that ensure the size is correct.
>>   From MDS side I didn't find any limitation why couldn't we optimize it.
>>
>>
> Well...I think we do still need to change the mtime in that case. We
> probably do still need to do a setattr or something that makes the MDS
> set it to its current local time, even if the size doesn't change.

Since there hasn't any change for the file data, will change the 'mtime' 
make sense here ? If so, then why in case the current client has the Fr 
caps and sizes are the same it won't.

In this case I found in the MDS side, it will also update the 'ctime' 
always even it will change nothing.

>>> Based on the kclient code, for size changes after a write that extends a
>>> file, it seems like Fw is sufficient to allow the client to buffer these
>>> things.
>> Since the MDS will only allow us to increase the file size, just like
>> the mtime/ctime/atime, so even the write size has exceed current file
>> size, the Fw is sufficient.
>>
> Good.
>
>>>    For a truncate (aka setattr) operation, we apparently need Fx.
>> In case one client is truncating the file, if the new size is larger
>> than or equal to current size, this should be okay and will behave just
>> like normal write case above.
>>
>> If the new size is smaller, the MDS will handle this in a different way.
>> When the MDS received the truncate request, it will first xlock the
>> filelock, which will switch the filelock state and in all these possible
>> interim or stable states, the Fw caps will be revoked from all the
>> clients, but the clients still could cache/buffer the file contents,
>> that means no client is allowed to change the size during the truncate
>> operation is on the way. After the truncate succeeds the MDS Locker will
>> issue_truncate() to all the clients and the clients will truncate the
>> caches/buffers, etc.
>>
>> And also the truncate operations will always be queued sequentially.
>>
>>
>>> It sort of makes sense, but the more I look over the existing code, the
>>> less sure I am about how this is intended to work. I think before we
>>> make any changes for fscrypt, we need to make sure we understand what's
>>> there now.
>> So if my understanding is correct, the Fx is not a must for the truncate
>> operation.
>>
> Basically, when a truncate up or down comes in, we have to make a
> determination of whether we can buffer that change, or whether we need
> to do it synchronously.
>
> The current kclient code bases that on whether it has Fx. I suspect
> that ensures that no other client has Frw, which sounds like what we
> probably want.

The Fx caps is only for the loner client and once the client has the Fx 
caps it will always have the Fsrwcb at the same time. From the 
mds/locker.cc I didn't find it'll allow that.

If current client has the Fx caps, so when there has another client is 
trying to truncate the same file at the same time, the MDS will have to 
revoke the Fx caps first and during which the buffered truncate will be 
flushed and be finished first too. So when Fx caps is issued and new 
size equals to the current size, why couldn't we buffer it ?


>   We need to prevent anyone from doing writes that might
> extend the file at that time

Yeah, since the Fx is only for loner client, and all other clients will 
have zero file caps. so it won't happen.


>   _and_ need to ensure that stat/statx for
> the file size blocks until it's complete. ceph_getattr will issue a
> GETATTR to the MDS if it doesn't have Fs in that case, so that should
> be fine.
>
> I assume that the MDS will never give out Fx caps to one client and Fs
> to another.
Yeah, It won't happen.


>   What about Fw, though?

While for the Fw caps, it will allow it. If the filelock is in LOCK_MIX 
state, the filelock maybe in this state if there at least one client 
wants the Fw caps and some others want any of Fr and Fw caps.


>   Do Fx caps conflict with Frw the
> same way as they do with Fs?

Yeah, it will be the same with Fs.

Thanks.


  reply	other threads:[~2021-09-22  2:23 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-03  8:15 [PATCH RFC 0/2] ceph: size handling for the fscrypt xiubli
2021-09-03  8:15 ` [PATCH RFC 1/2] Revert "ceph: make client zero partial trailing block on truncate" xiubli
2021-09-03  8:15 ` [PATCH RFC 2/2] ceph: truncate the file contents when needed when file scrypted xiubli
2021-09-07 16:26   ` Jeff Layton
2021-09-08  9:37     ` Xiubo Li
2021-09-08 13:57       ` Jeff Layton
2021-09-09  3:38         ` Xiubo Li
2021-09-09 12:48           ` Jeff Layton
2021-09-10  2:30             ` Xiubo Li
2021-09-10 11:46               ` Jeff Layton
2021-09-13  5:42                 ` Xiubo Li
2021-09-13 14:05                   ` Jeff Layton
2021-09-14  5:43                     ` Xiubo Li
2021-09-13 19:34                   ` Jeff Layton
2021-09-14  5:40                     ` Xiubo Li
2021-09-14 14:24                       ` Jeff Layton
2021-09-16 10:02                         ` Xiubo Li
2021-09-17 17:19                           ` Jeff Layton
2021-09-20 14:32                             ` Xiubo Li
2021-09-20 19:24                               ` Jeff Layton
2021-09-22  2:23                                 ` Xiubo Li [this message]
2021-09-24 18:52                                   ` Jeff Layton
2021-09-25  1:02                                     ` Xiubo Li
2021-09-24 15:01                     ` Xiubo Li
2021-09-25  9:56                     ` Xiubo Li
2021-10-11 13:29                       ` Jeff Layton
2021-10-11 15:16                         ` Xiubo Li
2021-09-07 12:35 ` [PATCH RFC 0/2] ceph: size handling for the fscrypt Jeff Layton
2021-09-07 13:19   ` Xiubo Li
2021-09-07 20:58     ` Jeff Layton
2021-09-08 11:16       ` Xiubo Li
2021-09-08 14:12         ` Jeff Layton
2021-09-09  8:12           ` Xiubo Li
2021-09-08 11:17       ` Xiubo Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5cf666a9-3b33-ad54-3878-a975505c872e@redhat.com \
    --to=xiubli@redhat.com \
    --cc=ceph-devel@vger.kernel.org \
    --cc=idryomov@gmail.com \
    --cc=jlayton@kernel.org \
    --cc=pdonnell@redhat.com \
    --cc=ukernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.