From: Dan Carpenter <error27@gmail.com>
To: "Ismail, Mustafa" <mustafa.ismail@intel.com>
Cc: Leon Romanovsky <leon@kernel.org>,
"Latif, Faisal" <faisal.latif@intel.com>,
"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
Subject: Re: [bug report] iwpm: crash fix for large connections test
Date: Sat, 19 Nov 2022 10:31:38 +0300 [thread overview]
Message-ID: <Y3iGWkLzqm3B4ttV@kadam> (raw)
In-Reply-To: <PH7PR11MB640377FDDE4E242D31DE063E8B099@PH7PR11MB6403.namprd11.prod.outlook.com>
On Fri, Nov 18, 2022 at 08:44:11PM +0000, Ismail, Mustafa wrote:
> > > 432 register_pid_response_exit:
> > > 433 nlmsg_request->request_done = 1;
> > > 434 /* always for found nlmsg_request */
> > > 435 kref_put(&nlmsg_request->kref, iwpm_free_nlmsg_request);
> > >
> > > The iwpm_free_nlmsg_request() function will free "nlmsg_request"...
> > > It's not clear what the "/* always for found nlmsg_request */" comment
> > > means. Maybe it means that the refcount won't drop to zero so the
> > > free function won't be called?
> >
> > I think so. The nlmsg_request reference counter is elevated when it is found
> > in iwpm_find_nlmsg_request(). So I assume that it will be at least
> > 2 before call to kref_put(). Most likely, nlmsg_request->sem prevents from
> > parallel threads to decrease that reference counter.
> >
>
> I agree with Leon. The ref count should be 2 here.
> However, I don't see why the kref_put() can't be moved after the up(&nlmsg_request->sem) to get rid of the warning.
>
Let's not expend too much time trying to silence this warning. One way
to silence the warning would be to do:
kref_put(&nlmsg_request->kref, NULL);
I'm conficted about this approach, but no good can come from calling
iwpm_free_nlmsg_request() on this path.
A better way to silence the warning would be to do:
diff --git a/drivers/infiniband/core/iwpm_util.c b/drivers/infiniband/core/iwpm_util.c
index 358a2db38d23..4f819e6c1b09 100644
--- a/drivers/infiniband/core/iwpm_util.c
+++ b/drivers/infiniband/core/iwpm_util.c
@@ -357,7 +357,7 @@ struct iwpm_nlmsg_request *iwpm_find_nlmsg_request(__u32 echo_seq)
inprocess_list) {
if (nlmsg_request->nlmsg_seq == echo_seq) {
found_request = nlmsg_request;
- kref_get(&nlmsg_request->kref);
+ kref_get(&found_request->kref);
break;
}
}
But the best way would be to make Smatch parse iwpm_find_nlmsg_request()
correctly as-is.
regards,
dan carpenter
next prev parent reply other threads:[~2022-11-19 7:31 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-15 13:17 [bug report] iwpm: crash fix for large connections test Dan Carpenter
2022-11-17 9:24 ` Leon Romanovsky
2022-11-18 20:44 ` Ismail, Mustafa
2022-11-19 7:31 ` Dan Carpenter [this message]
2022-11-28 7:34 ` Dan Carpenter
2023-01-20 11:13 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y3iGWkLzqm3B4ttV@kadam \
--to=error27@gmail.com \
--cc=faisal.latif@intel.com \
--cc=leon@kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=mustafa.ismail@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.