Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1515738 > unrolled thread

Re: [PATCH 3/3] ovl: redirect on rename-dir

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 3/3] ovl: redirect on rename-dir Konstantin Khlebnikov <koct9i@gmail.com> - 2016-11-06 20:20 +0100
    Re: [PATCH 3/3] ovl: redirect on rename-dir Miklos Szeredi <miklos@szeredi.hu> - 2016-11-07 09:10 +0100
      Re: [PATCH 3/3] ovl: redirect on rename-dir Konstantin Khlebnikov <koct9i@gmail.com> - 2016-11-07 11:00 +0100
        Re: [PATCH 3/3] ovl: redirect on rename-dir Miklos Szeredi <miklos@szeredi.hu> - 2016-11-07 11:10 +0100
          Re: [PATCH 3/3] ovl: redirect on rename-dir Konstantin Khlebnikov <koct9i@gmail.com> - 2016-11-07 11:10 +0100
            Re: [PATCH 3/3] ovl: redirect on rename-dir Amir Goldstein <amir73il@gmail.com> - 2016-11-07 18:50 +0100
              Re: [PATCH 3/3] ovl: redirect on rename-dir Amir Goldstein <amir73il@gmail.com> - 2016-11-11 00:00 +0100
                Re: [PATCH 3/3] ovl: redirect on rename-dir Konstantin Khlebnikov <koct9i@gmail.com> - 2016-11-11 10:50 +0100
                  Re: [PATCH 3/3] ovl: redirect on rename-dir Miklos Szeredi <miklos@szeredi.hu> - 2016-11-11 11:10 +0100
                    Re: [PATCH 3/3] ovl: redirect on rename-dir Amir Goldstein <amir73il@gmail.com> - 2016-11-11 13:50 +0100
    Re: [PATCH 3/3] ovl: redirect on rename-dir Raphael Hertzog <hertzog@debian.org> - 2016-11-07 12:10 +0100
      Re: [PATCH 3/3] ovl: redirect on rename-dir Konstantin Khlebnikov <koct9i@gmail.com> - 2016-11-07 12:40 +0100
        Re: [PATCH 3/3] ovl: redirect on rename-dir Raphael Hertzog <hertzog@debian.org> - 2016-11-07 14:50 +0100
          Re: [PATCH 3/3] ovl: redirect on rename-dir Miklos Szeredi <miklos@szeredi.hu> - 2016-11-10 23:40 +0100
            Re: [PATCH 3/3] ovl: redirect on rename-dir Konstantin Khlebnikov <koct9i@gmail.com> - 2016-11-11 10:50 +0100

#1515738 — Re: [PATCH 3/3] ovl: redirect on rename-dir

On Wed, Oct 26, 2016 at 2:12 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Tue, Oct 25, 2016 at 1:57 PM, Raphael Hertzog <hertzog@debian.org> wrote:
>
>> Do you plan to make it the default in the future when it has been
>> available for a while?
>>
>> Barring any regression introduced by your patch, it seems that the feature
>> is best available by default since it allows legitimate operations to
>> succeed that are otherwise refused. I understand that it makes it
>> impossible to mount the overlay filesystem with an older kernel but is
>> that problem more widespread than the one we're fixing here?  On my side,
>> overlayfs is only used in scenarios where the kernel is always the same
>> (or newer compared to what created the initial filesystem).
>
> I think it would be safe to make it the default if upperdir is empty.
> Nonempty implies that it was created with old kernel (or it was
> crafted by hand).   But there should be a way to explicitly turn it
> off;  either because of the need for backward compatibility or because
> the old format is simply easier to work with for humans.
>
> How about:
>
> - If upper is nonempty, then leave redirect feature alone except when
> mount option "-oredirect=on" is used to force enabling it.
> - If upper is empty, then enable redirect feature except when mount
> option "-oredirect=off" is used to force disabling it.

I don't like this empty-nonempty upper logic.

I think this feature should be off by default and be enabled
explicitly in mount option.
Available features could be listed in sysfs /sys/fs/overlay/..., like ext4 does.

