From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D8762C433F5 for ; Fri, 8 Apr 2022 02:37:48 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4KZMqH2f91z3bfR for ; Fri, 8 Apr 2022 12:37:47 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=CrkVJRLO; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gmail.com (client-ip=2a00:1450:4864:20::42b; helo=mail-wr1-x42b.google.com; envelope-from=ecree.xilinx@gmail.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=CrkVJRLO; dkim-atps=neutral Received: from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com [IPv6:2a00:1450:4864:20::42b]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4KZ7xf2WWGz2xn9 for ; Fri, 8 Apr 2022 03:42:28 +1000 (AEST) Received: by mail-wr1-x42b.google.com with SMTP id c7so9030236wrd.0 for ; Thu, 07 Apr 2022 10:42:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=Qf5/HY0H/HARlpQawjWcubI38eU259kawMq8gp9O7d4=; b=CrkVJRLO1aSJixV9DWKUTF746y41oFcj/ZYQXCMA93sWiB0r8cbHO/f8LZz8KS6V2R VtJYpBNgDsp6YUKM35f2uF3N+8gfMhpVgeHmhzuz8xy4T8pCvh4xoKIQ25X6AicMAZXP deImoshAMsKtF6YC9AZ2M7IJUaUdxIiycCgtpArV5rA0mTPv8LpJEcJyarvYcnZXmNCI aWlfhIlk0mpOrWQ6OdxS5X8MjD75utFVj5Sl/xPB855dp78ch/LkHjf2soCCgGydwfVJ cTIojrIudwVz04FEA14NfrVFmBYZG7vKl9ICFrjbOEbVg9rmja7sZ7v3ms1J2iW6OXWv jt3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Qf5/HY0H/HARlpQawjWcubI38eU259kawMq8gp9O7d4=; b=cS6e6luwShKJ87MAMehi+LpKGtNxll5BrjT2/IVSVbIVXk+3n+LZEJu3/X9iHm2QUl 4ECrrz/Ig34fJgprD/zJ0WJ78+KVkxLKQM/rB6V5wL0QsoFTk6elW6lBhYH8c3kUsb7s A5TfyccvvTDPLbC8XjXucxN3i+imtE8/0msCiCe+j6nLFyHlIvgmz0fVhRJvszlz5oKR ObOIApN9LiZSC71tAU9U5fduKjRoVQ8tBkXfD5vjvTCx+RHo+N/vyRrO3uceltNiz1oz myvtFWfU479UpeXXQ4IkMeYBYsn9t6oeuk6lK6vDR08Awn8Sw1fzmoZ57EHrZu30u4a0 6yHg== X-Gm-Message-State: AOAM532OZpUNi7vDqEVJURHYG7g9TOpZRH2es+kzYe5zgi2zpAdt/DM7 x/kWLewqk30ZQBZFWiEpp6E= X-Google-Smtp-Source: ABdhPJygSojkbPQ9T6nmuvpmWKE0eBxm5eOSmqhSZlDa5KHyWLaKpOYDGTp1qDvg6zbYeK2/pSW5Og== X-Received: by 2002:adf:d222:0:b0:206:17ba:5f76 with SMTP id k2-20020adfd222000000b0020617ba5f76mr11454903wrh.484.1649353339722; Thu, 07 Apr 2022 10:42:19 -0700 (PDT) Received: from [192.168.1.122] (cpc159313-cmbg20-2-0-cust161.5-4.cable.virginm.net. [82.0.78.162]) by smtp.gmail.com with ESMTPSA id y6-20020a05600015c600b00203fa70b4ebsm21873677wry.53.2022.04.07.10.42.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 07 Apr 2022 10:42:19 -0700 (PDT) Subject: Re: [PATCH net-next 11/15] sfc: Remove usage of list iterator for list_add() after the loop body To: Jakob Koschel , "David S. Miller" References: <20220407102900.3086255-1-jakobkoschel@gmail.com> <20220407102900.3086255-12-jakobkoschel@gmail.com> From: Edward Cree Message-ID: <4520e9c5-8871-b281-f621-ac737e64333b@gmail.com> Date: Thu, 7 Apr 2022 18:42:17 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 MIME-Version: 1.0 In-Reply-To: <20220407102900.3086255-12-jakobkoschel@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 8bit X-Mailman-Approved-At: Fri, 08 Apr 2022 12:37:11 +1000 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrew Lunn , Cristiano Giuffrida , Eric Dumazet , Paul Mackerras , Ariel Elior , Florian Fainelli , Manish Chopra , Steen Hegelund , "Bos, H.J." , linux-arm-kernel@lists.infradead.org, Martin Habets , Paolo Abeni , Vivien Didelot , Bjarni Jonasson , Jiri Pirko , Arnd Bergmann , Brian Johannesmeyer , Christophe JAILLET , Jakub Kicinski , Di Zhu , Lars Povlsen , Colin Ian King , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, UNGLinuxDriver@microchip.com, Michael Walle , Xu Wang , Vladimir Oltean , linuxppc-dev@lists.ozlabs.org, Casper Andersson , Mike Rapoport Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On 07/04/2022 11:28, Jakob Koschel wrote: > In preparation to limit the scope of a list iterator to the list > traversal loop, use a dedicated pointer to point to the found element [1]. > > Before, the code implicitly used the head when no element was found > when using &pos->list. Since the new variable is only set if an > element was found, the list_add() is performed within the loop > and only done after the loop if it is done on the list head directly. > > Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1] > Signed-off-by: Jakob Koschel The commit message doesn't accurately describe the patch; it states that "the list_add() is performed within the loop", which doesn't appear to be the case. Also it seems a bit subtle to use `head` as both the head of the list to iterate over and the found entry/gap to insert before; a comment explaining that wouldn't go amiss. (I'd question whether this change is really an improvement in this case, where the iterator really does hold the thing we want at the end of the search and so there's no if(found) special-casing — we're not even abusing the type system, because efx->rss_context is of the same type as all the list entries, so ctx really is a valid pointer and there shouldn't be any issues with speculative accesses or whatever — but it seems Linus has already pronounced in favour of the scope limiting, and far be it from me to gainsay him.) -ed > --- > drivers/net/ethernet/sfc/rx_common.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/sfc/rx_common.c b/drivers/net/ethernet/sfc/rx_common.c > index 1b22c7be0088..a8822152ff83 100644 > --- a/drivers/net/ethernet/sfc/rx_common.c > +++ b/drivers/net/ethernet/sfc/rx_common.c > @@ -563,8 +563,10 @@ struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx) > > /* Search for first gap in the numbering */ > list_for_each_entry(ctx, head, list) { > - if (ctx->user_id != id) > + if (ctx->user_id != id) { > + head = &ctx->list; > break; > + } > id++; > /* Check for wrap. If this happens, we have nearly 2^32 > * allocated RSS contexts, which seems unlikely. > @@ -582,7 +584,7 @@ struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx) > > /* Insert the new entry into the gap */ > new->user_id = id; > - list_add_tail(&new->list, &ctx->list); > + list_add_tail(&new->list, head); > return new; > } > >