Project

General

Profile

Bug #11069

disk topo enumerator miscalculates disk capacity

Added by Rob Johnston 5 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Start date:
2019-05-24
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:

Description

The disk topo enumerator module creates a property on disk nodes which represents the total capacity of the drive in bytes.

While working on an unrelated project, I noticed that the reported capacity for some newer drives was incorrect. For example, the following capacity was being reported for a 10TB HGST hard drive:

hc://:product-id=SMC-SC846P:server-id=:chassis-id=500304801861347f:serial=8DGGWD8H:part=HGST-HUH721212AL4200:revision=A3D0/ses-enclosure=0/bay=0/disk=0
    capacity-in-bytes string    1500017328128

That number (1500017328128) is an order of magitude off. It should have been 12000138625024.

I took a look at how the module was computing the capacity and found that it was multiplying the values of the "device-nblocks" and "device-blksize" devinfo properties. That makes sense.

However, on closer inspection, the problem is in the code for looking up the value of the "device-blksize" devinfo property:

        /*
         * To save kernel memory, the driver may not define
         * "device-dblksize" when its value is default DEV_BSIZE.
         */
        if (di_prop_lookup_ints(DDI_DEV_T_ANY, node,
            "device-dblksize", &dblksizep) > 0)
            dblksize = (uint_t)*dblksizep;
        else
            dblksize = DEV_BSIZE;        /* default value */

If it fails to lookup the block size, it falls back to the default value of DEV_BSIZE (512). And, as it turns out, this lookup always fails because the code is looking for "device-dblksize", but the actual devinfo property is "device-blksize" - as shown in the excerpt below from the cmlb kernel module (which contains the common code for working with disk labels)
/*
 * Declare the dynamic properties implemented in prop_op(9E) implementation
 * that we want to have show up in a di_init(3DEVINFO) device tree snapshot
 * of drivers that call cmlb_attach().
 */
static i_ddi_prop_dyn_t cmlb_prop_dyn[] = {
    {"Nblocks",        DDI_PROP_TYPE_INT64,    S_IFBLK},
    {"Size",        DDI_PROP_TYPE_INT64,    S_IFCHR},
    {"device-nblocks",    DDI_PROP_TYPE_INT64},
    {"device-blksize",    DDI_PROP_TYPE_INT},
    {"device-solid-state",    DDI_PROP_TYPE_INT},
    {"device-rotational",    DDI_PROP_TYPE_INT},
    {NULL}
};

On this 10TB disk, that makes a big difference as its block size is actually 4096 bytes (not 512). Correcting the above code causes the disk module to compute the correct capacity:
hc://:product-id=SMC-SC846P:server-id=:chassis-id=500304801861347f:serial=8DGGWD8H:part=HGST-HUH721212AL4200:
revision=A3D0/ses-enclosure=0/bay=0/disk=0
    capacity-in-bytes string    12000138625024

History

#1

Updated by Rob Johnston 5 months ago

Note that this issue has already been fixed in illumos-joyent via the commit below:

commit ab6a47af7ee9daefa937f7e8ca0531e68d003686
Author: Rob Johnston <rob.johnston@joyent.com>
Date:   Thu May 23 21:02:59 2019 +0000

    OS-7808 disk topo enumerator miscalculates disk capacity
    Reviewed by: Jordan Hendricks <jordan.hendricks@joyent.com>
    Approved by: Jason King <jason.king@joyent.com>

So this issue is simply to track getting the above change pushed upstream to master.

#2

Updated by Rob Johnston 5 months ago

See the original SmartOS bug for testing notes:

https://smartos.org/bugview/OS-7808

#3

Updated by Electric Monk 5 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 4a97b82e5d7af90029b0dc4e822c3b0b5e9bfa71

commit  4a97b82e5d7af90029b0dc4e822c3b0b5e9bfa71
Author: Rob Johnston <rob.johnston@joyent.com>
Date:   2019-05-28T23:35:45.000Z

    11069 disk topo enumerator miscalculates disk capacity
    Reviewed by: Jordan Hendricks <jordan.hendricks@joyent.com>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Reviewed by: Igor Kozhukhov <igor@dilos.org>
    Reviewed by: Gergő Doma <domag02@gmail.com>
    Approved by: Gordon Ross <gwr@nexenta.com>

Also available in: Atom PDF