public inbox for initramfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Philippe Troin <phil-zSL7SSj9W616jAk/7fyWoAC/G2K4zDHf@public.gmane.org>
To: initramfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Buglets in shutdown module in HEAD
Date: Wed, 18 Jan 2017 10:34:06 -0800	[thread overview]
Message-ID: <1484764446.31841.5.camel@coldcreektech.com> (raw)

Hi Dracut maintainers,

Note that I'm not subscribed to the mailing list, a CC is appreciated.

I've spotted a couple of issues in the latest shutdown module,
specifically this file: http://git.kernel.org/cgit/boot/dracut/dracut.g
it/tree/modules.d/99shutdown/shutdown.sh
918a8a4f123578e39f38404670d5cbe3e3436bb3

The unmount loop seems to be doing the opposite of what it intends to.

    _cnt=0
    while [ $_cnt -le 40 ]; do
        umount_a 2>/dev/null || break
        _cnt=$(($_cnt+1))
    done

doesn't do what's it's supposed to do as umount_a returns 0 in case of
success and 1 in case of failure.
The || should be changed into &&:

    _cnt=0
    while [ $_cnt -le 40 ]; do
        umount_a 2>/dev/null && break
        _cnt=$(($_cnt+1))
    done

[2] A little sleeping in between attempts helps too in my experience
I'd suggest changing that code to:

    _cnt=0
    while [ $_cnt -le 40 ]; do
        umount_a 2>/dev/null && break
        sleep 0.25
        _cnt=$(($_cnt+1))
    done

[3] The check_shutdown loop running all the hooks can be very verbose
and needlessly scary when luks+lvm is involved.  It generally scrolls
lots of messages about being unable to disable dm mappings before
finally succeeding.

I'd suggest using the same logic as the unmount loop:  loop quietly,
and if it fails 40 times in a row, run one last time verbosely.

So changing:

    _cnt=0
    while [ $_cnt -le 40 ]; do
        _check_shutdown && break
        _cnt=$(($_cnt+1))
    done
    [ $_cnt -ge 40 ] && _check_shutdown final

to

    _cnt=0
    while [ $_cnt -le 40 ]; do
        _check_shutdown 2> /dev/null && break
        _cnt=$(($_cnt+1))
        sleep 0.25
    done
    [ $_cnt -ge 40 ] && _check_shutdown final

Again, the chances of a clean(er) shutdown may be improved by sleeping
in between attempts.

I believe [1] is a true bug, while [2] and [3] will be the maintainers'
call.

Phil.

                 reply	other threads:[~2017-01-18 18:34 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=1484764446.31841.5.camel@coldcreektech.com \
    --to=phil-zsl7ssj9w616jak/7fywoac/g2k4zdhf@public.gmane.org \
    --cc=initramfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox