Re: [PATCH] ieee1275/ofdisk: retry on open and read failure

[Michael Chang]

On Tue, Mar 28, 2023 at 11:00:01AM +0530, Mukesh Kumar Chaurasiya wrote:
> Sometimes, when booting from a very busy SAN, the access to the
> disk can fail and then grub will eventually drop to grub prompt.
> This scenario is more frequent when deploying many machines at
> the same time using the same SAN.
> This patch aims to force the ofdisk module to retry the open or
> read function after it fails. We use MAX_RETRIES to specify the
> amount of times it will try to access the disk before it
> definitely fails.

To clarify this is a continuation of previous patch [1]. Obviously the
count of retries, MAX_RETRIES, has been replaced by a timeout,
RETRY_DEFAULT_TIMEOUT, which is 15000 milliseconds or fifteen seconds.
It appears that the description was not updated accordingly and needs to
be amended.

[1] https://www.mail-archive.com/grub-devel@gnu.org/msg32174.html

Thanks,
Michael

> 
> Signed-off-by: Mukesh Kumar Chaurasiya <mchauras@linux.vnet.ibm.com>
> ---
>  grub-core/disk/ieee1275/ofdisk.c | 65 +++++++++++++++++++++++++++++++-
>  1 file changed, 63 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/disk/ieee1275/ofdisk.c 
> b/grub-core/disk/ieee1275/ofdisk.c
> index c6cba0c8a..f4183a531 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -24,6 +24,9 @@
>  #include <grub/ieee1275/ofdisk.h>
>  #include <grub/i18n.h>
>  #include <grub/time.h>
> +#include <grub/env.h>
> +
> +#define RETRY_DEFAULT_TIMEOUT 15000
>  
>  static char *last_devpath;
>  static grub_ieee1275_ihandle_t last_ihandle;
> @@ -452,7 +455,7 @@ compute_dev_path (const char *name)
>  }
>  
>  static grub_err_t
> -grub_ofdisk_open (const char *name, grub_disk_t disk)
> +grub_ofdisk_open_real (const char *name, grub_disk_t disk)
>  {
>    grub_ieee1275_phandle_t dev;
>    char *devpath;
> @@ -525,6 +528,41 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
>    return 0;
>  }
>  
> +static grub_uint64_t
> +grub_ofdisk_disk_timeout(void)
> +{
> +   if(grub_env_get("ofdisk_retry_timeout") != NULL)
> +     {
> +     grub_uint64_t retry = 
> grub_strtoul(grub_env_get("ofdisk_retry_timeout"), 0, 10);
> +     if(retry)
> +       return retry;
> +     }
> +
> +   return RETRY_DEFAULT_TIMEOUT;
> +}
> +
> +static grub_err_t
> +grub_ofdisk_open (const char *name, grub_disk_t disk)
> +{
> +  grub_err_t err;
> +  grub_uint64_t timeout = grub_get_time_ms () + grub_ofdisk_disk_timeout();
> +
> + retry:
> +  err = grub_ofdisk_open_real (name, disk);
> +
> +  if (err == GRUB_ERR_UNKNOWN_DEVICE)
> +    {
> +      if (grub_get_time_ms () < timeout)
> +        {
> +          grub_dprintf ("ofdisk","Failed to open disk %s. Retrying...\n", 
> name);
> +          grub_errno = GRUB_ERR_NONE;
> +          goto retry;
> +     }
> +    }
> +
> +  return err;
> +}
> +
>  static void
>  grub_ofdisk_close (grub_disk_t disk)
>  {
> @@ -568,7 +606,7 @@ grub_ofdisk_prepare (grub_disk_t disk, grub_disk_addr_t 
> sector)
>  }
>  
>  static grub_err_t
> -grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
> +grub_ofdisk_read_real (grub_disk_t disk, grub_disk_addr_t sector,
>                 grub_size_t size, char *buf)
>  {
>    grub_err_t err;
> @@ -587,6 +625,29 @@ grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t 
> sector,
>    return 0;
>  }
>  
> +static grub_err_t
> +grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
> +               grub_size_t size, char *buf)
> +{
> +  grub_err_t err;
> +  grub_uint64_t timeout = grub_get_time_ms () + grub_ofdisk_disk_timeout();
> +
> + retry:
> +  err = grub_ofdisk_read_real (disk, sector, size, buf);
> +
> +  if (err == GRUB_ERR_READ_ERROR)
> +    {
> +      if (grub_get_time_ms () < timeout)
> +        {
> +          grub_dprintf ("ofdisk","Failed to read disk %s. Retrying...\n", 
> (char*)disk->data);
> +          grub_errno = GRUB_ERR_NONE;
> +          goto retry;
> +     }
> +    }
> +
> +  return err;
> +}
> +
>  static grub_err_t
>  grub_ofdisk_write (grub_disk_t disk, grub_disk_addr_t sector,
>                  grub_size_t size, const char *buf)
> -- 
> 2.31.1
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel