From patchwork Sat Feb 10 00:36:38 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Benjamin Marzinski X-Patchwork-Id: 10210119 X-Patchwork-Delegate: snitzer@redhat.com 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 B38C8602D8 for ; Sat, 10 Feb 2018 00:44:13 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9836229378 for ; Sat, 10 Feb 2018 00:44:13 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8CE0629A24; Sat, 10 Feb 2018 00:44:13 +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 06BE429378 for ; Sat, 10 Feb 2018 00:44:12 +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 B125DC056861; Sat, 10 Feb 2018 00:44:11 +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 5D5785C26B; Sat, 10 Feb 2018 00:44:11 +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 09DBE4A46D; Sat, 10 Feb 2018 00:44:11 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w1A0agYd004478 for ; Fri, 9 Feb 2018 19:36:42 -0500 Received: by smtp.corp.redhat.com (Postfix) id 2B25E101005D; Sat, 10 Feb 2018 00:36:42 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from redhat.com (octiron.msp.redhat.com [10.15.80.209]) by smtp.corp.redhat.com (Postfix) with SMTP id D480D1010060; Sat, 10 Feb 2018 00:36:38 +0000 (UTC) Received: by redhat.com (sSMTP sendmail emulation); Fri, 09 Feb 2018 18:36:38 -0600 From: "Benjamin Marzinski" Date: Fri, 9 Feb 2018 18:36:38 -0600 To: Martin Wilck Message-ID: <20180210003638.GY14513@octiron.msp.redhat.com> References: <1518134167-15938-1-git-send-email-bmarzins@redhat.com> <1518134167-15938-2-git-send-email-bmarzins@redhat.com> <1518208256.2937.16.camel@suse.com> <20180209230449.GW14513@octiron.msp.redhat.com> <1518218931.2937.20.camel@suse.com> <1518219365.2937.22.camel@suse.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1518219365.2937.22.camel@suse.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-loop: dm-devel@redhat.com Cc: Bart Van Assche , device-mapper development Subject: Re: [dm-devel] [PATCH v2 1/7] libmultipath: fix tur checker locking 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.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Sat, 10 Feb 2018 00:44:12 +0000 (UTC) X-Virus-Scanned: ClamAV using ClamSMTP On Sat, Feb 10, 2018 at 12:36:05AM +0100, Martin Wilck wrote: > On Sat, 2018-02-10 at 00:28 +0100, Martin Wilck wrote: > > Maybe it's easier than we thought. Attached is a patch on top of > > yours that I think might work, please have a look. > > > > That one didn't even compile. This one is better. > > Martin How about this one instead. The idea is that once we are in the cleanup handler, we just cleanup and exit. But before we enter it, we atomically exchange running, and if running was 0, we pause(), since the checker is either about to cancel us, or already has. > > -- > Dr. Martin Wilck , Tel. +49 (0)911 74053 2107 > SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) > From afb9c7de3658d49c4f28f6b9ee618a87b806ecdd Mon Sep 17 00:00:00 2001 > From: Martin Wilck > Date: Sat, 10 Feb 2018 00:22:17 +0100 > Subject: [PATCH] tur checker: make sure pthread_cancel isn't called for exited > thread > > If we enter the cleanup function as the result of a pthread_cancel by another > thread, we don't need to wait for a cancellation any more. If we exit > regularly, just tell the other thread not to try to cancel us. > --- > libmultipath/checkers/tur.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c > index 894ad41c89c3..5d2b36bfa883 100644 > --- a/libmultipath/checkers/tur.c > +++ b/libmultipath/checkers/tur.c > @@ -214,15 +214,13 @@ retry: > > static void cleanup_func(void *data) > { > - int running, holders; > + int holders; > struct tur_checker_context *ct = data; > > - running = uatomic_xchg(&ct->running, 0); > + uatomic_set(&ct->running, 0); > holders = uatomic_sub_return(&ct->holders, 1); > if (!holders) > cleanup_context(ct); > - if (!running) > - pause(); > } > > static int tur_running(struct tur_checker_context *ct) > @@ -266,6 +264,9 @@ static void *tur_thread(void *ctx) > pthread_cond_signal(&ct->active); > pthread_mutex_unlock(&ct->lock); > > + /* Tell main checker thread not to cancel us, as we exit anyway */ > + uatomic_set(&ct->running, 0); > + > condlog(3, "%s: tur checker finished, state %s", > tur_devt(devt, sizeof(devt), ct), checker_state_name(state)); > tur_thread_cleanup_pop(ct); > -- > 2.16.1 > --- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c index 894ad41..3774a17 100644 --- a/libmultipath/checkers/tur.c +++ b/libmultipath/checkers/tur.c @@ -214,15 +214,12 @@ retry: static void cleanup_func(void *data) { - int running, holders; + int holders; struct tur_checker_context *ct = data; - running = uatomic_xchg(&ct->running, 0); holders = uatomic_sub_return(&ct->holders, 1); if (!holders) cleanup_context(ct); - if (!running) - pause(); } static int tur_running(struct tur_checker_context *ct) @@ -242,7 +239,7 @@ static void copy_msg_to_tcc(void *ct_p, const char *msg) static void *tur_thread(void *ctx) { struct tur_checker_context *ct = ctx; - int state; + int state, running; char devt[32]; condlog(3, "%s: tur checker starting up", @@ -268,6 +265,11 @@ static void *tur_thread(void *ctx) condlog(3, "%s: tur checker finished, state %s", tur_devt(devt, sizeof(devt), ct), checker_state_name(state)); + + running = uatomic_xchg(&ct->running, 0); + if (!running) + pause(); + tur_thread_cleanup_pop(ct); return ((void *)0);