Overlayfs mounting anyway is complicated operation.
User must know a lot about it and provide persistent state for each mount:
list layers in correct order, work and uppder directory on the same disk, etc.
Enabled features is a part of this state.

Probably this could be solved in userspace tool "mount.overlay" - it could load
features and layers from config file or xattr and set required mount
options automatically.

--
Konstantin

[toc] | [next] | [standalone]

#1515894

On Sun, Nov 6, 2016 at 8:14 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> On Wed, Oct 26, 2016 at 2:12 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Tue, Oct 25, 2016 at 1:57 PM, Raphael Hertzog <hertzog@debian.org> wrote:
>>
>>> Do you plan to make it the default in the future when it has been
>>> available for a while?
>>>
>>> Barring any regression introduced by your patch, it seems that the feature
>>> is best available by default since it allows legitimate operations to
>>> succeed that are otherwise refused. I understand that it makes it
>>> impossible to mount the overlay filesystem with an older kernel but is
>>> that problem more widespread than the one we're fixing here?  On my side,
>>> overlayfs is only used in scenarios where the kernel is always the same
>>> (or newer compared to what created the initial filesystem).
>>
>> I think it would be safe to make it the default if upperdir is empty.
>> Nonempty implies that it was created with old kernel (or it was
>> crafted by hand).   But there should be a way to explicitly turn it
>> off;  either because of the need for backward compatibility or because
>> the old format is simply easier to work with for humans.
>>
>> How about:
>>
>> - If upper is nonempty, then leave redirect feature alone except when
>> mount option "-oredirect=on" is used to force enabling it.
>> - If upper is empty, then enable redirect feature except when mount
>> option "-oredirect=off" is used to force disabling it.
>
> I don't like this empty-nonempty upper logic.
>
> I think this feature should be off by default and be enabled
> explicitly in mount option.
> Available features could be listed in sysfs /sys/fs/overlay/..., like ext4 does.
>
> Overlayfs mounting anyway is complicated operation.
> User must know a lot about it and provide persistent state for each mount:
> list layers in correct order, work and uppder directory on the same disk, etc.
> Enabled features is a part of this state.
>
> Probably this could be solved in userspace tool "mount.overlay" - it could load
> features and layers from config file or xattr and set required mount
> options automatically.

It seems there are some conflicting opinions here.  I have mine too,
but I'll let this simmer and concentrate on actual features for now.

Thanks,
Miklos

[toc] | [prev] | [next] | [standalone]

#1515961

On Mon, Nov 7, 2016 at 11:07 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Sun, Nov 6, 2016 at 8:14 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>> On Wed, Oct 26, 2016 at 2:12 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>> On Tue, Oct 25, 2016 at 1:57 PM, Raphael Hertzog <hertzog@debian.org> wrote:
>>>
>>>> Do you plan to make it the default in the future when it has been
>>>> available for a while?
>>>>
>>>> Barring any regression introduced by your patch, it seems that the feature
>>>> is best available by default since it allows legitimate operations to
>>>> succeed that are otherwise refused. I understand that it makes it
>>>> impossible to mount the overlay filesystem with an older kernel but is
>>>> that problem more widespread than the one we're fixing here?  On my side,
>>>> overlayfs is only used in scenarios where the kernel is always the same
>>>> (or newer compared to what created the initial filesystem).
>>>
>>> I think it would be safe to make it the default if upperdir is empty.
>>> Nonempty implies that it was created with old kernel (or it was
>>> crafted by hand).   But there should be a way to explicitly turn it
>>> off;  either because of the need for backward compatibility or because
>>> the old format is simply easier to work with for humans.
>>>
>>> How about:
>>>
>>> - If upper is nonempty, then leave redirect feature alone except when
>>> mount option "-oredirect=on" is used to force enabling it.
>>> - If upper is empty, then enable redirect feature except when mount
>>> option "-oredirect=off" is used to force disabling it.
>>
>> I don't like this empty-nonempty upper logic.
>>
>> I think this feature should be off by default and be enabled
>> explicitly in mount option.
>> Available features could be listed in sysfs /sys/fs/overlay/..., like ext4 does.
>>
>> Overlayfs mounting anyway is complicated operation.
>> User must know a lot about it and provide persistent state for each mount:
>> list layers in correct order, work and uppder directory on the same disk, etc.
>> Enabled features is a part of this state.
>>
>> Probably this could be solved in userspace tool "mount.overlay" - it could load
>> features and layers from config file or xattr and set required mount
>> options automatically.
>
> It seems there are some conflicting opinions here.  I have mine too,
> but I'll let this simmer and concentrate on actual features for now.

