On Tue, 26 Apr 2005, Jeff Moyer wrote:
> ==> Regarding Re: [patch] get rid of is_mounted checks in cache_clean; Ian
> Kent <[EMAIL PROTECTED]> adds:
>
> raven> On Mon, 25 Apr 2005, Jeff Moyer wrote:
> >> Hi, Ian,
> >>
> >> I'm not sure what purpose the following check serves in the cache_clean
> >> function:
> >>
> >> if (is_mounted(_PATH_MOUNTED, path)) { free(path); continue; }
> >>
> >> In the event that the map was updated, we signal the daemon to do a
> >> re-read of the entire map. If the mount finishes before the daemon is
> >> able to process this signal, we end up not pruning the old entry for the
> >> given path. Now, because we append entries to the cache, we end up with
> >> the stale entry first, followed by the updated entry. This means that
> >> subsequent lookups will give the old entry.
I see the problem.
Side affect from a previous bug fix.
>
> raven> This was originally meant to stop busy (currently mounted) entries
> raven> from disappearing from the cache.
>
> raven> I'll check whether this is actually needed and get back.
>
> raven> One thing that comes to mind is that the direct mount rework design
> raven> might need this.
>
> Well, unfortunately I cannot comment on that. We had have a very precise
> definite of what it means to be in the cache, then, if there are to be
> side-effects of removing entries.
That's a bit unclear?
>
> Anyway, the patch I sent was somewhat incomplete. Look at this:
>
> if (me->age < age) {
> pred->next = me->next;
> free(me->key);
> free(me->mapent);
> free(me);
> me = pred;
> rmdir_path(path);
> }
>
> The rmdir_path is unneeded, and I would argue has no business being here.
> It is the job of the umount_multi code to clean up the directory tree, not
> the cache code. Lucky for us, the umount_multi path doesn't error out if a
> directory is removed for it (thinking of odd races, here). Anyway, the
> attached patch (diffed against current cvs) is more complete in this
> respect. It gets rid of these rmdir_path calls.
Agreed.
But I remember I put that in there to fix an obsucure problem I was
having. Removing it may cause undesirable side effects. Unfortuneatly, I
can't remember what the symptoms were.
It's probably better to fix the problem as you suggest above though.
I'll spend some time to see if I can work out why I thought I had to
have it there. A lot has changed since then so maybe removing it will
work fine.
The direct mount rework is driven by the cache. If it's mounted then it
needs to stay in the cache. To fix the problem above we could mark the
entry as stale and skip it on lookup. We could do this by adding a field,
stale, to the cache struct or by simply setting age to zero and adding the
needed tests.
Ian
_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs
