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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 CC8EDC352A1 for ; Wed, 7 Dec 2022 08:26:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:CC:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=xDE5knXIGv6wSZJ3sN/gFdJ1BR7k33WjQ9xmzVoxZoI=; b=23KDovMcwl5vT/ gaf7kItTkZwaVTT30Nyl+cG2bzjrDOyLdz6qTtuI+FijfM16nWepmMAMM4GX02saWoM1wb6R0ZRjU HlZJuhNGq337AXvKZdiLyBgDFoeu/bdkh6n4+pjui0rLcFXULNB5txtWaY1v4WO77Z/b6gguZ8LZc 8IK+ctJI09TTA3Ze2R+xuWvcQ5tlYH0p1dGy+xZpwjpkH72h1QmFAeuNfwaBP3cxHszcp0SC1S4PR ciYax/KUNaChrKRE+IBg4h53+UxFn6K/5d8O+fD+4jW54fylVz81GtzufTToXoZrmsYoNZXUQcdM1 kQ+mKJcw6W7ggI1+IH4Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p2pk6-00F1xW-RT; Wed, 07 Dec 2022 08:25:26 +0000 Received: from esa.microchip.iphmx.com ([68.232.154.123]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1p2pk1-00F1fL-91 for linux-arm-kernel@lists.infradead.org; Wed, 07 Dec 2022 08:25:24 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1670401521; x=1701937521; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=P7OrcBboMlV8Os7KJfShQiBKCP7NXDohE0LvqziHmvE=; b=rMsFE+FSOE2Hiie8vK+iG6AYrzgE4+oHxIkrpovfsQYxIp8BvHF9Bjnv nHmRGWyvXerfNZ16HRB0z2dR0Ro33GqPJPVUFwtIP/opbe5DtAtR4djGH OzPEHXh9JZtJ9rkmhm8fXvv0jDvwgKPYDlFuTvx9UorSjVIWFMzqpwjrs +LpP1E6fYN7Kd8t3Xp2ouvZCaLGsLsSkLRNngfKFXrLnkpWsDiGqPGnKY lVHbJaBPiyl/XIcvMqPqreL9Vc/BiJhxJJJb7+dH55nN/csYfOCCuLE5o kvvF0wquU9KSnj7/UP25uqyGXz7XmsiZu+lHft5knHsqDb6x1so25b8dI w==; X-IronPort-AV: E=Sophos;i="5.96,223,1665471600"; d="scan'208";a="126897538" Received: from unknown (HELO email.microchip.com) ([170.129.1.10]) by esa6.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 07 Dec 2022 01:25:10 -0700 Received: from chn-vm-ex03.mchp-main.com (10.10.85.151) by chn-vm-ex01.mchp-main.com (10.10.85.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.12; Wed, 7 Dec 2022 01:25:08 -0700 Received: from localhost (10.10.115.15) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server id 15.1.2507.12 via Frontend Transport; Wed, 7 Dec 2022 01:25:07 -0700 Date: Wed, 7 Dec 2022 09:30:14 +0100 From: Horatiu Vultur To: Paolo Abeni CC: , , , , , , , , , , , Subject: Re: [PATCH net-next v3 1/4] net: microchip: vcap: Add vcap_get_rule Message-ID: <20221207083014.mipiiu6dzgpuayz5@soft-dev3-1> References: <20221203104348.1749811-1-horatiu.vultur@microchip.com> <20221203104348.1749811-2-horatiu.vultur@microchip.com> <256f533f019b6392b41701707eb7aa056b2f16c0.camel@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <256f533f019b6392b41701707eb7aa056b2f16c0.camel@redhat.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221207_002522_450519_DB3DCA93 X-CRM114-Status: GOOD ( 19.40 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org The 12/06/2022 13:31, Paolo Abeni wrote: > > Hello, Hi Paolo, > > On Sat, 2022-12-03 at 11:43 +0100, Horatiu Vultur wrote: > > @@ -632,32 +264,22 @@ static int vcap_show_admin(struct vcap_control *vctrl, > > struct vcap_admin *admin, > > struct vcap_output_print *out) > > { > > - struct vcap_rule_internal *elem, *ri; > > + struct vcap_rule_internal *elem; > > + struct vcap_rule *vrule; > > int ret = 0; > > > > vcap_show_admin_info(vctrl, admin, out); > > - mutex_lock(&admin->lock); > > list_for_each_entry(elem, &admin->rules, list) { > > Not strictly related to this patch, as the patter is AFAICS already > there in other places, but I'd like to understand the admin->rules > locking schema. According to the commit message that introduced this lock [0] and the comment to the lock, this lock is used to protect the access to the admin->rules, admin->enabled and the caches which means the access to the HW (to read/write the rules). > > It looks like addition/removal are protected by admin->lock, but > traversal is usually lockless, which in turn looks buggy ?!? Thanks for the observation! You are correct, there seems to be some bugs regarding the usage of this lock. We will look over this and will send a patch to fix this. > > Note: as this patch does not introduce the mentioned behavior, I'm not > going to block the series for the above. [0] 71c9de995260 ("net: microchip: sparx5: Add VCAP locking to protect rules") > > Thanks, > > Paolo > > - ri = vcap_dup_rule(elem); > > - if (IS_ERR(ri)) { > > - ret = PTR_ERR(ri); > > - goto err_unlock; > > + vrule = vcap_get_rule(vctrl, elem->data.id); > > + if (IS_ERR_OR_NULL(vrule)) { > > + ret = PTR_ERR(vrule); > > + break; > > } > > - /* Read data from VCAP */ > > - ret = vcap_read_rule(ri); > > - if (ret) > > - goto err_free_rule; > > + > > out->prf(out->dst, "\n"); > > - vcap_show_admin_rule(vctrl, admin, out, ri); > > - vcap_free_rule((struct vcap_rule *)ri); > > + vcap_show_admin_rule(vctrl, admin, out, to_intrule(vrule)); > > + vcap_free_rule(vrule); > > } > > - mutex_unlock(&admin->lock); > > - return 0; > > - > > -err_free_rule: > > - vcap_free_rule((struct vcap_rule *)ri); > > -err_unlock: > > - mutex_unlock(&admin->lock); > > return ret; > > } > -- /Horatiu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel