From mboxrd@z Thu Jan 1 00:00:00 1970 From: Leon Romanovsky Subject: Re: [PATCH rdma-next v2 13/17] RDMA/core: Get sum value of all counters when perform a sysfs stat read Date: Wed, 29 May 2019 14:05:24 +0300 Message-ID: <20190529110524.GU4633@mtr-leonro.mtl.com> References: <20190429083453.16654-1-leon@kernel.org> <20190429083453.16654-14-leon@kernel.org> <20190522172636.GF15023@ziepe.ca> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20190522172636.GF15023@ziepe.ca> Sender: netdev-owner@vger.kernel.org To: Jason Gunthorpe Cc: Doug Ledford , RDMA mailing list , Majd Dibbiny , Mark Zhang , Saeed Mahameed , linux-netdev List-Id: linux-rdma@vger.kernel.org On Wed, May 22, 2019 at 02:26:36PM -0300, Jason Gunthorpe wrote: > On Mon, Apr 29, 2019 at 11:34:49AM +0300, Leon Romanovsky wrote: > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > > index c56ffc61ab1e..8ae4906a60e7 100644 > > +++ b/drivers/infiniband/core/device.c > > @@ -1255,7 +1255,11 @@ int ib_register_device(struct ib_device *device, const char *name) > > goto dev_cleanup; > > } > > > > - rdma_counter_init(device); > > + ret = rdma_counter_init(device); > > + if (ret) { > > + dev_warn(&device->dev, "Couldn't initialize counter\n"); > > + goto sysfs_cleanup; > > + } > > Don't put this things randomly, if there is some reason it should be > after sysfs it needs a comment, otherwise if it is just allocating > memory it belongs earlier, and the unwind should be done in release. > > I also think it is very strange/wrong that both sysfs and counters are > allocating the same alloc_hw_stats object > > Why can't they share? They can, but we wanted to separate "legacy" counters which were exposed through sysfs and "new" counters which can be enabled/disable automatically. Thanks > > Jason