Postpone copy-up until first real write?

Postpone copy-up until first real write?

[cgxu519]

Hi guys,

I have a simple question about copy-up timing after implementing stacked 
file operation.
Could we postpone copy-up until first real write happens?

The background is consuming much time on open is unexpected for some of 
our applications.

Thanks,
Chengguang

Re: Postpone copy-up until first real write?

[Amir Goldstein]

On Fri, Jun 29, 2018 at 6:35 AM, cgxu519 <cgxu519@gmx.com> wrote:
> Hi guys,
>
> I have a simple question about copy-up timing after implementing stacked
> file operation.
> Could we postpone copy-up until first real write happens?
>
> The background is consuming much time on open is unexpected for some of our
> applications.
>

I need this behavior as well for snapshots.
On first glance it looks feasible to me.
Seems like we could relax copy up on open(O_RDWR) if there were no special
open flags (O_TRUNC, O_CREATE, O_APPEND...), then mask out O_ACCMODE
if opening a lower inode and defer copy up to ovl_real_fdget_meta() in case the
O_ACCMODE flags of file and real->file don't match.

Then we can introduce ovl_real_fdget_read() call to be used in ovl_read_iter()
and ovl_copyfile() (in_file) in which cases the mismatch O_ACCMODE flags
doesn't need to be corrected with copy up.

Miklos? Does this sound reasonable to you?

Chengguang,

Will you take a swing at implementing this if there are no holes found
in the plan?
Do you know if your applications typically open files with O_RDWR? any
other flags?
Deferring copy up to first write with O_WRONLY seems a bit more tricky.

Thanks,
Amir.

Re: Postpone copy-up until first real write?

[cgxu519]

On 06/29/2018 06:28 PM, Amir Goldstein wrote:
> On Fri, Jun 29, 2018 at 6:35 AM, cgxu519 <cgxu519@gmx.com> wrote:
>> Hi guys,
>>
>> I have a simple question about copy-up timing after implementing stacked
>> file operation.
>> Could we postpone copy-up until first real write happens?
>>
>> The background is consuming much time on open is unexpected for some of our
>> applications.
>>
> I need this behavior as well for snapshots.
> On first glance it looks feasible to me.
> Seems like we could relax copy up on open(O_RDWR) if there were no special
> open flags (O_TRUNC, O_CREATE, O_APPEND...), then mask out O_ACCMODE
> if opening a lower inode and defer copy up to ovl_real_fdget_meta() in case the
> O_ACCMODE flags of file and real->file don't match.
>
> Then we can introduce ovl_real_fdget_read() call to be used in ovl_read_iter()
> and ovl_copyfile() (in_file) in which cases the mismatch O_ACCMODE flags
> doesn't need to be corrected with copy up.
>
> Miklos? Does this sound reasonable to you?
>
> Chengguang,
>
> Will you take a swing at implementing this if there are no holes found
> in the plan?
Sure, I'll do if the behavior is welcomed.

> Do you know if your applications typically open files with O_RDWR? any
> other flags?
> Deferring copy up to first write with O_WRONLY seems a bit more tricky.

Most of them open with O_RDWR but I'm not sure there is no O_WRONLY with 
complain in the future. :-/

Thanks,
Chengguang

Re: Postpone copy-up until first real write?

[Miklos Szeredi]

On Fri, Jun 29, 2018 at 12:28 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Fri, Jun 29, 2018 at 6:35 AM, cgxu519 <cgxu519@gmx.com> wrote:
>> Hi guys,
>>
>> I have a simple question about copy-up timing after implementing stacked
>> file operation.
>> Could we postpone copy-up until first real write happens?
>>
>> The background is consuming much time on open is unexpected for some of our
>> applications.
>>
>
> I need this behavior as well for snapshots.
> On first glance it looks feasible to me.
> Seems like we could relax copy up on open(O_RDWR) if there were no special
> open flags (O_TRUNC, O_CREATE, O_APPEND...), then mask out O_ACCMODE
> if opening a lower inode and defer copy up to ovl_real_fdget_meta() in case the
> O_ACCMODE flags of file and real->file don't match.
>
> Then we can introduce ovl_real_fdget_read() call to be used in ovl_read_iter()
> and ovl_copyfile() (in_file) in which cases the mismatch O_ACCMODE flags
> doesn't need to be corrected with copy up.
>
> Miklos? Does this sound reasonable to you?

So now that the stacked file ops are merged we can take a look at
these somewhat related issues:

 - shared r/o mmap of lower file
 - delayed copy up until first write
 - delayed copy up until first write fault on shared r/w mmap.

My idea would be to have three (or four) states for each in core overlay inode:

  LOWER:  opened only for read or not opened at all and mmaped with
