[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH 4/5] Wait for all DASDs to be online after autodetection (#558881).



On 02/10/2010 11:14 PM, David Cantrell wrote:
> When a user boots with cio_ignore=all,!0.0.0009 on s390x, it tells the
> kernel to only bring device 0.0.0009 online before booting. 

Please don't mix up cio_ignore and onlining. These are two different
things. cio_ignore hides devices from the ccw bus so they won't appear
in sysfs and not allocate kernel object memory. Only devices not ignored
may then be set online to activate (probe and provide real device
objects) or offline to deactivate again.

> When a DASD scan happens and each one is brought online, make sure we
> wait until the devices enter the 'active' or 'unformatted' states.  The
> other states indicate we should continue waiting for the device to come
> up.

Did you test this with cio_ignore=all,!0.0.0009 and not using the
workarounds I described for Jan in bug 558881 comment 6?

It seems to me as if we're still missing the part to wait for cio_ignore
to finish, i.e. the usage of dasd_cio_free. I fear we might see one of
the following linuxrc messages without explicitly waiting:
"DASD $devbusid does not provide attribute $attr"
"DASD $devbusid not found"

Only after that we can start setting DASDs online. And that also happens
asynchronously which is the reason of dasd_settle, namely to wait until
the device is fully online.

What's inside this patch seems to only care for the latter, i.e. bug
558881 comment 8.

Or am I on the wrong track?

> ---
>  loader/linuxrc.s390 |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/loader/linuxrc.s390 b/loader/linuxrc.s390
> index 82a741c..47a769f 100644
> --- a/loader/linuxrc.s390
> +++ b/loader/linuxrc.s390
> @@ -149,6 +149,16 @@ function dasd_settle() {
>      return 1
>  }
> 
> +function dasd_settle_all() {
> +    for dasdccw in $(cut -d '(' -f 1 /proc/dasd/devices) ; do

Nice. I like the use of this to go over all DASDs without having to
refactor parse_dasd.

> +        if ! dasd_settle $dasdccw ; then
> +            echo $"Could not access DASD $dasdccw in time"
> +            return 0

errorlevel != 0 means error case:
return 1

> +        fi
> +    done
> +    return 1

errorlevel 0 means successful case:
return 0

> +}
> +
>  function startinetd()
>  {
>      echo
> @@ -2519,6 +2529,7 @@ function parse_dasd() {
>      done < <(echo $DASD | sed 's/,/\n/g')
>      if [ "$handle" = "yes" ]; then
>          udevadm settle
> +        dasd_settle_all || return 0

dasd_settle_all || return 1

>          echo $"Activated DASDs:"
>          cat /proc/dasd/devices | sed -e 's/ at ([^)]*) is//' -e 's/ at/,/'
>      fi
> @@ -2713,6 +2724,7 @@ function final_check() {
>                  fi
>                  ;;
>              d) # show active DASDs with some useful details
> +                dasd_settle_all || return 0

Since this really only shows the current state, it should not paper over
possibly missed dasd_settle at the places where they are actually set
online. I would like to remove this added line here.

>                  echo $"Activated DASDs:"
>                  cat /proc/dasd/devices|sed -e 's/ at ([^)]*) is//' -e 's/ at/,/'
>                  ;;

Steffen

Linux on System z Development

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]