From patchwork Sun Aug 14 08:41:33 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Christie X-Patchwork-Id: 9278635 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 CC748600CB for ; Sun, 14 Aug 2016 08:45:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BA87C289D8 for ; Sun, 14 Aug 2016 08:45:26 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id AADB828A17; Sun, 14 Aug 2016 08:45:26 +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=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 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.wl.linuxfoundation.org (Postfix) with ESMTPS id 374A9289D8 for ; Sun, 14 Aug 2016 08:45:25 +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 u7E8faR2005010; Sun, 14 Aug 2016 04:41:37 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u7E8fZCx016355 for ; Sun, 14 Aug 2016 04:41:35 -0400 Received: from [10.10.57.52] (vpn-57-52.rdu2.redhat.com [10.10.57.52]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u7E8fYIB012308; Sun, 14 Aug 2016 04:41:34 -0400 To: Bart Van Assche , "dm-devel@redhat.com" , "christophe.varoqui@opensvc.com" References: <1470657710-28081-1-git-send-email-mchristi@redhat.com> <1470657710-28081-3-git-send-email-mchristi@redhat.com> <9d5dcfcd-2550-c9e9-94dc-47c34ebdb039@sandisk.com> <57ACE12F.20700@redhat.com> From: Mike Christie Message-ID: <57B02EBD.9090206@redhat.com> Date: Sun, 14 Aug 2016 03:41:33 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-loop: dm-devel@redhat.com Subject: Re: [dm-devel] [PATCH 2/4] multipath-tools: add checker callout to repair path 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-Virus-Scanned: ClamAV using ClamSMTP On 08/11/2016 04:41 PM, Bart Van Assche wrote: > On 08/11/2016 01:33 PM, Mike Christie wrote: >> Could you try the attached patch. I found two segfaults. If check_path >> returns less than 0 then we free the path and so we cannot call repair >> on it. If libcheck_init fails it memsets the checker, so we cannot call >> repair on it too. >> >> I moved the repair call to the specific paths that the path is down. > > Hello Mike, > > Thanks for the patch. Unfortunately even with this patch applied I can > still trigger a segfault sporadically: > Ok. This should fix all of them. Attached patch fixes: 1. If check_path returns less than 0 then we free the path and so we cannot call repair on it of course. 2. If libcheck_init fails it memsets the checker, so we cannot call repair on it too. 3. We can hit a race where when pathinfo is setting up a path, the path could have gone down. In the DI_CHECKER chunk we then do not run get_state and attach a checker. Later when check_path is run path_offline we could still return PATH_DOWN or PATH_REMOVED and get_state is again not run so we do not get to attach a checker again. I was then running repair_path since the state was PATH_DOWN, and kaboom. Attached patch should fix these issues. --- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c index 8976c89..fd999b0 100644 --- a/libmultipath/checkers.c +++ b/libmultipath/checkers.c @@ -213,7 +213,7 @@ void checker_put (struct checker * dst) void checker_repair (struct checker * c) { - if (!c) + if (!c || !checker_selected(c)) return; c->message[0] = '\0'; diff --git a/multipathd/main.c b/multipathd/main.c index f5e9a01..c4ffe6f 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -1442,6 +1442,16 @@ int update_path_groups(struct multipath *mpp, struct vectors *vecs, int refresh) return 0; } +void repair_path(struct path * pp) +{ + if (pp->state != PATH_DOWN) + return; + + checker_repair(&pp->checker); + if (strlen(checker_message(&pp->checker))) + LOG_MSG(1, checker_message(&pp->checker)); +} + /* * Returns '1' if the path has been checked, '-1' if it was blacklisted * and '0' otherwise @@ -1606,6 +1616,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) pp->mpp->failback_tick = 0; pp->mpp->stat_path_failures++; + repair_path(pp); return 1; } @@ -1700,7 +1711,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) } pp->state = newstate; - + repair_path(pp); if (pp->mpp->wait_for_udev) return 1; @@ -1725,14 +1736,6 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) return 1; } -void repair_path(struct vectors * vecs, struct path * pp) -{ - if (pp->state != PATH_DOWN) - return; - - checker_repair(&pp->checker); -} - static void * checkerloop (void *ap) { @@ -1804,7 +1807,6 @@ checkerloop (void *ap) i--; } else num_paths += rc; - repair_path(vecs, pp); } lock_cleanup_pop(vecs->lock); }