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 333EAA0542; Tue, 31 May 2022 13:53:13 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 260C040A87; Tue, 31 May 2022 13:53:13 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 53D40400D6 for ; Tue, 31 May 2022 13:53:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1653997989; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5MBvHpkC0beMVdNfGLy7geABtmwid6as5eUURNSdQgA=; b=WzcDyhGQaYdr2md4Q+Uog4VxFystPGhUaAVyHMEgMrC16pm5yar/qNclbfjxKGyqvQaCzf yNNLPS1GSzweS364BlqMUYIc+wWeUXw/hut5mp6dDvU7bfUm26yyW6QNs/ugN7zlTOZcu6 V6GPZNlBIPO6d5iSQVDUOP6m2cLU3BA= Received: from mail-lj1-f197.google.com (mail-lj1-f197.google.com [209.85.208.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-228-S_5elioCMOCyOot1D-LwZw-1; Tue, 31 May 2022 07:53:06 -0400 X-MC-Unique: S_5elioCMOCyOot1D-LwZw-1 Received: by mail-lj1-f197.google.com with SMTP id k5-20020a2e6f05000000b002555a5d11e4so403090ljc.18 for ; Tue, 31 May 2022 04:53:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=5MBvHpkC0beMVdNfGLy7geABtmwid6as5eUURNSdQgA=; b=jDbyuSHw6Q4M8Vt/t5Z+aHeMQwXZlIw8GoW07rNPEPPLjXa+m1LWW0wlI07V4pA/2c BF6S5mvowm7p1RUhXiSDFzxASUuSDy7BkPOEAkw/OQOF1ryFSqBsla+hzJLnxat/gwiI WY/q7DfZUEcZWIUxkddNFowJLwwjTgip1dw8ekKBRgIuQoTvmvLXOeoBjHel797cOe9H +A8qs4Cy6lxY8wJwpMWr1GpNZPi58T6a6rMu9lUH7OGP/mqqiz4W+tbNTUk5Fl1s29dB oqKEeTy+O/8M7SFDlRXjCZu/JB4UNhLjCWuYCJaX5aGpJDDH5Xelqjp4UqgQRmO9uw2T c9rQ== X-Gm-Message-State: AOAM533y9fd5z40DpoMwoBWFjnqWYHYxn7kfWmMe6Xj0RsT6wNaOo7b0 lpI536cpRC4fRyTCEwfPDstDSv8KL0IHoyFqDffX8cP3Q0tWPNzWCqBu1pkyG7fARRv3dBaizCe bKJoYCd+Q0j9E2CVBSWA= X-Received: by 2002:a05:6512:2a9a:b0:477:caea:4ba with SMTP id dt26-20020a0565122a9a00b00477caea04bamr41873674lfb.575.1653997985227; Tue, 31 May 2022 04:53:05 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzJNcgBVqfwKNIDBMWL4ruvicsuYY4ZPT/D4gbUcZf+HHKHSlK0OB/53wnf6mZoSSXH5Z6VuDjVt4QQJvXfQ2I= X-Received: by 2002:a05:6512:2a9a:b0:477:caea:4ba with SMTP id dt26-20020a0565122a9a00b00477caea04bamr41873665lfb.575.1653997984943; Tue, 31 May 2022 04:53:04 -0700 (PDT) MIME-Version: 1.0 References: <20220523113111.366599-1-mattias.ronnblom@ericsson.com> <20220523142346.366902-1-mattias.ronnblom@ericsson.com> In-Reply-To: <20220523142346.366902-1-mattias.ronnblom@ericsson.com> From: David Marchand Date: Tue, 31 May 2022 13:52:52 +0200 Message-ID: Subject: Re: [PATCH v9] eal: add seqlock To: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= Cc: Thomas Monjalon , dev , onar.olsen@ericsson.com, Honnappa Nagarahalli , nd , "Ananyev, Konstantin" , =?UTF-8?Q?Morten_Br=C3=B8rup?= , Stephen Hemminger , =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , Chengwen Feng , Ola Liljedahl Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Mon, May 23, 2022 at 4:24 PM Mattias R=C3=B6nnblom wrote: > > A sequence lock (seqlock) is a synchronization primitive which allows > for data-race free, low-overhead, high-frequency reads, suitable for > data structures shared across many cores and which are updated > relatively infrequently. > > A seqlock permits multiple parallel readers. A spinlock is used to > serialize writers. In cases where there is only a single writer, or > writer-writer synchronization is done by some external means, the > "raw" sequence counter type (and accompanying rte_seqcount_*() > functions) may be used instead. > > To avoid resource reclamation and other issues, the data protected by > a seqlock is best off being self-contained (i.e., no pointers [except > to constant data]). > > One way to think about seqlocks is that they provide means to perform > atomic operations on data objects larger than what the native atomic > machine instructions allow for. > > DPDK seqlocks (and the underlying sequence counters) are not > preemption safe on the writer side. A thread preemption affects > performance, not correctness. > > A seqlock contains a sequence number, which can be thought of as the > generation of the data it protects. > > A reader will > 1. Load the sequence number (sn). > 2. Load, in arbitrary order, the seqlock-protected data. > 3. Load the sn again. > 4. Check if the first and second sn are equal, and even numbered. > If they are not, discard the loaded data, and restart from 1. > > The first three steps need to be ordered using suitable memory fences. > > A writer will > 1. Take the spinlock, to serialize writer access. > 2. Load the sn. > 3. Store the original sn + 1 as the new sn. > 4. Perform load and stores to the seqlock-protected data. > 5. Store the original sn + 2 as the new sn. > 6. Release the spinlock. > > Proper memory fencing is required to make sure the first sn store, the > data stores, and the second sn store appear to the reader in the > mentioned order. > > The sn loads and stores must be atomic, but the data loads and stores > need not be. > > The original seqlock design and implementation was done by Stephen > Hemminger. This is an independent implementation, using C11 atomics. > > For more information on seqlocks, see > https://en.wikipedia.org/wiki/Seqlock The SoB and other tags were with annotations, just repeating them here (in the hope patchwork will catch them): Acked-by: Morten Br=C3=B8rup Acked-by: Konstantin Ananyev Reviewed-by: Ola Liljedahl Reviewed-by: Chengwen Feng Signed-off-by: Mattias R=C3=B6nnblom GHA had a hiccup on Ubuntu vm installation at the time of the v9 reception. I had run the checks on my side: https://github.com/david-marchand/dpdk/actions/runs/2373533127 I noticed a little issue in the example in the doxygen comments that I can fix myself: --- a/lib/eal/include/rte_seqlock.h +++ b/lib/eal/include/rte_seqlock.h @@ -63,7 +63,7 @@ extern "C" { * // An alternative to an immediate retry is to abort and * // try again at some later time, assuming progress is * // possible without the data. - * } while (rte_seqlock_read_retry(&config->lock)); + * } while (rte_seqlock_read_retry(&config->lock, sn)); * } * * // Accessor function for writing config fields. I also have some whitespace fixes here and there. I may shorten the release notes update to keep only the first paragraph, as the rest is too verbose and repeated in the documentation. Overall, comments were addressed and the patch looks ready. Last call before merging? --=20 David Marchand