[PATCH] hostdisk: Fix linux disk cache workaround on multipath disks

[PATCH] hostdisk: Fix linux disk cache workaround on multipath disks

[Michael Chang]

In hostdisk.c::grub_util_fd_open_device, there's a workaround to linux disk
cache described below.

"Linux has a bug that the disk cache for a whole disk is not consistent with
the one for a partition of the disk."

The workaround will result in using the partition device for writing the image
of which the address offset is calculated to be within it's range, to avoid the
cache problem of the whole disk device.

This patch fixed the linux disk cache workaround not being effective for
multipath/dm device because its partition device cannot be correctly determined
by grub_hostdisk_linux_find_partition function.

---
 grub-core/osdep/linux/hostdisk.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/grub-core/osdep/linux/hostdisk.c b/grub-core/osdep/linux/hostdisk.c
index 06179fca7..ed530bdc4 100644
--- a/grub-core/osdep/linux/hostdisk.c
+++ b/grub-core/osdep/linux/hostdisk.c
@@ -263,6 +263,12 @@ grub_hostdisk_linux_find_partition (char *dev, grub_disk_addr_t sector)
       p = real_dev + len;
       format = "-part%d";
     }
+  else if (strncmp (real_dev, "/dev/dm-",
+		    sizeof ("/dev/dm-") - 1) == 0)
+    {
+      p = real_dev + len - 1;
+      format = "%d";
+    }
   else if (real_dev[len - 1] >= '0' && real_dev[len - 1] <= '9')
     {
       p = real_dev + len;
-- 
2.13.6

Re: [PATCH] hostdisk: Fix linux disk cache workaround on multipath disks

[Daniel Kiper]

On Wed, Jul 25, 2018 at 04:49:15PM +0800, Michael Chang wrote:
> In hostdisk.c::grub_util_fd_open_device, there's a workaround to linux disk

s#hostdisk.c::grub_util_fd_open_device#grub-core/osdep/linux/hostdisk.c:grub_util_fd_open_device()#

> cache described below.
>
> "Linux has a bug that the disk cache for a whole disk is not consistent with
> the one for a partition of the disk."
>
> The workaround will result in using the partition device for writing the image

Which workaround are you referring to?

> of which the address offset is calculated to be within it's range, to avoid the
> cache problem of the whole disk device.

This sentence is convoluted. Please fix this.

> This patch fixed the linux disk cache workaround not being effective for

s/fixed/fixes/

> multipath/dm device because its partition device cannot be correctly determined
> by grub_hostdisk_linux_find_partition function.

In general I am not happy with the commit message. It is difficult to
understand where the problem is and why and how it is fixed.

And lack of SOB...

Daniel

Re: [PATCH] hostdisk: Fix linux disk cache workaround on multipath disks

[Michael Chang]

On Tue, Sep 04, 2018 at 04:46:31PM +0200, Daniel Kiper wrote:
> On Wed, Jul 25, 2018 at 04:49:15PM +0800, Michael Chang wrote:
> > In hostdisk.c::grub_util_fd_open_device, there's a workaround to linux disk
> 
> s#hostdisk.c::grub_util_fd_open_device#grub-core/osdep/linux/hostdisk.c:grub_util_fd_open_device()#

OK.

> 
> > cache described below.
> >
> > "Linux has a bug that the disk cache for a whole disk is not consistent with
> > the one for a partition of the disk."
> >
> > The workaround will result in using the partition device for writing the image
> 
> Which workaround are you referring to?

There's a comment in
grub-core/osdep/linux/hostdisk.c::grub_util_fd_open_device() that stats "Linux
has a bug ..." that we cannot use whole disk offset writing into a partition
device since it does not reliably work that the data may be lost.

  /* Linux has a bug that the disk cache for a whole disk is not consistent
     with the one for a partition of the disk.  */
  {
    ....
  } 

And as the input argument of grub_util_fd_open_device() is using address in
unit of sector size offset from the "disk", and in a bid to avoid Linux disk
cache inconsistency problem here grub translates the address again into the
address offset from partition that have enclosed it and then use that partition
device instead of disk device. 

> 
> > of which the address offset is calculated to be within it's range, to avoid the
> > cache problem of the whole disk device.
> 
> This sentence is convoluted. Please fix this.



> 
> > This patch fixed the linux disk cache workaround not being effective for
> 
> s/fixed/fixes/

OK.

> 
> > multipath/dm device because its partition device cannot be correctly determined
> > by grub_hostdisk_linux_find_partition function.

The grub_hostdisk_linux_find_partition() does the trick for finding the
partition device for a given partition start address and will be used in place
of whole disk device.

> 
> In general I am not happy with the commit message. It is difficult to
> understand where the problem is and why and how it is fixed.

The problem we encountered was that installing grub into multipath disk
partition didn't work reliably. And after debugging it boiled down to the disk
cache problem described above as strace result shown it was using whole disk
device. We then took another step forward in finding out the cause was missing
"/dev/dm-" name scheme handling in grub_hostdisk_linux_find_partition(). After
applying the fix the problem got fixed and we would like to have this fixing
patch upstreamed as it looks good material to be.

> 
> And lack of SOB...

Sorry. I will add SOB in later version.

Thanks,
Michael

> 
> Daniel
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel