Messages in this thread | | | Date | Tue, 26 Oct 2021 09:05:54 -0300 | From | Jason Gunthorpe <> | Subject | Re: [PATCH rdma-rc 2/2] RDMA/core: Initialize lock when allocate a rdma_hw_stats structure |
| |
On Tue, Oct 26, 2021 at 11:27:57AM +0300, Leon Romanovsky wrote: > On Mon, Oct 25, 2021 at 11:50:43AM -0300, Jason Gunthorpe wrote: > > On Sun, Oct 24, 2021 at 09:08:21AM +0300, Leon Romanovsky wrote: > > > From: Mark Zhang <markzhang@nvidia.com> > > > > > > Initialize the rdma_hw_stats "lock" field when do allocation, to fix the > > > warning below. Then we don't need to initialize it in sysfs, remove it. > > > > This is a fine cleanup, but this does not describe the bug properly, > > or have the right fixes line.. > > I think that this Fixes line should be instead. > Fixes: 0a0800ce2a6a ("RDMA/core: Add a helper API rdma_free_hw_stats_struct")
No, I don't think so, it should be the commit that added alloc_and_bind()
> > The issue is here: > > > > static struct rdma_counter *alloc_and_bind(struct ib_device *dev, u32 port, > > struct ib_qp *qp, > > enum rdma_nl_counter_mode mode) > > { > > counter->stats = dev->ops.counter_alloc_stats(counter); > > if (!counter->stats) > > goto err_stats; > > > > Which does not init counter->stat's mutex. > > This is exactly what Mark is doing here. > > alloc_and_bind() > -> dev->ops.counter_alloc_stats > -> mlx5_ib_counter_alloc_stats > -> do_alloc_stats() > -> rdma_alloc_hw_stats_struct() > -> mutex_init(&stats->lock); <- Mark's change.
Yes, I know, the patch is fine, the commit message just needs to be accurate
> > And trim the oops reports, don't include the usless ? fns, timestamps > > or other junk. > > I don't like when people "beatify" kernel reports, many times whey are > removing too much information.
There is too much junk in the raw oops messages
Jason
| |