From patchwork Thu Jul 25 21:32:13 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Marzinski X-Patchwork-Id: 2833686 Return-Path: X-Original-To: patchwork-dm-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 975A19F4D4 for ; Thu, 25 Jul 2013 21:37:00 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9F14820365 for ; Thu, 25 Jul 2013 21:36:59 +0000 (UTC) Received: from mx3-phx2.redhat.com (mx3-phx2.redhat.com [209.132.183.24]) by mail.kernel.org (Postfix) with ESMTP id 6E5E820363 for ; Thu, 25 Jul 2013 21:36:58 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx3-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r6PLWraI030752; Thu, 25 Jul 2013 17:32:56 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r6PLWqFC003339 for ; Thu, 25 Jul 2013 17:32:52 -0400 Received: from dhcp80-209.msp.redhat.com (octiron.msp.redhat.com [10.15.80.209]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r6PLWpUF018890 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 25 Jul 2013 17:32:51 -0400 Received: from dhcp80-209.msp.redhat.com (localhost [127.0.0.1]) by dhcp80-209.msp.redhat.com (8.14.7/8.14.7) with ESMTP id r6PLWEH1020848; Thu, 25 Jul 2013 16:32:14 -0500 Received: (from bmarzins@localhost) by dhcp80-209.msp.redhat.com (8.14.7/8.14.7/Submit) id r6PLWDQo020847; Thu, 25 Jul 2013 16:32:13 -0500 Date: Thu, 25 Jul 2013 16:32:13 -0500 From: Benjamin Marzinski To: device-mapper development Message-ID: <20130725213212.GH2113@dhcp80-209.msp.redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 X-loop: dm-devel@redhat.com Cc: Christophe Varoqui Subject: [dm-devel] [PATCH] multipath: rport tmo cleanup X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk Reply-To: device-mapper development List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The current code to set fast_io_fail_tmo and dev_loss_tmo fails if you want to set fast_io_fail_tmo from "off" to 600. Also, it often unnecessarily sets dev_loss_tmo to itself before setting it to the final value. This patch cleans up these issues. Signed-off-by: Benjamin Marzinski --- libmultipath/discovery.c | 79 ++++++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 37 deletions(-) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel Index: multipath-tools-130724/libmultipath/discovery.c =================================================================== --- multipath-tools-130724.orig/libmultipath/discovery.c +++ multipath-tools-130724/libmultipath/discovery.c @@ -322,9 +322,12 @@ sysfs_set_rport_tmo(struct multipath *mp struct udev_device *rport_dev = NULL; char value[16]; char rport_id[32]; - unsigned long long tmo = 0; + int delay_fast_io_fail = 0; + int current_dev_loss = 0; int ret; + if (!mpp->dev_loss && mpp->fast_io_fail == MP_FAST_IO_FAIL_UNSET) + return; sprintf(rport_id, "rport-%d:%d-%d", pp->sg_id.host_no, pp->sg_id.channel, pp->sg_id.transport_id); rport_dev = udev_device_new_from_subsystem_sysname(conf->udev, @@ -337,22 +340,8 @@ sysfs_set_rport_tmo(struct multipath *mp condlog(4, "target%d:%d:%d -> %s", pp->sg_id.host_no, pp->sg_id.channel, pp->sg_id.scsi_id, rport_id); - /* - * This is tricky. - * dev_loss_tmo will be limited to 600 if fast_io_fail - * is _not_ set. - * fast_io_fail will be limited by the current dev_loss_tmo - * setting. - * So to get everything right we first need to increase - * dev_loss_tmo to the fast_io_fail setting (if present), - * then set fast_io_fail, and _then_ set dev_loss_tmo - * to the correct value. - */ memset(value, 0, 16); - if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET && - mpp->fast_io_fail != MP_FAST_IO_FAIL_ZERO && - mpp->fast_io_fail != MP_FAST_IO_FAIL_OFF) { - /* Check if we need to temporarily increase dev_loss_tmo */ + if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET) { ret = sysfs_attr_get_value(rport_dev, "dev_loss_tmo", value, 16); if (ret <= 0) { @@ -360,38 +349,40 @@ sysfs_set_rport_tmo(struct multipath *mp "error %d", rport_id, -ret); goto out; } - if (sscanf(value, "%llu\n", &tmo) != 1) { + if (sscanf(value, "%u\n", ¤t_dev_loss) != 1) { condlog(0, "%s: Cannot parse dev_loss_tmo " "attribute '%s'", rport_id, value); goto out; } - if (mpp->fast_io_fail >= tmo) { - snprintf(value, 16, "%u", mpp->fast_io_fail + 1); - } - } else if (mpp->dev_loss > 600) { - condlog(3, "%s: limiting dev_loss_tmo to 600, since " - "fast_io_fail is not set", rport_id); - snprintf(value, 16, "%u", 600); - } else { - snprintf(value, 16, "%u", mpp->dev_loss); - } - if (strlen(value)) { - ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo", - value, strlen(value)); - if (ret <= 0) { - if (ret == -EBUSY) - condlog(3, "%s: rport blocked", rport_id); + if ((mpp->dev_loss && + mpp->fast_io_fail >= (int)mpp->dev_loss) || + (!mpp->dev_loss && + mpp->fast_io_fail >= (int)current_dev_loss)) { + condlog(3, "%s: limiting fast_io_fail_tmo to %d, since " + "it must be less than dev_loss_tmo", + rport_id, mpp->dev_loss - 1); + if (mpp->dev_loss) + mpp->fast_io_fail = mpp->dev_loss - 1; else - condlog(0, "%s: failed to set dev_loss_tmo to %s, error %d", - rport_id, value, -ret); - goto out; + mpp->fast_io_fail = current_dev_loss - 1; } + if (mpp->fast_io_fail >= (int)current_dev_loss) + delay_fast_io_fail = 1; + } + if (mpp->dev_loss > 600 && + (mpp->fast_io_fail == MP_FAST_IO_FAIL_OFF || + mpp->fast_io_fail == MP_FAST_IO_FAIL_UNSET)) { + condlog(3, "%s: limiting dev_loss_tmo to 600, since " + "fast_io_fail is unset or off", rport_id); + mpp->dev_loss = 600; } if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET) { if (mpp->fast_io_fail == MP_FAST_IO_FAIL_OFF) sprintf(value, "off"); else if (mpp->fast_io_fail == MP_FAST_IO_FAIL_ZERO) sprintf(value, "0"); + else if (delay_fast_io_fail) + snprintf(value, 16, "%u", current_dev_loss - 1); else snprintf(value, 16, "%u", mpp->fast_io_fail); ret = sysfs_attr_set_value(rport_dev, "fast_io_fail_tmo", @@ -402,9 +393,10 @@ sysfs_set_rport_tmo(struct multipath *mp else condlog(0, "%s: failed to set fast_io_fail_tmo to %s, error %d", rport_id, value, -ret); + goto out; } } - if (tmo > 0) { + if (mpp->dev_loss) { snprintf(value, 16, "%u", mpp->dev_loss); ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, strlen(value)); @@ -414,6 +406,19 @@ sysfs_set_rport_tmo(struct multipath *mp else condlog(0, "%s: failed to set dev_loss_tmo to %s, error %d", rport_id, value, -ret); + goto out; + } + } + if (delay_fast_io_fail) { + snprintf(value, 16, "%u", mpp->fast_io_fail); + ret = sysfs_attr_set_value(rport_dev, "fast_io_fail_tmo", + value, strlen(value)); + if (ret <= 0) { + if (ret == -EBUSY) + condlog(3, "%s: rport blocked", rport_id); + else + condlog(0, "%s: failed to set fast_io_fail_tmo to %s, error %d", + rport_id, value, -ret); } } out: