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 9D5FDA034C; Thu, 24 Feb 2022 18:45:03 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 37EB34113D; Thu, 24 Feb 2022 18:45:03 +0100 (CET) Received: from mail-pg1-f178.google.com (mail-pg1-f178.google.com [209.85.215.178]) by mails.dpdk.org (Postfix) with ESMTP id 24A1440696 for ; Thu, 24 Feb 2022 18:45:02 +0100 (CET) Received: by mail-pg1-f178.google.com with SMTP id o26so1698047pgb.8 for ; Thu, 24 Feb 2022 09:45:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=LYgdKPU7Got4ZrzoMA0Twcdob70VRsY48EEktfY4sfw=; b=BnIOwcJtd66VIypRMvYe+7I5ECYygwpkz0v2EGznSSgYzknpZEYQhOS6dpdecIoc+z DkeFsRDMNkNiI3jUaFnwBvflRxpBLkmC+ZuOBzEIKNmuYJzQ5qhpJVvM2yCM3SeM68G+ BLewkibXYcrcxrjtT/WXfRKSLQbF3dfcTNaIh/+ehrtiXXwvJ8hvVyFdlAg4tswEfBf7 CI/698+R6wJOO9OkFNRxQeyR+3vOrfRDPg18NQvsBRZEK13kxsCZIAmUfr/gN7EwWd6Z Q/+lXN/V0sYVGL2HPF+DqnHkpxvD2TbmM0Z7mBOzaawUdiPPT6LUrCfur3onBql/f4u4 pd2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=LYgdKPU7Got4ZrzoMA0Twcdob70VRsY48EEktfY4sfw=; b=HN0hDVg/1l/cfz1UF9AggwtaBXDI/ZOjuf3IBBZGjPb807WKz8cnIfXpLuzHArubNT 9RklmcCF7zCx0gk6LSOiRw486m+gzIiZGc98v8mdeqb/9UBldH77Wsfk/fxaYJ42EHnx gKFwu7Srq0D0OCUA1R9QbuQULLEp9LopePwvWKAb2n4LIg4kKK3BgGxail51F5Az0jlD Lo6uZU1aGqpFPOpjfpamlaHbVIYcPifgO7fH9Ryv4NLzdfWI8U9cj4EEs69j0hMboGcp pWT9LDs1rusgsCl+bZDp15ucEYxqUAcestiC3C2op00PDkuiS4UMTRCr7u05WEBWsy6n jrSA== X-Gm-Message-State: AOAM5323QsQf4vS3DJ9TV3CfGACO2BkqWtkBtZh1NMgHuy4VbojkrPWV xlWARz0/2wkDo+KHf2RWeIBhyA== X-Google-Smtp-Source: ABdhPJwpbg9mWBoIWtozjb/a6bkxy62Ttaik7yvzxSPDkung6Y5M3Q7VNY+8LHTL0OHv8Vnnc/jThg== X-Received: by 2002:a05:6a00:16d6:b0:4bf:325:de2f with SMTP id l22-20020a056a0016d600b004bf0325de2fmr3845964pfc.7.1645724701229; Thu, 24 Feb 2022 09:45:01 -0800 (PST) Received: from hermes.local (204-195-112-199.wavecable.com. [204.195.112.199]) by smtp.gmail.com with ESMTPSA id w16-20020aa79a10000000b004e1bb6ea986sm93100pfj.133.2022.02.24.09.45.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Feb 2022 09:45:00 -0800 (PST) Date: Thu, 24 Feb 2022 09:44:57 -0800 From: Stephen Hemminger To: "Ananyev, Konstantin" Cc: Dmitry Kozlyuk , Narcisa Ana Maria Vasile , "Richardson, Bruce" , "david.marchand@redhat.com" , "dev@dpdk.org" , "dmitrym@microsoft.com" , "khot@microsoft.com" , "navasile@microsoft.com" , "ocardona@microsoft.com" , "Kadam, Pallavi" , "roretzla@microsoft.com" , "talshn@nvidia.com" , "thomas@monjalon.net" Subject: Re: [PATCH v18 8/8] eal: implement functions for mutex management Message-ID: <20220224094457.1e6721d8@hermes.local> In-Reply-To: References: <20220209024755.GA9377@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20220221005605.3c11746e@sovereign> <20220223200854.29910906@sovereign> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 On Thu, 24 Feb 2022 17:29:03 +0000 "Ananyev, Konstantin" wrote: > Hi Dmitry, > > > 2022-02-21 00:56 (UTC+0300), Dmitry Kozlyuk: > > > 2022-02-09 13:57 (UTC+0000), Ananyev, Konstantin: > > > > > > Actually, please scrap that comment. > > > > > > Obviously it wouldn't work for static variables, > > > > > > and doesn't make much sense. > > > > > > Though few thoughts remain: > > > > > > for posix we probably don't need an indirection and > > > > > > rte_thread_mutex can be just typedef of pthread_mutex_t. > > > > > > also for posix we don't need RTE_INIT constructor for each > > > > > > static mutex initialization. > > > > > > Something like: > > > > > > #define RTE_STATIC_INITIALIZED_MUTEX(mx) \ > > > > > > rte_thread_mutex_t mx = PTHREAD_MUTEX_INITIALIZER > > > > > > should work, I think. > > > > > > Konstantin > > > > > > > > > > Thank you for reviewing, Konstantin! > > > > > Some context for the current representation of mutex > > > > > can be found in v9, patch 7/10 of this patchset. > > > > > > > > > > Originally we've typedef'ed the pthread_mutex_t on POSIX, just > > > > > like you are suggesting here. > > > > > However, on Windows there's no static initializer similar to the pthread > > > > > one. Still, we want ABI compatibility and same thread behavior between > > > > > platforms. The most elegant solution we found was the current representation, > > > > > as suggested by Dmitry K. > > > > > > > > Yes, I agree it is a problem with Windows for static initializer. > > > > But why we can't have different structs typedef for mutex > > > > for posix and windows platforms? > > > > > > Yes, I agree that having different mutex types on *nix and Windows > > > is a great idea. It will avoid ABI change for *nix > > > and will guarantee no performance impact. > > > > > > Maybe wrap pthread_mutex_t into a struct to have a distinct type > > > and to force using only DPDK API with it? > > > > > > [...] > > > > Yes, on Windows rte_thread_mutex still wouldn't work for MP, > > > > but that's the same as with current design. > > > > > > MP support is not planned for Windows and it is unknown if it ever will be, > > > so it's not an issue. > > > Data location is. > > > The reason rte_thread_mutex_t is not a typedef of CRITICAL_SECTION > > > (akin to pthread_mutex_t) is to avoid including Windows headers > > > into DPDK public headers, because Windows headers can break user code > > > by some macros they define. > > > Maybe instead of a pointer it could be an opaque array: > > > > > > #define RTE_PTHREAD_MUTEX_SIZE 40 > > > > > > struct rte_pthread_mutex_t { > > > uint8_t opaque[RTE_PTHREAD_MUTEX_SIZE]; > > > }; > > > > > > where RTE_PTHREAD_MUTEX_SIZE is actually sizeof(CRITICAL_SECTION). > > > Win32 ABI is remarkably stable, I don't think this constant will ever change, > > > it would break all the Windows user space. > > > Naty, DmitryM, Tyler, what do you think? > > > > Conclusion from offline call: yes, this is OK to do so. > > > > However, DmitryM suggested using Slim Reader-Writer lock (SRW): > > https://docs.microsoft.com/en-us/windows/win32/sync/slim-reader-writer--srw--locks > > instead of CRITICAL_SECTION. > > It seems to be a much better option: > > > > * sizeof(SRWLOCK) == 8 (technically "size of a pointer"), > > same as sizeof(pthread_mutex_t) on a typical Linux. > > Layout of data structures containing rte_thread_mutex_t > > can be the same on Windows and Unix, > > which simplifies design and promises similar less performance differences. > > > > * Can be taken by multiple readers and one writer, > > which is semantically similar to pthread_mutex_t > > Not sure I understand you here: > pthread_mutex provides only mutually-exclusive lock semantics. > For RW lock there exists: pthread_rwlock_t. > Off-course you can use rwlock fo exclusive locking too - > if all callers will use only writer locks, but usually that's no point to do that - > mutexes are simpler and faster. > That's for posix-like systems, don't know much about Windows environment :) > > > (CRITICAL_SECTION can only be taken by a single thread). > > > > Technically it can be a "typedef uintptr_t" or a structure wrapping it. > > Again can't say much about Windows, but pthread_mutex_t > can (and is) bigger then then 8 bytes. > > There seems to be some confusion here: pthread_mutex put thread to sleep if contended and on linux are built on the futex system call. pthread_rwlock are the reader/writer versions of these. The DPDK has primitives for multiple types of locks: spinlock, rwlock, ticketlock, pflock, etc these are build using atomic primitives (no syscall). these are platform independent these spin if contended Not sure about Windows, but it looks like slim rwlocks came from Windows NT and are an implementation of the same kind of spinning lock DPDK already has.