Re: [PATCH 2/7] proc: Implement support for automounts in taskdirectories

[Andrew Morton]

On Thu, 06 Nov 2008 18:05:46 -0800 ebiederm@xxxxxxxxxxxx (Eric W. Biederman) wrote:

> Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> writes:
> 
> > On Thu, 06 Nov 2008 02:48:35 -0800
> > ebiederm@xxxxxxxxxxxx (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.

> 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.

> >> +		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?

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/