From patchwork Fri Mar 2 13:59:40 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 10254703 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 265C760211 for ; Fri, 2 Mar 2018 14:02:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1367C284FF for ; Fri, 2 Mar 2018 14:02:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 080642899F; Fri, 2 Mar 2018 14:02:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 6E67E284FF for ; Fri, 2 Mar 2018 14:02:03 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3B415C04BE00; Fri, 2 Mar 2018 14:02:01 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A8D845D70C; Fri, 2 Mar 2018 14:01:59 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 9A5C54A46F; Fri, 2 Mar 2018 14:01:56 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w22E0ATC005537 for ; Fri, 2 Mar 2018 09:00:10 -0500 Received: by smtp.corp.redhat.com (Postfix) id B317B5D9CC; Fri, 2 Mar 2018 14:00:10 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from mx1.redhat.com (ext-mx08.extmail.prod.ext.phx2.redhat.com [10.5.110.32]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D520D5D965; Fri, 2 Mar 2018 14:00:00 +0000 (UTC) Received: from prv3-mh.provo.novell.com (prv3-mh.provo.novell.com [137.65.250.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 65126C0587E3; Fri, 2 Mar 2018 13:59:58 +0000 (UTC) Received: from apollon.mittagstun.de (prv-ext-foundry1int.gns.novell.com [137.65.251.240]) by prv3-mh.provo.novell.com with ESMTP (TLS encrypted); Fri, 02 Mar 2018 06:59:44 -0700 Message-ID: <1519999180.4169.60.camel@suse.com> From: Martin Wilck To: Benjamin Marzinski Date: Fri, 02 Mar 2018 14:59:40 +0100 In-Reply-To: <20180228234016.GI14513@octiron.msp.redhat.com> References: <20180220132658.22295-1-mwilck@suse.com> <20180220132658.22295-6-mwilck@suse.com> <20180228234016.GI14513@octiron.msp.redhat.com> Mime-Version: 1.0 X-Greylist: Sender passed SPF test, Sender IP whitelisted by DNSRBL, ACL 207 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 02 Mar 2018 13:59:59 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 02 Mar 2018 13:59:59 +0000 (UTC) for IP:'137.65.250.26' DOMAIN:'prv3-mh.provo.novell.com' HELO:'prv3-mh.provo.novell.com' FROM:'mwilck@suse.com' RCPT:'' X-RedHat-Spam-Score: -2.301 (RCVD_IN_DNSWL_MED, SPF_PASS) 137.65.250.26 prv3-mh.provo.novell.com 137.65.250.26 prv3-mh.provo.novell.com X-Scanned-By: MIMEDefang 2.78 on 10.5.110.32 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: dm-devel@redhat.com Cc: dm-devel@redhat.com Subject: Re: [dm-devel] [RFC PATCH 05/20] libmultipath: don't update path groups when printing X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk 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-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 02 Mar 2018 14:02:02 +0000 (UTC) X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 2018-02-28 at 17:40 -0600, Benjamin Marzinski wrote: > On Tue, Feb 20, 2018 at 02:26:43PM +0100, Martin Wilck wrote: > > Updating the prio values for printing makes no sense. The user > > wants to see > > the prio values multipath is actually using for path group > > selection, and > > updating the values here means actually lying to the user if the > > prio values > > have changed, but multipathd hasn't updated them internally. > > > > If we really don't update the pathgroup prios when we need to, this > > should be > > fixed elsewhere. The current wrong output would just hide that if > > it occured. > > > > Moreover, correctness forbids changing properties so deeply in a > > code path > > that's supposed to print them only. Finally, this piece of code > > prevents the > > print.c code to be converted to proper "const" usage. > > Well, it is true that we've only been updating the path group > priority > when we've needed it, and we've only need it to be uptodate when we > are > picking a new pathgroup, or are printing it out. When failback is set > to > "manual", we rarely are picking a new pathgroup, so we rarely update > the > pathgroup prio. > > [...] > > I'd be fine with simply updating the path group priority whever we > change a > path's priority, if we aren't updating it when printing it. The > bigger > work of actually making sure that the path group order it the table > is always uptodate needs to happen, but it doesn't need to happen in > this patchset. I just reviewed the code flow in check_path(), and here's what I see: 1. calls update_prio(pp, new_path_up) -> updates path's prio, or all paths' prios if the path was reinstated Now it calls either 2a. update_path_groups() (for group_by_prio and failback immediate) (-> reload_map() -> setup_map() -> select_path_group() -> path_group_prio_update()), or 2b. need_switch_pathgroup() (otherwise) So all we need to make sure is that need_switch_pathgroup() calls select_path_group(). It does that already, except for the "failback manual" case. So all we need is IMO the attached patch. Tell me what you think. [All of the above is only called if (!mpp->wait_for_udev), but if wait_for_udev is set, either when the event arrives or the wait times out, we'll call reconfigure() which makes sure all priorities are correct]. Best, Martin Reviewed-by: Benjamin Marzinski From 149f4458d138d504ee5947aaa6e38d134b21368a Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 2 Mar 2018 14:51:52 +0100 Subject: [PATCH] multipathd: update path group prio in check_path The previous patch "libmultipath: don't update path groups when printing" removed the call to path_group_prio_update() in the printing code path. To compensate for that, recalculate path group prio also when it's not strictly necessary (i.e. if failback "manual" is set). Signed-off-by: Martin Wilck --- multipathd/main.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/multipathd/main.c b/multipathd/main.c index e1d98861a841..61739ac6ea59 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -252,8 +252,9 @@ need_switch_pathgroup (struct multipath * mpp, int refresh) struct path * pp; unsigned int i, j; struct config *conf; + int bestpg; - if (!mpp || mpp->pgfailback == -FAILBACK_MANUAL) + if (!mpp) return 0; /* @@ -272,8 +273,11 @@ need_switch_pathgroup (struct multipath * mpp, int refresh) if (!mpp->pg || VECTOR_SIZE(mpp->paths) == 0) return 0; - mpp->bestpg = select_path_group(mpp); + bestpg = select_path_group(mpp); + if (mpp->pgfailback == -FAILBACK_MANUAL) + return 0; + mpp->bestpg = bestpg; if (mpp->bestpg != mpp->nextpg) return 1; -- 2.16.1