Ok =) let's try keep as simple as possible.

I've stumbled on somehow related problem - concurrent copy-ups are
strictly serialized by rename locks.
Obviously, file copying could be done in parallel: locks are required
only for final rename.
Because of that overlay slower that aufs for some workloads.

[toc] | [prev] | [next] | [standalone]

#1515966

On Mon, Nov 7, 2016 at 10:58 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:

> I've stumbled on somehow related problem - concurrent copy-ups are
> strictly serialized by rename locks.
> Obviously, file copying could be done in parallel: locks are required
> only for final rename.
> Because of that overlay slower that aufs for some workloads.

Easy to fix: for each copy up create a separate subdir of "work".
Then the contention is only for the time of creating the subdir, which
is very short.

Thanks,
Miklos

[toc] | [prev] | [next] | [standalone]

#1515971

On Mon, Nov 7, 2016 at 1:04 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Mon, Nov 7, 2016 at 10:58 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>
>> I've stumbled on somehow related problem - concurrent copy-ups are
>> strictly serialized by rename locks.
>> Obviously, file copying could be done in parallel: locks are required
>> only for final rename.
>> Because of that overlay slower that aufs for some workloads.
>
> Easy to fix: for each copy up create a separate subdir of "work".
> Then the contention is only for the time of creating the subdir, which
> is very short.

Yeah, but lock_rename() also takes per-sb s_vfs_rename_mutex (kludge by Al Viro)
I think proper synchronization for concurrent copy-up (for example
round flag on ovl_entry) and  locking rename only for rename could be
better.

[toc] | [prev] | [next] | [standalone]

#1516378

On Mon, Nov 7, 2016 at 12:08 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> On Mon, Nov 7, 2016 at 1:04 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Mon, Nov 7, 2016 at 10:58 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>
>>> I've stumbled on somehow related problem - concurrent copy-ups are
>>> strictly serialized by rename locks.
>>> Obviously, file copying could be done in parallel: locks are required
>>> only for final rename.
>>> Because of that overlay slower that aufs for some workloads.
>>
>> Easy to fix: for each copy up create a separate subdir of "work".
>> Then the contention is only for the time of creating the subdir, which
>> is very short.
>
> Yeah, but lock_rename() also takes per-sb s_vfs_rename_mutex (kludge by Al Viro)
> I think proper synchronization for concurrent copy-up (for example
> round flag on ovl_entry) and  locking rename only for rename could be
> better.

Removing s_vfs_rename_mutex from copy-up path is something I have been
pondering about.
Assuming that I understand Al's comment above vfs_rename() correctly,
the sole purpose of per-sb serialization is to prevent loop creations.
However, how can one create a loop by moving a non-directory?
So it looks like at least for the non-dir copy up case, a much finer grained
lock is in order.

Anyway, it's on my todo list, as concurrent operation performance on overlayfs
is important to out use case.

Amir.

[toc] | [prev] | [next] | [standalone]

#1519424

