From patchwork Wed Apr 27 11:10:57 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hannes Reinecke X-Patchwork-Id: 8955531 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.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 0400D9F6CD for ; Wed, 27 Apr 2016 11:15:32 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B8118202EB for ; Wed, 27 Apr 2016 11:15:25 +0000 (UTC) Received: from mx5-phx2.redhat.com (mx5-phx2.redhat.com [209.132.183.37]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D84E720263 for ; Wed, 27 Apr 2016 11:15:23 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx5-phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3RBC4QS012478; Wed, 27 Apr 2016 07:12:04 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u3RBBO94012546 for ; Wed, 27 Apr 2016 07:11:24 -0400 Received: from mx1.redhat.com (ext-mx04.extmail.prod.ext.phx2.redhat.com [10.5.110.28]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3RBBOAw031534 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 27 Apr 2016 07:11:24 -0400 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 601E585541; Wed, 27 Apr 2016 11:11:23 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 2E897ADEF; Wed, 27 Apr 2016 11:11:11 +0000 (UTC) From: Hannes Reinecke To: Christophe Varoqui Date: Wed, 27 Apr 2016 13:10:57 +0200 Message-Id: <1461755458-29225-57-git-send-email-hare@suse.de> In-Reply-To: <1461755458-29225-1-git-send-email-hare@suse.de> References: <1461755458-29225-1-git-send-email-hare@suse.de> X-RedHat-Spam-Score: -0.607 (BAYES_50, DCC_REPUT_00_12, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS) 195.135.220.15 mx2.suse.de 195.135.220.15 mx2.suse.de X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Scanned-By: MIMEDefang 2.75 on 10.5.110.28 X-loop: dm-devel@redhat.com Cc: dm-devel@redhat.com, Mike Snitzer Subject: [dm-devel] [PATCH 56/57] multipathd: push down lock in checkerloop() 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: , MIME-Version: 1.0 Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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 Instead of grabbing the lock at the start of the checkerloop and releasing it at the end we should be holding it only during the time when we actually need it. Signed-off-by: Hannes Reinecke --- multipathd/main.c | 136 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 87 insertions(+), 49 deletions(-) diff --git a/multipathd/main.c b/multipathd/main.c index 41b5a49..132101f 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -416,7 +416,11 @@ uev_add_map (struct uevent * uev, struct vectors * vecs) return 1; } } + pthread_cleanup_push(cleanup_lock, &vecs->lock); + lock(vecs->lock); + pthread_testcancel(); rc = ev_add_map(uev->kernel, alias, vecs); + lock_cleanup_pop(vecs->lock); FREE(alias); return rc; } @@ -512,6 +516,10 @@ uev_remove_map (struct uevent * uev, struct vectors * vecs) return 0; } minor = uevent_get_minor(uev); + + pthread_cleanup_push(cleanup_lock, &vecs->lock); + lock(vecs->lock); + pthread_testcancel(); mpp = find_mp_by_minor(vecs->mpvec, minor); if (!mpp) { @@ -528,10 +536,12 @@ uev_remove_map (struct uevent * uev, struct vectors * vecs) orphan_paths(vecs->pathvec, mpp); remove_map_and_stop_waiter(mpp, vecs, 1); out: + lock_cleanup_pop(vecs->lock); FREE(alias); return 0; } +/* Called from CLI handler */ int ev_remove_map (char * devname, char * alias, int minor, struct vectors * vecs) { @@ -567,6 +577,9 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) return 1; } + pthread_cleanup_push(cleanup_lock, &vecs->lock); + lock(vecs->lock); + pthread_testcancel(); pp = find_path_by_dev(vecs->pathvec, uev->kernel); if (pp) { int r; @@ -594,8 +607,10 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) ret = 1; } } - return ret; } + lock_cleanup_pop(vecs->lock); + if (pp) + return ret; /* * get path vital state @@ -608,6 +623,9 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) condlog(3, "%s: failed to get path info", uev->kernel); return 1; } + pthread_cleanup_push(cleanup_lock, &vecs->lock); + lock(vecs->lock); + pthread_testcancel(); ret = store_path(vecs->pathvec, pp); if (!ret) { pp->checkint = conf->checkint; @@ -619,7 +637,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) free_path(pp); ret = 1; } - + lock_cleanup_pop(vecs->lock); return ret; } @@ -687,12 +705,12 @@ rescan: */ start_waiter = 1; } - else + if (!start_waiter) goto fail; /* leave path added to pathvec */ } - /* persistent reseravtion check*/ - mpath_pr_event_handle(pp); + /* persistent reservation check*/ + mpath_pr_event_handle(pp); /* * push the map to the device-mapper @@ -720,7 +738,7 @@ retry: * deal with asynchronous uevents :(( */ if (mpp->action == ACT_RELOAD && retries-- > 0) { - condlog(0, "%s: uev_add_path sleep", mpp->alias); + condlog(0, "%s: ev_add_path sleep", mpp->alias); sleep(1); update_mpp_paths(mpp, vecs->pathvec); goto rescan; @@ -749,8 +767,7 @@ retry: condlog(2, "%s [%s]: path added to devmap %s", pp->dev, pp->dev_t, mpp->alias); return 0; - } - else + } else goto fail; fail_map: @@ -764,17 +781,22 @@ static int uev_remove_path (struct uevent *uev, struct vectors * vecs) { struct path *pp; + int ret; condlog(2, "%s: remove path (uevent)", uev->kernel); + pthread_cleanup_push(cleanup_lock, &vecs->lock); + lock(vecs->lock); + pthread_testcancel(); pp = find_path_by_dev(vecs->pathvec, uev->kernel); - + if (pp) + ret = ev_remove_path(pp, vecs); + lock_cleanup_pop(vecs->lock); if (!pp) { /* Not an error; path might have been purged earlier */ condlog(0, "%s: path already removed", uev->kernel); return 0; } - - return ev_remove_path(pp, vecs); + return ret; } int @@ -877,35 +899,50 @@ static int uev_update_path (struct uevent *uev, struct vectors * vecs) { int ro, retval = 0; - struct path * pp; - - pp = find_path_by_dev(vecs->pathvec, uev->kernel); - if (!pp) { - condlog(0, "%s: spurious uevent, path not found", - uev->kernel); - return 1; - } - - if (pp->initialized == INIT_REQUESTED_UDEV) - return uev_add_path(uev, vecs); ro = uevent_get_disk_ro(uev); if (ro >= 0) { + struct path * pp; + struct multipath *mpp = NULL; + condlog(2, "%s: update path write_protect to '%d' (uevent)", uev->kernel, ro); - if (pp->mpp) { - if (pp->mpp->wait_for_udev) { - pp->mpp->wait_for_udev = 2; - return 0; + pthread_cleanup_push(cleanup_lock, &vecs->lock); + lock(vecs->lock); + pthread_testcancel(); + /* + * pthread_mutex_lock() and pthread_mutex_unlock() + * need to be at the same indentation level, hence + * this slightly convoluted codepath. + */ + pp = find_path_by_dev(vecs->pathvec, uev->kernel); + if (pp) { + if (pp->initialized == INIT_REQUESTED_UDEV) { + retval = 2; + } else { + mpp = pp->mpp; + if (mpp && mpp->wait_for_udev) { + mpp->wait_for_udev = 2; + mpp = NULL; + retval = 0; + } } + if (mpp) { + retval = reload_map(vecs, mpp, 0); - retval = reload_map(vecs, pp->mpp, 0); - - condlog(2, "%s: map %s reloaded (retval %d)", - uev->kernel, pp->mpp->alias, retval); + condlog(2, "%s: map %s reloaded (retval %d)", + uev->kernel, mpp->alias, retval); + } } - + lock_cleanup_pop(vecs->lock); + if (!pp) { + condlog(0, "%s: spurious uevent, path not found", + uev->kernel); + return 1; + } + if (retval == 2) + return uev_add_path(uev, vecs); } return retval; @@ -1002,10 +1039,6 @@ uev_trigger (struct uevent * uev, void * trigger_data) if (running_state == DAEMON_SHUTDOWN) return 0; - pthread_cleanup_push(cleanup_lock, &vecs->lock); - lock(vecs->lock); - pthread_testcancel(); - /* * device map event * Add events are ignored here as the tables @@ -1044,7 +1077,6 @@ uev_trigger (struct uevent * uev, void * trigger_data) } out: - lock_cleanup_pop(vecs->lock); return r; } @@ -1627,17 +1659,6 @@ checkerloop (void *ap) if (gettimeofday(&start_time, NULL) != 0) start_time.tv_sec = 0; - - rc = set_config_state(DAEMON_RUNNING); - if (rc == ETIMEDOUT) { - condlog(4, "timeout waiting for DAEMON_IDLE"); - continue; - } - - pthread_cleanup_push(cleanup_lock, &vecs->lock); - lock(vecs->lock); - pthread_testcancel(); - strict_timing = conf->strict_timing; if (start_time.tv_sec && last_time.tv_sec) { timersub(&start_time, &last_time, &diff_time); condlog(4, "tick (%lu.%06lu secs)", @@ -1653,28 +1674,45 @@ checkerloop (void *ap) if (use_watchdog) sd_notify(0, "WATCHDOG=1"); #endif + rc = set_config_state(DAEMON_RUNNING); + if (rc == ETIMEDOUT) { + condlog(4, "timeout waiting for DAEMON_IDLE"); + continue; + } + strict_timing = conf->strict_timing; if (vecs->pathvec) { + pthread_cleanup_push(cleanup_lock, &vecs->lock); + lock(vecs->lock); + pthread_testcancel(); vector_foreach_slot (vecs->pathvec, pp, i) { num_paths += check_path(vecs, pp, ticks); } + lock_cleanup_pop(vecs->lock); } if (vecs->mpvec) { + pthread_cleanup_push(cleanup_lock, &vecs->lock); + lock(vecs->lock); + pthread_testcancel(); defered_failback_tick(vecs->mpvec); retry_count_tick(vecs->mpvec); missing_uev_wait_tick(vecs); + lock_cleanup_pop(vecs->lock); } if (count) count--; else { + pthread_cleanup_push(cleanup_lock, &vecs->lock); + lock(vecs->lock); + pthread_testcancel(); condlog(4, "map garbage collection"); mpvec_garbage_collector(vecs); count = MAPGCINT; + lock_cleanup_pop(vecs->lock); } - lock_cleanup_pop(vecs->lock); diff_time.tv_usec = 0; if (start_time.tv_sec && - gettimeofday(&end_time, NULL)) { + gettimeofday(&end_time, NULL) == 0) { timersub(&end_time, &start_time, &diff_time); if (num_paths) { condlog(3, "checked %d path%s in %lu.%06lu secs",