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 X-Spam-Level: X-Spam-Status: No, score=-6.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1F13EC47094 for ; Mon, 7 Jun 2021 12:41:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EF2A56108C for ; Mon, 7 Jun 2021 12:41:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230233AbhFGMnd (ORCPT ); Mon, 7 Jun 2021 08:43:33 -0400 Received: from mail.kernel.org ([198.145.29.99]:58832 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230212AbhFGMnc (ORCPT ); Mon, 7 Jun 2021 08:43:32 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 469136108C; Mon, 7 Jun 2021 12:41:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1623069701; bh=lu2ZH+zANWlmtDzF/msjapOw9uj69eZMY5m11K67Rxg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=BU+WD7cvjzRz1qIKJw94BmkSep/woow9BS2l3V+9yR94wwiE9aAlSzO7nj5OS+fG3 mqTIinkkY9pjBwYLQn9FCf+ctyq6oLwnyVoTpoRcWBZ8L5q68uWE2jiqLOCUB45rmt fhR6wu+MmOru9HRkelRrTFfa9RDCYid0o/XVpfYB13ezITJfrhSer+9/U9dkEcnHna CZW4ItXfscl/qD6DDhbS17wHDi3C+dRXzCq6oRgJ73t7QJ+xOzvFqrt5OtQDkpSmgs 39HMWIK+A0fFkmn9uUVkC606ZgrwMlcFh6HDVSyE9H6V9ewyLtcTV5LJuEWEMXSw+u uw5MGUfEG2xWQ== Date: Mon, 7 Jun 2021 15:41:37 +0300 From: Leon Romanovsky To: Greg KH Cc: Doug Ledford , Jason Gunthorpe , Kees Cook , Nathan Chancellor , Adit Ranadive , Ariel Elior , Christian Benvenuti , clang-built-linux@googlegroups.com, Dennis Dalessandro , Devesh Sharma , Gal Pressman , linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org, Michal Kalderon , Mike Marciniszyn , Mustafa Ismail , Naresh Kumar PBS , Nelson Escobar , Nick Desaulniers , Potnuri Bharat Teja , Selvin Xavier , Shiraz Saleem , VMware PV-Drivers , Yishai Hadas , Zhu Yanjun Subject: Re: [PATCH rdma-next v1 10/15] RDMA/cm: Use an attribute_group on the ib_port_attribute intead of kobj's Message-ID: References: <00e578937f557954d240bc0856f45b3f752d6cba.1623053078.git.leonro@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org On Mon, Jun 07, 2021 at 02:08:27PM +0200, Greg KH wrote: > On Mon, Jun 07, 2021 at 02:37:16PM +0300, Leon Romanovsky wrote: > > On Mon, Jun 07, 2021 at 01:22:26PM +0200, Greg KH wrote: > > > On Mon, Jun 07, 2021 at 01:51:36PM +0300, Leon Romanovsky wrote: > > > > On Mon, Jun 07, 2021 at 12:25:03PM +0200, Greg KH wrote: > > > > > On Mon, Jun 07, 2021 at 11:17:35AM +0300, Leon Romanovsky wrote: > > > > > > From: Jason Gunthorpe > > > > > > > > > > > > This code is trying to attach a list of counters grouped into 4 groups to > > > > > > the ib_port sysfs. Instead of creating a bunch of kobjects simply express > > > > > > everything naturally as an ib_port_attribute and add a single > > > > > > attribute_groups list. > > > > > > > > > > > > Remove all the naked kobject manipulations. > > > > > > > > > > Much nicer. > > > > > > > > > > But why do you need your counters to be atomic in the first place? What > > > > > are they counting that requires this? Given that they are just a > > > > > statistic for userspace, making them be a u64 should work just the same, > > > > > right? > > > > > > > > The statistic counters are per-port, while the cm.c flows run in > > > > asynchronically in parallel for every CM connection. > > > > > > > > We need atomic variable to ensure that "write to u64" is not > > > > interrupted. > > > > > > On what system is "write to u64" interruptable? > > > > On 32 bits, and yes, we have a customer who still uses such system. > > So you will see what, a "tear"? Or a stale value? Does it really make any difference for the users? The end result is that value is incorrect. > > > > As these are per-port, do multiple threads try to increment these at > > > the same time? > > > > Yes, CM connection can be seen as thread. Bottom line everything in parallel. > > > > > And even if they do, what happens if one is 'dropped' somehow because of this? > > > > Probably nothing, we increment the statistics only. > > So you are hitting cache lines for no good reason, probably not a good > idea, you are wasting cpu cycles for nothing :( I prefer correctness over performance optimization in paths that far from being hot. The thing is that we have such correctness for free without any extra code complexity by simply using proper data structure. Why shouldn't we use atomics? Also, we aren't talking about some driver code, but about core infrastructure for whole RDMA subsystem. Thanks > > thanks, > > greg k-h