From: Jason Gunthorpe <jgg@nvidia.com>
To: Anand Khoje <anand.a.khoje@oracle.com>
Cc: linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org,
dledford@redhat.com, haakon.bugge@oracle.com, leon@kernel.org
Subject: Re: [PATCH v5 for-next 3/3] IB/core: Obtain subnet_prefix from cache in IB devices
Date: Thu, 24 Jun 2021 14:54:58 -0300 [thread overview]
Message-ID: <20210624175458.GR2371267@nvidia.com> (raw)
In-Reply-To: <012d6cd2-5167-ed81-80db-444fd2741ea8@oracle.com>
On Wed, Jun 23, 2021 at 06:33:32PM +0530, Anand Khoje wrote:
> On 6/22/2021 5:19 AM, Jason Gunthorpe wrote:
> > On Wed, Jun 16, 2021 at 09:15:09PM +0530, Anand Khoje wrote:
> > > @@ -1523,13 +1524,21 @@ static int config_non_roce_gid_cache(struct ib_device *device,
> > > device->port_data[port].cache.lmc = tprops->lmc;
> > > device->port_data[port].cache.port_state = tprops->state;
> > > - device->port_data[port].cache.subnet_prefix = tprops->subnet_prefix;
> > > + ret = rdma_query_gid(device, port, 0, &gid);
> > > + if (ret) {
> >
> > This is quite a bit different than just calling ops.query_gid() - why
> > are you changing it? I'm not sure all the additional tests will pass,
> > the 0 gid entry is not required to be valid..
> >
> Hi Jason,
>
> We have opted for rdma_query_gid(), as during ib_cache_update() the code
> calls ops.query_gid() earlier in config_non_roce_gid_cache(), thereby
> updating the value of GID in cache. We utilize this updated value, instead
> of calling ops->query_gid() again.
Uhhhh, so just store the subnet prefix at that point then?
diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index c9e9fc81447e89..5c554ebd000e89 100644
--- a/drivers/infiniband/core/cache.c
+++ b/drivers/infiniband/core/cache.c
@@ -1428,8 +1428,8 @@ int rdma_read_gid_l2_fields(const struct ib_gid_attr *attr,
}
EXPORT_SYMBOL(rdma_read_gid_l2_fields);
-static int config_non_roce_gid_cache(struct ib_device *device,
- u32 port, int gid_tbl_len)
+static int config_non_roce_gid_cache(struct ib_device *device, u32 port,
+ struct ib_port_attr *tprops)
{
struct ib_gid_attr gid_attr = {};
struct ib_gid_table *table;
@@ -1441,7 +1441,7 @@ static int config_non_roce_gid_cache(struct ib_device *device,
table = rdma_gid_table(device, port);
mutex_lock(&table->lock);
- for (i = 0; i < gid_tbl_len; ++i) {
+ for (i = 0; i < tprops->gid_tbl_len; ++i) {
if (!device->ops.query_gid)
continue;
ret = device->ops.query_gid(device, port, i, &gid_attr.gid);
@@ -1452,6 +1452,8 @@ static int config_non_roce_gid_cache(struct ib_device *device,
goto err;
}
gid_attr.index = i;
+ tprops->subnet_prefix =
+ be64_to_cpu(gid_attr.global.subnet_prefix);
add_modify_gid(table, &gid_attr);
}
err:
@@ -1484,7 +1486,7 @@ ib_cache_update(struct ib_device *device, u32 port, bool update_gids,
if (!rdma_protocol_roce(device, port) && update_gids) {
ret = config_non_roce_gid_cache(device, port,
- tprops->gid_tbl_len);
+ tprops);
if (ret)
goto err;
}
> > And I would much prefer things be re-organized so the cache can be
> > valid sooner to adding this variable. What is the earlier call that is
> > motivating this?
>
> During device load and when cache is yet to be updated, ib_query_port()
> should have a mechanism to identify if the cache entry is valid or invalid
> (uninitialized), we have added this variable just to ensure the validity of
> cache.
Unless there is an actual user of ib_query_port() before
config_non_roce_gid_cache() that I can't see, don't bother, returning
0 is fine.
Jason
next prev parent reply other threads:[~2021-06-24 17:55 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-16 15:45 [PATCH v5 for-next 0/3] IB/core: Obtaining subnet_prefix from cache in Anand Khoje
2021-06-16 15:45 ` [PATCH v5 for-next 1/3] IB/core: Removed port validity check from ib_get_cached_subnet_prefix Anand Khoje
2021-06-16 15:45 ` [PATCH v5 for-next 2/3] IB/core: Shuffle locks in ib_port_data to save memory Anand Khoje
2021-06-16 15:45 ` [PATCH v5 for-next 3/3] IB/core: Obtain subnet_prefix from cache in IB devices Anand Khoje
2021-06-17 6:41 ` Leon Romanovsky
2021-06-21 23:49 ` Jason Gunthorpe
2021-06-23 13:03 ` Anand Khoje
2021-06-24 17:54 ` Jason Gunthorpe [this message]
2021-06-25 6:03 ` Anand Khoje
2021-06-25 12:48 ` Jason Gunthorpe
2021-06-21 23:52 ` [PATCH v5 for-next 0/3] IB/core: Obtaining subnet_prefix from cache in Jason Gunthorpe
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=20210624175458.GR2371267@nvidia.com \
--to=jgg@nvidia.com \
--cc=anand.a.khoje@oracle.com \
--cc=dledford@redhat.com \
--cc=haakon.bugge@oracle.com \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).