Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 21 Dec 2015 18:07:38 +0100
From:      Jilles Tjoelker <jilles@stack.nl>
To:        Edward Tomasz Napierala <trasz@FreeBSD.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r290197 - in head: etc/defaults etc/rc.d sys/kern
Message-ID:  <20151221170738.GA19579@stack.nl>
In-Reply-To: <201510301552.t9UFqAtv073641@repo.freebsd.org>
References:  <201510301552.t9UFqAtv073641@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Oct 30, 2015 at 03:52:10PM +0000, Edward Tomasz Napierala wrote:
> Author: trasz
> Date: Fri Oct 30 15:52:10 2015
> New Revision: 290197
> URL: https://svnweb.freebsd.org/changeset/base/290197

> Log:
>   After r290196, the kernel won't wait for stuff like gmirror nodes
>   if they are not required for mounting rootfs.  However, it's possible
>   that some setups try to mount them in mountcritlocal (ie from fstab).

>   Export the list of current root mount holds using a new sysctl,
>   vfs.root_mount_hold, and make mountcritlocal retry if "mount -a" fails
>   and the list is not empty.

>   MFC after:	1 month
>   Sponsored by:	The FreeBSD Foundation
>   Differential Revision:	https://reviews.freebsd.org/D3709

I like the faster startup, but the rc.d script clearly will not wait as
intended. See below.

> Modified:
>   head/etc/defaults/rc.conf
>   head/etc/rc.d/mountcritlocal
>   head/sys/kern/vfs_mountroot.c

> [snip]
> Modified: head/etc/rc.d/mountcritlocal
> ==============================================================================
> --- head/etc/rc.d/mountcritlocal	Fri Oct 30 15:35:04 2015	(r290196)
> +++ head/etc/rc.d/mountcritlocal	Fri Oct 30 15:52:10 2015	(r290197)
> @@ -15,7 +15,7 @@ stop_cmd=sync
>  
>  mountcritlocal_start()
>  {
> -	local err
> +	local err holders waited
>  
>  	# Set up the list of network filesystem types for which mounting
>  	# should be delayed until after network initialization.
> @@ -35,8 +35,42 @@ mountcritlocal_start()
>  		mount_excludes="${mount_excludes}${fstype},"
>  	done
>  	mount_excludes=${mount_excludes%,}
> +
> +	# Originally, root mount hold had to be released before mounting the root
> +	# filesystem.  This delayed the boot, so it was changed to only wait if
> +	# the root device isn't readily available.  This can result in this script
> +	# executing before all the devices - such as graid(8) - are available.
> +	# Thus, should the mount fail, we will wait for the root mount hold release
> +	# and retry.

These lines are a bit too long.

>  	mount -a -t ${mount_excludes}
>  	err=$?
> +	if [ $? -ne 0 ]; then

The assignment will set $? to 0, so the new code will never be executed.
"$err" should be tested instead of $?.

> +		echo
> +		echo 'Mounting /etc/fstab filesystems failed,' \
> +		    'will retry after root mount hold release'
> +
> +		waited=0
> +		while [ ${waited} -lt ${root_hold_delay} ]; do
> +			holders="$(sysctl -n vfs.root_mount_hold)"
> +			if [ -z "${holders}" ]; then
> +				break;
> +			fi
> +			if [ ${waited} -eq 0 ]; then
> +				echo -n "Waiting ${root_hold_delay}s" \
> +				"for the root mount holders: ${holders}"
> +			else
> +				echo -n .
> +			fi
> +			if [ ${waited} -eq ${root_hold_delay} ]; then
> +				break 2
> +			fi
> +			sleep 1
> +			waited=$(($waited + 1))
> +		done
> +		mount -a -t ${mount_excludes}
> +		err=$?
> +	fi
> +
>  	check_startmsgs && echo '.'
>  
>  	case ${err} in
> @@ -44,7 +78,7 @@ mountcritlocal_start()
>  		;;
>  	*)
>  		echo 'Mounting /etc/fstab filesystems failed,' \
> -		    ' startup aborted'
> +		    'startup aborted'
>  		stop_boot true
>  		;;
>  	esac
> [snip]

-- 
Jilles Tjoelker



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20151221170738.GA19579>