On Mon, Jan 16, 2012 at 4:56 PM, Leonardo Chiquitto
<leonardo.lists@xxxxxxxxx> wrote:
> On Thu, Dec 15, 2011 at 12:46 PM, Ian Kent <raven@xxxxxxxxxx> wrote:
>> On Thu, 2011-12-15 at 23:43 +0800, Ian Kent wrote:
>>> >
>>> > So -- if I understood correctly the case you wanted to test -- the answer
>>> > is yes: "s1" is unmounted when "v1" is in use.
>>>
>>> OK, thanks, I'll check this out when I get to it.
>>
>> Although that might actually be the correct behavior for deeper levels
>> of nesting. I've suspected that the handling of nesting may have been
>> broken at some point for a while but hadn't had cause to look closely at
>> it.
>
> Hello Ian,
>
> I've returned to this issue today, to investigate why the mount triggers
> on busy containers are being unmounted when AutoFS is stopped. I've
> found the following code in umount_multi_triggers():
>
> 1560 /*
> 1561 * Special case.
> 1562 * If we can't umount the root container then we can't
> 1563 * delete the offsets from the cache and we need to put
> 1564 * the offset triggers back.
> 1565 */
> 1566 if (is_mounted(_PATH_MOUNTED, root, MNTS_REAL)) {
> 1567 info(ap->logopt, "unmounting dir = %s", root);
> 1568 if (umount_ent(ap, root)) {
> 1569 if (mount_multi_triggers(ap, me, root,
> strlen(root), "/") < 0)
> 1570 warn(ap->logopt,
> 1571 "failed to remount offset triggers");
> 1572 return left++;
> 1573 }
> 1574 }
>
> So the automounter really tries to recreate the triggers, but this
> call to mount_multi_triggers() in line 1569 is failing. As far as I can
> see, the shut down sequence when a busy container exists is:
>
> - <state changes to ST_SHUTDOWN>
> - umount_autofs()
> - umount_autofs_direct() => closes ap->kpipefd and sets it to -1
> - do_umount_autofs_direct()
> - umount_multi()
> - umount_subtree_mounts()
> - umount_multi_triggers() => successfully umounts all offsets (/nfs/{v1,v2})
> but fails to unmount /nfs because it is busy. Calls mount_multi_triggers()
> to recreate the offsets.
> - mount_multi_triggers()
>
> Here are the (instrumented) logs of the attempt to create the /nfs/v1 trigger:
>
> Jan 16 16:28:38 n1 automount[14190]: mount_multi_triggers root: /nfs
> start: 4 base: /
> Jan 16 16:28:38 n1 automount[14190]: calling cache_get_offset()
> Jan 16 16:28:38 n1 automount[14190]: mount_multi_triggers: mount
> offset /nfs/v1 at /nfs
> Jan 16 16:28:38 n1 automount[14190]: calling mount_autofs_offset()
> Jan 16 16:28:38 n1 automount[14190]: entering mount_autofs_offset()
> Jan 16 16:28:38 n1 automount[14190]: ops->version: 1317877376,
> ap->flags: 1, ap->kpipefd: -1
> Jan 16 16:28:38 n1 automount[14190]: mountpoint: /nfs/v1 options:
> fd=-1,pgrp=14190,minproto=5,maxproto=5,offset
> Jan 16 16:28:38 n1 automount[14190]: calling mkdir_path(/nfs/v1)
> Jan 16 16:28:38 n1 automount[14190]: errno: 17
> Jan 16 16:28:38 n1 automount[14190]: mount_autofs_offset: calling
> mount -t autofs -s -o fd=-1,pgrp=14190,minproto=5,maxproto=5,offset
> automount /nfs/v1
> Jan 16 16:28:38 n1 kernel: [22093.824826] autofs: called with bogus options
> Jan 16 16:28:38 n1 automount[14190]: mount_autofs_offset: failed to
> mount offset trigger /nfs/v1 at /nfs/v1
> Jan 16 16:28:38 n1 automount[14190]: failed to mount offset
>
> The important error here is from the kernel: "autofs: called with
> bogus options".
> The bogus option it's complaining about is "fd=-1". So it looks like we
> closed the kpipefd too soon, and it's needed to recreate the mount triggers.
>
> To test this theory, I've wrote a patch to postpone closing kpipefd, so
> it would still be available when we need to call mount_multi_triggers().
> This resolved the problem here (at least with my basic test cases)
> and the mount triggers are correctly recreated when unmounting the
> container failed.
>
> Ian, please, could you take a look and tell me if I'm on the right path
> here?
>
> Thanks,
> Leonardo
>
> Index: autofs/daemon/direct.c
> ===================================================================
> --- autofs.orig/daemon/direct.c
> +++ autofs/daemon/direct.c
> @@ -198,7 +198,7 @@ int umount_autofs_direct(struct autofs_p
> if (ap->pipefd >= 0)
> close(ap->pipefd);
> if (ap->kpipefd >= 0) {
> - close(ap->kpipefd);
> + ap->kpipefd_save = ap->kpipefd;
> ap->kpipefd = -1;
> }
>
> @@ -228,6 +228,11 @@ int umount_autofs_direct(struct autofs_p
> pthread_cleanup_pop(1);
> map = map->next;
> }
> + if (ap->kpipefd_save >= 0) {
> + close(ap->kpipefd_save);
> + ap->kpipefd_save = -1;
> + }
> +
> pthread_cleanup_pop(1);
> pthread_cleanup_pop(1);
>
> Index: autofs/lib/mounts.c
> ===================================================================
> --- autofs.orig/lib/mounts.c
> +++ autofs/lib/mounts.c
> @@ -1566,6 +1566,8 @@ int umount_multi_triggers(struct autofs_
> if (is_mounted(_PATH_MOUNTED, root, MNTS_REAL)) {
> info(ap->logopt, "unmounting dir = %s", root);
> if (umount_ent(ap, root)) {
> + if (ap->kpipefd == -1 && ap->kpipefd_save != -1)
> + ap->kpipefd = ap->kpipefd_save;
> if (mount_multi_triggers(ap, me, root, strlen(root), "/") < 0)
> warn(ap->logopt,
> "failed to remount offset triggers");
> Index: autofs/include/automount.h
> ===================================================================
> --- autofs.orig/include/automount.h
> +++ autofs/include/automount.h
> @@ -456,6 +456,7 @@ struct autofs_point {
> char *path; /* Mount point name */
> int pipefd; /* File descriptor for pipe */
> int kpipefd; /* Kernel end descriptor for pipe */
> + int kpipefd_save; /* Kernel end descriptor for pipe */
> int ioctlfd; /* File descriptor for ioctls */
> int logpri_fifo; /* FIFO used for changing log levels */
> dev_t dev; /* "Device" number assigned by kernel */
Hello Ian,
Now that the deadlock on nested mounts is fixed, could you review the
sanity of this patch? I've tested it again today with the latest git checkout
plus the remount-deadlock patch and it seems to work fine.
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
