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 AB38CA050B; Fri, 8 Apr 2022 15:45:38 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7D79E4067E; Fri, 8 Apr 2022 15:45:38 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id EBEE14003F for ; Fri, 8 Apr 2022 15:45:37 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id B0E19946 for ; Fri, 8 Apr 2022 15:45:36 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id AF7A6945; Fri, 8 Apr 2022 15:45:36 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-3.6 required=5.0 tests=ALL_TRUSTED, AWL, NICE_REPLY_A, T_SCC_BODY_TEXT_LINE autolearn=disabled version=3.4.6 X-Spam-Score: -3.6 Received: from [192.168.1.59] (unknown [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 2CBACA30; Fri, 8 Apr 2022 15:45:33 +0200 (CEST) Message-ID: Date: Fri, 8 Apr 2022 15:45:32 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [PATCH v3] eal: add seqlock Content-Language: en-US To: Honnappa Nagarahalli , "dev@dpdk.org" Cc: "thomas@monjalon.net" , David Marchand , "onar.olsen@ericsson.com" , nd , "konstantin.ananyev@intel.com" , "mb@smartsharesystems.com" , "stephen@networkplumber.org" , Ola Liljedahl , =?UTF-8?Q?Mattias_R=c3=b6nnblom?= References: <20220401150749.136921-1-mattias.ronnblom@ericsson.com> From: =?UTF-8?Q?Mattias_R=c3=b6nnblom?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Virus-Scanned: ClamAV using ClamSMTP 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 2022-04-03 19:37, Honnappa Nagarahalli wrote: > > >> >>>> + * Example usage: >>>> + * @code{.c} >>>> + * #define MAX_Y_LEN (16) >>>> + * // Application-defined example data structure, protected by a seqlock. >>>> + * struct config { >>>> + * rte_seqlock_t lock; >>>> + * int param_x; >>>> + * char param_y[MAX_Y_LEN]; >>>> + * }; >>>> + * >>>> + * // Accessor function for reading config fields. >>>> + * void >>>> + * config_read(const struct config *config, int *param_x, char >>>> +*param_y) >>>> + * { >>>> + * // Temporary variables, just to improve readability. >>> I think the above comment is not necessary. It is beneficial to copy the >> protected data to keep the read side critical section small. >>> >> >> The data here would be copied into the buffers supplied by config_read() >> anyways, so it's a copy regardless. > I see what you mean here. I would think the local variables add confusion, the copy can happen to the passed parameters directly. I will leave it to you to decide. > I'll remove the temp variables. >> >>>> + * int tentative_x; >>>> + * char tentative_y[MAX_Y_LEN]; >>>> + * uint32_t sn; >>>> + * >>>> + * sn = rte_seqlock_read_lock(&config->lock); >>>> + * do { >>>> + * // Loads may be atomic or non-atomic, as in this example. >>>> + * tentative_x = config->param_x; >>>> + * strcpy(tentative_y, config->param_y); >>>> + * } while (!rte_seqlock_read_tryunlock(&config->lock, &sn)); >>>> + * // An application could skip retrying, and try again later, if >>>> + * // progress is possible without the data. >>>> + * >>>> + * *param_x = tentative_x; >>>> + * strcpy(param_y, tentative_y); >>>> + * } >>>> + * >>>> + * // Accessor function for writing config fields. >>>> + * void >>>> + * config_update(struct config *config, int param_x, const char >>>> +*param_y) >>>> + * { >>>> + * rte_seqlock_write_lock(&config->lock); >>>> + * // Stores may be atomic or non-atomic, as in this example. >>>> + * config->param_x = param_x; >>>> + * strcpy(config->param_y, param_y); >>>> + * rte_seqlock_write_unlock(&config->lock); >>>> + * } >>>> + * @endcode >>>> + * >>>> + * @see >>>> + * https://en.wikipedia.org/wiki/Seqlock. >>>> + */ >>>> + >>>> +#include >>>> +#include >>>> + >>>> +#include >>>> +#include >>>> +#include >>>> + >>>> +/** >>>> + * The RTE seqlock type. >>>> + */ >>>> +typedef struct { >>>> + uint32_t sn; /**< A sequence number for the protected data. */ >>>> + rte_spinlock_t lock; /**< Spinlock used to serialize writers. */ } >>> Suggest using ticket lock for the writer side. It should have low overhead >> when there is a single writer, but provides better functionality when there are >> multiple writers. >>> >> >> Is a seqlock the synchronization primitive of choice for high-contention cases? >> I would say no, but I'm not sure what you would use instead. > I think Stephen has come across some use cases of high contention writers with readers, maybe Stephen can provide some input. > > IMO, there is no harm/perf issues in using ticket lock. > OK. I will leave at as spinlock for now (PATCH v4).