On Mon, Nov 7, 2016 at 3:38 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Nov 7, 2016 at 12:08 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>> On Mon, Nov 7, 2016 at 1:04 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>> On Mon, Nov 7, 2016 at 10:58 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>>
>>>> I've stumbled on somehow related problem - concurrent copy-ups are
>>>> strictly serialized by rename locks.
>>>> Obviously, file copying could be done in parallel: locks are required
>>>> only for final rename.
>>>> Because of that overlay slower that aufs for some workloads.
>>>
>>> Easy to fix: for each copy up create a separate subdir of "work".
>>> Then the contention is only for the time of creating the subdir, which
>>> is very short.
>>
>> Yeah, but lock_rename() also takes per-sb s_vfs_rename_mutex (kludge by Al Viro)
>> I think proper synchronization for concurrent copy-up (for example
>> round flag on ovl_entry) and  locking rename only for rename could be
>> better.
>
> Removing s_vfs_rename_mutex from copy-up path is something I have been
> pondering about.
> Assuming that I understand Al's comment above vfs_rename() correctly,
> the sole purpose of per-sb serialization is to prevent loop creations.
> However, how can one create a loop by moving a non-directory?
> So it looks like at least for the non-dir copy up case, a much finer grained
> lock is in order.
>


I posted patches to relax the s_vfs_rename_mutex for copy-up and
whiteout in some use cases.

Konstantin,

It would be useful to know if those patches help with your use case.

Thanks,
Amir.

[toc] | [prev] | [next] | [standalone]

#1519642

On Fri, Nov 11, 2016 at 1:56 AM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Nov 7, 2016 at 3:38 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> On Mon, Nov 7, 2016 at 12:08 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>> On Mon, Nov 7, 2016 at 1:04 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>> On Mon, Nov 7, 2016 at 10:58 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>>>
>>>>> I've stumbled on somehow related problem - concurrent copy-ups are
>>>>> strictly serialized by rename locks.
>>>>> Obviously, file copying could be done in parallel: locks are required
>>>>> only for final rename.
>>>>> Because of that overlay slower that aufs for some workloads.
>>>>
>>>> Easy to fix: for each copy up create a separate subdir of "work".
>>>> Then the contention is only for the time of creating the subdir, which
>>>> is very short.
>>>
>>> Yeah, but lock_rename() also takes per-sb s_vfs_rename_mutex (kludge by Al Viro)
>>> I think proper synchronization for concurrent copy-up (for example
>>> round flag on ovl_entry) and  locking rename only for rename could be
>>> better.
>>
>> Removing s_vfs_rename_mutex from copy-up path is something I have been
>> pondering about.
>> Assuming that I understand Al's comment above vfs_rename() correctly,
>> the sole purpose of per-sb serialization is to prevent loop creations.
>> However, how can one create a loop by moving a non-directory?
>> So it looks like at least for the non-dir copy up case, a much finer grained
>> lock is in order.
>>
>
>
> I posted patches to relax the s_vfs_rename_mutex for copy-up and
> whiteout in some use cases.
>
> Konstantin,
>
> It would be useful to know if those patches help with your use case.
>

Well.. I think relaxing only s_vfs_rename_mutex wouldn't help much here.
Copying is still serialized by i_mutex on workdir?
Data copying should be done without rename locks at all.

[toc] | [prev] | [next] | [standalone]

#1519658