MAP_PRIVATE  or not mapped at all
  LOWER_SHARED: opened for read/write or mmaped MAP_SHARED
  UPPER_SHARED: copied up
  (UPPER:  copied up and all open files referring to the inode were
opened in the UPPER state)

The LOWER_SHARED state is basically in anticipation of copy-up,
without actually committing to the upper layer.

The UPPER state could just be an optimization, that would be otherwise
equivalent to the UPPER_SHARED state.

I/O paths for the different states would be:

LOWER/UPPER: just like now: read/write are stacked, mmap goes directly
to "realfile"
*_SHARED: just like a normal filesystem: uses
generic_file_{read|write}_iter, generic_mmap, defines own a_ops,
vm_ops.

So basically the _SHARED mode loses the cache sharing property of
LOWER, but gains the consistency offered by a unified page cache
regardless of which layer the file is stored in.

Thoughts?

Does this make sense wrt. overlay-snapshots?

One detail is whether we should ever make the LOWER_SHARED -> LOWER
transition while the inode is cached.  My gut feeling is that we
probably needn't bother.

Also I'm not sure we'd actually need the UPPER state at all, since (if
implemented right) UPPER_SHARED should offer a very similar cache
footprint and performance.

Thanks,
Miklos

Re: Postpone copy-up until first real write?

[Amir Goldstein]

On Mon, Oct 8, 2018 at 5:41 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Fri, Jun 29, 2018 at 12:28 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Fri, Jun 29, 2018 at 6:35 AM, cgxu519 <cgxu519@gmx.com> wrote:
> >> Hi guys,
> >>
> >> I have a simple question about copy-up timing after implementing stacked
> >> file operation.
> >> Could we postpone copy-up until first real write happens?
> >>
> >> The background is consuming much time on open is unexpected for some of our
> >> applications.
> >>
> >
> > I need this behavior as well for snapshots.
> > On first glance it looks feasible to me.
> > Seems like we could relax copy up on open(O_RDWR) if there were no special
> > open flags (O_TRUNC, O_CREATE, O_APPEND...), then mask out O_ACCMODE
> > if opening a lower inode and defer copy up to ovl_real_fdget_meta() in case the
> > O_ACCMODE flags of file and real->file don't match.
> >
> > Then we can introduce ovl_real_fdget_read() call to be used in ovl_read_iter()
> > and ovl_copyfile() (in_file) in which cases the mismatch O_ACCMODE flags
> > doesn't need to be corrected with copy up.
> >
> > Miklos? Does this sound reasonable to you?
>
> So now that the stacked file ops are merged we can take a look at
> these somewhat related issues:
>
>  - shared r/o mmap of lower file
>  - delayed copy up until first write
>  - delayed copy up until first write fault on shared r/w mmap.
>
> My idea would be to have three (or four) states for each in core overlay inode:
>
>   LOWER:  opened only for read or not opened at all and mmaped with
> MAP_PRIVATE  or not mapped at all
>   LOWER_SHARED: opened for read/write or mmaped MAP_SHARED
>   UPPER_SHARED: copied up
>   (UPPER:  copied up and all open files referring to the inode were
> opened in the UPPER state)
>
> The LOWER_SHARED state is basically in anticipation of copy-up,
> without actually committing to the upper layer.
>
> The UPPER state could just be an optimization, that would be otherwise
> equivalent to the UPPER_SHARED state.
>
> I/O paths for the different states would be:
>
> LOWER/UPPER: just like now: read/write are stacked, mmap goes directly
> to "realfile"
> *_SHARED: just like a normal filesystem: uses
> generic_file_{read|write}_iter, generic_mmap, defines own a_ops,
> vm_ops.
>
> So basically the _SHARED mode loses the cache sharing property of
> LOWER, but gains the consistency offered by a unified page cache
> regardless of which layer the file is stored in.
>
> Thoughts?

Sounds good and wishfully not too complicated?
I am not familiar enough with mm to understand what we are up
against.

Does IO from upper fs backing inode goes directly to overlay inode
page cache or first to upper fs inode page cache and then moving
pages to overlay inode page cache?
Am I over complicating this?
I suppose sharing pages with LOWER left for another time?

>
> Does this make sense wrt. overlay-snapshots?
>

Snapshots needs the a_ops/vm_ops to COW on page faults
and need delayed copy up on first write (after snapshot take),
so all this will be very useful.

> One detail is whether we should ever make the LOWER_SHARED -> LOWER
> transition while the inode is cached.  My gut feeling is that we
> probably needn't bother.
>
> Also I'm not sure we'd actually need the UPPER state at all, since (if
> implemented right) UPPER_SHARED should offer a very similar cache
> footprint and performance.
>

If we don't have UPPER state then we won't need to call
sync_filesystem(upper_sb) in ovl_sync_fs(), only
sb->s_op->sync_fs(), right? so that is a win for simplicity.

