I do have a holistic view on this and similar patches to avoid calling
ib_query_xyz().
Caching logic has been introduced in ibcore (cache.c). It does,
however, provide a non-transparent call to ib_query_xyz() attributes,
e.g., ib_get_cached_gid().
On my opinion, caching should be transparent.
(When caching in CPUs were introduced, we could still use the same
instructions, right?)
So, my proposal would be to make the ib_query_xyz() fast, by have
_its_ implementations using a cache.
This can be done by changing a single statement per query, and it will
take effect for all clients and ULPs:
Change ib_query_xyz() to call ib_get_cache_xyz() instead of
device->query_xyz().
This way, all legacy calls to ib_query_xyz() will use the cached
version and no changes are required in ULPs / clients.
The complication is to update the cache properly before events are
forwarded, but I think that can be done failry simply.
My 2 cents.
Thxs, Håkon
> On 17. des. 2015, at 14.19, Or Gerlitz <ogerlitz@xxxxxxxxxxxx> wrote:
>
> Instead, use the cached copy of the attributes present on the device.
>
> Based on a patch from Christoph Hellwig <hch@xxxxxx>
>
> Signed-off-by: Or Gerlitz <ogerlitz@xxxxxxxxxxxx>
> ---
> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c | 21 +--------------------
> 1 file changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> index 7c730e3..7231ef8 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> @@ -2070,32 +2070,13 @@ static int kiblnd_net_init_pools(kib_net_t *net, __u32 *cpts, int ncpts)
>
> static int kiblnd_hdev_get_attr(kib_hca_dev_t *hdev)
> {
> - struct ib_device_attr *attr;
> - int rc;
> -
> /* It's safe to assume a HCA can handle a page size
> * matching that of the native system */
> hdev->ibh_page_shift = PAGE_SHIFT;
> hdev->ibh_page_size = 1 << PAGE_SHIFT;
> hdev->ibh_page_mask = ~((__u64)hdev->ibh_page_size - 1);
>
> - LIBCFS_ALLOC(attr, sizeof(*attr));
> - if (attr == NULL) {
> - CERROR("Out of memory\n");
> - return -ENOMEM;
> - }
> -
> - rc = ib_query_device(hdev->ibh_ibdev, attr);
> - if (rc == 0)
> - hdev->ibh_mr_size = attr->max_mr_size;
> -
> - LIBCFS_FREE(attr, sizeof(*attr));
> -
> - if (rc != 0) {
> - CERROR("Failed to query IB device: %d\n", rc);
> - return rc;
> - }
> -
> + hdev->ibh_mr_size = hdev->ibh_ibdev->attrs.max_mr_size;
> if (hdev->ibh_mr_size == ~0ULL) {
> hdev->ibh_mr_shift = 64;
> return 0;
> --
> 2.3.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
