All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Ian Kent <raven@themaw.net>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	autofs mailing list <autofs@vger.kernel.org>,
	Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Omar Sandoval <osandov@osandov.com>
Subject: Re: [PATCH 1/7] vfs - merge path_is_mountpoint() and path_is_mountpoint_rcu()
Date: Thu, 08 Dec 2016 10:30:24 +1300	[thread overview]
Message-ID: <8737hzv34f.fsf@xmission.com> (raw)
In-Reply-To: <1481017056.4589.21.camel@themaw.net> (Ian Kent's message of "Tue, 06 Dec 2016 17:37:36 +0800")

Ian Kent <raven@themaw.net> writes:

> On Sat, 2016-12-03 at 05:13 +0000, Al Viro wrote:
>> 	FWIW, I've folded that pile into vfs.git#work.autofs.
>> 
>> Problems:
>
> snip ...
>
>> 	* the last one (propagation-related) is too ugly to live - at the
>> very least, its pieces should live in fs/pnode.c; exposing propagate_next()
>> is simply wrong.  I hadn't picked that one at all, and I would suggest
>> coordinating anything in that area with ebiederman - he has some work
>> around fs/pnode.c and you risk stepping on his toes.
>
> The earlier patches seem to be ok now so how about we talk a little about this
> last one.
>
> Eric, Al mentioned that you are working with fs/pnode.c and recommended I co-
> ordinate with you.
>
> So is my working on this this (which is most likely going to live in pnode.c if
> I can can get something acceptable) going to cause complications for you?
> Is what your doing at a point were it would be worth doing as Al
> suggests?
>
> Anyway, the problem that this patch is supposed to solve is to check if any of
> the list of mnt_mounts or any of the mounts propagated from each are in use.
>
> One obvious problem with it is the propagation could be very large.
>
> But now I look at it again there's no reason to have to every tree because if
> one tree is busy then the the set of trees is busy. But every tree would be
> visited if the not busy so it's perhaps still a problem.
>
> The difficult thing is working out if a tree is busy, more so because there
> could be a struct path holding references within any the trees so I don't know
> of a simpler, more efficient way to check for this.

So coordination seems to be in order.  Not so much because of your
current implementation (which won't tell you what you want to know)
but because an implementation that tells you what you are looking for
has really bad > O(N^2) complexity walking the propagation tree in
the worst case.

To be correct you code would need to use next_mnt and walk the tree and
on each mount use propagate_mnt_busy to see if that entry can be
unmounted.  Possibly with small variations to match your case.  Mounts
in one part of the tree may propagate to places that mounts in other
parts of the tree will not.

I am assuming you are not worried about MNT_DETACH, as that case would
not need may_umount_tree at all.

I am researching how to make walking propagation for multiple mounts
efficient but it is a non-trivial problem.

> Anyone have any suggestions at all?

In the short term I recommend just not doing this, but if you want to
see if you can find an efficient algorithm with me, I am happy to bring
you up to speed on all of the gory details.

Eric



  reply	other threads:[~2016-12-07 21:30 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-28  2:11 [PATCH 1/7] vfs - merge path_is_mountpoint() and path_is_mountpoint_rcu() Ian Kent
2016-11-28  2:11 ` Ian Kent
2016-11-28  2:11 ` [PATCH 2/7] autofs - make struct path const in autofs4_dir_open() Ian Kent
2016-11-28  2:11 ` [PATCH 3/7] autofs - change struct path to const in autofs4_expire_wait() and autofs4_wait() Ian Kent
2016-11-28  2:11   ` Ian Kent
2016-11-28  2:12 ` [PATCH 4/7] vfs - change struct path to const in d_manage() Ian Kent
2016-11-28  2:12   ` Ian Kent
2016-11-28  2:12 ` [PATCH 5/7] vfs - constify path parameter of path_has_submounts() Ian Kent
2016-11-28  2:12 ` [PATCH 6/7] autofs - dont hold spin lock over direct mount expire Ian Kent
2016-11-28  2:12   ` Ian Kent
2016-11-28  2:12 ` [PATCH 7/7] vfs - make may_umount_tree() mount propogation aware Ian Kent
2016-11-30 22:22 ` [PATCH 1/7] vfs - merge path_is_mountpoint() and path_is_mountpoint_rcu() Andrew Morton
2016-11-30 22:22   ` Andrew Morton
2016-12-01  0:13   ` Ian Kent
2016-12-01  0:13     ` Ian Kent
2016-12-03  5:13 ` Al Viro
2016-12-03  5:13   ` Al Viro
2016-12-03 23:29   ` Al Viro
2016-12-03 23:29     ` Al Viro
2016-12-04  2:18     ` Ian Kent
2016-12-05  2:50       ` Ian Kent
2016-12-05  2:50         ` Ian Kent
2016-12-04  2:07   ` Ian Kent
2016-12-06  9:37   ` Ian Kent
2016-12-07 21:30     ` Eric W. Biederman [this message]
2016-12-08  3:29       ` Ian Kent
2016-12-08  3:29         ` Ian Kent
2016-12-08  4:28         ` Eric W. Biederman
2016-12-08  4:28           ` Eric W. Biederman
2016-12-08  6:18           ` Ian Kent
2016-12-08  6:18             ` Ian Kent

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=8737hzv34f.fsf@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=autofs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=osandov@osandov.com \
    --cc=raven@themaw.net \
    --cc=viro@ZenIV.linux.org.uk \
    /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.