From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 17375A034F; Tue, 8 Jun 2021 22:48:30 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8DBC340689; Tue, 8 Jun 2021 22:48:29 +0200 (CEST) Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) by mails.dpdk.org (Postfix) with ESMTP id 29B144067A for ; Tue, 8 Jun 2021 22:48:28 +0200 (CEST) Received: by mail-pf1-f169.google.com with SMTP id z26so16652647pfj.5 for ; Tue, 08 Jun 2021 13:48:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ui5iHN/8lWhFT966mR3pM6/Ve2r19MaMCG3xXn7wSWo=; b=ZBpY2mxcmmGFOjLvWncnObqoeU7G7PjFXiUwxfpDqthq2L1/Z07iR+Q+ZDpOLWpJqw FGP3gAIT8shK3bI+Isf7QwKvExSIsW8v/6uAPFQFZU0wt61Ah2XQN4S3sA1zA7kvWti/ PvJ8dGApDMKgweyTgtRkHJEIJElDueH2MkTfVWJHTVVbQW7HlBONinTlYA3/ifbhlPOR Cl+z4x8hTSee1IwJ+zVOy++Ht7/7Z69pzN4RFxPxCtpS1x7YdtXoK8MamvIhrg7AimK6 PE3/35VezihKmqRLc0HRTf011ME+rcON6yyHODua3Am2knUkJuMN+a6xsRt6qg3il2M4 WQog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ui5iHN/8lWhFT966mR3pM6/Ve2r19MaMCG3xXn7wSWo=; b=fm/opN5vC/H5T4ll+RXz444jkOtcuZ9JU38MNd2ISHvxEa2zGiPWT3Yvo5//qr5tnB PsVn6xh8FAdgD5HpeSMBQDP4+uvWgHry32cfau9Uz91OXm/xuQbSDZbYuJp66Qr4PZ5T Dm9J446kxXxNVKCqwmC00u/yJ0sQSTlj0VCtJzTyuOH3RmeVl6pW0yf3KXPVw7B8AIri XEXz1z/tSVp5yiuszQ4wLCPu/3JVBGlqYCzNtQcjT6JFZq6nW2/gN8FYKSsHus2pLRHr /KhXyzVxvkk5Ngnr7niWFRmDMaNhSTaPRFqqsepmGzqdJZOGYIoDx4kXRYCgBKeEfimp t2IQ== X-Gm-Message-State: AOAM533TuOr7gk7go7moyTzh2peoHBHvWWE08LFH8DXC6p/I2UO/Adi6 nRo16YrAc4wrOiI+m6n8NMDzjg== X-Google-Smtp-Source: ABdhPJy2R1x6rVe4i787/mmNE3qMhB502EocVZEYkaVCBfZ0VaqfNNDJ0AlcM40mUM2x2lFIhpmgtg== X-Received: by 2002:a62:3344:0:b029:28c:6f0f:cb90 with SMTP id z65-20020a6233440000b029028c6f0fcb90mr1635663pfz.58.1623185308022; Tue, 08 Jun 2021 13:48:28 -0700 (PDT) Received: from hermes.local (76-14-218-44.or.wavecable.com. [76.14.218.44]) by smtp.gmail.com with ESMTPSA id a9sm11110745pfo.69.2021.06.08.13.48.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Jun 2021 13:48:27 -0700 (PDT) Date: Tue, 8 Jun 2021 13:48:24 -0700 From: Stephen Hemminger To: Andrew Rybchenko Cc: Thomas Monjalon , dev@dpdk.org, matan@mellanox.com, Gaetan Rivet Message-ID: <20210608134824.3181b063@hermes.local> In-Reply-To: References: <20210315192722.35490-1-stephen@networkplumber.org> <20210315192722.35490-3-stephen@networkplumber.org> <6747934.v7ilQdk43l@thomas> <20210608084210.73d05f60@hermes.local> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 2/2] net/failsafe: fix primary/secondary mutex X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, 8 Jun 2021 18:55:17 +0300 Andrew Rybchenko wrote: > On 6/8/21 6:42 PM, Stephen Hemminger wrote: > > On Tue, 8 Jun 2021 11:00:37 +0300 > > Andrew Rybchenko wrote: > > > >> On 4/19/21 8:08 PM, Thomas Monjalon wrote: > >>> About the title, better to speak about multi-process, > >>> it is less confusing than primary/secondary. > >>> > >>> 15/03/2021 20:27, Stephen Hemminger: > >>>> Set mutex used in failsafe driver to protect when used by > >>>> both primary and secondary process. Without this fix, the failsafe > >>>> lock is not really locking when there are multiple secondary processes. > >>>> > >>>> Bugzilla ID: 662 > >>>> Signed-off-by: Stephen Hemminger > >>>> Fixes: 655fcd68c7d2 ("net/failsafe: fix hotplug races") > >>>> Cc: matan@mellanox.com > >>> > >>> The correct order for above lines is: > >>> > >>> Bugzilla ID: 662 > >>> Fixes: 655fcd68c7d2 ("net/failsafe: fix hotplug races") > >>> > >>> Signed-off-by: Stephen Hemminger > >>> > >>>> --- > >>>> --- a/drivers/net/failsafe/failsafe.c > >>>> +++ b/drivers/net/failsafe/failsafe.c > >>>> @@ -140,6 +140,11 @@ fs_mutex_init(struct fs_priv *priv) > >>>> ERROR("Cannot initiate mutex attributes - %s", strerror(ret)); > >>>> return ret; > >>>> } > >>>> + /* Allow mutex to protect primary/secondary */ > >>>> + ret = pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED); > >>>> + if (ret) > >>>> + ERROR("Cannot set mutex shared - %s", strerror(ret)); > >>> > >>> Why not returning an error here? > >> > >> +1 > >> > >> I think it would be safer to return an error here. > > > > Ok but it never happens. > > > > May I ask why? 'man pthread_mutexattr_setpshared' says that it > is possible. > The glibc implementation of pthread_mutexattr_setpshared is: int pthread_mutexattr_setpshared (pthread_mutexattr_t *attr, int pshared) { struct pthread_mutexattr *iattr; int err = futex_supports_pshared (pshared); if (err != 0) return err; iattr = (struct pthread_mutexattr *) attr; if (pshared == PTHREAD_PROCESS_PRIVATE) iattr->mutexkind &= ~PTHREAD_MUTEXATTR_FLAG_PSHARED; else iattr->mutexkind |= PTHREAD_MUTEXATTR_FLAG_PSHARED; return 0; } And /* FUTEX_SHARED is always supported by the Linux kernel. */ static __always_inline int futex_supports_pshared (int pshared) { if (__glibc_likely (pshared == PTHREAD_PROCESS_PRIVATE)) return 0; else if (pshared == PTHREAD_PROCESS_SHARED) return 0; else return EINVAL; } There for the code as written can not return an error. The check was only because someone could report a bogus issue from a broken c library.