Same for implementing ovl_freeze(). It should mostly "just work".
Snapshots need freeze, but snapshots can turn off the UPPER state
optimization if it is at all implemented.

Thanks,
Amir.

Re: Postpone copy-up until first real write?

[Amir Goldstein]

On Mon, Oct 8, 2018 at 5:41 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Fri, Jun 29, 2018 at 12:28 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Fri, Jun 29, 2018 at 6:35 AM, cgxu519 <cgxu519@gmx.com> wrote:
> >> Hi guys,
> >>
> >> I have a simple question about copy-up timing after implementing stacked
> >> file operation.
> >> Could we postpone copy-up until first real write happens?
> >>
> >> The background is consuming much time on open is unexpected for some of our
> >> applications.
> >>
> >

Getting back to this with some more questions...

> > I need this behavior as well for snapshots.
> > On first glance it looks feasible to me.
> > Seems like we could relax copy up on open(O_RDWR) if there were no special
> > open flags (O_TRUNC, O_CREATE, O_APPEND...), then mask out O_ACCMODE
> > if opening a lower inode and defer copy up to ovl_real_fdget_meta() in case the
> > O_ACCMODE flags of file and real->file don't match.
> >
> > Then we can introduce ovl_real_fdget_read() call to be used in ovl_read_iter()
> > and ovl_copyfile() (in_file) in which cases the mismatch O_ACCMODE flags
> > doesn't need to be corrected with copy up.
> >
> > Miklos? Does this sound reasonable to you?
>
> So now that the stacked file ops are merged we can take a look at
> these somewhat related issues:
>
>  - shared r/o mmap of lower file
>  - delayed copy up until first write
>  - delayed copy up until first write fault on shared r/w mmap.
>

These 3 issues map closely together, but do not cover all "delayed copy up"
cases. Even if we do implement copy up on first write fault, we'd still need
to perform delayed copy up for fallocate(),clone_range(),dedupe_range(),
so perhaps my suggestion above for simpler(?) first step implementation of
"delayed copy up until first write" without bundling it together with the
shared maps issues still makes some sense?

> My idea would be to have three (or four) states for each in core overlay inode:
>
>   LOWER:  opened only for read or not opened at all and mmaped with
> MAP_PRIVATE  or not mapped at all
>   LOWER_SHARED: opened for read/write or mmaped MAP_SHARED
>   UPPER_SHARED: copied up
>   (UPPER:  copied up and all open files referring to the inode were
> opened in the UPPER state)
>
> The LOWER_SHARED state is basically in anticipation of copy-up,
> without actually committing to the upper layer.
>
> The UPPER state could just be an optimization, that would be otherwise
> equivalent to the UPPER_SHARED state.
>
> I/O paths for the different states would be:
>
> LOWER/UPPER: just like now: read/write are stacked, mmap goes directly
> to "realfile"
> *_SHARED: just like a normal filesystem: uses
> generic_file_{read|write}_iter, generic_mmap, defines own a_ops,
> vm_ops.
>
> So basically the _SHARED mode loses the cache sharing property of
> LOWER, but gains the consistency offered by a unified page cache
> regardless of which layer the file is stored in.
>
> Thoughts?
>

Did you give any consideration to the locking order of copy up
from write fault context?

I suppose you are aware of this hence the proposal of pre_mmap op:
https://marc.info/?l=linux-fsdevel&m=152760695502640&w=2

Do we gain anything (simplicity? relaxed locking order) if we always
copy up metadata on open O_RDWR and only delay data copy up
until first write/page fault?

Chengguang,

For your use case, is it acceptable if we copy up metadata
(and parents) before first write?

Anyway, if no one else is working on this I am willing to take
a swing at it, but it seems to me like it would be best to leave
stacked a_ops for phase two. Thoughts?

Thanks,
Amir.

Re: Postpone copy-up until first real write?

[cgxu519]

