From patchwork Tue Mar 6 21:18:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 10262991 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 3824460211 for ; Tue, 6 Mar 2018 21:19:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2630B29082 for ; Tue, 6 Mar 2018 21:19:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1AA3A291A0; Tue, 6 Mar 2018 21:19:40 +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 3D0B329082 for ; Tue, 6 Mar 2018 21:19:38 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8CC107FDCA; Tue, 6 Mar 2018 21:19:37 +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 26191508E7; Tue, 6 Mar 2018 21:19:37 +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 A99934A46E; Tue, 6 Mar 2018 21:19:34 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w26LJWiN009196 for ; Tue, 6 Mar 2018 16:19:32 -0500 Received: by smtp.corp.redhat.com (Postfix) id D32CF60C89; Tue, 6 Mar 2018 21:19:32 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from mx1.redhat.com (ext-mx04.extmail.prod.ext.phx2.redhat.com [10.5.110.28]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0E00E60BE6; Tue, 6 Mar 2018 21:19:30 +0000 (UTC) Received: from smtp.nue.novell.com (smtp.nue.novell.com [195.135.221.5]) (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 698B77ECE6; Tue, 6 Mar 2018 21:19:27 +0000 (UTC) Received: from emea4-mta.ukb.novell.com ([10.120.13.87]) by smtp.nue.novell.com with ESMTP (TLS encrypted); Tue, 06 Mar 2018 22:19:25 +0100 Received: from apollon.suse.de.de (nwb-a10-snat.microfocus.com [10.120.13.202]) by emea4-mta.ukb.novell.com with ESMTP (TLS encrypted); Tue, 06 Mar 2018 21:19:04 +0000 From: Martin Wilck To: Christophe Varoqui , Guan Junxiong Date: Tue, 6 Mar 2018 22:18:42 +0100 Message-Id: <20180306211842.2556-2-mwilck@suse.com> In-Reply-To: <20180306211842.2556-1-mwilck@suse.com> References: <20180306211842.2556-1-mwilck@suse.com> 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.28]); Tue, 06 Mar 2018 21:19:28 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Tue, 06 Mar 2018 21:19:28 +0000 (UTC) for IP:'195.135.221.5' DOMAIN:'smtp.nue.novell.com' HELO:'smtp.nue.novell.com' FROM:'mwilck@suse.com' RCPT:'' X-RedHat-Spam-Score: -2.301 (RCVD_IN_DNSWL_MED, SPF_PASS) 195.135.221.5 smtp.nue.novell.com 195.135.221.5 smtp.nue.novell.com X-Scanned-By: MIMEDefang 2.78 on 10.5.110.28 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: dm-devel@redhat.com Cc: dm-devel@redhat.com, Martin Wilck Subject: [dm-devel] [PATCH v2 2/2] multipathd: start marginal path checker thread lazily 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-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 06 Mar 2018 21:19:38 +0000 (UTC) X-Virus-Scanned: ClamAV using ClamSMTP I noticed that the io_error checker thread accounts for most of the activity of multipathd even if the marginal path checking paramters are not set (which is still the default in most installations I assume). Therefore, start the io_error checker thread only if there's at least one map with marginal error path checking configured. Also, make sure the thread is really up when start_io_err_stat_thread() returns. This requires adding a "vecs" argument to setup_map, because vecs needs to be passed to the io_error checking code. Signed-off-by: Martin Wilck Reviewed-by: Hannes Reinecke --- libmultipath/configure.c | 14 +++++++++--- libmultipath/configure.h | 3 ++- libmultipath/io_err_stat.c | 55 ++++++++++++++++++++++++++++++++++++++++++---- libmultipath/structs_vec.c | 3 ++- multipathd/cli_handlers.c | 2 +- multipathd/main.c | 8 ++----- 6 files changed, 69 insertions(+), 16 deletions(-) diff --git a/libmultipath/configure.c b/libmultipath/configure.c index 42b7c896ee65..fa6e21cb31af 100644 --- a/libmultipath/configure.c +++ b/libmultipath/configure.c @@ -41,6 +41,7 @@ #include "uxsock.h" #include "wwids.h" #include "sysfs.h" +#include "io_err_stat.h" /* group paths in pg by host adapter */ @@ -255,7 +256,8 @@ int rr_optimize_path_order(struct pathgroup *pgp) return 0; } -int setup_map(struct multipath *mpp, char *params, int params_size) +int setup_map(struct multipath *mpp, char *params, int params_size, + struct vectors *vecs) { struct pathgroup * pgp; struct config *conf; @@ -315,6 +317,12 @@ int setup_map(struct multipath *mpp, char *params, int params_size) sysfs_set_scsi_tmo(mpp, conf->checkint); put_multipath_config(conf); + + if (mpp->marginal_path_double_failed_time > 0 && + mpp->marginal_path_err_sample_time > 0 && + mpp->marginal_path_err_recheck_gap_time > 0 && + mpp->marginal_path_err_rate_threshold >= 0) + start_io_err_stat_thread(vecs); /* * assign paths to path groups -- start with no groups and all paths * in mpp->paths @@ -1019,7 +1027,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid, verify_paths(mpp, vecs); params[0] = '\0'; - if (setup_map(mpp, params, PARAMS_SIZE)) { + if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { remove_map(mpp, vecs, 0); continue; } @@ -1348,7 +1356,7 @@ int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh, } } } - if (setup_map(mpp, params, PARAMS_SIZE)) { + if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { condlog(0, "%s: failed to setup map", mpp->alias); return 1; } diff --git a/libmultipath/configure.h b/libmultipath/configure.h index 0f5d30a540ca..27a7e6f60a63 100644 --- a/libmultipath/configure.h +++ b/libmultipath/configure.h @@ -28,7 +28,8 @@ enum actions { struct vectors; -int setup_map (struct multipath * mpp, char * params, int params_size ); +int setup_map (struct multipath * mpp, char * params, int params_size, + struct vectors *vecs ); int domap (struct multipath * mpp, char * params, int is_daemon); int reinstate_paths (struct multipath *mpp); int coalesce_paths (struct vectors *vecs, vector curmp, char * refwwid, int force_reload, enum mpath_cmds cmd); diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c index 536ba87968fd..ac81b4b9390d 100644 --- a/libmultipath/io_err_stat.c +++ b/libmultipath/io_err_stat.c @@ -74,6 +74,10 @@ struct io_err_stat_path { pthread_t io_err_stat_thr; pthread_attr_t io_err_stat_attr; +static pthread_mutex_t io_err_thread_lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t io_err_thread_cond = PTHREAD_COND_INITIALIZER; +static int io_err_thread_running = 0; + static struct io_err_stat_pathvec *paths; struct vectors *vecs; io_context_t ioctx; @@ -316,6 +320,9 @@ int io_err_stat_handle_pathfail(struct path *path) struct timespec curr_time; int res; + if (uatomic_read(&io_err_thread_running) == 0) + return 1; + if (path->io_err_disable_reinstate) { io_err_stat_log(3, "%s: reinstate is already disabled", path->dev); @@ -380,6 +387,8 @@ int hit_io_err_recheck_time(struct path *pp) struct timespec curr_time; int r; + if (uatomic_read(&io_err_thread_running) == 0) + return 0; if (pp->mpp->nr_active <= 0) { io_err_stat_log(2, "%s: recover path early", pp->dev); goto recover; @@ -690,6 +699,16 @@ static void service_paths(void) pthread_mutex_unlock(&paths->mutex); } +static void cleanup_unlock(void *arg) +{ + pthread_mutex_unlock((pthread_mutex_t*) arg); +} + +static void cleanup_exited(void *arg) +{ + uatomic_set(&io_err_thread_running, 0); +} + static void *io_err_stat_loop(void *data) { sigset_t set; @@ -698,10 +717,18 @@ static void *io_err_stat_loop(void *data) pthread_cleanup_push(rcu_unregister, NULL); rcu_register_thread(); + pthread_cleanup_push(cleanup_exited, NULL); + sigfillset(&set); sigdelset(&set, SIGUSR2); mlockall(MCL_CURRENT | MCL_FUTURE); + + pthread_mutex_lock(&io_err_thread_lock); + uatomic_set(&io_err_thread_running, 1); + pthread_cond_broadcast(&io_err_thread_cond); + pthread_mutex_unlock(&io_err_thread_lock); + while (1) { struct timespec ts; @@ -716,12 +743,18 @@ static void *io_err_stat_loop(void *data) pselect(1, NULL, NULL, NULL, &ts, &set); } + pthread_cleanup_pop(1); pthread_cleanup_pop(1); return NULL; } int start_io_err_stat_thread(void *data) { + int ret; + + if (uatomic_read(&io_err_thread_running) == 1) + return 0; + if (io_setup(CONCUR_NR_EVENT, &ioctx) != 0) { io_err_stat_log(4, "io_setup failed"); return 1; @@ -730,12 +763,24 @@ int start_io_err_stat_thread(void *data) if (!paths) goto destroy_ctx; - if (pthread_create(&io_err_stat_thr, &io_err_stat_attr, - io_err_stat_loop, data)) { + pthread_mutex_lock(&io_err_thread_lock); + pthread_cleanup_push(cleanup_unlock, &io_err_thread_lock); + + ret = pthread_create(&io_err_stat_thr, &io_err_stat_attr, + io_err_stat_loop, data); + + while (!ret && !uatomic_read(&io_err_thread_running) && + pthread_cond_wait(&io_err_thread_cond, + &io_err_thread_lock) == 0); + + pthread_cleanup_pop(1); + + if (ret) { io_err_stat_log(0, "cannot create io_error statistic thread"); goto out_free; } - io_err_stat_log(3, "thread started"); + + io_err_stat_log(2, "io_error statistic thread started"); return 0; out_free: @@ -748,7 +793,9 @@ destroy_ctx: void stop_io_err_stat_thread(void) { - pthread_cancel(io_err_stat_thr); + if (uatomic_read(&io_err_thread_running) == 1) + pthread_cancel(io_err_stat_thr); + pthread_join(io_err_stat_thr, NULL); free_io_err_pathvec(paths); io_destroy(ioctx); diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c index 3aafee7b217b..b3b94678438a 100644 --- a/libmultipath/structs_vec.c +++ b/libmultipath/structs_vec.c @@ -18,6 +18,7 @@ #include "prio.h" #include "configure.h" #include "libdevmapper.h" +#include "io_err_stat.h" /* * creates or updates mpp->paths reading mpp->pg @@ -426,7 +427,7 @@ retry: mpp->action = ACT_RELOAD; extract_hwe_from_path(mpp); - if (setup_map(mpp, params, PARAMS_SIZE)) { + if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { condlog(0, "%s: failed to setup new map in update", mpp->alias); retries = -1; goto fail; diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c index c0ae54aae841..7169e8d3d7ed 100644 --- a/multipathd/cli_handlers.c +++ b/multipathd/cli_handlers.c @@ -894,7 +894,7 @@ int resize_map(struct multipath *mpp, unsigned long long size, mpp->size = size; update_mpp_paths(mpp, vecs->pathvec); - setup_map(mpp, params, PARAMS_SIZE); + setup_map(mpp, params, PARAMS_SIZE, vecs); mpp->action = ACT_RESIZE; mpp->force_udev_reload = 1; if (domap(mpp, params, 1) <= 0) { diff --git a/multipathd/main.c b/multipathd/main.c index 09a59292245f..ccfc41a1f680 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -734,7 +734,7 @@ rescan: /* * push the map to the device-mapper */ - if (setup_map(mpp, params, PARAMS_SIZE)) { + if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { condlog(0, "%s: failed to setup map for addition of new " "path %s", mpp->alias, pp->dev); goto fail_map; @@ -873,7 +873,7 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) */ } - if (setup_map(mpp, params, PARAMS_SIZE)) { + if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { condlog(0, "%s: failed to setup map for" " removal of path %s", mpp->alias, pp->dev); goto fail; @@ -2489,10 +2489,6 @@ child (void * param) /* * start threads */ - rc = start_io_err_stat_thread(vecs); - if (rc) - goto failed; - if ((rc = pthread_create(&check_thr, &misc_attr, checkerloop, vecs))) { condlog(0,"failed to create checker loop thread: %d", rc); goto failed;