Re: [REVIEW][PATCH] mnt: Tuck mounts under others instead of creating shadow/side mounts.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Andrei Vagin <avagin@xxxxxxxxxxxxx> writes:

> On Sun, May 14, 2017 at 04:26:18AM -0500, Eric W. Biederman wrote:
>> ebiederm@xxxxxxxxxxxx (Eric W. Biederman) writes:
>> 
>> > Andrei Vagin <avagin@xxxxxxxxxxxxx> writes:
>> >
>> >> Hi Eric,
>> >>
>> >> I found one issue about this patch. Take a look at the next script. The
>> >> idea is simple, we call mount twice and than we call umount twice, but
>> >> the second umount fails.
>> >
>> > Definitely an odd.  I will take a look.
>> 
>> After a little more looking I have to say the only thing I can see wrong
>> in the behavior is that the first umount doesn't unmount everything.
>> 
>> Changing the mount propagation tree while it is being traversed
>> apparently prevents a complete traversal of the propgation tree.
>
> Is it be enough to find topper which will not be umounted? I attached a
> patch which does this. I can't find a reason why this will not work.
>
> The idea of the patch is that we try to find a topper, check whether it is
> marked, and if it is marked, then we try to find the next one. All
> marked toppers are umounted and the first unmarked topper is attached to
> the parent mount.

That isn't completely wrong.  But I don't believe the it is the proper
logic.

Fundamentally the issue is that we are reparenting while remounting.
This results in mounts that should be unmounted moving before they
are unmounted.

Which means that if we leave all of the mnt_change_mountpoint work
for a final pass over the mounts everything works correctly.

Which is fabulous news.

That means multiple passes through a mount propagation tree for a given
mountpoint can no longer change what winds up unmounted.  Which
means we can skip subtrees that have already seen mount propagation.
Which means the optimizations I was playing with earlier fundamentally
will work without changing the semantics of MNT_DETACH.

> And I think we need to add tests in tools/testing/selftests/...

Feel free.

Patch to sort this immediate issue out in a moment...

Eric




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux