LKML Archive on
help / color / mirror / Atom feed
From: (Eric W. Biederman)
To: Andrew Morton <>
Subject: Re: [PATCH 2/7] proc: Implement support for automounts in task directories
Date: Thu, 06 Nov 2008 19:51:23 -0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <> (Andrew Morton's message of "Thu, 6 Nov 2008 18:49:45 -0800")

Andrew Morton <> writes:

> On Thu, 06 Nov 2008 18:05:46 -0800 (Eric W. Biederman)
> wrote:
>> Andrew Morton <> writes:
>> > On Thu, 06 Nov 2008 02:48:35 -0800
>> > (Eric W. Biederman) wrote:
>> >
>> >> +void proc_shrink_automounts(void)
>> >> +{
>> >> +	struct list_head *list = &proc_automounts;
>> >> +
>> >> +	mark_mounts_for_expiry(list);
>> >> +	mark_mounts_for_expiry(list);
>> >
>> > Strange.  In case the first attempt didn't work?
>> Yes.  I'd like to say.  Mount just go away but it takes two passes before
>> a mount is actually removed.
> hm.  I stared at mark_mounts_for_expiry() for a while trying to work
> out how that can happen and what it semantically *means*, and failed.
> I guess I'm just not smart/experienced enough.

The semantics are a classic two pass exiry.  The mount is not in use
and it was not in use last time we came by remove the mount.

The code is fs/namespace.c is especially clever.  It took me a long
time starring at it to figure out what is going on.  The bug fix comes
from when I was working out what that code does.

>> For NFS which does the whole expiry of all inodes where it comes from it
>> is a good fit.  For /proc where we don't have to guess it isn't the best
>> fit but it isn't shabby either.
>> >
>> >> +	if (list_empty(list))
> So even after two passes through mark_mounts_for_expiry(), there can
> still be mounts on our list.

Only if the mounts are actually in use.

>> >> +		return;
>> >> +
>> >> +	schedule_delayed_work(&proc_automount_task, proc_automount_timeout);
> And this causes proc_shrink_automounts() to be called every 500 seconds
> for ever and ever, until proc_automounts is empty.


> Again, I just don't know how the reader of this file is to understand
> why this is done this way.  What is the thinking behind it?  What is
> the expected dynamic behaviour?  Under what circumstances will this
> very unusual repeated polling activity be triggered?

Basically if someone does:

cd /proc/<pid>/net/stat  or something like that and holds one of the dentries
alive.  The mount is not allowed to go away.

When a process exits release_task is called which in turn calls
proc_shrink_automounts().  In a normal case it will remove mounts that
are under that process.  

Unfortunately if one of these mounts is busy because the path to a dentry
references it we can not free it (although d_revalidate will cause the
dentry to become unhashed so we can't find the mount going in the other

So we have to periodically check, is the mount free yet.

If we could do all of this with reference counting so that the
mount would persist exactly until the last user of it has gone
away without a periodic poll I would love it.  But the infrastructure
doesn't support that today, and where this is at least partially
a bug fix I would rather not have the change depend on enhancing
the VFS.

The algorithm is actually very aggressive and in practice you don't
see any /proc/<pid>/net showing up as a mount point.

> Obviously, that becomes clearer as one spends more time with the code,
> but I wonder whether this has all been made as maintainble as it
> possibly could be.

Good question.

In the sense of will we have to go through and futz with the code all
of the time.  The abstraction seems good.   You put a mount on
the proc_automounts list with do_add_mounts and it goes away eventually
with all of the vfs rules maintained.

In the sense of can the code be read?    Perhaps it could be better.
I expect it helps to have run the code and see /proc/net as a filesystem.
that is magically mounted.

In the long term it also seems to make sense maintenance wise to split
/proc into separate parts that can go different directions, and having
the automounting of the old pieces, helps a lot with that.

Unless there are some real bugs my preference is to evolve the code from
where it is today.  The code is at least simple and stupid.


  reply	other threads:[~2008-11-07  3:55 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-06 10:38 [PATCH 1/7] vfs: Fix shrink_submounts Eric W. Biederman
2008-11-06 10:48 ` [PATCH 2/7] proc: Implement support for automounts in task directories Eric W. Biederman
2008-11-06 10:49   ` [PATCH 3/7] proc: Support multiple filesystems using the proc generic infrastructure Eric W. Biederman
2008-11-06 10:53     ` [PATCH 4/7] proc: Make /proc/net it's own filesystem Eric W. Biederman
2008-11-06 10:56       ` [PATCH 5/7] proc_net: Don't show the wrong /proc/net after unshare Eric W. Biederman
2008-11-06 10:57         ` [PATCH 6/7] proc_net: Simplify network namespace lookup Eric W. Biederman
2008-11-06 10:58           ` [PATCH 7/7] proc: Cleanup proc_flush_task Eric W. Biederman
2008-11-07  1:25   ` [PATCH 2/7] proc: Implement support for automounts in task directories Andrew Morton
2008-11-07  2:02     ` Eric W. Biederman
2008-11-07  1:26   ` Andrew Morton
2008-11-07  2:05     ` Eric W. Biederman
2008-11-07  2:49       ` Andrew Morton
2008-11-07  3:51         ` Eric W. Biederman [this message]
2008-11-07  4:28           ` Andrew Morton
2008-11-07 15:51             ` Eric W. Biederman
2008-11-07 16:05               ` Andrew Morton
2008-11-07 16:58                 ` Eric W. Biederman
2008-11-13 23:39                 ` Eric W. Biederman
2008-11-19  0:07                   ` Alexey Dobriyan
2008-11-19  2:35                     ` Alexey Dobriyan
2008-11-19 13:20                       ` Eric W. Biederman
2008-11-07  4:41   ` Alexey Dobriyan
2008-11-07 16:04     ` [PATCH] proc: Supply proc_shrink_automounts when CONFIG_PROC_FS=N Eric W. Biederman
2008-11-07  1:22 ` [PATCH 1/7] vfs: Fix shrink_submounts Andrew Morton
2008-11-07  2:06   ` Eric W. Biederman

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:

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

  git send-email \ \ \ \ \ \ \ \
    --subject='Re: [PATCH 2/7] proc: Implement support for automounts in task directories' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).