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 9BDFCA00C2; Wed, 23 Feb 2022 18:08:58 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3067440E5A; Wed, 23 Feb 2022 18:08:58 +0100 (CET) Received: from mail-lj1-f180.google.com (mail-lj1-f180.google.com [209.85.208.180]) by mails.dpdk.org (Postfix) with ESMTP id 7730240DF6 for ; Wed, 23 Feb 2022 18:08:56 +0100 (CET) Received: by mail-lj1-f180.google.com with SMTP id u7so18114289ljk.13 for ; Wed, 23 Feb 2022 09:08:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ZPueZw2VOKc6oqWGtAda+gkJoIsdhU8dgeOK//5BFQk=; b=Czh+HyjrNumN/Lb95V5d4pzJ9EoOd0Kktg7s6HJvdFHOuq9am2Puy8Gn83BTh7dOtB zgx2vxSsZnb0z/zNOlRtsALIgPiCC3sPtEcZcbxFRQMzsXYfwMxa2XhW9cSshRnN+OrV Hl8I+uznYykzP4/Nwcd1aFPHrjs1WsMcJJRyCw9DKzaOLxBOTUG5osQwn9S+mU9jdEtR mtzPt1dHJ4OdW833GT+Wbmmv5Cy/xgrwbBVsUZEUmwiXWefW/jjwofmWByM6HvAkInQk pmSo6gUzX3SH/7d29U152u5k0+wm3bDWvUlQAqJE8/cYxNeTWwvZaj7lGDrxU5ImlVrN V1Ag== 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=ZPueZw2VOKc6oqWGtAda+gkJoIsdhU8dgeOK//5BFQk=; b=mUVMcctxtijgVxYVYLnNLXjqkU1eKWwS33OoQSjYK8HLc5gVEyPQBGvlJpWhXIOPbf pLB/BaMazJfd8PvI18KIBmjAM7kKueAfRlxfW5oLJKL1PMnlSKyywzBED/K2WscjLFM/ aGS15bqy48/E9mkRwtVv2IjUp7SXUvO9LQ9z7ATPEafb5OxBChFxzmkDBk0nJie7hop1 avvgrm45EF8KdpUzZoZtAMEHdVCTgJpcBsCbIvJ63KF64Pb3SkmYW7/2msA1KqxinHF9 aDJdewrVzXDVTKfz4uQ4tv67ZsJybkB0kaN93jSfXczsDfgwWDJE2dMeFkTjqdGKzM8X qH7g== X-Gm-Message-State: AOAM5332/yqSoAN5Gzw+2wYDUfQPJc4dKBZ80XcvB1Flz9mimR4KsIp3 VUo5KRL3nlhU+0KWWe/+WPA= X-Google-Smtp-Source: ABdhPJxlp7dSMyNM43jN4Bc74fdkkRSXKtF1Ueqdmn4FFW09l+uZAaHYlN3eeOiR4mk4/zWW6gtM/Q== X-Received: by 2002:a05:651c:1595:b0:244:8cab:808d with SMTP id h21-20020a05651c159500b002448cab808dmr272953ljq.238.1645636135781; Wed, 23 Feb 2022 09:08:55 -0800 (PST) Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id v29sm25248ljv.72.2022.02.23.09.08.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Feb 2022 09:08:55 -0800 (PST) Date: Wed, 23 Feb 2022 20:08:54 +0300 From: Dmitry Kozlyuk To: "Ananyev, Konstantin" Cc: 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: <20220223200854.29910906@sovereign> In-Reply-To: <20220221005605.3c11746e@sovereign> References: <20220209024755.GA9377@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20220221005605.3c11746e@sovereign> X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) 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 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 (CRITICAL_SECTION can only be taken by a single thread). Technically it can be a "typedef uintptr_t" or a structure wrapping it.