On Fri, Nov 11, 2016 at 10:46 AM, Konstantin Khlebnikov
<koct9i@gmail.com> wrote:
> On Fri, Nov 11, 2016 at 1:56 AM, Amir Goldstein <amir73il@gmail.com> wrote:
>> On Mon, Nov 7, 2016 at 3:38 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>> On Mon, Nov 7, 2016 at 12:08 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>>> On Mon, Nov 7, 2016 at 1:04 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>>> On Mon, Nov 7, 2016 at 10:58 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>>>>
>>>>>> I've stumbled on somehow related problem - concurrent copy-ups are
>>>>>> strictly serialized by rename locks.
>>>>>> Obviously, file copying could be done in parallel: locks are required
>>>>>> only for final rename.
>>>>>> Because of that overlay slower that aufs for some workloads.
>>>>>
>>>>> Easy to fix: for each copy up create a separate subdir of "work".
>>>>> Then the contention is only for the time of creating the subdir, which
>>>>> is very short.
>>>>
>>>> Yeah, but lock_rename() also takes per-sb s_vfs_rename_mutex (kludge by Al Viro)
>>>> I think proper synchronization for concurrent copy-up (for example
>>>> round flag on ovl_entry) and  locking rename only for rename could be
>>>> better.
>>>
>>> Removing s_vfs_rename_mutex from copy-up path is something I have been
>>> pondering about.
>>> Assuming that I understand Al's comment above vfs_rename() correctly,
>>> the sole purpose of per-sb serialization is to prevent loop creations.
>>> However, how can one create a loop by moving a non-directory?
>>> So it looks like at least for the non-dir copy up case, a much finer grained
>>> lock is in order.
>>>
>>
>>
>> I posted patches to relax the s_vfs_rename_mutex for copy-up and
>> whiteout in some use cases.
>>
>> Konstantin,
>>
>> It would be useful to know if those patches help with your use case.
>>
>
> Well.. I think relaxing only s_vfs_rename_mutex wouldn't help much here.
> Copying is still serialized by i_mutex on workdir?
> Data copying should be done without rename locks at all.

We do need something to prevent multiple copy-ups starting up in
parallel on the same file, though.

Thanks,
Miklos

[toc] | [prev] | [next] | [standalone]

#1519773

On Fri, Nov 11, 2016 at 12:06 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Fri, Nov 11, 2016 at 10:46 AM, Konstantin Khlebnikov
> <koct9i@gmail.com> wrote:
>> On Fri, Nov 11, 2016 at 1:56 AM, Amir Goldstein <amir73il@gmail.com> wrote:
>>> On Mon, Nov 7, 2016 at 3:38 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>>> On Mon, Nov 7, 2016 at 12:08 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>>>> On Mon, Nov 7, 2016 at 1:04 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>>>> On Mon, Nov 7, 2016 at 10:58 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>>>>>
>>>>>>> I've stumbled on somehow related problem - concurrent copy-ups are
>>>>>>> strictly serialized by rename locks.
>>>>>>> Obviously, file copying could be done in parallel: locks are required
>>>>>>> only for final rename.
>>>>>>> Because of that overlay slower that aufs for some workloads.
>>>>>>
>>>>>> Easy to fix: for each copy up create a separate subdir of "work".
>>>>>> Then the contention is only for the time of creating the subdir, which
>>>>>> is very short.
>>>>>
>>>>> Yeah, but lock_rename() also takes per-sb s_vfs_rename_mutex (kludge by Al Viro)
>>>>> I think proper synchronization for concurrent copy-up (for example
>>>>> round flag on ovl_entry) and  locking rename only for rename could be
>>>>> better.
>>>>
>>>> Removing s_vfs_rename_mutex from copy-up path is something I have been
>>>> pondering about.
>>>> Assuming that I understand Al's comment above vfs_rename() correctly,
>>>> the sole purpose of per-sb serialization is to prevent loop creations.
>>>> However, how can one create a loop by moving a non-directory?
>>>> So it looks like at least for the non-dir copy up case, a much finer grained
>>>> lock is in order.
>>>>
>>>
>>>
>>> I posted patches to relax the s_vfs_rename_mutex for copy-up and
>>> whiteout in some use cases.
>>>
>>> Konstantin,
>>>
>>> It would be useful to know if those patches help with your use case.
>>>
>>
>> Well.. I think relaxing only s_vfs_rename_mutex wouldn't help much here.
>> Copying is still serialized by i_mutex on workdir?
>> Data copying should be done without rename locks at all.
>
> We do need something to prevent multiple copy-ups starting up in
> parallel on the same file, though.
>

I guess an inode_lock on the copy-up victim should suffice?
I will look into it as soon as I am done with profiling.
So far I ran only 2 rm -rf threads on 2 different overlay mounts
on the same underlying fs
and s_vfs_rename_mutex was contended about ~4% of the time.

