Multipath was setting max_sectors_kb on the multipath device and all its path devices both when the device was created, and when it was reloaded. The problem with this is that while this would set max_sectors_kb on all the devices under multipath, it couldn't set this on devices on top of multipath. This meant that if a user lowered max_sectors_kb on an already existing multipath device with a LV on top of it, the LV could send down IO that was too large for the new max_sectors_kb value, because the LV was still using the old value. The solution to this is to only set max_sectors_kb to the configured value when the device is originally created, not when it is later reloaded. Since not all paths may be present when the device is original created, on reloads multipath still needs to make sure that the max_sectors_kb value on all the path devices is the same as the value of the multipath device. But if this value doesn't match the configuration value, that's o.k. This means that the max_sectors_kb value for a multipath device won't change after it have been initially created. All of the devices created on top of the multipath device will inherit that value, and all of the devices will use it all the way down, so IOs will never be mis-sized. I also moved sysfs_set_max_sectors_kb to configure.c, since it is only called from there, and it it makes use of static functions from there. Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> --- libmultipath/configure.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++-- libmultipath/discovery.c | 23 ------------------- libmultipath/discovery.h | 1 - 3 files changed, 58 insertions(+), 26 deletions(-) diff --git a/libmultipath/configure.c b/libmultipath/configure.c index d955455..3eb1b7a 100644 --- a/libmultipath/configure.c +++ b/libmultipath/configure.c @@ -302,7 +302,6 @@ int setup_map(struct multipath *mpp, char *params, int params_size) select_max_sectors_kb(conf, mpp); sysfs_set_scsi_tmo(mpp, conf->checkint); - sysfs_set_max_sectors_kb(mpp); put_multipath_config(conf); /* * assign paths to path groups -- start with no groups and all paths @@ -432,6 +431,58 @@ is_mpp_known_to_udev(const struct multipath *mpp) return ret; } +static int +sysfs_set_max_sectors_kb(struct multipath *mpp, int is_reload) +{ + struct pathgroup * pgp; + struct path *pp; + char buff[11]; + int i, j, ret, err = 0; + struct udev_device *udd; + int max_sectors_kb; + + if (mpp->max_sectors_kb == MAX_SECTORS_KB_UNDEF) + return 0; + max_sectors_kb = mpp->max_sectors_kb; + if (is_reload) { + if (!mpp->dmi && dm_get_info(mpp->alias, &mpp->dmi) != 0) { + condlog(1, "failed to get dm info for %s", mpp->alias); + return 1; + } + udd = get_udev_for_mpp(mpp); + if (!udd) { + condlog(1, "failed to get udev device to set max_sectors_kb for %s", mpp->alias); + return 1; + } + ret = sysfs_attr_get_value(udd, "queue/max_sectors_kb", buff, + sizeof(buff)); + udev_device_unref(udd); + if (ret <= 0) { + condlog(1, "failed to get current max_sectors_kb from %s", mpp->alias); + return 1; + } + if (sscanf(buff, "%u\n", &max_sectors_kb) != 1) { + condlog(1, "can't parse current max_sectors_kb from %s", + mpp->alias); + return 1; + } + } + snprintf(buff, 11, "%d", max_sectors_kb); + + vector_foreach_slot (mpp->pg, pgp, i) { + vector_foreach_slot(pgp->paths, pp, j) { + ret = sysfs_attr_set_value(pp->udev, + "queue/max_sectors_kb", + buff, strlen(buff)); + if (ret < 0) { + condlog(1, "failed setting max_sectors_kb on %s : %s", pp->dev, strerror(-ret)); + err = 1; + } + } + } + return err; +} + static void select_action (struct multipath * mpp, vector curmp, int force_reload) { @@ -703,16 +754,19 @@ int domap(struct multipath *mpp, char *params, int is_daemon) return DOMAP_RETRY; } + sysfs_set_max_sectors_kb(mpp, 0); r = dm_addmap_create(mpp, params); lock_multipath(mpp, 0); break; case ACT_RELOAD: + sysfs_set_max_sectors_kb(mpp, 1); r = dm_addmap_reload(mpp, params, 0); break; case ACT_RESIZE: + sysfs_set_max_sectors_kb(mpp, 1); r = dm_addmap_reload(mpp, params, 1); break; @@ -728,8 +782,10 @@ int domap(struct multipath *mpp, char *params, int is_daemon) r = dm_rename(mpp->alias_old, mpp->alias, conf->partition_delim, mpp->skip_kpartx); put_multipath_config(conf); - if (r) + if (r) { + sysfs_set_max_sectors_kb(mpp, 1); r = dm_addmap_reload(mpp, params, 0); + } break; default: diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c index bfb46b1..8c51254 100644 --- a/libmultipath/discovery.c +++ b/libmultipath/discovery.c @@ -214,29 +214,6 @@ declare_sysfs_get_str(vendor); declare_sysfs_get_str(model); declare_sysfs_get_str(rev); -int -sysfs_set_max_sectors_kb(struct multipath *mpp) -{ - struct path *pp; - char buff[11]; - int i, ret, err = 0; - - if (mpp->max_sectors_kb == MAX_SECTORS_KB_UNDEF) - return 0; - snprintf(buff, 11, "%d", mpp->max_sectors_kb); - - vector_foreach_slot(mpp->paths, pp, i) { - ret = sysfs_attr_set_value(pp->udev, "queue/max_sectors_kb", - buff, strlen(buff)); - if (ret < 0) { - condlog(0, "failed setting max_sectors_kb on %s : %s", - pp->dev, strerror(-ret)); - err = 1; - } - } - return err; -} - ssize_t sysfs_get_vpd (struct udev_device * udev, int pg, unsigned char * buff, size_t len) diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h index d16a69a..0563bfd 100644 --- a/libmultipath/discovery.h +++ b/libmultipath/discovery.h @@ -50,7 +50,6 @@ ssize_t sysfs_get_vpd (struct udev_device * udev, int pg, unsigned char * buff, int sysfs_get_asymmetric_access_state(struct path *pp, char *buff, int buflen); int get_uid(struct path * pp, int path_state, struct udev_device *udev); -int sysfs_set_max_sectors_kb(struct multipath *mpp); /* * discovery bitmask -- 1.8.3.1 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel