On Mon, 2013-04-08 at 22:24 -0300, Leonardo Chiquitto wrote:
> On Mon, Apr 8, 2013 at 4:48 AM, Ian Kent <raven@xxxxxxxxxx> wrote:
> > On Fri, 2013-03-29 at 17:33 -0300, Leonardo Chiquitto wrote:
> >>
> >> Ian: do you think the "occasional deadlocks" mentioned in the
> >> expire-specific-submount-only patch might have been addressed
> >> by other changes (so we can use the patch below)? Or do you
> >> see another way to fix the problem?
> >
> > Think so.
> >
> >>
> >> diff --git a/lib/master.c b/lib/master.c
> >> index a0e62f2..99b1092 100644
> >> --- a/lib/master.c
> >> +++ b/lib/master.c
> >> @@ -906,8 +906,10 @@ int master_notify_submount(struct autofs_point
> >> *ap, const char *path, enum state
> >> p = p->prev;
> >>
> >> if (!master_submount_list_empty(this)) {
> >> - mounts_mutex_unlock(ap);
> >> - return master_notify_submount(this, path, state);
> >> + if (!master_notify_submount(this, path, state)) {
> >> + ret = 0;
> >> + break;
> >> + }
> >> }
> >>
> >> /* path not the same */
> >
> > I think the mounts mutex needs to be re-taken and then check if the
> > submount has gone away.
> >
> > The list curor for the next one to check (set to p->prev) should always
> > be ok because there's only one expire process for a given mount but the
> > one we are checking (this) could go away, in which case we're done.
> >
> > This patch seems to work.
> >
> > autofs-5.0.7 - fix submount tree not all expiring
> >
> > From: Ian Kent <ikent@xxxxxxxxxx>
> >
> > Due to the change in the expire-specific-submount-only patch, sub-mounts
> > within an indirect mount that follow a submount (in the check order) won't
> > be expired if that submount is busy.
> > ---
> > lib/master.c | 24 +++++++++++++++---------
> > 1 file changed, 15 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/master.c b/lib/master.c
> > index a0e62f2..64dbcb1 100644
> > --- a/lib/master.c
> > +++ b/lib/master.c
> > @@ -905,15 +905,24 @@ int master_notify_submount(struct autofs_point *ap, const char *path, enum state
> > this = list_entry(p, struct autofs_point, mounts);
> > p = p->prev;
> >
> > - if (!master_submount_list_empty(this)) {
> > - mounts_mutex_unlock(ap);
> > - return master_notify_submount(this, path, state);
> > - }
> > -
> > /* path not the same */
> > if (strcmp(this->path, path))
> > continue;
> >
> > + if (!master_submount_list_empty(this)) {
> > + char *this_path = strdup(this->path);
> > + if (this_path) {
> > + mounts_mutex_unlock(ap);
> > + master_notify_submount(this, path, state);
> > + mounts_mutex_lock(ap);
> > + if (!__master_find_submount(ap, this_path)) {
> > + free(this_path);
> > + continue;
> > + }
> > + free(this_path);
> > + }
> > + }
> > +
> > /* Now we have found the submount we want to expire */
> >
> > st_mutex_lock();
> > @@ -959,10 +968,7 @@ int master_notify_submount(struct autofs_point *ap, const char *path, enum state
> > st_mutex_lock();
> > }
> > st_mutex_unlock();
> > - mounts_mutex_unlock(ap);
> > -
> > - return ret;
> > -
> > + break;
> > }
> >
> > mounts_mutex_unlock(ap);
>
> Hi Ian,
>
> I tested the patch and verified that it resolves the problem.
Thanks, I'll commit it next time I push an update to the repo.
>
> Thanks,
> Leonardo
--
To unsubscribe from this list: send the line "unsubscribe autofs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