In this test, copy-up is not dominant - only ~2% for the directory
copy-ups, but vfs_whiteouts take 20% and the vfs_rename itself 10%,
both with s_vfs_rename_mutex held.

Amir.

[toc] | [prev] | [next] | [standalone]

#1516007

Hello,

On Sun, 06 Nov 2016, Konstantin Khlebnikov wrote:
> > - If upper is nonempty, then leave redirect feature alone except when
> > mount option "-oredirect=on" is used to force enabling it.
> > - If upper is empty, then enable redirect feature except when mount
> > option "-oredirect=off" is used to force disabling it.
> 
> I don't like this empty-nonempty upper logic.

Why? (I don't have the feeling that your subsequent paragraphs answer this
question... unless "overlayfs mounting is hard, let's complicate it even
more" is your answer)

> I think this feature should be off by default and be enabled
> explicitly in mount option.
> Available features could be listed in sysfs /sys/fs/overlay/..., like ext4 does.

TTBOMK in ext4, they are set at mkfs time and the default feature set
comes from /etc/mke2fs.conf. There's nothing like that for overlayfs.

> Overlayfs mounting anyway is complicated operation.
> User must know a lot about it and provide persistent state for each mount:
> list layers in correct order, work and uppder directory on the same disk, etc.
> Enabled features is a part of this state.

A large part of the users are not direct overlayfs users, they use
application (like schroot and live-build in my case) that rely on
overlayfs to offer some user-modifiable throw-away chroots or some
persistency on top of a read-only image. In both cases, the upper
directory start empty.

I find it highly disturbing to have to modify all those applications just
to get the correct semantics to rename a directory.

> Probably this could be solved in userspace tool "mount.overlay" - it could load
> features and layers from config file or xattr and set required mount
> options automatically.

I'm all for having a better API to mount overlayfs, but I don't think
blocking on the "redirect=on" by default is a good way to get this.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: http://www.freexian.com/services/debian-lts.html
Learn to master Debian: http://debian-handbook.info/get/

[toc] | [prev] | [next] | [standalone]

#1516019

On Mon, Nov 7, 2016 at 2:03 PM, Raphael Hertzog <hertzog@debian.org> wrote:
> Hello,
>
> On Sun, 06 Nov 2016, Konstantin Khlebnikov wrote:
>> > - If upper is nonempty, then leave redirect feature alone except when
>> > mount option "-oredirect=on" is used to force enabling it.
>> > - If upper is empty, then enable redirect feature except when mount
>> > option "-oredirect=off" is used to force disabling it.
>>
>> I don't like this empty-nonempty upper logic.
>
> Why? (I don't have the feeling that your subsequent paragraphs answer this
> question... unless "overlayfs mounting is hard, let's complicate it even
> more" is your answer)

Mixing flags from mount options, xattrs and emptiness of upper layer
doesn't make it simpler.

We have clear statement that options in /proc/mounts describes overlay
instance, let's keep feeding state in this way.

>
>> I think this feature should be off by default and be enabled
>> explicitly in mount option.
>> Available features could be listed in sysfs /sys/fs/overlay/..., like ext4 does.
>
> TTBOMK in ext4, they are set at mkfs time and the default feature set
> comes from /etc/mke2fs.conf. There's nothing like that for overlayfs.

/etc/mke2fs.conf is used by mkfs.ext4 - state is saved in super-block
inside filesystem.

overlayfs have no persistent super-block.

>
>> Overlayfs mounting anyway is complicated operation.
>> User must know a lot about it and provide persistent state for each mount:
>> list layers in correct order, work and uppder directory on the same disk, etc.
>> Enabled features is a part of this state.
>
> A large part of the users are not direct overlayfs users, they use
> application (like schroot and live-build in my case) that rely on
> overlayfs to offer some user-modifiable throw-away chroots or some
> persistency on top of a read-only image. In both cases, the upper
> directory start empty.
>
> I find it highly disturbing to have to modify all those applications just
> to get the correct semantics to rename a directory.

Other application are already aware about overlay layout and use it.
We cannot enable by default new backward incompatible features.

Returning -EXDEV is a completely correct semantics for rename,
most applications employ broken assumptions about this syscall =)

>
>> Probably this could be solved in userspace tool "mount.overlay" - it could load
>> features and layers from config file or xattr and set required mount
>> options automatically.
>
> I'm all for having a better API to mount overlayfs, but I don't think
> blocking on the "redirect=on" by default is a good way to get this.
>
> Cheers,
> --
> Raphaël Hertzog ◈ Debian Developer
>
> Support Debian LTS: http://www.freexian.com/services/debian-lts.html
> Learn to master Debian: http://debian-handbook.info/get/

[toc] | [prev] | [next] | [standalone]

#1516207

On Mon, 07 Nov 2016, Konstantin Khlebnikov wrote:
> > Why? (I don't have the feeling that your subsequent paragraphs answer this
> > question... unless "overlayfs mounting is hard, let's complicate it even
> > more" is your answer)
> 
> Mixing flags from mount options, xattrs and emptiness of upper layer
> doesn't make it simpler.

It depends for whom. It does make it simpler for applications that just
want overlayfs to work like other normal filesystems.

> We have clear statement that options in /proc/mounts describes overlay
> instance, let's keep feeding state in this way.

Didn't you say above that xattrs provide flags too?

> > I find it highly disturbing to have to modify all those applications just
> > to get the correct semantics to rename a directory.
> 
> Other application are already aware about overlay layout and use it.
> We cannot enable by default new backward incompatible features.

On the opposite, if we have to modify those applications to add a new
mount option, then they will no longer work with older versions of
overlayfs... so you move the complexity down to applications if they want
to work with multiple kernel versions.

There's no technical problem to enable a new backward incompatible
feature. It's just that you don't want to do it in case the user
wants to mount it again with an older kernel. So this is just policy.

So what about a new mount option that defines a compatibility level?

0: initial feature set
1: with renamedir flag

It would default to 1 but the user can set it to "0" to keep compatibility
with older versions of overlayfs. In the future, as more backward
incompatible changes are added, you add new levels and define the values
of the various flags based on this setting.

> Returning -EXDEV is a completely correct semantics for rename,
> most applications employ broken assumptions about this syscall =)

Maybe (I don't know what standards say), but then what matters is
real-life. And in real-life, it's somewhat unexpected to get back -EXDEV
when the rename() happens in the same directory (and has therefore no
chance to cross any mount boundary).

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: http://www.freexian.com/services/debian-lts.html
Learn to master Debian: http://debian-handbook.info/get/

[toc] | [prev] | [next] | [standalone]

#1519405

New version is at:

git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git #redirect

News:
- it actually should work in all cases
- when rename is not cross directory, just store the new name instead
of a full path, as suggested by Amir
- when redirect path is too long fall back to EXDEV (the max length
should probably be a module param)

About turning the feature on or off.  Yes, maybe the empty checking is
too complicated.  Going one simpler:

 - default to old behavior, turn on with mount option
 - add module option and kernel compile option to turn on the feature by default

I guess distros wil simply enable this by default, since back
compatibility is basically a non-issue.

Thanks,
Miklos

[toc] | [prev] | [next] | [standalone]

#1519645

On Fri, Nov 11, 2016 at 1:39 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> New version is at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git #redirect
>
> News:
> - it actually should work in all cases
> - when rename is not cross directory, just store the new name instead
> of a full path, as suggested by Amir
> - when redirect path is too long fall back to EXDEV (the max length
> should probably be a module param)
>
> About turning the feature on or off.  Yes, maybe the empty checking is
> too complicated.  Going one simpler:
>
>  - default to old behavior, turn on with mount option
>  - add module option and kernel compile option to turn on the feature by default
>
> I guess distros wil simply enable this by default, since back
> compatibility is basically a non-issue.

Looks good.

I suppose module parameter exposed in /sys/module/overlay/parameters/ could be
documented as recommended way for detecting presence of that feature.

--
Konstantin

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web