On 1/13/19 7:25 PM, Amir Goldstein wrote:
> On Mon, Oct 8, 2018 at 5:41 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Fri, Jun 29, 2018 at 12:28 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>> On Fri, Jun 29, 2018 at 6:35 AM, cgxu519 <cgxu519@gmx.com> wrote:
>>>> Hi guys,
>>>>
>>>> I have a simple question about copy-up timing after implementing stacked
>>>> file operation.
>>>> Could we postpone copy-up until first real write happens?
>>>>
>>>> The background is consuming much time on open is unexpected for some of our
>>>> applications.
>>>>
> Getting back to this with some more questions...
>
>>> I need this behavior as well for snapshots.
>>> On first glance it looks feasible to me.
>>> Seems like we could relax copy up on open(O_RDWR) if there were no special
>>> open flags (O_TRUNC, O_CREATE, O_APPEND...), then mask out O_ACCMODE
>>> if opening a lower inode and defer copy up to ovl_real_fdget_meta() in case the
>>> O_ACCMODE flags of file and real->file don't match.
>>>
>>> Then we can introduce ovl_real_fdget_read() call to be used in ovl_read_iter()
>>> and ovl_copyfile() (in_file) in which cases the mismatch O_ACCMODE flags
>>> doesn't need to be corrected with copy up.
>>>
>>> Miklos? Does this sound reasonable to you?
>> So now that the stacked file ops are merged we can take a look at
>> these somewhat related issues:
>>
>>   - shared r/o mmap of lower file
>>   - delayed copy up until first write
>>   - delayed copy up until first write fault on shared r/w mmap.
>>
> These 3 issues map closely together, but do not cover all "delayed copy up"
> cases. Even if we do implement copy up on first write fault, we'd still need
> to perform delayed copy up for fallocate(),clone_range(),dedupe_range(),
> so perhaps my suggestion above for simpler(?) first step implementation of
> "delayed copy up until first write" without bundling it together with the
> shared maps issues still makes some sense?
>
>> My idea would be to have three (or four) states for each in core overlay inode:
>>
>>    LOWER:  opened only for read or not opened at all and mmaped with
>> MAP_PRIVATE  or not mapped at all
>>    LOWER_SHARED: opened for read/write or mmaped MAP_SHARED
>>    UPPER_SHARED: copied up
>>    (UPPER:  copied up and all open files referring to the inode were
>> opened in the UPPER state)
>>
>> The LOWER_SHARED state is basically in anticipation of copy-up,
>> without actually committing to the upper layer.
>>
>> The UPPER state could just be an optimization, that would be otherwise
>> equivalent to the UPPER_SHARED state.
>>
>> I/O paths for the different states would be:
>>
>> LOWER/UPPER: just like now: read/write are stacked, mmap goes directly
>> to "realfile"
>> *_SHARED: just like a normal filesystem: uses
>> generic_file_{read|write}_iter, generic_mmap, defines own a_ops,
>> vm_ops.
>>
>> So basically the _SHARED mode loses the cache sharing property of
>> LOWER, but gains the consistency offered by a unified page cache
>> regardless of which layer the file is stored in.
>>
>> Thoughts?
>>
> Did you give any consideration to the locking order of copy up
> from write fault context?
>
> I suppose you are aware of this hence the proposal of pre_mmap op:
> https://marc.info/?l=linux-fsdevel&m=152760695502640&w=2
>
> Do we gain anything (simplicity? relaxed locking order) if we always
> copy up metadata on open O_RDWR and only delay data copy up
> until first write/page fault?
>
> Chengguang,
>
> For your use case, is it acceptable if we copy up metadata
> (and parents) before first write?

In our use case that will be fine if copy up metadata(and parents) is 
quick enough.



>
> Anyway, if no one else is working on this I am willing to take
> a swing at it, but it seems to me like it would be best to leave
> stacked a_ops for phase two. Thoughts?

I'm sorry that I could not focus on this because now I'm in some
other urgent projects. So please take it up if you can.

Thanks,
Chengguang.

Re: Postpone copy-up until first real write?

[Miklos Szeredi]

On Sun, Jan 13, 2019 at 12:25 PM Amir Goldstein <amir73il@gmail.com> wrote:

> Did you give any consideration to the locking order of copy up
> from write fault context?
>
> I suppose you are aware of this hence the proposal of pre_mmap op:
> https://marc.info/?l=linux-fsdevel&m=152760695502640&w=2

Apparently ->fault() is allowed to release mmap_sem and return
VM_FAULT_RETRY, which is perfect for doing the copy up.

> Do we gain anything (simplicity? relaxed locking order) if we always
> copy up metadata on open O_RDWR and only delay data copy up
> until first write/page fault?

Yeah, maybe doing that as a start makes sense.

Thanks,
Miklos

Re: Postpone copy-up until first real write?

[Amir Goldstein]

On Mon, Jan 14, 2019 at 10:21 AM cgxu519 <cgxu519@gmx.com> wrote:
[...]
> > Do we gain anything (simplicity? relaxed locking order) if we always
> > copy up metadata on open O_RDWR and only delay data copy up
> > until first write/page fault?
> >
> > Chengguang,
> >
> > For your use case, is it acceptable if we copy up metadata
> > (and parents) before first write?
>
> In our use case that will be fine if copy up metadata(and parents) is
> quick enough.
>

Chengguang,

I posted an RFC patch (sorry forgot to CC you).
It seems to be working as expected short of the mmap case,
but I only tested manually - have not written any tests yet.

You may test the RFC patch and let me know if it meets the requirements
of your use case.

Thanks,
Amir.