Skip to content

Commit

Permalink
libmultipath: don't set max_sectors_kb on reloads
Browse files Browse the repository at this point in the history
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@redhat.com>
  • Loading branch information
bmarzins authored and cvaroqui committed Apr 12, 2017
1 parent e6811f3 commit 8fd4868
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 26 deletions.
60 changes: 58 additions & 2 deletions libmultipath/configure.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;

Expand All @@ -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:
Expand Down
23 changes: 0 additions & 23 deletions libmultipath/discovery.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion libmultipath/discovery.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 8fd4868

Please sign in to comment.